All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] efi_loader: correct shortening of device-paths
@ 2023-03-26 10:25 Heinrich Schuchardt
  2023-03-26 10:45 ` Mark Kettenis
  0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2023-03-26 10:25 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: u-boot, Heinrich Schuchardt

We use short device-paths in boot options so that a file on a block device
can be found independent of the port into which the device is plugged.

Usb() device-path nodes only contain port and interface information and
therefore cannot identify a block device.
UsbWwi() device-path nodes contain the serial number of USB devices.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 include/efi_api.h                |  1 +
 lib/efi_loader/efi_device_path.c | 21 ++++++---------------
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index c4512eeb86..dc6e5ce236 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -604,6 +604,7 @@ struct efi_device_path_acpi_path {
 #  define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR	0x0b
 #  define DEVICE_PATH_SUB_TYPE_MSG_UART		0x0e
 #  define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS	0x0f
+#  define DEVICE_PATH_SUB_TYPE_MSG_USB_WWI	0x10
 #  define DEVICE_PATH_SUB_TYPE_MSG_SATA		0x12
 #  define DEVICE_PATH_SUB_TYPE_MSG_NVME		0x17
 #  define DEVICE_PATH_SUB_TYPE_MSG_URI		0x18
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 288baa1ca7..9ed5e6273d 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -92,17 +92,13 @@ int efi_dp_match(const struct efi_device_path *a,
 /**
  * efi_dp_shorten() - shorten device-path
  *
- * We can have device paths that start with a USB WWID or a USB Class node,
- * and a few other cases which don't encode the full device path with bus
- * hierarchy:
+ * When creating a short boot option we want to use a device-path that is
+ * independent of the location where the block device is plugged in.
  *
- * * MESSAGING:USB_WWID
- * * MESSAGING:USB_CLASS
- * * MEDIA:FILE_PATH
- * * MEDIA:HARD_DRIVE
- * * MESSAGING:URI
+ * UsbWwi() nodes contain a serial number, hard drive paths a partition
+ * UUID. Both should be unique.
  *
- * See UEFI spec (section 3.1.2, about short-form device-paths)
+ * See UEFI spec, section 3.1.2 for "short-form device path".
  *
  * @dp:		original device-path
  * @Return:	shortened device-path or NULL
@@ -110,12 +106,7 @@ int efi_dp_match(const struct efi_device_path *a,
 struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp)
 {
 	while (dp) {
-		/*
-		 * TODO: Add MESSAGING:USB_WWID and MESSAGING:URI..
-		 * in practice fallback.efi just uses MEDIA:HARD_DRIVE
-		 * so not sure when we would see these other cases.
-		 */
-		if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
+		if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_WWI) ||
 		    EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
 		    EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
 			return dp;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] efi_loader: correct shortening of device-paths
  2023-03-26 10:25 [PATCH 1/1] efi_loader: correct shortening of device-paths Heinrich Schuchardt
@ 2023-03-26 10:45 ` Mark Kettenis
  2023-03-26 18:42   ` Heinrich Schuchardt
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Kettenis @ 2023-03-26 10:45 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: ilias.apalodimas, u-boot, heinrich.schuchardt

> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Date: Sun, 26 Mar 2023 12:25:54 +0200
> 
> We use short device-paths in boot options so that a file on a block device
> can be found independent of the port into which the device is plugged.
> 
> Usb() device-path nodes only contain port and interface information and
> therefore cannot identify a block device.
> UsbWwi() device-path nodes contain the serial number of USB devices.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  include/efi_api.h                |  1 +
>  lib/efi_loader/efi_device_path.c | 21 ++++++---------------
>  2 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index c4512eeb86..dc6e5ce236 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -604,6 +604,7 @@ struct efi_device_path_acpi_path {
>  #  define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR	0x0b
>  #  define DEVICE_PATH_SUB_TYPE_MSG_UART		0x0e
>  #  define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS	0x0f
> +#  define DEVICE_PATH_SUB_TYPE_MSG_USB_WWI	0x10
>  #  define DEVICE_PATH_SUB_TYPE_MSG_SATA		0x12
>  #  define DEVICE_PATH_SUB_TYPE_MSG_NVME		0x17
>  #  define DEVICE_PATH_SUB_TYPE_MSG_URI		0x18
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 288baa1ca7..9ed5e6273d 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -92,17 +92,13 @@ int efi_dp_match(const struct efi_device_path *a,
>  /**
>   * efi_dp_shorten() - shorten device-path
>   *
> - * We can have device paths that start with a USB WWID or a USB Class node,
> - * and a few other cases which don't encode the full device path with bus
> - * hierarchy:
> + * When creating a short boot option we want to use a device-path that is
> + * independent of the location where the block device is plugged in.
>   *
> - * * MESSAGING:USB_WWID
> - * * MESSAGING:USB_CLASS
> - * * MEDIA:FILE_PATH
> - * * MEDIA:HARD_DRIVE
> - * * MESSAGING:URI
> + * UsbWwi() nodes contain a serial number, hard drive paths a partition
> + * UUID. Both should be unique.

This sentence makes no sense.  Do they contain a serial number, a hard
drive path *and* a partition UUID?  But then all *three* should be
unique.

>   *
> - * See UEFI spec (section 3.1.2, about short-form device-paths)
> + * See UEFI spec, section 3.1.2 for "short-form device path".
>   *
>   * @dp:		original device-path
>   * @Return:	shortened device-path or NULL
> @@ -110,12 +106,7 @@ int efi_dp_match(const struct efi_device_path *a,
>  struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp)
>  {
>  	while (dp) {
> -		/*
> -		 * TODO: Add MESSAGING:USB_WWID and MESSAGING:URI..
> -		 * in practice fallback.efi just uses MEDIA:HARD_DRIVE
> -		 * so not sure when we would see these other cases.
> -		 */
> -		if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
> +		if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_WWI) ||
>  		    EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
>  		    EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
>  			return dp;
> -- 
> 2.39.2
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] efi_loader: correct shortening of device-paths
  2023-03-26 10:45 ` Mark Kettenis
@ 2023-03-26 18:42   ` Heinrich Schuchardt
  2023-03-27  6:39     ` Ilias Apalodimas
  0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2023-03-26 18:42 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: ilias.apalodimas, u-boot



On 3/26/23 12:45, Mark Kettenis wrote:
>> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> Date: Sun, 26 Mar 2023 12:25:54 +0200
>>
>> We use short device-paths in boot options so that a file on a block device
>> can be found independent of the port into which the device is plugged.
>>
>> Usb() device-path nodes only contain port and interface information and
>> therefore cannot identify a block device.
>> UsbWwi() device-path nodes contain the serial number of USB devices.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>   include/efi_api.h                |  1 +
>>   lib/efi_loader/efi_device_path.c | 21 ++++++---------------
>>   2 files changed, 7 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index c4512eeb86..dc6e5ce236 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -604,6 +604,7 @@ struct efi_device_path_acpi_path {
>>   #  define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR	0x0b
>>   #  define DEVICE_PATH_SUB_TYPE_MSG_UART		0x0e
>>   #  define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS	0x0f
>> +#  define DEVICE_PATH_SUB_TYPE_MSG_USB_WWI	0x10
>>   #  define DEVICE_PATH_SUB_TYPE_MSG_SATA		0x12
>>   #  define DEVICE_PATH_SUB_TYPE_MSG_NVME		0x17
>>   #  define DEVICE_PATH_SUB_TYPE_MSG_URI		0x18
>> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
>> index 288baa1ca7..9ed5e6273d 100644
>> --- a/lib/efi_loader/efi_device_path.c
>> +++ b/lib/efi_loader/efi_device_path.c
>> @@ -92,17 +92,13 @@ int efi_dp_match(const struct efi_device_path *a,
>>   /**
>>    * efi_dp_shorten() - shorten device-path
>>    *
>> - * We can have device paths that start with a USB WWID or a USB Class node,
>> - * and a few other cases which don't encode the full device path with bus
>> - * hierarchy:
>> + * When creating a short boot option we want to use a device-path that is
>> + * independent of the location where the block device is plugged in.
>>    *
>> - * * MESSAGING:USB_WWID
>> - * * MESSAGING:USB_CLASS
>> - * * MEDIA:FILE_PATH
>> - * * MEDIA:HARD_DRIVE
>> - * * MESSAGING:URI
>> + * UsbWwi() nodes contain a serial number, hard drive paths a partition
>> + * UUID. Both should be unique.
> 
> This sentence makes no sense.  Do they contain a serial number, a hard
> drive path *and* a partition UUID?  But then all *three* should be
> unique.

* UsbWWi() nodes contain a unique serial number
* HD() nodes contain a unique UUID.

Shortened device paths for files will only be created if the device path 
is not a block device. Currently neither the eficonfig nor the efidebug 
command allow to specify such a device path.

Best regards

Heinrich

>>    *
>> - * See UEFI spec (section 3.1.2, about short-form device-paths)
>> + * See UEFI spec, section 3.1.2 for "short-form device path".
>>    *
>>    * @dp:		original device-path
>>    * @Return:	shortened device-path or NULL
>> @@ -110,12 +106,7 @@ int efi_dp_match(const struct efi_device_path *a,
>>   struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp)
>>   {
>>   	while (dp) {
>> -		/*
>> -		 * TODO: Add MESSAGING:USB_WWID and MESSAGING:URI..
>> -		 * in practice fallback.efi just uses MEDIA:HARD_DRIVE
>> -		 * so not sure when we would see these other cases.
>> -		 */
>> -		if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
>> +		if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_WWI) ||
>>   		    EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
>>   		    EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
>>   			return dp;
>> -- 
>> 2.39.2
>>
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] efi_loader: correct shortening of device-paths
  2023-03-26 18:42   ` Heinrich Schuchardt
@ 2023-03-27  6:39     ` Ilias Apalodimas
  0 siblings, 0 replies; 4+ messages in thread
From: Ilias Apalodimas @ 2023-03-27  6:39 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Mark Kettenis, u-boot

On Sun, 26 Mar 2023 at 21:42, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
>
>
> On 3/26/23 12:45, Mark Kettenis wrote:
> >> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> Date: Sun, 26 Mar 2023 12:25:54 +0200
> >>
> >> We use short device-paths in boot options so that a file on a block device
> >> can be found independent of the port into which the device is plugged.
> >>
> >> Usb() device-path nodes only contain port and interface information and
> >> therefore cannot identify a block device.
> >> UsbWwi() device-path nodes contain the serial number of USB devices.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >>   include/efi_api.h                |  1 +
> >>   lib/efi_loader/efi_device_path.c | 21 ++++++---------------
> >>   2 files changed, 7 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/include/efi_api.h b/include/efi_api.h
> >> index c4512eeb86..dc6e5ce236 100644
> >> --- a/include/efi_api.h
> >> +++ b/include/efi_api.h
> >> @@ -604,6 +604,7 @@ struct efi_device_path_acpi_path {
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR        0x0b
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_UART            0x0e
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS       0x0f
> >> +#  define DEVICE_PATH_SUB_TYPE_MSG_USB_WWI  0x10
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_SATA            0x12
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_NVME            0x17
> >>   #  define DEVICE_PATH_SUB_TYPE_MSG_URI             0x18
> >> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> >> index 288baa1ca7..9ed5e6273d 100644
> >> --- a/lib/efi_loader/efi_device_path.c
> >> +++ b/lib/efi_loader/efi_device_path.c
> >> @@ -92,17 +92,13 @@ int efi_dp_match(const struct efi_device_path *a,
> >>   /**
> >>    * efi_dp_shorten() - shorten device-path
> >>    *
> >> - * We can have device paths that start with a USB WWID or a USB Class node,
> >> - * and a few other cases which don't encode the full device path with bus
> >> - * hierarchy:
> >> + * When creating a short boot option we want to use a device-path that is
> >> + * independent of the location where the block device is plugged in.
> >>    *
> >> - * * MESSAGING:USB_WWID
> >> - * * MESSAGING:USB_CLASS
> >> - * * MEDIA:FILE_PATH
> >> - * * MEDIA:HARD_DRIVE
> >> - * * MESSAGING:URI
> >> + * UsbWwi() nodes contain a serial number, hard drive paths a partition
> >> + * UUID. Both should be unique.
> >
> > This sentence makes no sense.  Do they contain a serial number, a hard
> > drive path *and* a partition UUID?  But then all *three* should be
> > unique.
>
> * UsbWWi() nodes contain a unique serial number
> * HD() nodes contain a unique UUID.
>
> Shortened device paths for files will only be created if the device path
> is not a block device. Currently neither the eficonfig nor the efidebug
> command allow to specify such a device path.

Mark just wants the comment adjusted, which I think makes sense.
Maybe we could use the EFI spec as-is? IOW
" For USB WWID, the boot manager must use the device vendor ID, device
product id, and serial number, and must match any USB device in the
system that contains this information"

Regards
/Ilias
>
> Best regards
>
> Heinrich
>
> >>    *
> >> - * See UEFI spec (section 3.1.2, about short-form device-paths)
> >> + * See UEFI spec, section 3.1.2 for "short-form device path".
> >>    *
> >>    * @dp:            original device-path
> >>    * @Return:        shortened device-path or NULL
> >> @@ -110,12 +106,7 @@ int efi_dp_match(const struct efi_device_path *a,
> >>   struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp)
> >>   {
> >>      while (dp) {
> >> -            /*
> >> -             * TODO: Add MESSAGING:USB_WWID and MESSAGING:URI..
> >> -             * in practice fallback.efi just uses MEDIA:HARD_DRIVE
> >> -             * so not sure when we would see these other cases.
> >> -             */
> >> -            if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
> >> +            if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_WWI) ||
> >>                  EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
> >>                  EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
> >>                      return dp;
> >> --
> >> 2.39.2
> >>
> >>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-03-27  6:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-26 10:25 [PATCH 1/1] efi_loader: correct shortening of device-paths Heinrich Schuchardt
2023-03-26 10:45 ` Mark Kettenis
2023-03-26 18:42   ` Heinrich Schuchardt
2023-03-27  6:39     ` Ilias Apalodimas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.