On 02.12.17 13:42, Heinrich Schuchardt wrote:

The SD cards and eMMC devices have different device nodes. The current coding interpretes all MMC devices as eMMC.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_device_path.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index b4e2f933cb..42fe6e1185 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -36,6 +36,24 @@ static const struct efi_device_path_vendor ROOT = { .guid = U_BOOT_GUID, };

+#if defined(CONFIG_DM_MMC) && defined(CONFIG_MMC) +/*

• • Determine if an MMC device is an SD card.
• • @desc block device descriptor
• • @return true if the device is an SD card
• */

+static bool is_sd(struct blk_desc *desc) +{

• struct mmc *mmc = find_mmc_device(desc->devnum);
• if (!mmc)

• return false;
  

• return IS_SD(mmc) != 0U;

+} +#endif

static void *dp_alloc(size_t sz) { void *buf; @@ -298,9 +316,9 @@ static void *dp_fill(void *buf, struct udevice *dev) struct blk_desc *desc = mmc_get_blk_desc(mmc);

sddp->dp.type     = DEVICE_PATH_TYPE_MESSAGING_DEVICE;

• sddp->dp.sub_type = (desc->if_type == IF_TYPE_MMC) ?
  

• 	DEVICE_PATH_SUB_TYPE_MSG_MMC :
  

• 	DEVICE_PATH_SUB_TYPE_MSG_SD;
  

• sddp->dp.sub_type = is_sd(desc) ?
  

• 	DEVICE_PATH_SUB_TYPE_MSG_SD :
  

• 	DEVICE_PATH_SUB_TYPE_MSG_MMC;
  

So in general, MMC != SD != eMMC. Or rather eMMC "is like" SD "is like" MMC maybe?

I really don't know how edk2 distinguishes between them. In general, eMMC is an SD card on steroids. Can you double-check in the edk2 code and then we just do whatever they do?

Alex

On Sunday, December 3, 2017 8:24:39 PM CET Mark Kettenis wrote:

Heinrich Schuchardt schreef op 2017-12-02 13:42:

...

For each block device we have to setup:

• a device path for the block device

• a partition device path for the block device with

  partition number 0

Do x86 UEFI implementations actually generate these partition number 0 paths?

Thanks for reviewing.

I do not have access the an x86 computer with UEFI firmware. Maybe somebody else on the list has.

Another reference point is EDK2.

Part of the relevant coding is in function PartitionInstallMbrChildHandles(), MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c:47.

Another part is in PartitionInstallChildHandle(), MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c.

I could not identify any line referring to partition 0.

The standard says that 0 can be used in tis fashion but doesn't seem to prescribe their use.

<cite>A partition number of zero can be used to represent the raw hard drive or a raw extended partition.</cite>

Do you think we should not generate this entry?

Regards

Heinrich

...

• a partition device path for each partition

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_device_path.c | 11 +++++++---- lib/efi_loader/efi_disk.c | 7 +++++-- 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 42fe6e1185..d0d62ff428 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -412,15 +412,18 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part)

buf = &udp[1];

#endif

• if (part == 0) /* the actual disk, not a partition */

• if (part == -1) /* the actual disk, not a partition */

  return buf;

• part_get_info(desc, part, &info);

• if (part == 0)

• part_get_info_whole_disk(desc, &info);
  

• else

• part_get_info(desc, part, &info);
  
  

  if (desc->part_type == PART_TYPE_ISO) {

  struct efi_device_path_cdrom_path *cddp = buf;

• cddp->boot_entry = part - 1;
  

• cddp->boot_entry = part;
  
  

  cddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE; cddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CDROM_PATH; cddp->dp.length = sizeof(*cddp);

@@ -434,7 +437,7 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part)

hddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
hddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH;
hddp->dp.length = sizeof(*hddp);

• hddp->partition_number = part - 1;
  

• hddp->partition_number = part;
  
  

  hddp->partition_start = info.start; hddp->partition_end = info.size; if (desc->part_type == PART_TYPE_EFI)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 4e457a841b..19f75aa919 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -274,6 +274,9 @@ static int efi_disk_create_partitions(struct blk_desc *desc,

disk_partition_t info; int part;

• /* Add devices for disk */

• snprintf(devname, sizeof(devname), "%s", pdevname);

• efi_disk_add_dev(devname, if_typename, desc, diskid, info.start, 0);

  /* Add devices for each partition */ for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {

  if (part_get_info(desc, part, &info))

@@ -315,7 +318,7 @@ int efi_disk_register(void)

/* Add block device for the full device */
efi_disk_add_dev(dev->name, if_typename, desc,

• 		 desc->devnum, 0, 0);
  

• 		 desc->devnum, 0, -1);
  
  

  disks++;

@@ -351,7 +354,7 @@ int efi_disk_register(void)

		 if_typename, i);
	
	/* Add block device for the full device */

• 	efi_disk_add_dev(devname, if_typename, desc, i, 0, 0);
  

• 	efi_disk_add_dev(devname, if_typename, desc, i, 0, -1);
  
  disks++;
  

  /* Partitions show up as block devices in EFI */

xypron.glpk@gmx.de schreef op 2017-12-03 22:20:

On Sunday, December 3, 2017 8:24:39 PM CET Mark Kettenis wrote:

...

Heinrich Schuchardt schreef op 2017-12-02 13:42:

...

For each block device we have to setup:

• a device path for the block device

• a partition device path for the block device with

  partition number 0

Do x86 UEFI implementations actually generate these partition number 0 paths?

Thanks for reviewing.

I do not have access the an x86 computer with UEFI firmware. Maybe somebody else on the list has.

Another reference point is EDK2.

Part of the relevant coding is in function PartitionInstallMbrChildHandles(), MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c:47.

Another part is in PartitionInstallChildHandle(), MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c.

I could not identify any line referring to partition 0.

...

The standard says that 0 can be used in tis fashion but doesn't seem to prescribe their use.

<cite>A partition number of zero can be used to represent the raw hard drive or a raw extended partition.</cite>

Do you think we should not generate this entry?

I think it would be better to not generate this entry if other UEFI implementations don't generate it either. Your analysis of the EDK2 code suggests that they don't, so U-Boot probably shouldn't do this either.

Was going to check whether the EDK2-based firmware on my Overdrive 1000 provides a partition 0, but I managed to hang it in the process and I'll have to power-cycle it when I get home later today...

...

...

• a partition device path for each partition

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_device_path.c | 11 +++++++---- lib/efi_loader/efi_disk.c | 7 +++++-- 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 42fe6e1185..d0d62ff428 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -412,15 +412,18 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part)

buf = &udp[1];

#endif

• if (part == 0) /* the actual disk, not a partition */

• if (part == -1) /* the actual disk, not a partition */

  return buf;

• part_get_info(desc, part, &info);

• if (part == 0)

• part_get_info_whole_disk(desc, &info);
  

• else

• part_get_info(desc, part, &info);
  
  

  if (desc->part_type == PART_TYPE_ISO) {

  struct efi_device_path_cdrom_path *cddp = buf;

• cddp->boot_entry = part - 1;
  

• cddp->boot_entry = part;
  
  

  cddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE; cddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CDROM_PATH; cddp->dp.length = sizeof(*cddp);

@@ -434,7 +437,7 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part)

hddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
hddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH;
hddp->dp.length = sizeof(*hddp);

• hddp->partition_number = part - 1;
  

• hddp->partition_number = part;
  
  

  hddp->partition_start = info.start; hddp->partition_end = info.size; if (desc->part_type == PART_TYPE_EFI)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 4e457a841b..19f75aa919 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -274,6 +274,9 @@ static int efi_disk_create_partitions(struct blk_desc *desc,

disk_partition_t info; int part;

• /* Add devices for disk */

• snprintf(devname, sizeof(devname), "%s", pdevname);

• efi_disk_add_dev(devname, if_typename, desc, diskid, info.start, 0);

  /* Add devices for each partition */ for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {

  if (part_get_info(desc, part, &info))

@@ -315,7 +318,7 @@ int efi_disk_register(void)

/* Add block device for the full device */
efi_disk_add_dev(dev->name, if_typename, desc,

• 		 desc->devnum, 0, 0);
  

• 		 desc->devnum, 0, -1);
  
  

  disks++;

@@ -351,7 +354,7 @@ int efi_disk_register(void)

		 if_typename, i);

	/* Add block device for the full device */

• 	efi_disk_add_dev(devname, if_typename, desc, i, 0, 0);
  

• 	efi_disk_add_dev(devname, if_typename, desc, i, 0, -1);
  
  disks++;
  
  /* Partitions show up as block devices in EFI */