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 <[email protected]> > > > Signed-off-by: Alexander Graf <[email protected]> > > > > 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 [email protected]... > > efi_disk_register BLK efi_disk_add_dev [email protected], > > if_typename=mmc_blk, dev_index=0 offset=0, part=0 > > efi_disk_create_partitions efi_disk_add_dev [email protected]:1, > > if_typename=mmc_blk, dev_index=0 offset=8192, part=1 > > efi_disk_create_partitions efi_disk_add_dev [email protected]: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 [email protected]... > > efi_disk_register BLK efi_disk_add_dev [email protected], > > if_typename=mmc_blk, dev_index=0 offset=0, part=1 > > efi_disk_register BLK efi_disk_add_dev [email protected], > > if_typename=mmc_blk, dev_index=0 offset=0, part=4 > > efi_disk_register BLK efi_disk_add_dev [email protected], > > 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 > > > [email protected] > > > https://lists.denx.de/listinfo/u-boot > > > > _______________________________________________ > U-Boot mailing list > [email protected] > https://lists.denx.de/listinfo/u-boot _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
