| Message ID | 20230326102554.14622-1-heinrich.schuchardt@canonical.com |
|---|---|
| State | Accepted, archived |
| Commit | a9203b0fefca4627096779e4eb4b1efbea43ec35 |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | [1/1] efi_loader: correct shortening of device-paths | expand |
> 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 > >
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 >> >>
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 > >> > >>
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;
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(-)