[U-Boot] [PATCH] efi_loader: Fix partition offsets
Jonathan Gray
jsg at jsg.id.au
Fri Dec 8 07:27:04 UTC 2017
On Fri, Dec 08, 2017 at 08:09:46AM +0100, Heinrich Schuchardt wrote:
> On 12/07/2017 08:00 AM, Jonathan Gray wrote:
> > On Fri, Dec 01, 2017 at 04:10:33PM +0100, Alexander Graf wrote:
> > > Commit 884bcf6f65 (efi_loader: use proper device-paths for partitions) tried
> > > to introduce the el torito scheme to all partition table types: Spawn
> > > individual disk objects for each partition on a disk.
> > >
> > > Unfortunately, that code ended up creating partitions with offset=0 which meant
> > > that anyone accessing these objects gets data from the raw block device instead
> > > of the partition.
>
> Hello Jonathan,
>
> according to the UEFI standard the whole disk may be represented by as
> partition number 0 (with offset 0).
You are quoting Alexander there not me.
>
> UEFI spec 2.7:
> 10.3.6.1 Hard Drive
> A partition number of zero can be used to represent the raw hard drive or a
> raw extended partition.
>
> Do you have access to the source of the BSD loader that you are calling?
> Please, check that it complies to this aspect of the UEFI API.
Already mentioned in this thread, but again:
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/arm64/stand/efiboot/
Which works fine with the EDK2 based firmware on the overdrive 1000.
And has found various regressions in changes made to the efi loader in
U-Boot over the last few months.
>
> Best regards
>
> Heinrich
>
>
> > >
> > > Furthermore, all the el torito logic to spawn devices for partitions was
> > > duplicated. So let's merge the two code paths and give partition disk objects
> > > good offsets to work from, so that payloads can actually make use of them.
> > >
> > > Fixes: 884bcf6f65 (efi_loader: use proper device-paths for partitions)
> > > Reported-by: Yousaf Kaukab <yousaf.kaukab at suse.com>
> > > Signed-off-by: Alexander Graf <agraf at suse.de>
> >
> > This once again broke being able to find a DEVICE_PATH_TYPE_MEDIA_DEVICE
> > node with the loaded image protocol on rpi_3 with mmc/usb.
> >
> > broken
> > ## Starting EFI application at 01000000 ...
> > Scanning disk sdhci at 7e300000.blk...
> > efi_disk_register BLK efi_disk_add_dev name=sdhci at 7e300000.blk, if_typename=mmc_blk, dev_index=0 offset=0, part=0
> > efi_disk_create_partitions efi_disk_add_dev name=sdhci at 7e300000.blk:1, if_typename=mmc_blk, dev_index=0 offset=8192, part=1
> > efi_disk_create_partitions efi_disk_add_dev name=sdhci at 7e300000.blk:4, if_typename=mmc_blk, dev_index=0 offset=16384, part=4
> > Scanning disk usb_mass_storage.lun0...
> > efi_disk_register BLK efi_disk_add_dev name=usb_mass_storage.lun0, if_typename=usb_storage_blk, dev_index=0 offset=0, part=0
> > efi_disk_create_partitions efi_disk_add_dev name=usb_mass_storage.lun0:1, if_typename=usb_storage_blk, dev_index=0 offset=8192, part=1
> > efi_disk_create_partitions efi_disk_add_dev name=usb_mass_storage.lun0:4, if_typename=usb_storage_blk, dev_index=0 offset=40960, part=4
> > Found 6 disks
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 11 type 3 b: len 11 type: 3
> > efi_dp_match a: len 11 type 3 b: len 11 type: 3
> > efi_dp_match a: len 11 type 3 b: len 11 type: 3
> > 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 no match for type 4
> > depth=-1
> >
> > working (this commit reverted)
> > ## Starting EFI application at 01000000 ...
> > Scanning disk sdhci at 7e300000.blk...
> > efi_disk_register BLK efi_disk_add_dev name=sdhci at 7e300000.blk, if_typename=mmc_blk, dev_index=0 offset=0, part=1
> > efi_disk_register BLK efi_disk_add_dev name=sdhci at 7e300000.blk, if_typename=mmc_blk, dev_index=0 offset=0, part=4
> > efi_disk_register BLK efi_disk_add_dev name=sdhci at 7e300000.blk, if_typename=mmc_blk, dev_index=0 offset=0, part=0
> > Scanning disk usb_mass_storage.lun0...
> > efi_disk_register BLK efi_disk_add_dev name=usb_mass_storage.lun0, if_typename=usb_storage_blk, dev_index=0 offset=0, part=1
> > efi_disk_register BLK efi_disk_add_dev name=usb_mass_storage.lun0, if_typename=usb_storage_blk, dev_index=0 offset=0, part=4
> > efi_disk_register BLK efi_disk_add_dev name=usb_mass_storage.lun0, if_typename=usb_storage_blk, dev_index=0 offset=0, part=0
> > Found 2 disks
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 20 type 1 b: len 20 type: 1
> > efi_dp_match a: len 11 type 3 b: len 11 type: 3
> > efi_dp_match a: len 11 type 3 b: len 11 type: 3
> > efi_dp_match a: len 11 type 3 b: len 11 type: 3
> > efi_dp_match a: len 42 type 4 b: len 42 type: 4
> > 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
> >
> > > ---
> > > lib/efi_loader/efi_disk.c | 60 ++++++++++-------------------------------------
> > > 1 file changed, 13 insertions(+), 47 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > > index 68ba2cf7b2..4e457a841b 100644
> > > --- a/lib/efi_loader/efi_disk.c
> > > +++ b/lib/efi_loader/efi_disk.c
> > > @@ -264,21 +264,17 @@ out_of_memory:
> > > printf("ERROR: Out of memory\n");
> > > }
> > > -static int efi_disk_create_eltorito(struct blk_desc *desc,
> > > - const char *if_typename,
> > > - int diskid,
> > > - const char *pdevname)
> > > +static int efi_disk_create_partitions(struct blk_desc *desc,
> > > + const char *if_typename,
> > > + int diskid,
> > > + const char *pdevname)
> > > {
> > > int disks = 0;
> > > -#if CONFIG_IS_ENABLED(ISO_PARTITION)
> > > char devname[32] = { 0 }; /* dp->str is u16[32] long */
> > > disk_partition_t info;
> > > int part;
> > > - if (desc->part_type != PART_TYPE_ISO)
> > > - return 0;
> > > -
> > > - /* and devices for each partition: */
> > > + /* Add devices for each partition */
> > > for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> > > if (part_get_info(desc, part, &info))
> > > continue;
> > > @@ -289,10 +285,6 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
> > > disks++;
> > > }
> > > - /* ... and add block device: */
> > > - efi_disk_add_dev(devname, if_typename, desc, diskid, 0, 0);
> > > -#endif
> > > -
> > > return disks;
> > > }
> > > @@ -318,31 +310,18 @@ int efi_disk_register(void)
> > > uclass_next_device_check(&dev)) {
> > > struct blk_desc *desc = dev_get_uclass_platdata(dev);
> > > const char *if_typename = dev->driver->name;
> > > - disk_partition_t info;
> > > - int part;
> > > printf("Scanning disk %s...\n", dev->name);
> > > - /* add devices for each partition: */
> > > - for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> > > - if (part_get_info(desc, part, &info))
> > > - continue;
> > > - efi_disk_add_dev(dev->name, if_typename, desc,
> > > - desc->devnum, 0, part);
> > > - }
> > > -
> > > - /* ... and add block device: */
> > > + /* Add block device for the full device */
> > > efi_disk_add_dev(dev->name, if_typename, desc,
> > > desc->devnum, 0, 0);
> > > disks++;
> > > - /*
> > > - * El Torito images show up as block devices in an EFI world,
> > > - * so let's create them here
> > > - */
> > > - disks += efi_disk_create_eltorito(desc, if_typename,
> > > - desc->devnum, dev->name);
> > > + /* Partitions show up as block devices in EFI */
> > > + disks += efi_disk_create_partitions(desc, if_typename,
> > > + desc->devnum, dev->name);
> > > }
> > > #else
> > > int i, if_type;
> > > @@ -361,8 +340,6 @@ int efi_disk_register(void)
> > > for (i = 0; i < 4; i++) {
> > > struct blk_desc *desc;
> > > char devname[32] = { 0 }; /* dp->str is u16[32] long */
> > > - disk_partition_t info;
> > > - int part;
> > > desc = blk_get_devnum_by_type(if_type, i);
> > > if (!desc)
> > > @@ -373,24 +350,13 @@ int efi_disk_register(void)
> > > snprintf(devname, sizeof(devname), "%s%d",
> > > if_typename, i);
> > > - /* add devices for each partition: */
> > > - for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> > > - if (part_get_info(desc, part, &info))
> > > - continue;
> > > - efi_disk_add_dev(devname, if_typename, desc,
> > > - i, 0, part);
> > > - }
> > > -
> > > - /* ... and add block device: */
> > > + /* Add block device for the full device */
> > > efi_disk_add_dev(devname, if_typename, desc, i, 0, 0);
> > > disks++;
> > > - /*
> > > - * El Torito images show up as block devices
> > > - * in an EFI world, so let's create them here
> > > - */
> > > - disks += efi_disk_create_eltorito(desc, if_typename,
> > > - i, devname);
> > > + /* Partitions show up as block devices in EFI */
> > > + disks += efi_disk_create_partitions(desc, if_typename,
> > > + i, devname);
> > > }
> > > }
> > > #endif
> > > --
> > > 2.12.3
> > >
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot
> >
>
> _______________________________________________
> 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