[U-Boot] [U-Boot, 3/4] efi_loader: make disk objects for partitions

Rob Clark
2017-07-23T19:48:20Z

On Sun, Jul 23, 2017 at 6:25 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 07/21/2017 08:43 PM, Rob Clark wrote:
>> Normally a EFI application, like grub2, is going to expect to see
>
> %s/a/an/
>
>> block-io devices for both the actual disk and for each partition
>> (with the disk object being parent of the partition objects).
>>
>> Now instead of seeing devices like:
>>
>> /File(sdhci at 07864000.blk)/EndEntire
>> /File(usb_mass_storage.lun0)/EndEntire
>>
>> You see:
>>
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,0000000000000000,1,0)/EndEntire
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,0000000000000000,1,0)/EndEntire
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,0000000000000000,1,0)/EndEntire
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,0000000000000000,1,0)/EndEntire
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,0000000000000000,1,0)/EndEntire
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,0000000000000000,1,0)/EndEntire
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,0000000000000000,1,0)/EndEntire
>>
>> This is on a board with single USB disk and single sd-card. The
>> UnknownMessaging(1d) node in the device-path is the MMC device,
>> but grub_efi_print_device_path() hasn't been updated yet for some
>> of the newer device-path sub-types.
>>
>> This patch is somewhat based on a patch originally from Peter Jones,
>> but re-worked to use efi_devpath, so it doesn't much resemble the
>> original.
>>
>> This depends on efi_devpath, which depends on CONFIG_DM, so legacy
>> devices will still get the old broken behavior.
>>
>> There are a couple TODOs:
>> 1) how to find slot # for mmc device
>> 2) how to find parent port # and interface # for usb device
>>
>> Also I don't seem to manage to get u-boot to recognize my usb-eth
>> dongle (config issue?) so I haven't been able to test the net-boot
>> path.. so it likely needs some work.
>>
>> Cc: Peter Jones <pjones@redhat.com>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> lib/efi_loader/efi_disk.c | 73 ++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 53 insertions(+), 20 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> index 09697936b5..44f72c93e2 100644
>> --- a/lib/efi_loader/efi_disk.c
>> +++ b/lib/efi_loader/efi_disk.c
>> @@ -28,7 +28,7 @@ struct efi_disk_obj {
>> /* EFI Interface Media descriptor struct, referenced by ops */
>> struct efi_block_io_media media;
>> /* EFI device path to this block device */
>> - struct efi_device_path_file_path *dp;
>> + struct efi_device_path *dp;
>> /* Offset into disk for simple partitions */
>> lbaint_t offset;
>> /* Internal block device */
>> @@ -193,21 +193,47 @@ static const struct efi_block_io block_io_disk_template = {
>> .flush_blocks = &efi_disk_flush_blocks,
>> };
>>
>> +static struct efi_device_path *blk2dp(struct blk_desc *desc,
>> + const char *name, unsigned part)
>> +{
>> +#ifdef CONFIG_DM
>> + /* part==0 is the case for parent block device representing
>> + * entire disk, and part>=1 is for child per-partition
>> + * MEDIA_DEVICE objects:
>> + */
>> + if (part == 0)
>> + return efi_dp_from_dev(desc->bdev->parent);
>> + return efi_dp_from_part(desc, part);
>> +#else
>> + struct efi_device_path_file_path *dp = calloc(2, sizeof(*dp));
>> +
>> + dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>> + dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>> + dp[0].dp.length = sizeof(*dp);
>> + ascii2unicode(dp[0].str, name);
>> +
>> + dp[1].dp.type = DEVICE_PATH_TYPE_END;
>> + dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
>> + dp[1].dp.length = sizeof(*dp);
>> +
>> + return dp;
>> +#endif
>> +}
>> +
>> static void efi_disk_add_dev(const char *name,
>> const char *if_typename,
>> - const struct blk_desc *desc,
>> + struct blk_desc *desc,
>> int dev_index,
>> - lbaint_t offset)
>> + lbaint_t offset,
>> + unsigned part)
>> {
>> struct efi_disk_obj *diskobj;
>> - struct efi_device_path_file_path *dp;
>> - int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
>>
>> /* Don't add empty devices */
>> if (!desc->lba)
>> return;
>>
>> - diskobj = calloc(1, objlen);
>> + diskobj = calloc(1, sizeof(*diskobj));
>>
>> /* Fill in object data */
>> diskobj->parent.protocols[0].guid = &efi_block_io_guid;
>
> I am unable to apply this patch to the efi-next branch of
> git://github.com/agraf/u-boot.git
>
> Applying: efi_loader: make disk objects for partitions
> error: patch failed: lib/efi_loader/efi_disk.c:193
> error: lib/efi_loader/efi_disk.c: patch does not apply
> Patch failed at 0001 efi_loader: make disk objects for partitions
>
> Could you, please, resubmit your patch series rebased on efi-next.

sorry, wasn't aware of efi-next tree.. I'll rebase soon. I have some
more updates[1] (including some parts that I'd like to squash into
this patch) to get shim.efi and fallback.efi working, but I'll send a
new version of these patches when I have that working.

[1] https://github.com/robclark/u-boot/commits/wip-enough-uefi-for-shim
<-- very much work-in-progress

BR,
-R

> You somehow forgot to CC the maintainer Alexander Graf in your patch.
>
> Please, always use scripts/get_maintainer.pl to determine whom a patch
> should be addressed to.
>
> Best regards
>
> Heinrich Schuchardt
>
>> @@ -228,18 +254,7 @@ static void efi_disk_add_dev(const char *name,
>> diskobj->media.io_align = desc->blksz;
>> diskobj->media.last_block = desc->lba - offset;
>> diskobj->ops.media = &diskobj->media;
>> -
>> - /* Fill in device path */
>> - dp = (void*)&diskobj[1];
>> - diskobj->dp = dp;
>> - dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>> - dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>> - dp[0].dp.length = sizeof(*dp);
>> - ascii2unicode(dp[0].str, name);
>> -
>> - dp[1].dp.type = DEVICE_PATH_TYPE_END;
>> - dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
>> - dp[1].dp.length = sizeof(*dp);
>> + diskobj->dp = blk2dp(desc, name, part);
>>
>> /* Hook up to the device list */
>> list_add_tail(&diskobj->parent.link, &efi_obj_list);
>> @@ -259,11 +274,16 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
>> if (desc->part_type != PART_TYPE_ISO)
>> return 0;
>>
>> +#ifdef CONFIG_DM
>> + /* add block device: */
>> + efi_disk_add_dev(devname, if_typename, desc, diskid, 0, 0);
>> +#endif
>> + /* ... and devices for each partition: */
>> while (!part_get_info(desc, part, &info)) {
>> snprintf(devname, sizeof(devname), "%s:%d", pdevname,
>> part);
>> efi_disk_add_dev(devname, if_typename, desc, diskid,
>> - info.start);
>> + info.start, part);
>> part++;
>> disks++;
>> }
>> @@ -290,9 +310,22 @@ int efi_disk_register(void)
>> uclass_next_device(&dev)) {
>> struct blk_desc *desc = dev_get_uclass_platdata(dev);
>> const char *if_typename = dev->driver->name;
>> + disk_partition_t info;
>> + int part = 1;
>>
>> printf("Scanning disk %s...\n", dev->name);
>> - efi_disk_add_dev(dev->name, if_typename, desc, desc->devnum, 0);
>> +
>> +#ifdef CONFIG_DM
>> + /* add block device: */
>> + efi_disk_add_dev(dev->name, if_typename, desc,
>> + desc->devnum, 0, 0);
>> +#endif
>> + /* ... and devices for each partition: */
>> + while (!part_get_info(desc, part, &info)) {
>> + efi_disk_add_dev(dev->name, if_typename, desc,
>> + desc->devnum, 0, part);
>> + part++;
>> + }
>> disks++;
>>
>> /*
>>
>