[U-Boot] [PATCH] efi_loader: Fix disk dp's for pre-DM/legacy devices

Jonathan Gray jsg at jsg.id.au
Mon Oct 9 17:48:01 UTC 2017


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.

## 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: */


More information about the U-Boot mailing list