[U-Boot] [PATCH 2/3] efi_loader: correctly setup device paths for block devices

[xypron.glpk at gmx.de <xypron.glpk@gmx.de>]

On Sunday, December 3, 2017 8:24:39 PM CET Mark Kettenis wrote:
> Heinrich Schuchardt schreef op 2017-12-02 13:42:
> > For each block device we have to setup:
> > - a device path for the block device
> > - a partition device path for the block device with
> > 
> >   partition number 0
> 
> Do x86 UEFI implementations actually generate these partition number 0
> paths?

Thanks for reviewing.

I do not have access the an x86 computer with UEFI firmware.
Maybe somebody else on the list has.

Another reference point is EDK2.

Part of the relevant coding is in function PartitionInstallMbrChildHandles(),
MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c:47.

Another part is in PartitionInstallChildHandle(),
MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c.

I could not identify any line referring to partition 0.

> The standard says that 0 can be used in tis fashion but doesn't seem to
> prescribe their use.

<cite>A partition number of  zero can be used to represent the raw hard drive 
or a raw extended partition.</cite>

Do you think we should not generate this entry?

Regards

Heinrich

> 
> > - a partition device path for each partition
> > 
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> > 
> >  lib/efi_loader/efi_device_path.c | 11 +++++++----
> >  lib/efi_loader/efi_disk.c        |  7 +++++--
> >  2 files changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_device_path.c
> > b/lib/efi_loader/efi_device_path.c
> > index 42fe6e1185..d0d62ff428 100644
> > --- a/lib/efi_loader/efi_device_path.c
> > +++ b/lib/efi_loader/efi_device_path.c
> > @@ -412,15 +412,18 @@ static void *dp_part_fill(void *buf, struct
> > blk_desc *desc, int part)
> > 
> >  	buf = &udp[1];
> >  
> >  #endif
> > 
> > -	if (part == 0) /* the actual disk, not a partition */
> > +	if (part == -1) /* the actual disk, not a partition */
> > 
> >  		return buf;
> > 
> > -	part_get_info(desc, part, &info);
> > +	if (part == 0)
> > +		part_get_info_whole_disk(desc, &info);
> > +	else
> > +		part_get_info(desc, part, &info);
> > 
> >  	if (desc->part_type == PART_TYPE_ISO) {
> >  	
> >  		struct efi_device_path_cdrom_path *cddp = buf;
> > 
> > -		cddp->boot_entry = part - 1;
> > +		cddp->boot_entry = part;
> > 
> >  		cddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> >  		cddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CDROM_PATH;
> >  		cddp->dp.length = sizeof(*cddp);
> > 
> > @@ -434,7 +437,7 @@ static void *dp_part_fill(void *buf, struct
> > blk_desc *desc, int part)
> > 
> >  		hddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> >  		hddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH;
> >  		hddp->dp.length = sizeof(*hddp);
> > 
> > -		hddp->partition_number = part - 1;
> > +		hddp->partition_number = part;
> > 
> >  		hddp->partition_start = info.start;
> >  		hddp->partition_end = info.size;
> >  		if (desc->part_type == PART_TYPE_EFI)
> > 
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 4e457a841b..19f75aa919 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -274,6 +274,9 @@ static int efi_disk_create_partitions(struct
> > blk_desc *desc,
> > 
> >  	disk_partition_t info;
> >  	int part;
> > 
> > +	/* Add devices for disk */
> > +	snprintf(devname, sizeof(devname), "%s", pdevname);
> > +	efi_disk_add_dev(devname, if_typename, desc, diskid, info.start, 0);
> > 
> >  	/* Add devices for each partition */
> >  	for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> >  	
> >  		if (part_get_info(desc, part, &info))
> > 
> > @@ -315,7 +318,7 @@ int efi_disk_register(void)
> > 
> >  		/* Add block device for the full device */
> >  		efi_disk_add_dev(dev->name, if_typename, desc,
> > 
> > -				 desc->devnum, 0, 0);
> > +				 desc->devnum, 0, -1);
> > 
> >  		disks++;
> > 
> > @@ -351,7 +354,7 @@ int efi_disk_register(void)
> > 
> >  				 if_typename, i);
> >  			
> >  			/* Add block device for the full device */
> > 
> > -			efi_disk_add_dev(devname, if_typename, desc, i, 0, 0);
> > +			efi_disk_add_dev(devname, if_typename, desc, i, 0, -1);
> > 
> >  			disks++;
> >  			
> >  			/* Partitions show up as block devices in EFI */