[PATCH 1/1] efi_loader: support all uclasses in device path

[AKASHI Takahiro]

On Wed, Jul 19, 2023 at 06:43:08AM +0200, Heinrich Schuchardt wrote:
> On devices with multiple USB mass storage devices errors like
> 
>     Path /../USB(0x0,0x0)/USB(0x1,0x0)/Ctrl(0x0)
>     already installed.
> 
> are seen. This is due to creating non-unique device paths. To uniquely
> identify devices we must provide path nodes for all devices on the path
> from the root device.
> 
> Add support for generating device path nodes for all uclasses.

I think that this is the last resort for devices that cannot be classified/
identified by any other means. If possible, should we provide a more concrete
device path?
I have submitted a patch specifically for USB devices in the past:
https://lists.denx.de/pipermail/u-boot/2021-November/468216.html

# I'm no longer working on this patch, though.

-Takahiro Akashi


> Reported-by: Suniel Mahesh <sunil at amarulasolutions.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  include/efi_api.h                |  7 ++++
>  lib/efi_loader/efi_device_path.c | 56 ++++++++++++++------------------
>  2 files changed, 32 insertions(+), 31 deletions(-)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 55a4c989fc..8f5ef5f680 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -579,6 +579,13 @@ struct efi_device_path_vendor {
>  	u8 vendor_data[];
>  } __packed;
>  
> +struct efi_device_path_udevice {
> +	struct efi_device_path dp;
> +	efi_guid_t guid;
> +	int uclass_id;
> +	int dev_number;
> +} __packed;
> +
>  struct efi_device_path_controller {
>  	struct efi_device_path dp;
>  	u32 controller_number;
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 04ebb449ca..d0be869d94 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -10,6 +10,7 @@
>  #include <common.h>
>  #include <blk.h>
>  #include <dm.h>
> +#include <dm/root.h>
>  #include <log.h>
>  #include <net.h>
>  #include <usb.h>
> @@ -38,16 +39,6 @@ const struct efi_device_path END = {
>  	.length   = sizeof(END),
>  };
>  
> -/* template ROOT node: */
> -static const struct efi_device_path_vendor ROOT = {
> -	.dp = {
> -		.type     = DEVICE_PATH_TYPE_HARDWARE_DEVICE,
> -		.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR,
> -		.length   = sizeof(ROOT),
> -	},
> -	.guid = U_BOOT_GUID,
> -};
> -
>  #if defined(CONFIG_MMC)
>  /*
>   * Determine if an MMC device is an SD card.
> @@ -497,13 +488,12 @@ bool efi_dp_is_multi_instance(const struct efi_device_path *dp)
>  __maybe_unused static unsigned int dp_size(struct udevice *dev)
>  {
>  	if (!dev || !dev->driver)
> -		return sizeof(ROOT);
> +		return sizeof(struct efi_device_path_udevice);
>  
>  	switch (device_get_uclass_id(dev)) {
>  	case UCLASS_ROOT:
> -	case UCLASS_SIMPLE_BUS:
>  		/* stop traversing parents at this point: */
> -		return sizeof(ROOT);
> +		return sizeof(struct efi_device_path_udevice);
>  	case UCLASS_ETH:
>  		return dp_size(dev->parent) +
>  			sizeof(struct efi_device_path_mac_addr);
> @@ -582,8 +572,8 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev)
>  		return dp_size(dev->parent) +
>  			sizeof(struct efi_device_path_usb);
>  	default:
> -		/* just skip over unknown classes: */
> -		return dp_size(dev->parent);
> +		return dp_size(dev->parent) +
> +			sizeof(struct efi_device_path_udevice);
>  	}
>  }
>  
> @@ -600,13 +590,6 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
>  		return buf;
>  
>  	switch (device_get_uclass_id(dev)) {
> -	case UCLASS_ROOT:
> -	case UCLASS_SIMPLE_BUS: {
> -		/* stop traversing parents at this point: */
> -		struct efi_device_path_vendor *vdp = buf;
> -		*vdp = ROOT;
> -		return &vdp[1];
> -	}
>  #ifdef CONFIG_NETDEVICES
>  	case UCLASS_ETH: {
>  		struct efi_device_path_mac_addr *dp =
> @@ -811,11 +794,24 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
>  
>  		return &udp[1];
>  	}
> -	default:
> -		/* If the uclass driver is missing, this will show NULL */
> -		log_debug("unhandled device class: %s (%s)\n", dev->name,
> -			  dev_get_uclass_name(dev));
> -		return dp_fill(buf, dev->parent);
> +	default: {
> +		struct efi_device_path_udevice *vdp;
> +		enum uclass_id uclass_id = device_get_uclass_id(dev);
> +
> +		if (uclass_id == UCLASS_ROOT)
> +			vdp = buf;
> +		else
> +			vdp = dp_fill(buf, dev->parent);
> +
> +		vdp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
> +		vdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
> +		vdp->dp.length = sizeof(*vdp);
> +		memcpy(&vdp->guid, &efi_u_boot_guid, sizeof(efi_guid_t));
> +		vdp->uclass_id = uclass_id;
> +		vdp->dev_number = dev->seq_;
> +
> +		return &vdp[1];
> +	    }
>  	}
>  }
>  
> @@ -1052,14 +1048,12 @@ struct efi_device_path *efi_dp_from_uart(void)
>  {
>  	void *buf, *pos;
>  	struct efi_device_path_uart *uart;
> -	size_t dpsize = sizeof(ROOT) + sizeof(*uart) + sizeof(END);
> +	size_t dpsize = dp_size(dm_root()) + sizeof(*uart) + sizeof(END);
>  
>  	buf = efi_alloc(dpsize);
>  	if (!buf)
>  		return NULL;
> -	pos = buf;
> -	memcpy(pos, &ROOT, sizeof(ROOT));
> -	pos += sizeof(ROOT);
> +	pos = dp_fill(buf, dm_root());
>  	uart = pos;
>  	uart->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
>  	uart->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_UART;
> -- 
> 2.40.1
>