* [U-Boot] [PATCH v2 0/2] efi_loader: device_path: support Sandbox's host device @ 2019-09-12 4:52 AKASHI Takahiro 2019-09-12 4:52 ` [U-Boot] [PATCH v2 1/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro 2019-09-12 4:52 ` [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form AKASHI Takahiro 0 siblings, 2 replies; 11+ messages in thread From: AKASHI Takahiro @ 2019-09-12 4:52 UTC (permalink / raw) To: u-boot Changes in v2 (Sept 12, 2019) * use "vendor device path" with dedicate guid instead of LUN * add patch#2 for pretty printing AKASHI Takahiro (2): efi_loader: device_path: support Sandbox's "host" devices efi_loader: device_path: show a host device in understandable form include/efi_loader.h | 8 ++++++ lib/efi_loader/efi_device_path.c | 33 ++++++++++++++++++++++++ lib/efi_loader/efi_device_path_to_text.c | 5 ++++ 3 files changed, 46 insertions(+) -- 2.21.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] efi_loader: device_path: support Sandbox's "host" devices 2019-09-12 4:52 [U-Boot] [PATCH v2 0/2] efi_loader: device_path: support Sandbox's host device AKASHI Takahiro @ 2019-09-12 4:52 ` AKASHI Takahiro 2019-09-12 8:26 ` Heinrich Schuchardt 2019-09-12 4:52 ` [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form AKASHI Takahiro 1 sibling, 1 reply; 11+ messages in thread From: AKASHI Takahiro @ 2019-09-12 4:52 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 "vendor device path" to make it distinguishable. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- include/efi_loader.h | 8 ++++++++ lib/efi_loader/efi_device_path.c | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/include/efi_loader.h b/include/efi_loader.h index 5298ea7997f7..38330f2f5463 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -24,6 +24,10 @@ #define U_BOOT_GUID \ EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \ 0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b) +/* GUID used as host device on sandbox */ +#define U_BOOT_HOST_DEV_GUID \ + EFI_GUID(0xbbe4e671, 0x5773, 0x4ea1, \ + 0x9a, 0xab, 0x3a, 0x7d, 0xbf, 0x40, 0xc4, 0x82) /* Root node */ extern efi_handle_t efi_root; @@ -121,6 +125,10 @@ uint16_t *efi_dp_str(struct efi_device_path *dp); /* GUID of the U-Boot root node */ extern const efi_guid_t efi_u_boot_guid; +#ifdef CONFIG_SANDBOX +/* GUID of U-Boot host device on sandbox */ +extern const efi_guid_t efi_guid_host_dev; +#endif /* GUID of the EFI_BLOCK_IO_PROTOCOL */ extern const efi_guid_t efi_block_io_guid; extern const efi_guid_t efi_global_variable_guid; diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index eeeb80683607..611bab514286 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -12,8 +12,13 @@ #include <mmc.h> #include <efi_loader.h> #include <part.h> +#include <sandboxblockdev.h> #include <asm-generic/unaligned.h> +#ifdef CONFIG_SANDBOX +const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID; +#endif + /* template END node: */ static const struct efi_device_path END = { .type = DEVICE_PATH_TYPE_END, @@ -445,6 +450,16 @@ static unsigned dp_size(struct udevice *dev) case UCLASS_MMC: return dp_size(dev->parent) + sizeof(struct efi_device_path_sd_mmc_path); +#endif +#ifdef CONFIG_SANDBOX + case UCLASS_ROOT: + /* + * Sandbox's host device will be represented + * as vendor device with extra one byte for + * device number + */ + return dp_size(dev->parent) + + sizeof(struct efi_device_path_vendor) + 1; #endif default: return dp_size(dev->parent); @@ -505,6 +520,24 @@ 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 *dp = buf; + struct blk_desc *desc = dev_get_uclass_platdata(dev); + + dp_fill(buf, dev->parent); + dp = buf; + ++dp; + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; + dp->dp.length = sizeof(*dp) + 1; + memcpy(&dp->guid, &efi_guid_host_dev, + sizeof(efi_guid_t)); + dp->vendor_data[0] = desc->devnum; + return &dp->vendor_data[1]; + } +#endif #ifdef CONFIG_IDE case UCLASS_IDE: { struct efi_device_path_atapi *dp = -- 2.21.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/2] efi_loader: device_path: support Sandbox's "host" devices 2019-09-12 4:52 ` [U-Boot] [PATCH v2 1/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro @ 2019-09-12 8:26 ` Heinrich Schuchardt 0 siblings, 0 replies; 11+ messages in thread From: Heinrich Schuchardt @ 2019-09-12 8:26 UTC (permalink / raw) To: u-boot On 9/12/19 6:52 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 I applied you patch to efi/efi-2019-10 and executed 'bootefi selftest' Without host: Installed device path protocols: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f) /VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1) /VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbba2) /VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbba2)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbbc3) With host file bound as device: Installed device path protocols: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,02) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,02)/HD(1,MBR,0x9b2103f1,0x800,0x3fffe) /VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f) /VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1) /VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbba2) /VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbba2)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbbc3) => efidebug devices Scanning disk host2... Found 2 disks Device Device Path ================ ==================== 0000000015e008b0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) 0000000015e00ec0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,02) 0000000015e01050 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,02)/HD(1,MBR,0x9b2103f1,0x800,0x3fffe) This looks ok. Thanks for your contribution. Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > 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 "vendor device path" to make it distinguishable. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > include/efi_loader.h | 8 ++++++++ > lib/efi_loader/efi_device_path.c | 33 ++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 5298ea7997f7..38330f2f5463 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -24,6 +24,10 @@ > #define U_BOOT_GUID \ > EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \ > 0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b) > +/* GUID used as host device on sandbox */ > +#define U_BOOT_HOST_DEV_GUID \ > + EFI_GUID(0xbbe4e671, 0x5773, 0x4ea1, \ > + 0x9a, 0xab, 0x3a, 0x7d, 0xbf, 0x40, 0xc4, 0x82) > > /* Root node */ > extern efi_handle_t efi_root; > @@ -121,6 +125,10 @@ uint16_t *efi_dp_str(struct efi_device_path *dp); > > /* GUID of the U-Boot root node */ > extern const efi_guid_t efi_u_boot_guid; > +#ifdef CONFIG_SANDBOX > +/* GUID of U-Boot host device on sandbox */ > +extern const efi_guid_t efi_guid_host_dev; > +#endif > /* GUID of the EFI_BLOCK_IO_PROTOCOL */ > extern const efi_guid_t efi_block_io_guid; > extern const efi_guid_t efi_global_variable_guid; > diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c > index eeeb80683607..611bab514286 100644 > --- a/lib/efi_loader/efi_device_path.c > +++ b/lib/efi_loader/efi_device_path.c > @@ -12,8 +12,13 @@ > #include <mmc.h> > #include <efi_loader.h> > #include <part.h> > +#include <sandboxblockdev.h> > #include <asm-generic/unaligned.h> > > +#ifdef CONFIG_SANDBOX > +const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID; > +#endif > + > /* template END node: */ > static const struct efi_device_path END = { > .type = DEVICE_PATH_TYPE_END, > @@ -445,6 +450,16 @@ static unsigned dp_size(struct udevice *dev) > case UCLASS_MMC: > return dp_size(dev->parent) + > sizeof(struct efi_device_path_sd_mmc_path); > +#endif > +#ifdef CONFIG_SANDBOX > + case UCLASS_ROOT: > + /* > + * Sandbox's host device will be represented > + * as vendor device with extra one byte for > + * device number > + */ > + return dp_size(dev->parent) > + + sizeof(struct efi_device_path_vendor) + 1; > #endif > default: > return dp_size(dev->parent); > @@ -505,6 +520,24 @@ 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 *dp = buf; > + struct blk_desc *desc = dev_get_uclass_platdata(dev); > + > + dp_fill(buf, dev->parent); > + dp = buf; > + ++dp; > + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; > + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; > + dp->dp.length = sizeof(*dp) + 1; > + memcpy(&dp->guid, &efi_guid_host_dev, > + sizeof(efi_guid_t)); > + dp->vendor_data[0] = desc->devnum; > + return &dp->vendor_data[1]; > + } > +#endif > #ifdef CONFIG_IDE > case UCLASS_IDE: { > struct efi_device_path_atapi *dp = > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-09-12 4:52 [U-Boot] [PATCH v2 0/2] efi_loader: device_path: support Sandbox's host device AKASHI Takahiro 2019-09-12 4:52 ` [U-Boot] [PATCH v2 1/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro @ 2019-09-12 4:52 ` AKASHI Takahiro 2019-09-12 7:59 ` Heinrich Schuchardt 1 sibling, 1 reply; 11+ messages in thread From: AKASHI Takahiro @ 2019-09-12 4:52 UTC (permalink / raw) To: u-boot It would be better to give a user-friendly text to a host device on sandbox instead of just dumping its guid. => host bind 0 /opt/disk/uboot_sandbox_fat.img => efi devices Device Device Path ================ ==================== 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- lib/efi_loader/efi_device_path_to_text.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 96fd08971b73..40a06b70e08a 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) case DEVICE_PATH_SUB_TYPE_VENDOR: { struct efi_device_path_vendor *vdp = (struct efi_device_path_vendor *)dp; +#ifdef CONFIG_SANDBOX + if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) + s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); + else +#endif s += sprintf(s, "VenHw(%pUl)", &vdp->guid); break; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-09-12 4:52 ` [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form AKASHI Takahiro @ 2019-09-12 7:59 ` Heinrich Schuchardt 2019-09-12 9:07 ` AKASHI Takahiro 0 siblings, 1 reply; 11+ messages in thread From: Heinrich Schuchardt @ 2019-09-12 7:59 UTC (permalink / raw) To: u-boot On 9/12/19 6:52 AM, AKASHI Takahiro wrote: > It would be better to give a user-friendly text to a host device > on sandbox instead of just dumping its guid. > > => host bind 0 /opt/disk/uboot_sandbox_fat.img > => efi devices > Device Device Path > ================ ==================== > 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > lib/efi_loader/efi_device_path_to_text.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c > index 96fd08971b73..40a06b70e08a 100644 > --- a/lib/efi_loader/efi_device_path_to_text.c > +++ b/lib/efi_loader/efi_device_path_to_text.c > @@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) > case DEVICE_PATH_SUB_TYPE_VENDOR: { > struct efi_device_path_vendor *vdp = > (struct efi_device_path_vendor *)dp; > +#ifdef CONFIG_SANDBOX > + if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) > + s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); This does not conform to the UEFI spec. The purpose of the sandbox is testing. What would make sense to me is checking in a Python test that The VenHw() output contains the GUID and the drive number. Best regards Heinrich > + else > +#endif > s += sprintf(s, "VenHw(%pUl)", &vdp->guid); > break; > } > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-09-12 7:59 ` Heinrich Schuchardt @ 2019-09-12 9:07 ` AKASHI Takahiro 2019-09-12 9:50 ` Heinrich Schuchardt 0 siblings, 1 reply; 11+ messages in thread From: AKASHI Takahiro @ 2019-09-12 9:07 UTC (permalink / raw) To: u-boot On Thu, Sep 12, 2019 at 09:59:01AM +0200, Heinrich Schuchardt wrote: > On 9/12/19 6:52 AM, AKASHI Takahiro wrote: > > It would be better to give a user-friendly text to a host device > > on sandbox instead of just dumping its guid. > > > > => host bind 0 /opt/disk/uboot_sandbox_fat.img > > => efi devices > > Device Device Path > > ================ ==================== > > 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > > 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > lib/efi_loader/efi_device_path_to_text.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c > > index 96fd08971b73..40a06b70e08a 100644 > > --- a/lib/efi_loader/efi_device_path_to_text.c > > +++ b/lib/efi_loader/efi_device_path_to_text.c > > @@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) > > case DEVICE_PATH_SUB_TYPE_VENDOR: { > > struct efi_device_path_vendor *vdp = > > (struct efi_device_path_vendor *)dp; > > +#ifdef CONFIG_SANDBOX > > + if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) > > + s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); > > This does not conform to the UEFI spec. Okay, so I'd like to change the format again. Instead of 'Vendor' subtype, use 'Controller' subtype. This way, the example above can be seen as: 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Ctrl(0) Looks nice, doesn't it? -Takahiro Akashi > The purpose of the sandbox is testing. What would make sense to me is > checking in a Python test that The VenHw() output contains the GUID and > the drive number. > > Best regards > > Heinrich > > > + else > > +#endif > > s += sprintf(s, "VenHw(%pUl)", &vdp->guid); > > break; > > } > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-09-12 9:07 ` AKASHI Takahiro @ 2019-09-12 9:50 ` Heinrich Schuchardt 2019-09-13 0:20 ` AKASHI Takahiro 0 siblings, 1 reply; 11+ messages in thread From: Heinrich Schuchardt @ 2019-09-12 9:50 UTC (permalink / raw) To: u-boot On 9/12/19 11:07 AM, AKASHI Takahiro wrote: > On Thu, Sep 12, 2019 at 09:59:01AM +0200, Heinrich Schuchardt wrote: >> On 9/12/19 6:52 AM, AKASHI Takahiro wrote: >>> It would be better to give a user-friendly text to a host device >>> on sandbox instead of just dumping its guid. >>> >>> => host bind 0 /opt/disk/uboot_sandbox_fat.img >>> => efi devices >>> Device Device Path >>> ================ ==================== >>> 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) >>> 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> --- >>> lib/efi_loader/efi_device_path_to_text.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c >>> index 96fd08971b73..40a06b70e08a 100644 >>> --- a/lib/efi_loader/efi_device_path_to_text.c >>> +++ b/lib/efi_loader/efi_device_path_to_text.c >>> @@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) >>> case DEVICE_PATH_SUB_TYPE_VENDOR: { >>> struct efi_device_path_vendor *vdp = >>> (struct efi_device_path_vendor *)dp; >>> +#ifdef CONFIG_SANDBOX >>> + if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) >>> + s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); >> >> This does not conform to the UEFI spec. > > Okay, so I'd like to change the format again. > Instead of 'Vendor' subtype, use 'Controller' subtype. > This way, the example above can be seen as: > > 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Ctrl(0) > > Looks nice, doesn't it? A controller is a handle that bears the EFI_DRIVER_BINDING_PROTOCOL. Best regards Heinrich > > -Takahiro Akashi > >> The purpose of the sandbox is testing. What would make sense to me is >> checking in a Python test that The VenHw() output contains the GUID and >> the drive number. >> >> Best regards >> >> Heinrich >> >>> + else >>> +#endif >>> s += sprintf(s, "VenHw(%pUl)", &vdp->guid); >>> break; >>> } >>> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-09-12 9:50 ` Heinrich Schuchardt @ 2019-09-13 0:20 ` AKASHI Takahiro 2019-10-03 6:56 ` AKASHI Takahiro 0 siblings, 1 reply; 11+ messages in thread From: AKASHI Takahiro @ 2019-09-13 0:20 UTC (permalink / raw) To: u-boot On Thu, Sep 12, 2019 at 11:50:04AM +0200, Heinrich Schuchardt wrote: > On 9/12/19 11:07 AM, AKASHI Takahiro wrote: > > On Thu, Sep 12, 2019 at 09:59:01AM +0200, Heinrich Schuchardt wrote: > >> On 9/12/19 6:52 AM, AKASHI Takahiro wrote: > >>> It would be better to give a user-friendly text to a host device > >>> on sandbox instead of just dumping its guid. > >>> > >>> => host bind 0 /opt/disk/uboot_sandbox_fat.img > >>> => efi devices > >>> Device Device Path > >>> ================ ==================== > >>> 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>> 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) > >>> > >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >>> --- > >>> lib/efi_loader/efi_device_path_to_text.c | 5 +++++ > >>> 1 file changed, 5 insertions(+) > >>> > >>> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c > >>> index 96fd08971b73..40a06b70e08a 100644 > >>> --- a/lib/efi_loader/efi_device_path_to_text.c > >>> +++ b/lib/efi_loader/efi_device_path_to_text.c > >>> @@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) > >>> case DEVICE_PATH_SUB_TYPE_VENDOR: { > >>> struct efi_device_path_vendor *vdp = > >>> (struct efi_device_path_vendor *)dp; > >>> +#ifdef CONFIG_SANDBOX > >>> + if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) > >>> + s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); > >> > >> This does not conform to the UEFI spec. > > > > Okay, so I'd like to change the format again. > > Instead of 'Vendor' subtype, use 'Controller' subtype. > > This way, the example above can be seen as: > > > > 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > > 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Ctrl(0) > > > > Looks nice, doesn't it? > > A controller is a handle that bears the EFI_DRIVER_BINDING_PROTOCOL. To my best knowledge, I don't find such a definition in the specification. > >> The purpose of the sandbox is testing. What would make sense to me is > >> checking in a Python test that The VenHw() output contains the GUID and > >> the drive number. For *that* reason, we don't have to stick to strict *standardized* form for Sandbox host device. It would never cause any problems as such a text would never come up on any other platforms. -Takahiro Akashi > >> > >> Best regards > >> > >> Heinrich > >> > >>> + else > >>> +#endif > >>> s += sprintf(s, "VenHw(%pUl)", &vdp->guid); > >>> break; > >>> } > >>> > >> > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-09-13 0:20 ` AKASHI Takahiro @ 2019-10-03 6:56 ` AKASHI Takahiro 2019-10-03 13:58 ` Heinrich Schuchardt 0 siblings, 1 reply; 11+ messages in thread From: AKASHI Takahiro @ 2019-10-03 6:56 UTC (permalink / raw) To: u-boot Heinrich, On Fri, Sep 13, 2019 at 09:20:53AM +0900, AKASHI Takahiro wrote: > On Thu, Sep 12, 2019 at 11:50:04AM +0200, Heinrich Schuchardt wrote: > > On 9/12/19 11:07 AM, AKASHI Takahiro wrote: > > > On Thu, Sep 12, 2019 at 09:59:01AM +0200, Heinrich Schuchardt wrote: > > >> On 9/12/19 6:52 AM, AKASHI Takahiro wrote: > > >>> It would be better to give a user-friendly text to a host device > > >>> on sandbox instead of just dumping its guid. > > >>> > > >>> => host bind 0 /opt/disk/uboot_sandbox_fat.img > > >>> => efi devices > > >>> Device Device Path > > >>> ================ ==================== > > >>> 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > > >>> 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) > > >>> > > >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > >>> --- > > >>> lib/efi_loader/efi_device_path_to_text.c | 5 +++++ > > >>> 1 file changed, 5 insertions(+) > > >>> > > >>> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c > > >>> index 96fd08971b73..40a06b70e08a 100644 > > >>> --- a/lib/efi_loader/efi_device_path_to_text.c > > >>> +++ b/lib/efi_loader/efi_device_path_to_text.c > > >>> @@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) > > >>> case DEVICE_PATH_SUB_TYPE_VENDOR: { > > >>> struct efi_device_path_vendor *vdp = > > >>> (struct efi_device_path_vendor *)dp; > > >>> +#ifdef CONFIG_SANDBOX > > >>> + if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) > > >>> + s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); > > >> > > >> This does not conform to the UEFI spec. > > > > > > Okay, so I'd like to change the format again. > > > Instead of 'Vendor' subtype, use 'Controller' subtype. > > > This way, the example above can be seen as: > > > > > > 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > > > 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Ctrl(0) > > > > > > Looks nice, doesn't it? > > > > A controller is a handle that bears the EFI_DRIVER_BINDING_PROTOCOL. > > To my best knowledge, I don't find such a definition in the specification. Do you have any comments? > > > >> The purpose of the sandbox is testing. What would make sense to me is > > >> checking in a Python test that The VenHw() output contains the GUID and > > >> the drive number. > > For *that* reason, we don't have to stick to strict *standardized* > form for Sandbox host device. It would never cause any problems > as such a text would never come up on any other platforms. Any comments? -Takahiro Akashi > > > > >> > > >> Best regards > > >> > > >> Heinrich > > >> > > >>> + else > > >>> +#endif > > >>> s += sprintf(s, "VenHw(%pUl)", &vdp->guid); > > >>> break; > > >>> } > > >>> > > >> > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-10-03 6:56 ` AKASHI Takahiro @ 2019-10-03 13:58 ` Heinrich Schuchardt 2019-10-04 0:18 ` AKASHI Takahiro 0 siblings, 1 reply; 11+ messages in thread From: Heinrich Schuchardt @ 2019-10-03 13:58 UTC (permalink / raw) To: u-boot On 10/3/19 8:56 AM, AKASHI Takahiro wrote: > Heinrich, > > On Fri, Sep 13, 2019 at 09:20:53AM +0900, AKASHI Takahiro wrote: >> On Thu, Sep 12, 2019 at 11:50:04AM +0200, Heinrich Schuchardt wrote: >>> On 9/12/19 11:07 AM, AKASHI Takahiro wrote: >>>> On Thu, Sep 12, 2019 at 09:59:01AM +0200, Heinrich Schuchardt wrote: >>>>> On 9/12/19 6:52 AM, AKASHI Takahiro wrote: >>>>>> It would be better to give a user-friendly text to a host device >>>>>> on sandbox instead of just dumping its guid. >>>>>> >>>>>> => host bind 0 /opt/disk/uboot_sandbox_fat.img >>>>>> => efi devices >>>>>> Device Device Path >>>>>> ================ ==================== >>>>>> 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) >>>>>> 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) >>>>>> >>>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>>>>> --- >>>>>> lib/efi_loader/efi_device_path_to_text.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c >>>>>> index 96fd08971b73..40a06b70e08a 100644 >>>>>> --- a/lib/efi_loader/efi_device_path_to_text.c >>>>>> +++ b/lib/efi_loader/efi_device_path_to_text.c >>>>>> @@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) >>>>>> case DEVICE_PATH_SUB_TYPE_VENDOR: { >>>>>> struct efi_device_path_vendor *vdp = >>>>>> (struct efi_device_path_vendor *)dp; >>>>>> +#ifdef CONFIG_SANDBOX >>>>>> + if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) >>>>>> + s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); >>>>> >>>>> This does not conform to the UEFI spec. >>>> >>>> Okay, so I'd like to change the format again. >>>> Instead of 'Vendor' subtype, use 'Controller' subtype. >>>> This way, the example above can be seen as: >>>> >>>> 0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) >>>> 0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Ctrl(0) >>>> >>>> Looks nice, doesn't it? >>> >>> A controller is a handle that bears the EFI_DRIVER_BINDING_PROTOCOL. >> >> To my best knowledge, I don't find such a definition in the specification. > > Do you have any comments? > >> >>>>> The purpose of the sandbox is testing. What would make sense to me is >>>>> checking in a Python test that The VenHw() output contains the GUID and >>>>> the drive number. >> >> For *that* reason, we don't have to stick to strict *standardized* >> form for Sandbox host device. It would never cause any problems >> as such a text would never come up on any other platforms. > > Any comments? As the Sandbox host device is vendor specific VenHw() is a good fit. I see no benefit in changing the current solution and would prefer not to make this code more complicated. Best regards Heinrich > > -Takahiro Akashi >> >> >>>>> >>>>> Best regards >>>>> >>>>> Heinrich >>>>> >>>>>> + else >>>>>> +#endif >>>>>> s += sprintf(s, "VenHw(%pUl)", &vdp->guid); >>>>>> break; >>>>>> } >>>>>> >>>>> >>>> >>> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form 2019-10-03 13:58 ` Heinrich Schuchardt @ 2019-10-04 0:18 ` AKASHI Takahiro 0 siblings, 0 replies; 11+ messages in thread From: AKASHI Takahiro @ 2019-10-04 0:18 UTC (permalink / raw) To: u-boot On Thu, Oct 03, 2019 at 03:58:48PM +0200, Heinrich Schuchardt wrote: > On 10/3/19 8:56 AM, AKASHI Takahiro wrote: > >Heinrich, > > > >On Fri, Sep 13, 2019 at 09:20:53AM +0900, AKASHI Takahiro wrote: > >>On Thu, Sep 12, 2019 at 11:50:04AM +0200, Heinrich Schuchardt wrote: > >>>On 9/12/19 11:07 AM, AKASHI Takahiro wrote: > >>>>On Thu, Sep 12, 2019 at 09:59:01AM +0200, Heinrich Schuchardt wrote: > >>>>>On 9/12/19 6:52 AM, AKASHI Takahiro wrote: > >>>>>>It would be better to give a user-friendly text to a host device > >>>>>>on sandbox instead of just dumping its guid. > >>>>>> > >>>>>>=> host bind 0 /opt/disk/uboot_sandbox_fat.img > >>>>>>=> efi devices > >>>>>>Device Device Path > >>>>>>================ ==================== > >>>>>>0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>>>>>0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Hostdev(0) > >>>>>> > >>>>>>Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >>>>>>--- > >>>>>> lib/efi_loader/efi_device_path_to_text.c | 5 +++++ > >>>>>> 1 file changed, 5 insertions(+) > >>>>>> > >>>>>>diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c > >>>>>>index 96fd08971b73..40a06b70e08a 100644 > >>>>>>--- a/lib/efi_loader/efi_device_path_to_text.c > >>>>>>+++ b/lib/efi_loader/efi_device_path_to_text.c > >>>>>>@@ -62,6 +62,11 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) > >>>>>> case DEVICE_PATH_SUB_TYPE_VENDOR: { > >>>>>> struct efi_device_path_vendor *vdp = > >>>>>> (struct efi_device_path_vendor *)dp; > >>>>>>+#ifdef CONFIG_SANDBOX > >>>>>>+ if (!guidcmp(&vdp->guid, &efi_guid_host_dev)) > >>>>>>+ s += sprintf(s, "Hostdev(%d)", vdp->vendor_data[0]); > >>>>> > >>>>>This does not conform to the UEFI spec. > >>>> > >>>>Okay, so I'd like to change the format again. > >>>>Instead of 'Vendor' subtype, use 'Controller' subtype. > >>>>This way, the example above can be seen as: > >>>> > >>>>0000000015c1f3a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > >>>>0000000015c20f00 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Ctrl(0) > >>>> > >>>>Looks nice, doesn't it? > >>> > >>>A controller is a handle that bears the EFI_DRIVER_BINDING_PROTOCOL. > >> > >>To my best knowledge, I don't find such a definition in the specification. > > > >Do you have any comments? > > > >> > >>>>>The purpose of the sandbox is testing. What would make sense to me is > >>>>>checking in a Python test that The VenHw() output contains the GUID and > >>>>>the drive number. > >> > >>For *that* reason, we don't have to stick to strict *standardized* > >>form for Sandbox host device. It would never cause any problems > >>as such a text would never come up on any other platforms. > > > >Any comments? > > As the Sandbox host device is vendor specific VenHw() is a good fit. > I see no benefit in changing the current solution and would prefer not > to make this code more complicated. Benefit? This notation is intuitive and very easily-understandable instead of super-unfriendly GUID. By knowing a number as a host device, we can easily identify what the device is by "host info." The "reason d'etre" of UEFI's "display format" is for human beings, not machine. Moreover, the compatibility/portability is not a problem here as host devices appear only on sandbox which is mainly used for test purposes. -Takahiro Akashi > Best regards > > Heinrich > > > > >-Takahiro Akashi > >> > >> > >>>>> > >>>>>Best regards > >>>>> > >>>>>Heinrich > >>>>> > >>>>>>+ else > >>>>>>+#endif > >>>>>> s += sprintf(s, "VenHw(%pUl)", &vdp->guid); > >>>>>> break; > >>>>>> } > >>>>>> > >>>>> > >>>> > >>> > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-10-04 0:18 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-09-12 4:52 [U-Boot] [PATCH v2 0/2] efi_loader: device_path: support Sandbox's host device AKASHI Takahiro 2019-09-12 4:52 ` [U-Boot] [PATCH v2 1/2] efi_loader: device_path: support Sandbox's "host" devices AKASHI Takahiro 2019-09-12 8:26 ` Heinrich Schuchardt 2019-09-12 4:52 ` [U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form AKASHI Takahiro 2019-09-12 7:59 ` Heinrich Schuchardt 2019-09-12 9:07 ` AKASHI Takahiro 2019-09-12 9:50 ` Heinrich Schuchardt 2019-09-13 0:20 ` AKASHI Takahiro 2019-10-03 6:56 ` AKASHI Takahiro 2019-10-03 13:58 ` Heinrich Schuchardt 2019-10-04 0:18 ` AKASHI Takahiro
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox