[U-Boot] [PATCH v2 2/2] efi_loader: device_path: show a host device in understandable form

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

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;
> >>>>>>  	}
> >>>>>>
> >>>>>
> >>>>
> >>>
> >
>