Hi Heinrich,
On Mon, 20 Mar 2023 at 04:25, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
>
> EFI device paths for block devices must be unique. If a non-unique device
> path is discovered, probing of the block device fails.
>
> Currently we use UsbClass() device path nodes. As multiple devices may
> have the same vendor and product id these are non-unique. Instead we
> should use Usb() device path nodes. They include the USB port on the
> parent hub. Hence they are unique.
>
> A USB storage device may contain multiple logical units. These can be
> modeled as Ctrl() nodes.
>
> Reported-by: Patrick Delaunay <patrick.delau...@foss.st.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> ---
> lib/efi_loader/efi_device_path.c | 45 +++++++++++++++++++++++---------
> 1 file changed, 33 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass <s...@chromium.org>
[..]
>
> diff --git a/lib/efi_loader/efi_device_path.c
> b/lib/efi_loader/efi_device_path.c
> index 3b267b713e..b6dd575b13 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -147,7 +147,7 @@ struct efi_device_path *efi_dp_shorten(struct
> efi_device_path *dp)
> * 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_CLASS) ||
> + if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
> EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
> EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
> return dp;
> @@ -564,6 +564,11 @@ __maybe_unused static unsigned int dp_size(struct
> udevice *dev)
> return dp_size(dev->parent)
> + sizeof(struct efi_device_path_vendor) + 1;
> #endif
> +#ifdef CONFIG_USB
> + case UCLASS_MASS_STORAGE:
Can we do:
case UCLASS_MASS_STORAGE:
if (IS_ENABLED(CONFIG_USB)) {
...
}
?
and below too
> + return dp_size(dev->parent)
> + + sizeof(struct efi_device_path_controller);
> +#endif
> #ifdef CONFIG_VIRTIO_BLK
> case UCLASS_VIRTIO:
> /*
> @@ -585,7 +590,7 @@ __maybe_unused static unsigned int dp_size(struct udevice
> *dev)
> case UCLASS_MASS_STORAGE:
> case UCLASS_USB_HUB:
> return dp_size(dev->parent) +
> - sizeof(struct efi_device_path_usb_class);
> + sizeof(struct efi_device_path_usb);
> default:
> /* just skip over unknown classes: */
> return dp_size(dev->parent);
> @@ -741,6 +746,19 @@ __maybe_unused static void *dp_fill(void *buf, struct
> udevice *dev)
> memcpy(&dp->ns_id, &ns_id, sizeof(ns_id));
> return &dp[1];
> }
> +#endif
> +#if defined(CONFIG_USB)
> + case UCLASS_MASS_STORAGE: {
> + struct blk_desc *desc = desc =
> dev_get_uclass_plat(dev);
> + struct efi_device_path_controller *dp =
> + dp_fill(buf, dev->parent);
> +
> + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
> + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER;
> + dp->dp.length = sizeof(*dp);
> + dp->controller_number = desc->lun;
> + return &dp[1];
> + }
> #endif
[..]
Regards,
SImon
