Re: [U-Boot] [PATCH v1 08/15] efi_loader: use proper device-paths for partitions

[Alexander Graf]

> Am 12.08.2017 um 16:31 schrieb Rob Clark :
> 
>> On Sat, Aug 12, 2017 at 8:36 AM, Alexander Graf  wrote:
>> 
>> 
>>> On 10.08.17 20:29, Rob Clark wrote:
>>> 
>>> Also, create disk objects for the disk itself, in addition to the
>>> partitions.  (UEFI terminology is a bit confusing, a "disk" object is
>>> really a partition.)  This helps grub properly identify the boot device
>>> since it is trying to match up partition "disk" object with it's parent
>>> device.
>>> 
>>> Now instead of seeing devices like:
>>> 
>>>   /File(sd...@07864000.blk)/EndEntire
>>>   /File(usb_mass_storage.lun0)/EndEntire
>>> 
>>> You see:
>>> 
>>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>>> 
>>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c,1,1)/EndEntire
>>> 
>>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
>>> 
>>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,1,1)/EndEntire
>>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>>> 
>>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,6,38ca6802,1,1)/EndEntire
>>> 
>>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
>>> 
>>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
>>> 
>>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,1,1)/EndEntire
>>> 
>>> This is on a board with single USB disk and single sd-card.  The
>>> UnknownMessaging(1d) node in the device-path is the MMC device,
>>> but grub_efi_print_device_path() hasn't been updated yet for some
>>> of the newer device-path sub-types.
>>> 
>>> This patch is inspired by a patch originally from Peter Jones, but
>>> re-worked to use efi_device_path, so it doesn't much resemble the
>>> original.
>>> 
>>> Signed-off-by: Rob Clark 
>>> ---
>>>  lib/efi_loader/efi_disk.c | 54
>>> +++
>>>  1 file changed, 31 insertions(+), 23 deletions(-)
>>> 
>>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>>> index ed06485e33..eea65a402a 100644
>>> --- a/lib/efi_loader/efi_disk.c
>>> +++ b/lib/efi_loader/efi_disk.c
>>> @@ -28,11 +28,13 @@ struct efi_disk_obj {
>>>/* EFI Interface Media descriptor struct, referenced by ops */
>>>struct efi_block_io_media media;
>>>/* EFI device path to this block device */
>>> -   struct efi_device_path_file_path *dp;
>>> +   struct efi_device_path *dp;
>>> +   /* partition # */
>>> +   unsigned part;
>>>/* Offset into disk for simple partitions */
>>>lbaint_t offset;
>>>/* Internal block device */
>>> -   const struct blk_desc *desc;
>>> +   struct blk_desc *desc;
>>>  };
>>>static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,
>>> @@ -172,26 +174,26 @@ static const struct efi_block_io
>>> block_io_disk_template = {
>>>static void efi_disk_add_dev(const char *name,
>>> const char *if_typename,
>>> -const struct blk_desc *desc,
>>> +struct blk_desc *desc,
>>> int dev_index,
>>> -lbaint_t offset)
>>> +lbaint_t offset,
>>> +unsigned part)
>>>  {
>>>struct efi_disk_obj *diskobj;
>>> -   struct efi_device_path_file_path *dp;
>>> -   int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
>>>/* Don't add empty devices */
>>>if (!desc->lba)
>>>return;
>>>  - diskobj = calloc(1, objlen);
>>> +   diskobj = calloc(1, sizeof(*diskobj));
>>>/* Fill in object data */
>>> -   dp = (void *)[1];
>>> +   diskobj->dp = efi_dp_from_part(desc, part);
>>> +   diskobj->part = part;
>>>diskobj->parent.protocols[0].guid = _block_io_guid;
>>>diskobj->parent.protocols[0].protocol_interface = >ops;
>>>diskobj->parent.protocols[1].guid = _guid_device_path;
>>> -   diskobj->parent.protocols[1].protocol_interface = dp;
>>> +   diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
>>>diskobj->parent.handle = diskobj;
>>>diskobj->ops = block_io_disk_template;
>>>diskobj->ifname = if_typename;
>>> @@ -207,17 +209,6 @@ static void efi_disk_add_dev(const char *name,
>>>diskobj->media.last_block = desc->lba - offset;
>>>diskobj->ops.media = >media;
>>>  - /* Fill in device path */
>>> -   diskobj->dp = dp;
>>> -   dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>>> -   dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>>> -   dp[0].dp.length = sizeof(*dp);
>>> -   ascii2unicode(dp[0].str, name);
>>> -
>>> -   dp[1].dp.type = DEVICE_PATH_TYPE_END;
>>> -   dp[1].dp.sub_type = 

Re: [U-Boot] [PATCH v1 08/15] efi_loader: use proper device-paths for partitions

[Rob Clark]

On Sat, Aug 12, 2017 at 8:36 AM, Alexander Graf  wrote:
>
>
> On 10.08.17 20:29, Rob Clark wrote:
>>
>> Also, create disk objects for the disk itself, in addition to the
>> partitions.  (UEFI terminology is a bit confusing, a "disk" object is
>> really a partition.)  This helps grub properly identify the boot device
>> since it is trying to match up partition "disk" object with it's parent
>> device.
>>
>> Now instead of seeing devices like:
>>
>>/File(sd...@07864000.blk)/EndEntire
>>/File(usb_mass_storage.lun0)/EndEntire
>>
>> You see:
>>
>>/ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>>
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c,1,1)/EndEntire
>>
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
>>
>> /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,1,1)/EndEntire
>>/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>>
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,6,38ca6802,1,1)/EndEntire
>>
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
>>
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
>>
>> /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,1,1)/EndEntire
>>
>> This is on a board with single USB disk and single sd-card.  The
>> UnknownMessaging(1d) node in the device-path is the MMC device,
>> but grub_efi_print_device_path() hasn't been updated yet for some
>> of the newer device-path sub-types.
>>
>> This patch is inspired by a patch originally from Peter Jones, but
>> re-worked to use efi_device_path, so it doesn't much resemble the
>> original.
>>
>> Signed-off-by: Rob Clark 
>> ---
>>   lib/efi_loader/efi_disk.c | 54
>> +++
>>   1 file changed, 31 insertions(+), 23 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> index ed06485e33..eea65a402a 100644
>> --- a/lib/efi_loader/efi_disk.c
>> +++ b/lib/efi_loader/efi_disk.c
>> @@ -28,11 +28,13 @@ struct efi_disk_obj {
>> /* EFI Interface Media descriptor struct, referenced by ops */
>> struct efi_block_io_media media;
>> /* EFI device path to this block device */
>> -   struct efi_device_path_file_path *dp;
>> +   struct efi_device_path *dp;
>> +   /* partition # */
>> +   unsigned part;
>> /* Offset into disk for simple partitions */
>> lbaint_t offset;
>> /* Internal block device */
>> -   const struct blk_desc *desc;
>> +   struct blk_desc *desc;
>>   };
>> static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,
>> @@ -172,26 +174,26 @@ static const struct efi_block_io
>> block_io_disk_template = {
>> static void efi_disk_add_dev(const char *name,
>>  const char *if_typename,
>> -const struct blk_desc *desc,
>> +struct blk_desc *desc,
>>  int dev_index,
>> -lbaint_t offset)
>> +lbaint_t offset,
>> +unsigned part)
>>   {
>> struct efi_disk_obj *diskobj;
>> -   struct efi_device_path_file_path *dp;
>> -   int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
>> /* Don't add empty devices */
>> if (!desc->lba)
>> return;
>>   - diskobj = calloc(1, objlen);
>> +   diskobj = calloc(1, sizeof(*diskobj));
>> /* Fill in object data */
>> -   dp = (void *)[1];
>> +   diskobj->dp = efi_dp_from_part(desc, part);
>> +   diskobj->part = part;
>> diskobj->parent.protocols[0].guid = _block_io_guid;
>> diskobj->parent.protocols[0].protocol_interface = >ops;
>> diskobj->parent.protocols[1].guid = _guid_device_path;
>> -   diskobj->parent.protocols[1].protocol_interface = dp;
>> +   diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
>> diskobj->parent.handle = diskobj;
>> diskobj->ops = block_io_disk_template;
>> diskobj->ifname = if_typename;
>> @@ -207,17 +209,6 @@ static void efi_disk_add_dev(const char *name,
>> diskobj->media.last_block = desc->lba - offset;
>> diskobj->ops.media = >media;
>>   - /* Fill in device path */
>> -   diskobj->dp = dp;
>> -   dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>> -   dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>> -   dp[0].dp.length = sizeof(*dp);
>> -   ascii2unicode(dp[0].str, name);
>> -
>> -   dp[1].dp.type = DEVICE_PATH_TYPE_END;
>> -   dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
>> -   dp[1].dp.length = sizeof(*dp);
>> -
>> /* Hook up to the device list */
>> list_add_tail(>parent.link, _obj_list);
>>   }
>> @@ -236,14 

Re: [U-Boot] [PATCH v1 08/15] efi_loader: use proper device-paths for partitions

[Alexander Graf]

On 10.08.17 20:29, Rob Clark wrote:

Also, create disk objects for the disk itself, in addition to the
partitions.  (UEFI terminology is a bit confusing, a "disk" object is
really a partition.)  This helps grub properly identify the boot device
since it is trying to match up partition "disk" object with it's parent
device.

Now instead of seeing devices like:

   /File(sd...@07864000.blk)/EndEntire
   /File(usb_mass_storage.lun0)/EndEntire

You see:

   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
   
/ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c,1,1)/EndEntire
   
/ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
   
/ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,1,1)/EndEntire
   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
   
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,6,38ca6802,1,1)/EndEntire
   
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
   
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
   
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,1,1)/EndEntire

This is on a board with single USB disk and single sd-card.  The
UnknownMessaging(1d) node in the device-path is the MMC device,
but grub_efi_print_device_path() hasn't been updated yet for some
of the newer device-path sub-types.

This patch is inspired by a patch originally from Peter Jones, but
re-worked to use efi_device_path, so it doesn't much resemble the
original.

Signed-off-by: Rob Clark 
---
  lib/efi_loader/efi_disk.c | 54 +++
  1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index ed06485e33..eea65a402a 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -28,11 +28,13 @@ struct efi_disk_obj {
/* EFI Interface Media descriptor struct, referenced by ops */
struct efi_block_io_media media;
/* EFI device path to this block device */
-   struct efi_device_path_file_path *dp;
+   struct efi_device_path *dp;
+   /* partition # */
+   unsigned part;
/* Offset into disk for simple partitions */
lbaint_t offset;
/* Internal block device */
-   const struct blk_desc *desc;
+   struct blk_desc *desc;
  };
  
  static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,

@@ -172,26 +174,26 @@ static const struct efi_block_io block_io_disk_template = 
{
  
  static void efi_disk_add_dev(const char *name,

 const char *if_typename,
-const struct blk_desc *desc,
+struct blk_desc *desc,
 int dev_index,
-lbaint_t offset)
+lbaint_t offset,
+unsigned part)
  {
struct efi_disk_obj *diskobj;
-   struct efi_device_path_file_path *dp;
-   int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
  
  	/* Don't add empty devices */

if (!desc->lba)
return;
  
-	diskobj = calloc(1, objlen);

+   diskobj = calloc(1, sizeof(*diskobj));
  
  	/* Fill in object data */

-   dp = (void *)[1];
+   diskobj->dp = efi_dp_from_part(desc, part);
+   diskobj->part = part;
diskobj->parent.protocols[0].guid = _block_io_guid;
diskobj->parent.protocols[0].protocol_interface = >ops;
diskobj->parent.protocols[1].guid = _guid_device_path;
-   diskobj->parent.protocols[1].protocol_interface = dp;
+   diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
diskobj->parent.handle = diskobj;
diskobj->ops = block_io_disk_template;
diskobj->ifname = if_typename;
@@ -207,17 +209,6 @@ static void efi_disk_add_dev(const char *name,
diskobj->media.last_block = desc->lba - offset;
diskobj->ops.media = >media;
  
-	/* Fill in device path */

-   diskobj->dp = dp;
-   dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
-   dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
-   dp[0].dp.length = sizeof(*dp);
-   ascii2unicode(dp[0].str, name);
-
-   dp[1].dp.type = DEVICE_PATH_TYPE_END;
-   dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
-   dp[1].dp.length = sizeof(*dp);
-
/* Hook up to the device list */
list_add_tail(>parent.link, _obj_list);
  }
@@ -236,14 +227,18 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
if (desc->part_type != PART_TYPE_ISO)
return 0;
  
+	/* and devices for each partition: */

while (!part_get_info(desc, part, )) {
snprintf(devname, sizeof(devname), "%s:%d", pdevname,
 part);
efi_disk_add_dev(devname, 

[U-Boot] [PATCH v1 08/15] efi_loader: use proper device-paths for partitions

[Rob Clark]

Also, create disk objects for the disk itself, in addition to the
partitions.  (UEFI terminology is a bit confusing, a "disk" object is
really a partition.)  This helps grub properly identify the boot device
since it is trying to match up partition "disk" object with it's parent
device.

Now instead of seeing devices like:

  /File(sd...@07864000.blk)/EndEntire
  /File(usb_mass_storage.lun0)/EndEntire

You see:

  /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
  
/ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,dd904a8c,1,1)/EndEntire
  
/ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,20,dd904a8c,1,1)/EndEntire
  
/ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,dd904a8c,1,1)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
  
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,6,38ca6802,1,1)/EndEntire
  
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,38ca6802,1,1)/EndEntire
  
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,38ca6802,1,1)/EndEntire
  
/ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,38ca6802,1,1)/EndEntire

This is on a board with single USB disk and single sd-card.  The
UnknownMessaging(1d) node in the device-path is the MMC device,
but grub_efi_print_device_path() hasn't been updated yet for some
of the newer device-path sub-types.

This patch is inspired by a patch originally from Peter Jones, but
re-worked to use efi_device_path, so it doesn't much resemble the
original.

Signed-off-by: Rob Clark 
---
 lib/efi_loader/efi_disk.c | 54 +++
 1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index ed06485e33..eea65a402a 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -28,11 +28,13 @@ struct efi_disk_obj {
/* EFI Interface Media descriptor struct, referenced by ops */
struct efi_block_io_media media;
/* EFI device path to this block device */
-   struct efi_device_path_file_path *dp;
+   struct efi_device_path *dp;
+   /* partition # */
+   unsigned part;
/* Offset into disk for simple partitions */
lbaint_t offset;
/* Internal block device */
-   const struct blk_desc *desc;
+   struct blk_desc *desc;
 };
 
 static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,
@@ -172,26 +174,26 @@ static const struct efi_block_io block_io_disk_template = 
{
 
 static void efi_disk_add_dev(const char *name,
 const char *if_typename,
-const struct blk_desc *desc,
+struct blk_desc *desc,
 int dev_index,
-lbaint_t offset)
+lbaint_t offset,
+unsigned part)
 {
struct efi_disk_obj *diskobj;
-   struct efi_device_path_file_path *dp;
-   int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
 
/* Don't add empty devices */
if (!desc->lba)
return;
 
-   diskobj = calloc(1, objlen);
+   diskobj = calloc(1, sizeof(*diskobj));
 
/* Fill in object data */
-   dp = (void *)[1];
+   diskobj->dp = efi_dp_from_part(desc, part);
+   diskobj->part = part;
diskobj->parent.protocols[0].guid = _block_io_guid;
diskobj->parent.protocols[0].protocol_interface = >ops;
diskobj->parent.protocols[1].guid = _guid_device_path;
-   diskobj->parent.protocols[1].protocol_interface = dp;
+   diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
diskobj->parent.handle = diskobj;
diskobj->ops = block_io_disk_template;
diskobj->ifname = if_typename;
@@ -207,17 +209,6 @@ static void efi_disk_add_dev(const char *name,
diskobj->media.last_block = desc->lba - offset;
diskobj->ops.media = >media;
 
-   /* Fill in device path */
-   diskobj->dp = dp;
-   dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
-   dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
-   dp[0].dp.length = sizeof(*dp);
-   ascii2unicode(dp[0].str, name);
-
-   dp[1].dp.type = DEVICE_PATH_TYPE_END;
-   dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
-   dp[1].dp.length = sizeof(*dp);
-
/* Hook up to the device list */
list_add_tail(>parent.link, _obj_list);
 }
@@ -236,14 +227,18 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
if (desc->part_type != PART_TYPE_ISO)
return 0;
 
+   /* and devices for each partition: */
while (!part_get_info(desc, part, )) {
snprintf(devname, sizeof(devname), "%s:%d", pdevname,
 part);
efi_disk_add_dev(devname, if_typename, desc, diskid,
-