[U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type
This sub type may not be very useful for normal systems, but it will be used to support "host" devices on U-Boot sandbox build.
See UEFI Specification 2.8, section 10.3.4.8.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- include/efi_api.h | 6 ++++++ lib/efi_loader/efi_device_path_to_text.c | 6 ++++++ 2 files changed, 12 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h index d3fff3c57936..bb028546c864 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -427,6 +427,7 @@ struct efi_device_path_acpi_path { # define DEVICE_PATH_SUB_TYPE_MSG_USB 0x05 # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b # define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS 0x0f +# define DEVICE_PATH_SUB_TYPE_MSG_LUN 0x11 # define DEVICE_PATH_SUB_TYPE_MSG_SD 0x1a # define DEVICE_PATH_SUB_TYPE_MSG_MMC 0x1d
@@ -443,6 +444,11 @@ struct efi_device_path_scsi { u16 logical_unit_number; } __packed;
+struct efi_device_path_lun { + struct efi_device_path dp; + u8 logical_unit_number; +} __packed; + struct efi_device_path_usb { struct efi_device_path dp; u8 parent_port_number; diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 96fd08971b73..8aae8215e1af 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -107,6 +107,12 @@ static char *dp_msging(char *s, struct efi_device_path *dp) ide->logical_unit_number); break; } + case DEVICE_PATH_SUB_TYPE_MSG_LUN: { + struct efi_device_path_lun *lun = + (struct efi_device_path_lun *)dp; + s += sprintf(s, "LUN(%u)", lun->logical_unit_number); + break; + } case DEVICE_PATH_SUB_TYPE_MSG_USB: { struct efi_device_path_usb *udp = (struct efi_device_path_usb *)dp;
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@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 =
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@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.
Could you, please, assign the device paths you want to generate to the device tree nodes below. Hopefully we than can come up with a UEFI compliant solution.
@Simon: Why do we have siblings in the device tree with the same name (e.g. demo_shape_drv)? I thought siblings should always have different names.
Best regards
Heinrich
Class Index Probed Driver Name ----------------------------------------------------------- root 0 [ + ] root_driver root_driver demo 0 [ ] demo_shape_drv |-- demo_shape_drv demo 1 [ ] demo_simple_drv |-- demo_simple_drv demo 2 [ ] demo_shape_drv |-- demo_shape_drv demo 3 [ ] demo_simple_drv |-- demo_simple_drv demo 4 [ ] demo_shape_drv |-- demo_shape_drv test 0 [ ] test_drv |-- test_drv test 1 [ ] test_drv |-- test_drv test 2 [ ] test_drv |-- test_drv gpio 0 [ ] gpio_sandbox |-- gpio_sandbox rsa_mod_ex 0 [ ] mod_exp_sw |-- mod_exp_sw remoteproc 0 [ ] sandbox_test_proc |-- sandbox_test_proc timer 0 [ + ] sandbox_timer |-- sandbox_timer serial 0 [ + ] serial_sandbox |-- serial_sandbox sysreset 0 [ ] sysreset_sandbox |-- sysreset_sandbox audio-code 0 [ ] sandbox_codec |-- audio-codec cros-ec 0 [ + ] cros_ec_sandbox |-- cros-ec testfdt 0 [ ] testfdt_drv |-- a-test backlight 0 [ ] pwm_backlight |-- backlight testfdt 1 [ ] testfdt_drv |-- b-test phy 0 [ ] phy_sandbox |-- gen_phy@0 phy 1 [ ] phy_sandbox |-- gen_phy@1 simple_bus 0 [ ] generic_simple_bus |-- gen_phy_user testbus 0 [ ] testbus_drv |-- some-bus testfdt 2 [ ] testfdt_drv |-- d-test testfdt 3 [ ] testfdt_drv |-- e-test testfdt 4 [ ] testfdt_drv |-- f-test testfdt 5 [ ] testfdt_drv |-- g-test testfdt 6 [ ] testfdt1_drv |-- h-test clk 0 [ ] clk_sandbox |-- clk-sbox misc 0 [ ] sandbox_clk_test |-- clk-test clk 1 [ ] sandbox_clk_ccf |-- clk-ccf eth 0 [ + ] eth_sandbox |-- eth@10002000 eth 1 [ + ] eth_sandbox |-- eth@10003000 eth 2 [ + ] eth_sandbox |-- sbe5 eth 3 [ + ] eth_sandbox |-- eth@10004000 gpio 1 [ + ] gpio_sandbox |-- base-gpios gpio 2 [ ] gpio_sandbox |-- extra-gpios i2c 0 [ + ] i2c_sandbox |-- i2c@0 i2c_eeprom 0 [ ] i2c_eeprom | |-- eeprom@2c rtc 0 [ ] rtc-sandbox | |-- rtc@43 rtc 1 [ + ] rtc-sandbox | |-- rtc@61 i2c_emul_p 0 [ + ] i2c_emul_parent_drv | |-- emul i2c_emul 0 [ ] sandbox_i2c_eeprom_e | | |-- emul-eeprom i2c_emul 1 [ ] sandbox_i2c_rtc_emul | | |-- emul0 i2c_emul 2 [ + ] sandbox_i2c_rtc_emul | | |-- emull i2c_emul 3 [ ] sandbox_i2c_pmic_emu | | |-- pmic-emul0 i2c_emul 4 [ ] sandbox_i2c_pmic_emu | | `-- pmic-emul1 pmic 0 [ ] sandbox_pmic | |-- sandbox_pmic regulator 0 [ ] sandbox_buck | | |-- buck1 regulator 1 [ ] sandbox_buck | | |-- buck2 regulator 2 [ ] sandbox_ldo | | |-- ldo1 regulator 3 [ ] sandbox_ldo | | |-- ldo2 regulator 4 [ ] sandbox_buck | | `-- no_match_by_nodename pmic 1 [ ] mc34708_pmic | `-- pmic@41 bootcount 0 [ + ] bootcount-rtc |-- bootcount@0 adc 0 [ ] sandbox-adc |-- adc@0 video 0 [ + ] sdl_sandbox |-- lcd vidconsole 0 [ + ] vidconsole_tt | `-- lcd.vidconsole_tt led 0 [ + ] gpio_led |-- leds led 1 [ ] gpio_led | |-- iracibble led 2 [ ] gpio_led | |-- martinet led 3 [ + ] gpio_led | |-- default_on led 4 [ + ] gpio_led | `-- default_off mailbox 0 [ ] sandbox_mbox |-- mbox misc 1 [ ] sandbox_mbox_test |-- mbox-test cpu 0 [ ] cpu_sandbox |-- cpu-test1 cpu 1 [ ] cpu_sandbox |-- cpu-test2 cpu 2 [ ] cpu_sandbox |-- cpu-test3 i2s 0 [ ] sandbox_i2s |-- i2s nop 0 [ ] noptest1_drv |-- nop-test_0 nop 1 [ ] noptest2_drv | `-- nop-test_1 misc 2 [ ] misc_sandbox |-- misc-test mmc 0 [ + ] mmc_sandbox |-- mmc2 blk 0 [ ] mmc_blk | `-- mmc2.blk mmc 1 [ + ] mmc_sandbox |-- mmc1 blk 1 [ ] mmc_blk | `-- mmc1.blk mmc 2 [ + ] mmc_sandbox |-- mmc0 blk 2 [ ] mmc_blk | `-- mmc0.blk pch 0 [ ] sandbox-pch |-- pch pci 0 [ ] pci_sandbox |-- pci-controller0 pci_generi 0 [ ] pci_generic_drv | |-- pci@0,0 pci_emul 0 [ ] sandbox_swap_case_em | | `-- emul@0,0 pci_generi 1 [ ] pci_generic_drv | |-- pci@1,0 pci_emul 1 [ ] sandbox_swap_case_em | | `-- emul@0,0 pci_generi 2 [ ] pci_generic_drv | `-- pci@1f,0 pci_emul 2 [ ] sandbox_swap_case_em | `-- emul@1f,0 pci 1 [ ] pci_sandbox |-- pci-controller1 pci 2 [ ] pci_sandbox |-- pci-controller2 pci_generi 3 [ ] pci_generic_drv | `-- pci@1f,0 pci_emul 3 [ ] sandbox_swap_case_em | `-- emul@1f,0 pci_ep 0 [ ] pci_ep_sandbox |-- pci_ep simple_bus 1 [ ] generic_simple_bus |-- probing testprobe 0 [ ] testprobe_drv | |-- test1 testprobe 1 [ ] testprobe_drv | |-- test2 testprobe 2 [ ] testprobe_drv | |-- test3 testprobe 3 [ ] testprobe_drv | `-- test4 power_doma 0 [ ] sandbox_power_domain |-- power-domain misc 3 [ ] sandbox_power_domain |-- power-domain-test pwm 0 [ ] pwm_sandbox |-- pwm pwm 1 [ ] pwm_sandbox |-- pwm2 ram 0 [ ] ram_sandbox |-- ram sysreset 1 [ ] warm_sysreset_sandbo |-- reset@0 sysreset 2 [ ] sysreset_sandbox |-- reset@1 reset 0 [ ] sandbox_reset |-- reset-ctl misc 4 [ ] sandbox_reset_test |-- reset-ctl-test remoteproc 1 [ ] sandbox_test_proc |-- rproc@1 remoteproc 2 [ ] sandbox_test_proc |-- rproc@2 panel 0 [ ] simple_panel |-- panel smem 0 [ ] smem_sandbox |-- smem@0 sound 0 [ ] sandbox_sound |-- sound spi 0 [ ] spi_sandbox |-- spi@0 spi_flash 0 [ ] spi_flash_std | `-- spi.bin@0 syscon 0 [ ] sandbox_syscon |-- syscon@0 syscon 1 [ ] sandbox_syscon |-- syscon@1 simple_bus 2 [ ] generic_simple_bus |-- syscon@2 timer 1 [ ] sandbox_timer |-- timer tpm 0 [ ] sandbox_tpm2 |-- tpm2 serial 1 [ ] serial_sandbox |-- serial usb 0 [ ] usb_sandbox |-- usb@1 usb_hub 0 [ ] usb_hub | `-- hub usb_emul 0 [ ] usb_sandbox_hub | `-- hub-emul usb_emul 1 [ ] usb_sandbox_flash | |-- flash-stick@0 usb_emul 2 [ ] usb_sandbox_flash | |-- flash-stick@1 usb_emul 3 [ ] usb_sandbox_flash | |-- flash-stick@2 usb_emul 4 [ ] usb_sandbox_keyb | `-- keyb@3 spmi 0 [ ] sandbox_spmi |-- spmi@0 pmic 2 [ ] pmic_pm8916 | `-- pm8916@0 gpio 3 [ ] gpio_pm8916 | `-- gpios@c000 watchdog 0 [ + ] wdt_sandbox |-- wdt@0 axi 0 [ ] axi_sandbox_bus |-- axi@0 axi_emul 0 [ ] sandbox_axi_store | `-- store@0 testfdt 7 [ ] testfdt_drv |-- chosen-test simple_bus 3 [ ] generic_simple_bus |-- translation-test@8000 fdt-dummy 0 [ ] fdt_dummy_drv | |-- dev@0,0 fdt-dummy 1 [ ] fdt_dummy_drv | |-- dev@1,100 fdt-dummy 2 [ ] fdt_dummy_drv | |-- dev@2,200 simple_bus 4 [ ] generic_simple_bus | `-- noxlatebus@3,300 fdt-dummy 3 [ ] fdt_dummy_drv | `-- dev@42 video_osd 0 [ ] sandbox_osd_drv |-- osd board 0 [ ] board_sandbox |-- board tee 0 [ ] sandbox_tee |-- sandbox_tee virtio 0 [ ] virtio-sandbox1 |-- sandbox_virtio1 virtio 1 [ ] virtio-sandbox2 |-- sandbox_virtio2 pinctrl 0 [ + ] sandbox_pinctrl |-- pinctrl hwspinlock 0 [ ] hwspinlock_sandbox |-- hwspinlock@0 dma 0 [ ] sandbox-dma |-- dma mdio-mux 0 [ ] mdio_mux_sandbox |-- mdio-mux-test mdio 0 [ ] mdio-mux-bus-drv | |-- mdio-ch-test@0 mdio 1 [ ] mdio-mux-bus-drv | `-- mdio-ch-test@1 mdio 2 [ ] mdio_sandbox |-- mdio-test clk 2 [ ] fixed_rate_clock |-- clk-fixed clk 3 [ ] fixed_factor_clock |-- clk-fixed-factor clk 4 [ ] fixed_rate_clock |-- osc firmware 0 [ ] sandbox_firmware `-- sandbox-firmware
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@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
Could you, please, assign the device paths you want to generate to the device tree nodes below. Hopefully we than can come up with a UEFI compliant solution.
@Simon: Why do we have siblings in the device tree with the same name (e.g. demo_shape_drv)? I thought siblings should always have different names.
Best regards
Heinrich
Class Index Probed Driver Name
root 0 [ + ] root_driver root_driver demo 0 [ ] demo_shape_drv |-- demo_shape_drv demo 1 [ ] demo_simple_drv |-- demo_simple_drv demo 2 [ ] demo_shape_drv |-- demo_shape_drv demo 3 [ ] demo_simple_drv |-- demo_simple_drv demo 4 [ ] demo_shape_drv |-- demo_shape_drv test 0 [ ] test_drv |-- test_drv test 1 [ ] test_drv |-- test_drv test 2 [ ] test_drv |-- test_drv gpio 0 [ ] gpio_sandbox |-- gpio_sandbox rsa_mod_ex 0 [ ] mod_exp_sw |-- mod_exp_sw remoteproc 0 [ ] sandbox_test_proc |-- sandbox_test_proc timer 0 [ + ] sandbox_timer |-- sandbox_timer serial 0 [ + ] serial_sandbox |-- serial_sandbox sysreset 0 [ ] sysreset_sandbox |-- sysreset_sandbox audio-code 0 [ ] sandbox_codec |-- audio-codec cros-ec 0 [ + ] cros_ec_sandbox |-- cros-ec testfdt 0 [ ] testfdt_drv |-- a-test backlight 0 [ ] pwm_backlight |-- backlight testfdt 1 [ ] testfdt_drv |-- b-test phy 0 [ ] phy_sandbox |-- gen_phy@0 phy 1 [ ] phy_sandbox |-- gen_phy@1 simple_bus 0 [ ] generic_simple_bus |-- gen_phy_user testbus 0 [ ] testbus_drv |-- some-bus testfdt 2 [ ] testfdt_drv |-- d-test testfdt 3 [ ] testfdt_drv |-- e-test testfdt 4 [ ] testfdt_drv |-- f-test testfdt 5 [ ] testfdt_drv |-- g-test testfdt 6 [ ] testfdt1_drv |-- h-test clk 0 [ ] clk_sandbox |-- clk-sbox misc 0 [ ] sandbox_clk_test |-- clk-test clk 1 [ ] sandbox_clk_ccf |-- clk-ccf eth 0 [ + ] eth_sandbox |-- eth@10002000 eth 1 [ + ] eth_sandbox |-- eth@10003000 eth 2 [ + ] eth_sandbox |-- sbe5 eth 3 [ + ] eth_sandbox |-- eth@10004000 gpio 1 [ + ] gpio_sandbox |-- base-gpios gpio 2 [ ] gpio_sandbox |-- extra-gpios i2c 0 [ + ] i2c_sandbox |-- i2c@0 i2c_eeprom 0 [ ] i2c_eeprom | |-- eeprom@2c rtc 0 [ ] rtc-sandbox | |-- rtc@43 rtc 1 [ + ] rtc-sandbox | |-- rtc@61 i2c_emul_p 0 [ + ] i2c_emul_parent_drv | |-- emul i2c_emul 0 [ ] sandbox_i2c_eeprom_e | | |-- emul-eeprom i2c_emul 1 [ ] sandbox_i2c_rtc_emul | | |-- emul0 i2c_emul 2 [ + ] sandbox_i2c_rtc_emul | | |-- emull i2c_emul 3 [ ] sandbox_i2c_pmic_emu | | |-- pmic-emul0 i2c_emul 4 [ ] sandbox_i2c_pmic_emu | | `-- pmic-emul1 pmic 0 [ ] sandbox_pmic | |-- sandbox_pmic regulator 0 [ ] sandbox_buck | | |-- buck1 regulator 1 [ ] sandbox_buck | | |-- buck2 regulator 2 [ ] sandbox_ldo | | |-- ldo1 regulator 3 [ ] sandbox_ldo | | |-- ldo2 regulator 4 [ ] sandbox_buck | | `-- no_match_by_nodename pmic 1 [ ] mc34708_pmic | `-- pmic@41 bootcount 0 [ + ] bootcount-rtc |-- bootcount@0 adc 0 [ ] sandbox-adc |-- adc@0 video 0 [ + ] sdl_sandbox |-- lcd vidconsole 0 [ + ] vidconsole_tt | `-- lcd.vidconsole_tt led 0 [ + ] gpio_led |-- leds led 1 [ ] gpio_led | |-- iracibble led 2 [ ] gpio_led | |-- martinet led 3 [ + ] gpio_led | |-- default_on led 4 [ + ] gpio_led | `-- default_off mailbox 0 [ ] sandbox_mbox |-- mbox misc 1 [ ] sandbox_mbox_test |-- mbox-test cpu 0 [ ] cpu_sandbox |-- cpu-test1 cpu 1 [ ] cpu_sandbox |-- cpu-test2 cpu 2 [ ] cpu_sandbox |-- cpu-test3 i2s 0 [ ] sandbox_i2s |-- i2s nop 0 [ ] noptest1_drv |-- nop-test_0 nop 1 [ ] noptest2_drv | `-- nop-test_1 misc 2 [ ] misc_sandbox |-- misc-test mmc 0 [ + ] mmc_sandbox |-- mmc2 blk 0 [ ] mmc_blk | `-- mmc2.blk mmc 1 [ + ] mmc_sandbox |-- mmc1 blk 1 [ ] mmc_blk | `-- mmc1.blk mmc 2 [ + ] mmc_sandbox |-- mmc0 blk 2 [ ] mmc_blk | `-- mmc0.blk pch 0 [ ] sandbox-pch |-- pch pci 0 [ ] pci_sandbox |-- pci-controller0 pci_generi 0 [ ] pci_generic_drv | |-- pci@0,0 pci_emul 0 [ ] sandbox_swap_case_em | | `-- emul@0,0 pci_generi 1 [ ] pci_generic_drv | |-- pci@1,0 pci_emul 1 [ ] sandbox_swap_case_em | | `-- emul@0,0 pci_generi 2 [ ] pci_generic_drv | `-- pci@1f,0 pci_emul 2 [ ] sandbox_swap_case_em | `-- emul@1f,0 pci 1 [ ] pci_sandbox |-- pci-controller1 pci 2 [ ] pci_sandbox |-- pci-controller2 pci_generi 3 [ ] pci_generic_drv | `-- pci@1f,0 pci_emul 3 [ ] sandbox_swap_case_em | `-- emul@1f,0 pci_ep 0 [ ] pci_ep_sandbox |-- pci_ep simple_bus 1 [ ] generic_simple_bus |-- probing testprobe 0 [ ] testprobe_drv | |-- test1 testprobe 1 [ ] testprobe_drv | |-- test2 testprobe 2 [ ] testprobe_drv | |-- test3 testprobe 3 [ ] testprobe_drv | `-- test4 power_doma 0 [ ] sandbox_power_domain |-- power-domain misc 3 [ ] sandbox_power_domain |-- power-domain-test pwm 0 [ ] pwm_sandbox |-- pwm pwm 1 [ ] pwm_sandbox |-- pwm2 ram 0 [ ] ram_sandbox |-- ram sysreset 1 [ ] warm_sysreset_sandbo |-- reset@0 sysreset 2 [ ] sysreset_sandbox |-- reset@1 reset 0 [ ] sandbox_reset |-- reset-ctl misc 4 [ ] sandbox_reset_test |-- reset-ctl-test remoteproc 1 [ ] sandbox_test_proc |-- rproc@1 remoteproc 2 [ ] sandbox_test_proc |-- rproc@2 panel 0 [ ] simple_panel |-- panel smem 0 [ ] smem_sandbox |-- smem@0 sound 0 [ ] sandbox_sound |-- sound spi 0 [ ] spi_sandbox |-- spi@0 spi_flash 0 [ ] spi_flash_std | `-- spi.bin@0 syscon 0 [ ] sandbox_syscon |-- syscon@0 syscon 1 [ ] sandbox_syscon |-- syscon@1 simple_bus 2 [ ] generic_simple_bus |-- syscon@2 timer 1 [ ] sandbox_timer |-- timer tpm 0 [ ] sandbox_tpm2 |-- tpm2 serial 1 [ ] serial_sandbox |-- serial usb 0 [ ] usb_sandbox |-- usb@1 usb_hub 0 [ ] usb_hub | `-- hub usb_emul 0 [ ] usb_sandbox_hub | `-- hub-emul usb_emul 1 [ ] usb_sandbox_flash | |-- flash-stick@0 usb_emul 2 [ ] usb_sandbox_flash | |-- flash-stick@1 usb_emul 3 [ ] usb_sandbox_flash | |-- flash-stick@2 usb_emul 4 [ ] usb_sandbox_keyb | `-- keyb@3 spmi 0 [ ] sandbox_spmi |-- spmi@0 pmic 2 [ ] pmic_pm8916 | `-- pm8916@0 gpio 3 [ ] gpio_pm8916 | `-- gpios@c000 watchdog 0 [ + ] wdt_sandbox |-- wdt@0 axi 0 [ ] axi_sandbox_bus |-- axi@0 axi_emul 0 [ ] sandbox_axi_store | `-- store@0 testfdt 7 [ ] testfdt_drv |-- chosen-test simple_bus 3 [ ] generic_simple_bus |-- translation-test@8000 fdt-dummy 0 [ ] fdt_dummy_drv | |-- dev@0,0 fdt-dummy 1 [ ] fdt_dummy_drv | |-- dev@1,100 fdt-dummy 2 [ ] fdt_dummy_drv | |-- dev@2,200 simple_bus 4 [ ] generic_simple_bus | `-- noxlatebus@3,300 fdt-dummy 3 [ ] fdt_dummy_drv | `-- dev@42 video_osd 0 [ ] sandbox_osd_drv |-- osd board 0 [ ] board_sandbox |-- board tee 0 [ ] sandbox_tee |-- sandbox_tee virtio 0 [ ] virtio-sandbox1 |-- sandbox_virtio1 virtio 1 [ ] virtio-sandbox2 |-- sandbox_virtio2 pinctrl 0 [ + ] sandbox_pinctrl |-- pinctrl hwspinlock 0 [ ] hwspinlock_sandbox |-- hwspinlock@0 dma 0 [ ] sandbox-dma |-- dma mdio-mux 0 [ ] mdio_mux_sandbox |-- mdio-mux-test mdio 0 [ ] mdio-mux-bus-drv | |-- mdio-ch-test@0 mdio 1 [ ] mdio-mux-bus-drv | `-- mdio-ch-test@1 mdio 2 [ ] mdio_sandbox |-- mdio-test clk 2 [ ] fixed_rate_clock |-- clk-fixed clk 3 [ ] fixed_factor_clock |-- clk-fixed-factor clk 4 [ ] fixed_rate_clock |-- osc firmware 0 [ ] sandbox_firmware `-- sandbox-firmware
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@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:
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.
But for MMC, USB, SATA, and SCSI devices you would want to use another node type.
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.
Best regards
Heinrich
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@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
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.akashi@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
On 8/22/19 10:54 AM, AKASHI Takahiro wrote:
This sub type may not be very useful for normal systems, but it will be used to support "host" devices on U-Boot sandbox build.
See UEFI Specification 2.8, section 10.3.4.8.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
include/efi_api.h | 6 ++++++ lib/efi_loader/efi_device_path_to_text.c | 6 ++++++ 2 files changed, 12 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h index d3fff3c57936..bb028546c864 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -427,6 +427,7 @@ struct efi_device_path_acpi_path { # define DEVICE_PATH_SUB_TYPE_MSG_USB 0x05 # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b # define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS 0x0f +# define DEVICE_PATH_SUB_TYPE_MSG_LUN 0x11 # define DEVICE_PATH_SUB_TYPE_MSG_SD 0x1a # define DEVICE_PATH_SUB_TYPE_MSG_MMC 0x1d
@@ -443,6 +444,11 @@ struct efi_device_path_scsi { u16 logical_unit_number; } __packed;
+struct efi_device_path_lun {
- struct efi_device_path dp;
- u8 logical_unit_number;
+} __packed;
- struct efi_device_path_usb { struct efi_device_path dp; u8 parent_port_number;
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 96fd08971b73..8aae8215e1af 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -107,6 +107,12 @@ static char *dp_msging(char *s, struct efi_device_path *dp) ide->logical_unit_number); break; }
- case DEVICE_PATH_SUB_TYPE_MSG_LUN: {
struct efi_device_path_lun *lun =(struct efi_device_path_lun *)dp;s += sprintf(s, "LUN(%u)", lun->logical_unit_number);
The UEFI spec 2 has this output example: Unit(LUN)
In EDK2: MdePkg/Library/UefiDevicePathLib/DevicePathToText.c:1019: UefiDevicePathLibCatPrint (Str, L"Unit(0x%x)", LogicalUnit->Lun);
Please, correct the output format to match EDK2 (and the spec).
Best regards
Heinrich
break;- } case DEVICE_PATH_SUB_TYPE_MSG_USB: { struct efi_device_path_usb *udp = (struct efi_device_path_usb *)dp;
On Thu, Aug 22, 2019 at 08:44:49PM +0200, Heinrich Schuchardt wrote:
On 8/22/19 10:54 AM, AKASHI Takahiro wrote:
This sub type may not be very useful for normal systems, but it will be used to support "host" devices on U-Boot sandbox build.
See UEFI Specification 2.8, section 10.3.4.8.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
include/efi_api.h | 6 ++++++ lib/efi_loader/efi_device_path_to_text.c | 6 ++++++ 2 files changed, 12 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h index d3fff3c57936..bb028546c864 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -427,6 +427,7 @@ struct efi_device_path_acpi_path { # define DEVICE_PATH_SUB_TYPE_MSG_USB 0x05 # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b # define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS 0x0f +# define DEVICE_PATH_SUB_TYPE_MSG_LUN 0x11 # define DEVICE_PATH_SUB_TYPE_MSG_SD 0x1a # define DEVICE_PATH_SUB_TYPE_MSG_MMC 0x1d
@@ -443,6 +444,11 @@ struct efi_device_path_scsi { u16 logical_unit_number; } __packed;
+struct efi_device_path_lun {
- struct efi_device_path dp;
- u8 logical_unit_number;
+} __packed;
struct efi_device_path_usb { struct efi_device_path dp; u8 parent_port_number; diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 96fd08971b73..8aae8215e1af 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -107,6 +107,12 @@ static char *dp_msging(char *s, struct efi_device_path *dp) ide->logical_unit_number); break; }
- case DEVICE_PATH_SUB_TYPE_MSG_LUN: {
struct efi_device_path_lun *lun =(struct efi_device_path_lun *)dp;s += sprintf(s, "LUN(%u)", lun->logical_unit_number);The UEFI spec 2 has this output example: Unit(LUN)
In EDK2: MdePkg/Library/UefiDevicePathLib/DevicePathToText.c:1019: UefiDevicePathLibCatPrint (Str, L"Unit(0x%x)", LogicalUnit->Lun);
Please, correct the output format to match EDK2 (and the spec).
Good catch, thank you. Will fix.
-Takahiro Akashi
Best regards
Heinrich
break;- } case DEVICE_PATH_SUB_TYPE_MSG_USB: { struct efi_device_path_usb *udp = (struct efi_device_path_usb *)dp;
participants (2)
-
AKASHI Takahiro -
Heinrich Schuchardt