On 9/11/19 8:35 AM, AKASHI Takahiro wrote:
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.aka...@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?
The README calls it Live Device Tree.
./doc/driver-model/livetree.rst
But yes I mean the tree of the driver model.
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.
Previously you suggested using the logical unit class (LUN). I think a
vendor defined node is more appropriate.
Commit 8254f8feb71a93a4d87aa68d900660ef445d44cd
efi_loader: correct text conversion for vendor DP
ensures that you can print out the extra data identifying the individual
host number.
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.
Sorry, the mistake is on my side.
Best regards
Heinrich
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
