* [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type
@ 2019-08-22 8:54 AKASHI Takahiro
2019-08-22 8:54 ` [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro
2019-08-22 18:44 ` [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type Heinrich Schuchardt
0 siblings, 2 replies; 9+ messages in thread
From: AKASHI Takahiro @ 2019-08-22 8:54 UTC (permalink / raw)
To: u-boot
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;
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices
2019-08-22 8:54 [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type AKASHI Takahiro
@ 2019-08-22 8:54 ` AKASHI Takahiro
2019-08-22 18:19 ` Heinrich Schuchardt
2019-08-22 18:44 ` [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type Heinrich Schuchardt
1 sibling, 1 reply; 9+ messages in thread
From: AKASHI Takahiro @ 2019-08-22 8:54 UTC (permalink / raw)
To: u-boot
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 =
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices
2019-08-22 8:54 ` [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro
@ 2019-08-22 18:19 ` Heinrich Schuchardt
2019-08-22 23:34 ` AKASHI Takahiro
0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2019-08-22 18:19 UTC (permalink / raw)
To: u-boot
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 at 0
phy 1 [ ] phy_sandbox |-- gen_phy at 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 at 10002000
eth 1 [ + ] eth_sandbox |-- eth at 10003000
eth 2 [ + ] eth_sandbox |-- sbe5
eth 3 [ + ] eth_sandbox |-- eth at 10004000
gpio 1 [ + ] gpio_sandbox |-- base-gpios
gpio 2 [ ] gpio_sandbox |-- extra-gpios
i2c 0 [ + ] i2c_sandbox |-- i2c at 0
i2c_eeprom 0 [ ] i2c_eeprom | |-- eeprom at 2c
rtc 0 [ ] rtc-sandbox | |-- rtc at 43
rtc 1 [ + ] rtc-sandbox | |-- rtc at 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 at 41
bootcount 0 [ + ] bootcount-rtc |-- bootcount at 0
adc 0 [ ] sandbox-adc |-- adc at 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 at 0,0
pci_emul 0 [ ] sandbox_swap_case_em | | `-- emul at 0,0
pci_generi 1 [ ] pci_generic_drv | |-- pci at 1,0
pci_emul 1 [ ] sandbox_swap_case_em | | `-- emul at 0,0
pci_generi 2 [ ] pci_generic_drv | `-- pci at 1f,0
pci_emul 2 [ ] sandbox_swap_case_em | `-- emul at 1f,0
pci 1 [ ] pci_sandbox |-- pci-controller1
pci 2 [ ] pci_sandbox |-- pci-controller2
pci_generi 3 [ ] pci_generic_drv | `-- pci at 1f,0
pci_emul 3 [ ] sandbox_swap_case_em | `-- emul at 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 at 0
sysreset 2 [ ] sysreset_sandbox |-- reset at 1
reset 0 [ ] sandbox_reset |-- reset-ctl
misc 4 [ ] sandbox_reset_test |-- reset-ctl-test
remoteproc 1 [ ] sandbox_test_proc |-- rproc at 1
remoteproc 2 [ ] sandbox_test_proc |-- rproc at 2
panel 0 [ ] simple_panel |-- panel
smem 0 [ ] smem_sandbox |-- smem at 0
sound 0 [ ] sandbox_sound |-- sound
spi 0 [ ] spi_sandbox |-- spi at 0
spi_flash 0 [ ] spi_flash_std | `-- spi.bin at 0
syscon 0 [ ] sandbox_syscon |-- syscon at 0
syscon 1 [ ] sandbox_syscon |-- syscon at 1
simple_bus 2 [ ] generic_simple_bus |-- syscon at 2
timer 1 [ ] sandbox_timer |-- timer
tpm 0 [ ] sandbox_tpm2 |-- tpm2
serial 1 [ ] serial_sandbox |-- serial
usb 0 [ ] usb_sandbox |-- usb at 1
usb_hub 0 [ ] usb_hub | `-- hub
usb_emul 0 [ ] usb_sandbox_hub | `-- hub-emul
usb_emul 1 [ ] usb_sandbox_flash | |--
flash-stick at 0
usb_emul 2 [ ] usb_sandbox_flash | |--
flash-stick at 1
usb_emul 3 [ ] usb_sandbox_flash | |--
flash-stick at 2
usb_emul 4 [ ] usb_sandbox_keyb | `-- keyb at 3
spmi 0 [ ] sandbox_spmi |-- spmi at 0
pmic 2 [ ] pmic_pm8916 | `-- pm8916 at 0
gpio 3 [ ] gpio_pm8916 | `-- gpios at c000
watchdog 0 [ + ] wdt_sandbox |-- wdt at 0
axi 0 [ ] axi_sandbox_bus |-- axi at 0
axi_emul 0 [ ] sandbox_axi_store | `-- store at 0
testfdt 7 [ ] testfdt_drv |-- chosen-test
simple_bus 3 [ ] generic_simple_bus |-- translation-test at 8000
fdt-dummy 0 [ ] fdt_dummy_drv | |-- dev at 0,0
fdt-dummy 1 [ ] fdt_dummy_drv | |-- dev at 1,100
fdt-dummy 2 [ ] fdt_dummy_drv | |-- dev at 2,200
simple_bus 4 [ ] generic_simple_bus | `-- noxlatebus at 3,300
fdt-dummy 3 [ ] fdt_dummy_drv | `-- dev at 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 at 0
dma 0 [ ] sandbox-dma |-- dma
mdio-mux 0 [ ] mdio_mux_sandbox |-- mdio-mux-test
mdio 0 [ ] mdio-mux-bus-drv | |-- mdio-ch-test at 0
mdio 1 [ ] mdio-mux-bus-drv | `-- mdio-ch-test at 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type
2019-08-22 8:54 [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type AKASHI Takahiro
2019-08-22 8:54 ` [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro
@ 2019-08-22 18:44 ` Heinrich Schuchardt
2019-08-22 23:25 ` AKASHI Takahiro
1 sibling, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2019-08-22 18:44 UTC (permalink / raw)
To: u-boot
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;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type
2019-08-22 18:44 ` [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type Heinrich Schuchardt
@ 2019-08-22 23:25 ` AKASHI Takahiro
0 siblings, 0 replies; 9+ messages in thread
From: AKASHI Takahiro @ 2019-08-22 23:25 UTC (permalink / raw)
To: u-boot
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;
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices
2019-08-22 18:19 ` Heinrich Schuchardt
@ 2019-08-22 23:34 ` AKASHI Takahiro
2019-08-23 18:09 ` Heinrich Schuchardt
0 siblings, 1 reply; 9+ messages in thread
From: AKASHI Takahiro @ 2019-08-22 23:34 UTC (permalink / raw)
To: u-boot
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 at 0
> phy 1 [ ] phy_sandbox |-- gen_phy at 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 at 10002000
> eth 1 [ + ] eth_sandbox |-- eth at 10003000
> eth 2 [ + ] eth_sandbox |-- sbe5
> eth 3 [ + ] eth_sandbox |-- eth at 10004000
> gpio 1 [ + ] gpio_sandbox |-- base-gpios
> gpio 2 [ ] gpio_sandbox |-- extra-gpios
> i2c 0 [ + ] i2c_sandbox |-- i2c at 0
> i2c_eeprom 0 [ ] i2c_eeprom | |-- eeprom at 2c
> rtc 0 [ ] rtc-sandbox | |-- rtc at 43
> rtc 1 [ + ] rtc-sandbox | |-- rtc at 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 at 41
> bootcount 0 [ + ] bootcount-rtc |-- bootcount at 0
> adc 0 [ ] sandbox-adc |-- adc at 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 at 0,0
> pci_emul 0 [ ] sandbox_swap_case_em | | `-- emul at 0,0
> pci_generi 1 [ ] pci_generic_drv | |-- pci at 1,0
> pci_emul 1 [ ] sandbox_swap_case_em | | `-- emul at 0,0
> pci_generi 2 [ ] pci_generic_drv | `-- pci at 1f,0
> pci_emul 2 [ ] sandbox_swap_case_em | `-- emul at 1f,0
> pci 1 [ ] pci_sandbox |-- pci-controller1
> pci 2 [ ] pci_sandbox |-- pci-controller2
> pci_generi 3 [ ] pci_generic_drv | `-- pci at 1f,0
> pci_emul 3 [ ] sandbox_swap_case_em | `-- emul at 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 at 0
> sysreset 2 [ ] sysreset_sandbox |-- reset at 1
> reset 0 [ ] sandbox_reset |-- reset-ctl
> misc 4 [ ] sandbox_reset_test |-- reset-ctl-test
> remoteproc 1 [ ] sandbox_test_proc |-- rproc at 1
> remoteproc 2 [ ] sandbox_test_proc |-- rproc at 2
> panel 0 [ ] simple_panel |-- panel
> smem 0 [ ] smem_sandbox |-- smem at 0
> sound 0 [ ] sandbox_sound |-- sound
> spi 0 [ ] spi_sandbox |-- spi at 0
> spi_flash 0 [ ] spi_flash_std | `-- spi.bin at 0
> syscon 0 [ ] sandbox_syscon |-- syscon at 0
> syscon 1 [ ] sandbox_syscon |-- syscon at 1
> simple_bus 2 [ ] generic_simple_bus |-- syscon at 2
> timer 1 [ ] sandbox_timer |-- timer
> tpm 0 [ ] sandbox_tpm2 |-- tpm2
> serial 1 [ ] serial_sandbox |-- serial
> usb 0 [ ] usb_sandbox |-- usb at 1
> usb_hub 0 [ ] usb_hub | `-- hub
> usb_emul 0 [ ] usb_sandbox_hub | `-- hub-emul
> usb_emul 1 [ ] usb_sandbox_flash | |--
> flash-stick at 0
> usb_emul 2 [ ] usb_sandbox_flash | |--
> flash-stick at 1
> usb_emul 3 [ ] usb_sandbox_flash | |--
> flash-stick at 2
> usb_emul 4 [ ] usb_sandbox_keyb | `-- keyb at 3
> spmi 0 [ ] sandbox_spmi |-- spmi at 0
> pmic 2 [ ] pmic_pm8916 | `-- pm8916 at 0
> gpio 3 [ ] gpio_pm8916 | `-- gpios at c000
> watchdog 0 [ + ] wdt_sandbox |-- wdt at 0
> axi 0 [ ] axi_sandbox_bus |-- axi at 0
> axi_emul 0 [ ] sandbox_axi_store | `-- store at 0
> testfdt 7 [ ] testfdt_drv |-- chosen-test
> simple_bus 3 [ ] generic_simple_bus |-- translation-test at 8000
> fdt-dummy 0 [ ] fdt_dummy_drv | |-- dev at 0,0
> fdt-dummy 1 [ ] fdt_dummy_drv | |-- dev at 1,100
> fdt-dummy 2 [ ] fdt_dummy_drv | |-- dev at 2,200
> simple_bus 4 [ ] generic_simple_bus | `-- noxlatebus at 3,300
> fdt-dummy 3 [ ] fdt_dummy_drv | `-- dev at 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 at 0
> dma 0 [ ] sandbox-dma |-- dma
> mdio-mux 0 [ ] mdio_mux_sandbox |-- mdio-mux-test
> mdio 0 [ ] mdio-mux-bus-drv | |-- mdio-ch-test at 0
> mdio 1 [ ] mdio-mux-bus-drv | `-- mdio-ch-test at 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices
2019-08-22 23:34 ` AKASHI Takahiro
@ 2019-08-23 18:09 ` Heinrich Schuchardt
2019-09-11 6:35 ` AKASHI Takahiro
0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2019-08-23 18:09 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices
2019-08-23 18:09 ` Heinrich Schuchardt
@ 2019-09-11 6:35 ` AKASHI Takahiro
2019-09-11 17:22 ` Heinrich Schuchardt
0 siblings, 1 reply; 9+ messages in thread
From: AKASHI Takahiro @ 2019-09-11 6:35 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices
2019-09-11 6:35 ` AKASHI Takahiro
@ 2019-09-11 17:22 ` Heinrich Schuchardt
0 siblings, 0 replies; 9+ messages in thread
From: Heinrich Schuchardt @ 2019-09-11 17:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-09-11 17:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-22 8:54 [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type AKASHI Takahiro
2019-08-22 8:54 ` [U-Boot] [PATCH 2/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro
2019-08-22 18:19 ` Heinrich Schuchardt
2019-08-22 23:34 ` AKASHI Takahiro
2019-08-23 18:09 ` Heinrich Schuchardt
2019-09-11 6:35 ` AKASHI Takahiro
2019-09-11 17:22 ` Heinrich Schuchardt
2019-08-22 18:44 ` [U-Boot] [PATCH 1/2] efi_loader: device_path: add Device Logical Unit sub type Heinrich Schuchardt
2019-08-22 23:25 ` AKASHI Takahiro
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.