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

[Alexander Graf]

On 03.12.17 22:20, xypron.glpk at gmx.de wrote:
> 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?

Given that basically all major UEFI implementations are based on edk2, I
think we should just do it exactly the same way :)

  https://wiki.linaro.org/LEG/UEFIforQEMU

To give you example grub lsefi output for a simple disk with 2 MBR
partitions:

== U-Boot w/o this patch set (SD) ==

  /HardwareVendor(e61d73b9-a384-4acc-aeab-82e828f3628b)[0:
]/USB(6,0)/EndEntire
  block
  device path
Handle 0x9fef8f78
  /HardwareVendor(e61d73b9-a384-4acc-aeab-82e828f3628b)[0:
]/USB(6,0)/HD(0,800,32000,2de808cb00000000,1,1)/EndEntire
  block
  device path
  simple FS
Handle 0x9fef90a0
  /HardwareVendor(e61d73b9-a384-4acc-aeab-82e828f3628b)[0:
]/USB(6,0)/HD(1,32800,1cd800,2de808cb00000000,1,1)/EndEntire
  block
  device path
  simple FS

== U-Boot w/ this patch set (SD) ==

Handle 0x9fef7598
  /HardwareVendor(e61d73b9-a384-4acc-aeab-82e828f3628b)[0:
]/USB(6,0)/EndEntire
  block
  device path
  simple FS
Handle 0x9fef76c0
  /HardwareVendor(e61d73b9-a384-4acc-aeab-82e828f3628b)[0:
]/USB(6,0)/HD(0,0,200000,2de808cb00000000,1,1)/EndEntire
  block
  device path
Handle 0x9fef7020
  /HardwareVendor(e61d73b9-a384-4acc-aeab-82e828f3628b)[0:
]/USB(6,0)/HD(1,800,32000,2de808cb00000000,1,1)/EndEntire
  block
  device path
  simple FS
Handle 0x9fef7228
  /HardwareVendor(e61d73b9-a384-4acc-aeab-82e828f3628b)[0:
]/USB(6,0)/HD(2,32800,1cd800,2de808cb00000000,1,1)/EndEntire
  block
  device path
  simple FS

== edk2 (virtio) ==

Handle 0x43250d10
  /ACPI(a0341d0,0)/PCI(0,1)/EndEntire
  disk
  block
  fa920010-6785-4941-b6ec-498c579f160a
  PCI
  device path
Handle 0x4223af90
  /ACPI(a0341d0,0)/PCI(0,1)/HD(1,800,32000,2de808cb00000000,1,1)/EndEntire
  simple FS
  disk
  c12a7328-f81f-11d2-ba4b-00a0c93ec93b
  8cf2f62c-bc9b-4821-808d-ec9ec421a1a0
  block
  device path
Handle 0x4223ac90

/ACPI(a0341d0,0)/PCI(0,1)/HD(2,32800,1cd800,2de808cb00000000,1,1)/EndEntire
  simple FS
  disk
  8cf2f62c-bc9b-4821-808d-ec9ec421a1a0
  block
  device path


=====

So I think the only patch we really need is something like this:

diff --git a/lib/efi_loader/efi_device_path.c
b/lib/efi_loader/efi_device_path.c
index b4e2f933cb..9ba6b04e5e 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -416,7 +416,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)

But this can wait until after rc1, so I'll send the pull request as is
out now :).


Alex