[U-Boot] [PATCH] efi_loader: Fix disk dp's for pre-DM/legacy devices
Peter Robinson
pbrobinson at gmail.com
Mon Oct 9 18:36:24 UTC 2017
On Mon, Oct 9, 2017 at 7:20 PM, Rob Clark <robdclark at gmail.com> wrote:
> On Mon, Oct 9, 2017 at 1:48 PM, Jonathan Gray <jsg at jsg.id.au> wrote:
>> On Mon, Oct 09, 2017 at 01:06:26PM -0400, Rob Clark wrote:
>>> On Mon, Oct 9, 2017 at 12:41 PM, Jonathan Gray <jsg at jsg.id.au> wrote:
>>> > On Mon, Oct 09, 2017 at 12:06:24PM -0400, Rob Clark wrote:
>>> >> On Mon, Oct 9, 2017 at 11:35 AM, Jonathan Gray <jsg at jsg.id.au> wrote:
>>> >> > On Mon, Oct 09, 2017 at 10:17:18AM -0400, Rob Clark wrote:
>>> >> >> On Mon, Oct 9, 2017 at 8:25 AM, Jonathan Gray <jsg at jsg.id.au> wrote:
>>> >> >> > On Mon, Oct 09, 2017 at 06:52:18AM -0400, Rob Clark wrote:
>>> >> >> >> On Sun, Oct 8, 2017 at 11:33 PM, Jonathan Gray <jsg at jsg.id.au> wrote:
>>> >> >> >> > On Sun, Oct 08, 2017 at 11:33:08AM -0400, Rob Clark wrote:
>>> >> >> >> >> This fixes an issue with OpenBSD's bootloader, and I think should also
>>> >> >> >> >> fix a similar issue with grub2 on legacy devices. In the legacy case
>>> >> >> >> >> we were creating disk objects for the partitions, but not also the
>>> >> >> >> >> parent device.
>>> >> >> >> >>
>>> >> >> >> >> Reported-by: Jonathan Gray <jsg at jsg.id.au>
>>> >> >> >> >> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>> >> >> >> >
>>> >> >> >> > Thanks for looking into this. While this lets armv7/bootarm.efi
>>> >> >> >> > boot again on cubox-i and bbb it doesn't help rpi3.
>>> >> >> >> >
>>> >> >> >> > What is the easiest way to get U-Boot to display these paths
>>> >> >> >> > to be able to compare the current behaviour to 2017.09?
>>> >> >> >> >
>>> >> >> >>
>>> >> >> >> in grub, there is the lsefi command, not sure if the OpenBSD
>>> >> >> >> bootloader has something similar?
>>> >> >> >>
>>> >> >> >> u-boot implements that device-path to text protocol, so it should be
>>> >> >> >> pretty easy to implement something like this if not.
>>> >> >> >>
>>> >> >> >> BR,
>>> >> >> >> -R
>>> >> >> >
>>> >> >> > With git + your patch a node with type 4 (DEVICE_PATH_TYPE_MEDIA_DEVICE)
>>> >> >> > is no longer seen. Is it possible having U-Boot on mmc but directing
>>> >> >> > it to load off usb is somehow involved here?
>>> >> >>
>>> >> >> There is no requirement that efi payload and u-boot are on the same
>>> >> >> device. The distro bootcmd stuff will just look for
>>> >> >> /efi/boot/bootxyz.efi on all the devices/partitions until it finds
>>> >> >> one.
>>> >> >>
>>> >> >> The dp for the partition device should end in MEDIA_DEVICE:HARD_DRIVE
>>> >> >> or MEDIA_DEVICE:CDROM. What comes before that differs depending on DM
>>> >> >> or legacy boards, in the former case we can construct something more
>>> >> >> realistic. Although the bootloader shouldn't really care.
>>> >> >
>>> >> > I only see MEDIA_DEVICE with U-Boot 2017.09. The latest code
>>> >> > in master only gives types of 1 (Hardware Device Path) and
>>> >> > 3 (Messaging Device Path).
>>> >> >
>>> >> > It is DM in this case:
>>> >>
>>> >> Possibly something weird with your partition table? In
>>> >> efi_disk_register() it should create objects for the disk itself
>>> >> (part==0, no MEDIA_DEVICE nodes in the dp), as well as for each
>>> >> partition (part>=1, which would have same dp as the disk but
>>> >> additional MEDIA_DEVICE:HARD_DRIVE or MEDIA_DEVICE:CDROM node).
>>> >>
>>> >> If part_get_info() fails for part==1 then you won't get any of the
>>> >> partition devices. I suppose this could happen if you an unknown
>>> >> partition table type, or u-boot is not built w/ the correct partition
>>> >> driver.
>>> >>
>>> >> BR,
>>> >> -R
>>> >
>>> > It is partitioned mbr style.
>>> >
>>> > U-Boot> part list mmc 0
>>> >
>>> > Partition Map for MMC device 0 -- Partition Type: DOS
>>> >
>>> > Part Start Sector Num Sectors UUID Type
>>> > 1 8192 8192 00000000-01 0c Boot
>>> > 4 16384 26624 00000000-04 a6
>>> > U-Boot> part list usb 0
>>>
>>> perhaps jumping from partition #1 to #4 is what is confusing things
>>> here? It's possible you end up with a partition "diskobj" for
>>> partition #1 but not #4?
>>>
>>> Try adding a print in efi_disk_register() and see how many times we go
>>> thru the while(!part_get_info()) loop.
>>
>> Indeed, it seems to only end up calling efi_disk_add_dev() for
>> part 1 in that loop:
>>
>> ## Starting EFI application at 01000000 ...
>> Scanning disk sdhci at 7e300000.blk...
>> ## Valid DOS partition found ##
>> efi_disk_register calling efi_disk_add_dev for part 1
>> ## Valid DOS partition found ##
>> Scanning disk usb_mass_storage.lun0...
>> ## Valid DOS partition found ##
>> efi_disk_register calling efi_disk_add_dev for part 1
>> ## Valid DOS partition found ##
>> Found 2 disks
>>
>> If I change the code there to match other callers of part_get_info()
>> I get a MEDIA_DEVICE type and can boot.
>
> Ok, this makes sense now. There is one more loop to fix in the
> non-DM/BLK case (well, the one added by this patch, which I think
> agraf has already applied).
I think that also explains why I've seen issues on the Pine64 as we
currently have a extended partition.
> Care to send a patch?
>
> BR,
> -R
>
>> ## Starting EFI application at 01000000 ...
>> Scanning disk sdhci at 7e300000.blk...
>> ## Valid DOS partition found ##
>> efi_disk_register calling efi_disk_add_dev for part 1
>> ## Valid DOS partition found ##
>> ## Valid DOS partition found ##
>> efi_disk_register calling efi_disk_add_dev for part 4
>> ## Valid DOS partition found ##
>> Scanning disk usb_mass_storage.lun0...
>> ## Valid DOS partition found ##
>> efi_disk_register calling efi_disk_add_dev for part 1
>> ## Valid DOS partition found ##
>> ## Valid DOS partition found ##
>> efi_disk_register calling efi_disk_add_dev for part 4
>> ## Valid DOS partition found ##
>> Found 2 disks
>> efi_diskprobe
>> efi_device_path_depth looking for type 4 i=0 type 1
>> efi_device_path_depth looking for type 4 i=1 type 3
>> efi_device_path_depth looking for type 4 i=2 type 3
>> efi_device_path_depth looking for type 4 i=3 type 3
>> efi_device_path_depth looking for type 4 i=4 type 4
>> depth=4
>> i=0
>>
>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> index eb9ce772d1..d34a5b3e5e 100644
>> --- a/lib/efi_loader/efi_disk.c
>> +++ b/lib/efi_loader/efi_disk.c
>> @@ -254,18 +254,20 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
>> #if CONFIG_IS_ENABLED(ISO_PARTITION)
>> char devname[32] = { 0 }; /* dp->str is u16[32] long */
>> disk_partition_t info;
>> - int part = 1;
>> + int part, ret;
>>
>> if (desc->part_type != PART_TYPE_ISO)
>> return 0;
>>
>> /* and devices for each partition: */
>> - while (!part_get_info(desc, part, &info)) {
>> + for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
>> + ret = part_get_info(desc, part, &info);
>> + if (ret)
>> + continue;
>> snprintf(devname, sizeof(devname), "%s:%d", pdevname,
>> part);
>> efi_disk_add_dev(devname, if_typename, desc, diskid,
>> info.start, part);
>> - part++;
>> disks++;
>> }
>>
>> @@ -299,15 +301,18 @@ int efi_disk_register(void)
>> struct blk_desc *desc = dev_get_uclass_platdata(dev);
>> const char *if_typename = dev->driver->name;
>> disk_partition_t info;
>> - int part = 1;
>> + int part, ret;
>>
>> printf("Scanning disk %s...\n", dev->name);
>>
>> /* add devices for each partition: */
>> - while (!part_get_info(desc, part, &info)) {
>> + for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
>> + ret = part_get_info(desc, part, &info);
>> + if (ret)
>> + continue;
>> +printf("%s calling efi_disk_add_dev for part %d\n", __func__, part);
>> efi_disk_add_dev(dev->name, if_typename, desc,
>> desc->devnum, 0, part);
>> - part++;
>> }
>>
>> /* ... and add block device: */
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list