[PATCH] efi: fix semihosting EFI payload booting

[Heinrich Schuchardt]

On 5/10/23 16:13, Andre Przywara wrote:
> At the moment any naive attempt to boot an EFI payload that has just
> been loaded via "hostfs" (sandbox or semihosting) is met by a rather
> confusing error message:
> ===========
> VExpress64# load hostfs - $kernel_addr_r Image
> 52752896 bytes read in 8 ms (6.1 GiB/s)
> VExpress64# bootefi $kernel_addr_r
> No UEFI binary known at 0x80080000
> ===========
> Actually explicitly providing the filesize:
> VExpress64# bootefi $kernel_addr_r:$filesize
> works around that problem, but the issue lies deeper: the call to
> efi_set_bootdev() (as done by the generic load code) bails out at some
> point, leaving the image_addr and image_size variables unset, which
> triggers this message. The problem seems to be that "-" is not
> understood by the code creating an UEFI device path. We could try to fix
> just that, but actually semihosting seems to have some explicit support
> in UEFI (at least it does in EDK II): there is a separate GUID for it,
> and hostfs is significantly different in some aspects to justify special
> handling.
>
> Check for the device name being "hostfs" and create a specific UEFI device
> path for semihosting in this case. This uses the GUID used by EDK II for
> almost 15 years.
> This fixes the above load/bootefi sequence without requiring an explicit
> file size argument.
>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>

Could you, please, indicate how to invoke QEMU with semihosting enabled.
This information needs to be added to doc/usage/semihosting.rst.

Can there be multiple semihosting block devices?

> ---
>   lib/efi_loader/efi_device_path.c | 34 ++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
>
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index e2e98a39be1..b6d2074dd70 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -1079,6 +1079,35 @@ struct efi_device_path *efi_dp_from_uart(void)
>   	return buf;
>   }
>
> +#define SEMIHOSTING_GUID \
> +	EFI_GUID(0xc5b9c74a, 0x6d72, 0x4719, \
> +		 0x99, 0xab, 0xc5, 0x9f, 0x19, 0x90, 0x91, 0xeb)

Can semihosting be moved to the driver model?

Then we could create a generic catch all for device path nodes for all
uclasses.

Best regards

Heinrich

> +
> +struct efi_device_path *efi_dp_from_semihosting(void)
> +{
> +	const struct efi_device_path_vendor smh_vendor = {
> +		.dp = {
> +			.type     = DEVICE_PATH_TYPE_HARDWARE_DEVICE,
> +			.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR,
> +			.length   = sizeof(smh_vendor),
> +		},
> +		.guid = SEMIHOSTING_GUID,
> +	};
> +	void *buf, *pos;
> +	size_t dpsize = sizeof(smh_vendor) + sizeof(END);
> +
> +	buf = efi_alloc(dpsize);
> +	if (!buf)
> +		return NULL;
> +	pos = buf;
> +	memcpy(pos, &smh_vendor, sizeof(smh_vendor));
> +	pos += sizeof(smh_vendor);
> +
> +	memcpy(pos, &END, sizeof(END));
> +
> +	return buf;
> +}
> +
>   #ifdef CONFIG_NETDEVICES
>   struct efi_device_path *efi_dp_from_eth(void)
>   {
> @@ -1210,6 +1239,11 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   			*device = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
>   						  (uintptr_t)image_addr,
>   						  image_size);
> +	} else if (!strcmp(dev, "hostfs")) {
> +		efi_get_image_parameters(&image_addr, &image_size);
> +
> +		if (device)
> +			*device = efi_dp_from_semihosting();
>   	} else {
>   		part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
>   					       1);