[U-Boot] [PATCH] efi_loader: Fix partition offsets

Heinrich Schuchardt xypron.glpk at gmx.de
Sat Dec 2 04:53:10 UTC 2017



On 12/01/2017 04:10 PM, 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.
> 
> 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>
> ---
>   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++) {

include/configs/highbank.h allows 5 SCSI drives.
include/configs/qemu-arm.h allows 6 SCSI drives.

I guess you want to use drv->max_devs as upper limit.

>   			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
> 

I tested on an Odroid C2 with four partitions on an SD card (device 
mmc0). Device mmc1 is not installed but defined in the dtb.

Bootefi selftest (with some debug output added) showed:

=> bootefi selftest 

lib/efi_loader/efi_disk.c (311) efi_disk_register, dev = 
000000007df72600 

Scanning disk mmc at 72000.blk...
lib/efi_loader/efi_disk.c (320) efi_disk_register, disks = 0
lib/efi_loader/efi_disk.c (323) efi_disk_register, disks = 1
lib/efi_loader/efi_disk.c (287) efi_disk_create_partitions, disks = 4
lib/efi_loader/efi_disk.c (328) efi_disk_register, disks = 5

Up to this point everything is ok.
We created one device of the complete disk and four devices for the 
partions.
Now it starts going wrong:

Card did not respond to voltage select! 

mmc_init: -95, time 9
lib/efi_loader/efi_disk.c (311) efi_disk_register, dev = 
000000007df72930 

Scanning disk mmc at 74000.blk... 

lib/efi_loader/efi_disk.c (320) efi_disk_register, disks = 5 

lib/efi_loader/efi_disk.c (323) efi_disk_register, disks = 6 

lib/efi_loader/efi_disk.c (287) efi_disk_create_partitions, disks = 0 

lib/efi_loader/efi_disk.c (328) efi_disk_register, disks = 6 

Found 6 disks

So here we are creating a disk for an MMC device that does not 
physically exist. Can't we check the existence? I wonder why 
uclass_next_device_check does not skip the drive.

Only the following MMC related device paths are created:

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/MMC(Slot0)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/MMC(Slot0)/HD(Part0,Sig6fe30000)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/MMC(Slot0)/HD(Part1,Sig6fe30000)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/MMC(Slot0)/HD(Part2,Sig6fe30000)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/MMC(Slot0)/HD(Part3,Sig6fe30000)

The device paths with partition number 0 for the disk devices are 
missing and the partitions incorrectly numbered 0 to 3 instead of 1 to 4.

But I guess this problem is in another part of the code.

Best regards

Heinrich


More information about the U-Boot mailing list