[U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices
AKASHI Takahiro
takahiro.akashi at linaro.org
Wed Sep 11 06:35:31 UTC 2019
Heinrich,
Apologies for having not replied.
On Fri, Aug 23, 2019 at 08:09:45PM +0200, Heinrich Schuchardt wrote:
> On 8/23/19 1:34 AM, AKASHI Takahiro wrote:
> >On Thu, Aug 22, 2019 at 08:19:24PM +0200, Heinrich Schuchardt wrote:
> >>On 8/22/19 10:54 AM, AKASHI Takahiro wrote:
> >>>Sandbox's "host" devices are currently described as UCLASS_ROOT udevice
> >>>with DEV_IF_HOST block device. As the current implementation of
> >>>efi_device_path doesn't support such a type, any "host" device
> >>>on sandbox cannot be seen as a distinct object.
> >>>
> >>>For example,
> >>> => host bind 0 /foo/disk.img
> >>>
> >>> => efi devices
> >>> Scanning disk host0...
> >>> Found 1 disks
> >>> Device Device Path
> >>> ================ ====================
> >>> 0000000015c19970 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>> 0000000015c19d70 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>>
> >>> => efi dh
> >>> Handle Protocols
> >>> ================ ====================
> >>> 0000000015c19970 Device Path, Device Path To Text, Device Path Utilities, Unicode Collation 2, HII String, HII Database, HII Config Routing
> >>> 0000000015c19ba0 Driver Binding
> >>> 0000000015c19c10 Simple Text Output
> >>> 0000000015c19c80 Simple Text Input, Simple Text Input Ex
> >>> 0000000015c19d70 Block IO, Device Path, Simple File System
> >>>
> >>>As you can see here, efi_root (0x0000000015c19970) and host0 device
> >>>(0x0000000015c19d70) has the same representation of device path.
> >>>
> >>>This is not only inconvenient, but also confusing since two different
> >>>efi objects are associated with the same device path and
> >>>efi_dp_find_obj() will possibly return a wrong result.
> >>>
> >>>Solution:
> >>>Each "host" device should be given an additional device path node
> >>>of Logical Unit Number(LUN) to make it distinguishable. The result
> >>>would be:
> >>> Device Device Path
> >>> ================ ====================
> >>> 0000000015c19970 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>> 0000000015c19d70 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/LUN(0)
> >>>
> >>>Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> >>>---
> >>> lib/efi_loader/efi_device_path.c | 22 ++++++++++++++++++++++
> >>> 1 file changed, 22 insertions(+)
> >>>
> >>>diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> >>>index eeeb80683607..565bb6888fe1 100644
> >>>--- a/lib/efi_loader/efi_device_path.c
> >>>+++ b/lib/efi_loader/efi_device_path.c
> >>>@@ -12,6 +12,7 @@
> >>> #include <mmc.h>
> >>> #include <efi_loader.h>
> >>> #include <part.h>
> >>>+#include <sandboxblockdev.h>
> >>> #include <asm-generic/unaligned.h>
> >>>
> >>> /* template END node: */
> >>>@@ -422,6 +423,10 @@ static unsigned dp_size(struct udevice *dev)
> >>>
> >>> switch (dev->driver->id) {
> >>> case UCLASS_ROOT:
> >>>+#ifdef CONFIG_SANDBOX
> >>>+ /* stop traversing parents at this point: */
> >>>+ return sizeof(ROOT) + sizeof(struct efi_device_path_lun);
> >>>+#endif
> >>> case UCLASS_SIMPLE_BUS:
> >>> /* stop traversing parents at this point: */
> >>> return sizeof(ROOT);
> >>>@@ -505,6 +510,23 @@ static void *dp_fill(void *buf, struct udevice *dev)
> >>> #ifdef CONFIG_BLK
> >>> case UCLASS_BLK:
> >>> switch (dev->parent->uclass->uc_drv->id) {
> >>>+#ifdef CONFIG_SANDBOX
> >>>+ case UCLASS_ROOT: {
> >>>+ /* stop traversing parents at this point: */
> >>>+ struct efi_device_path_vendor *vdp = buf;
> >>>+ struct efi_device_path_lun *dp;
> >>>+ struct host_block_dev *host_dev = dev_get_platdata(dev);
> >>>+ struct blk_desc *desc = dev_get_uclass_platdata(dev);
> >>>+
> >>>+ *vdp++ = ROOT;
> >>>+ dp = (struct efi_device_path_lun *)vdp;
> >>>+ dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> >>>+ dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_LUN;
> >>>+ dp->dp.length = sizeof(*dp);
> >>>+ dp->logical_unit_number = desc->devnum;
> >>>+ return ++dp;
> >>>+ }
> >>>+#endif
> >>> #ifdef CONFIG_IDE
> >>> case UCLASS_IDE: {
> >>> struct efi_device_path_atapi *dp =
> >>>
> >>
> >>Hello Takahiro,
> >>
> >>thank you for pointing out that currently we generate the same device
> >>path twice. This for sure is something we need to fix.
> >>
> >>In the table below you will find the device tree for
> >>
> >>./u-boot -d arch/sandbox/dts/test.dtb
> >>
> >>In the device tree I see exactly one root node. I see no device called
> >>host0.
> >
> >"Host0" or whatever other host device is a pseudo device which will
> >be dynamically created, like other hot-pluggable devices, by "host bind"
> >command on sandbox U-Boot. So it won't appear in device tree.
> >
> >-Takahiro Akashi
>
> Thank you for the explanation.
>
> The host device is shown in the device tree:
"device tree" is a confusing term. Is "dm tree" better?
> make sandbox_defconfig
> make
> ./u-boot
> => host bind 0 ../sct-amd64.img
> => dm tree
> Class Index Probed Driver Name
> -----------------------------------------------------------
> root 0 [ + ] root_driver root_driver
> <snip />
> blk 0 [ + ] sandbox_host_blk `-- host0
>
> I guess the device node for host0 should be of type "Vendor-Defined
> Media Device Path". The field "Vendor Defined Data" can be used to
> differentiate between host0 - host3.
Okay, agreed.
But this will also create another gap between "dm tree"
and device path representation in UEFI.
> But for MMC, USB, SATA, and SCSI devices you would want to use another
> node type.
There is no issue that I'm aware of on those devices.
> The code in your patch should not depend on CONFIG_SANDBOX. Instead in
> the driver model you can simply walk up the device tree up to its root
> node and assign a device path node to each node on the device tree path
> according to its UCLASS (UCLASS_BLK here) and according to its interface
> type (IF_TYPE_HOST here) in the case of block devices.
I don't get your point.
IICU, what you claim above is the exact same as my patch does.
The issue is not 'walking dm tree,' which is already done
by efi_disk_register(), but adding an extra "device path" to
host device.
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
More information about the U-Boot
mailing list