Re: [U-Boot] [PATCH 2/3] efi_loader: correctly setup device paths for block devices

[Mark Kettenis]

xypron.g...@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.g...@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 */
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot