[PATCH 1/1] efi_loader: support all uclasses in device path
Heinrich Schuchardt
heinrich.schuchardt at canonical.com
Wed Jul 19 11:36:31 CEST 2023
On 19.07.23 07:00, AKASHI Takahiro wrote:
> 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.
UCLASS_USB models USB host controllers. The UEFI specification has this
example:
PciRoot(0)/PCI(31,2)/USB(0,0)
Here PCI(31,2) is the host controller and USB(0,0) is a device attached
to its port 0.
As can be seen from the example USB host controllers are not meant to be
modelled as USB() nodes. The UEFI specification has no specific node
type for devices in the SoC or directly attached to the SoC without PCIe
bus.
>
> -Takahiro Akashi
My patch creates VenHW() nodes for all uclasses that have no specific
device path node type assigned in our code. This does not exclude
providing a more specific modelling in future.
With my patch the PCI(31,2) node from the example above would be
represented as two nodes
VenHW()/USB(0,0)
where VenHW would be the PCIe device (UCLASS_USB) and USB(0,0) the
embedded USB root hub (UCLASS_USB_HUB).
Specifically PCIe devices need further work.
Best regards
Heinrich
>
>
>> 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
>>
More information about the U-Boot
mailing list