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

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

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