[U-Boot] [PATCH v3 0/6] efi_loader: correct media device paths

For each disk we need partition device path with partion number 0. The device node texts should match the UEFI spec.
v3: Add support for IDE and SCSI disks. Avoid an unaligned memory access. v2: Do not generate optional device path with partion number 0 for the whole block device.
Heinrich Schuchardt (6): efi_loader: correctly determine if an MMC device is an SD-card efi_loader: correctly setup device paths for block devices efi_loader: correct DeviceNodeToText for media types efi_loader: comments for dp_part_fill() efi_loader: create full device path for block devices efi_loader: support device path for IDE and SCSI disks
include/efi_api.h | 15 ++++ lib/efi_loader/efi_device_path.c | 119 ++++++++++++++++++++++++++++--- lib/efi_loader/efi_device_path_to_text.c | 57 +++++++++++---- 3 files changed, 171 insertions(+), 20 deletions(-)

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 --- v3 no change v2 no change --- 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; sddp->dp.length = sizeof(*sddp); sddp->slot_number = dev->seq;

According to the UEFI spec the numbering of partitions has to start with 1.
Partion number 0 is reserved for the optional device path for the complete block device.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 no change v2 Do not generate optional device path with partion number 0. --- lib/efi_loader/efi_device_path.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 42fe6e1185..6461ea9abc 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -420,7 +420,7 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part) 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 +434,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)

When converting device nodes and paths to text we should stick to the UEFI spec.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 Copy from packed structure to aligned memory to avoid unaligned memory access. v2 no change --- lib/efi_loader/efi_device_path_to_text.c | 43 +++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 7159c974d4..50d9e911c0 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -90,7 +90,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_MSG_USB: { struct efi_device_path_usb *udp = (struct efi_device_path_usb *)dp; - s += sprintf(s, "Usb(0x%x,0x%x)", udp->parent_port_number, + s += sprintf(s, "USB(0x%x,0x%x)", udp->parent_port_number, udp->usb_interface); break; } @@ -124,10 +124,10 @@ static char *dp_msging(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_MSG_MMC: { const char *typename = (dp->sub_type == DEVICE_PATH_SUB_TYPE_MSG_SD) ? - "SDCard" : "MMC"; + "SD" : "eMMC"; struct efi_device_path_sd_mmc_path *sddp = (struct efi_device_path_sd_mmc_path *)dp; - s += sprintf(s, "%s(Slot%u)", typename, sddp->slot_number); + s += sprintf(s, "%s(%u)", typename, sddp->slot_number); break; } default: @@ -137,6 +137,13 @@ static char *dp_msging(char *s, struct efi_device_path *dp) return s; }
+/* + * Convert a media device path node to text. + * + * @s output buffer + * @dp device path node + * @return next unused buffer address + */ static char *dp_media(char *s, struct efi_device_path *dp) { switch (dp->sub_type) { @@ -144,21 +151,33 @@ static char *dp_media(char *s, struct efi_device_path *dp) struct efi_device_path_hard_drive_path *hddp = (struct efi_device_path_hard_drive_path *)dp; void *sig = hddp->partition_signature; + u64 start; + u64 end; + + /* Copy from packed structure to aligned memory */ + memcpy(&start, &hddp->partition_start, sizeof(start)); + memcpy(&end, &hddp->partition_end, sizeof(end));
switch (hddp->signature_type) { - case SIG_TYPE_MBR: - s += sprintf(s, "HD(Part%d,Sig%08x)", - hddp->partition_number, - *(uint32_t *)sig); + case SIG_TYPE_MBR: { + u32 signature; + + memcpy(&signature, sig, sizeof(signature)); + s += sprintf( + s, "HD(%d,MBR,0x%08x,0x%llx,0x%llx)", + hddp->partition_number, signature, start, end); break; + } case SIG_TYPE_GUID: - s += sprintf(s, "HD(Part%d,Sig%pUl)", - hddp->partition_number, sig); + s += sprintf( + s, "HD(%d,GPT,%pUl,0x%llx,0x%llx)", + hddp->partition_number, sig, start, end); break; default: - s += sprintf(s, "HD(Part%d,MBRType=%02x,SigType=%02x)", - hddp->partition_number, hddp->partmap_type, - hddp->signature_type); + s += sprintf( + s, "HD(%d,0x%02x,0,0x%llx,0x%llx)", + hddp->partition_number, hddp->partmap_type, + start, end); break; }

Add a description for dp_part_fill(). Reword a comment in the function.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 new patch --- lib/efi_loader/efi_device_path.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 6461ea9abc..31cdd38773 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -384,6 +384,13 @@ static unsigned dp_part_size(struct blk_desc *desc, int part) return dpsize; }
+/* + * Create a device path for a block device or one of its partitions. + * + * @buf buffer to which the device path is wirtten + * @desc block device descriptor + * @part partition number, 0 identifies a block device + */ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part) { disk_partition_t info; @@ -396,7 +403,7 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part) * and handling all the different cases like we do for non- * legacy (ie CONFIG_BLK=y) case. But most important thing * is just to have a unique device-path for if_type+devnum. - * So map things to a fictional USB device: + * So map things to a fictitious USB device. */ struct efi_device_path_usb *udp;

When creating the device path of a block device it has to comprise the block device itself and should not end at its parent.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 new patch --- lib/efi_loader/efi_device_path.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 31cdd38773..492a7643e6 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -368,7 +368,14 @@ static unsigned dp_part_size(struct blk_desc *desc, int part) unsigned dpsize;
#ifdef CONFIG_BLK - dpsize = dp_size(desc->bdev->parent); + { + struct udevice *dev; + int ret = blk_find_device(desc->if_type, desc->devnum, &dev); + + if (ret) + dev = desc->bdev->parent; + dpsize = dp_size(dev); + } #else dpsize = sizeof(ROOT) + sizeof(struct efi_device_path_usb); #endif @@ -396,7 +403,14 @@ static void *dp_part_fill(void *buf, struct blk_desc *desc, int part) disk_partition_t info;
#ifdef CONFIG_BLK - buf = dp_fill(buf, desc->bdev->parent); + { + struct udevice *dev; + int ret = blk_find_device(desc->if_type, desc->devnum, &dev); + + if (ret) + dev = desc->bdev->parent; + buf = dp_fill(buf, dev); + } #else /* * We *could* make a more accurate path, by looking at if_type

Correctly create the device path for IDE and SCSI disks.
Support for SATA remains to be done in a future patch.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 new patch --- include/efi_api.h | 15 ++++++++ lib/efi_loader/efi_device_path.c | 64 ++++++++++++++++++++++++++++++++ lib/efi_loader/efi_device_path_to_text.c | 14 +++++++ 3 files changed, 93 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h index 584016dc30..46963f2891 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -329,12 +329,27 @@ struct efi_device_path_acpi_path { } __packed;
#define DEVICE_PATH_TYPE_MESSAGING_DEVICE 0x03 +# define DEVICE_PATH_SUB_TYPE_MSG_ATAPI 0x01 +# define DEVICE_PATH_SUB_TYPE_MSG_SCSI 0x02 # define DEVICE_PATH_SUB_TYPE_MSG_USB 0x05 # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b # define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS 0x0f # define DEVICE_PATH_SUB_TYPE_MSG_SD 0x1a # define DEVICE_PATH_SUB_TYPE_MSG_MMC 0x1d
+struct efi_device_path_atapi { + struct efi_device_path dp; + u8 primary_secondary; + u8 slave_master; + u16 logical_unit_number; +} __packed; + +struct efi_device_path_scsi { + struct efi_device_path dp; + u16 target_id; + u16 logical_unit_number; +} __packed; + struct efi_device_path_usb { struct efi_device_path dp; u8 parent_port_number; diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 492a7643e6..ed30f1cabf 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -282,6 +282,23 @@ static unsigned dp_size(struct udevice *dev) case UCLASS_SIMPLE_BUS: /* stop traversing parents at this point: */ return sizeof(ROOT); +#ifdef CONFIG_BLK + case UCLASS_BLK: + switch (dev->parent->uclass->uc_drv->id) { +#ifdef CONFIG_IDE + case UCLASS_IDE: + return dp_size(dev->parent) + + sizeof(struct efi_device_path_atapi); +#endif +#if defined(CONFIG_SCSI) && defined(CONFIG_DM_SCSI) + case UCLASS_SCSI: + return dp_size(dev->parent) + + sizeof(struct efi_device_path_scsi); +#endif + default: + return dp_size(dev->parent); + } +#endif case UCLASS_MMC: return dp_size(dev->parent) + sizeof(struct efi_device_path_sd_mmc_path); @@ -295,6 +312,13 @@ static unsigned dp_size(struct udevice *dev) } }
+/* + * Recursively build a device path. + * + * @buf pointer to the end of the device path + * @dev device + * @return pointer to the end of the device path + */ static void *dp_fill(void *buf, struct udevice *dev) { if (!dev || !dev->driver) @@ -308,6 +332,46 @@ static void *dp_fill(void *buf, struct udevice *dev) *vdp = ROOT; return &vdp[1]; } +#ifdef CONFIG_BLK + case UCLASS_BLK: + switch (dev->parent->uclass->uc_drv->id) { +#ifdef CONFIG_IDE + case UCLASS_IDE: { + struct efi_device_path_atapi *dp = + dp_fill(buf, dev->parent); + struct blk_desc *desc = dev_get_uclass_platdata(dev); + + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_ATAPI; + dp->dp.length = sizeof(*dp); + dp->logical_unit_number = desc->devnum; + dp->primary_secondary = IDE_BUS(desc->devnum); + dp->slave_master = desc->devnum % + (CONFIG_SYS_IDE_MAXDEVICE / + CONFIG_SYS_IDE_MAXBUS); + return &dp[1]; + } +#endif +#if defined(CONFIG_SCSI) && defined(CONFIG_DM_SCSI) + case UCLASS_SCSI: { + struct efi_device_path_scsi *dp = + dp_fill(buf, dev->parent); + struct blk_desc *desc = dev_get_uclass_platdata(dev); + + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SCSI; + dp->dp.length = sizeof(*dp); + dp->logical_unit_number = desc->lun; + dp->target_id = desc->target; + return &dp[1]; + } +#endif + default: + printf("unhandled parent class: %s (%u)\n", + dev->name, dev->driver->id); + return dp_fill(buf, dev->parent); + } +#endif #if defined(CONFIG_DM_MMC) && defined(CONFIG_MMC) case UCLASS_MMC: { struct efi_device_path_sd_mmc_path *sddp = diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 50d9e911c0..40eae730ed 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -87,6 +87,20 @@ static char *dp_acpi(char *s, struct efi_device_path *dp) static char *dp_msging(char *s, struct efi_device_path *dp) { switch (dp->sub_type) { + case DEVICE_PATH_SUB_TYPE_MSG_ATAPI: { + struct efi_device_path_atapi *ide = + (struct efi_device_path_atapi *)dp; + s += sprintf(s, "Ata(%d,%d,%d)", ide->primary_secondary, + ide->slave_master, ide->logical_unit_number); + break; + } + case DEVICE_PATH_SUB_TYPE_MSG_SCSI: { + struct efi_device_path_scsi *ide = + (struct efi_device_path_scsi *)dp; + s += sprintf(s, "Scsi(%u,%u)", ide->target_id, + ide->logical_unit_number); + break; + } case DEVICE_PATH_SUB_TYPE_MSG_USB: { struct efi_device_path_usb *udp = (struct efi_device_path_usb *)dp;
participants (1)
-
Heinrich Schuchardt