[PATCH] efi: fix semihosting EFI payload booting

[PATCH] efi: fix semihosting EFI payload booting

[Andre Przywara]

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@arm.com>
---
 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)
+
+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);
-- 
2.25.1

Re: [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@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);

Re: [PATCH] efi: fix semihosting EFI payload booting

[Andre Przywara]

On Wed, 10 May 2023 17:58:06 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

Hi,

> 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@arm.com>  
> 
> Could you, please, indicate how to invoke QEMU with semihosting enabled.
> This information needs to be added to doc/usage/semihosting.rst.

It's already there:
https://u-boot.readthedocs.io/en/latest/usage/semihosting.html#qemu

It's mostly just "-semihosting" on the QEMU side, but it's not enabled by
default in the QEMU defconfig, because it *was* needed to be provided in sync
with the QEMU option. It's a debug feature, so there is no good discovery
mechanism, really. I think Sean made this work, with this autodetect via
trap feature, so we might want to enable this option now, at least for the
filesystem part.

Regardless I was doing those experiments with the FVP fastmodel, which is
the most original semihosting provider, I think, and always enables
semihosting.

> Can there be multiple semihosting block devices?

There is no semihosting *block device*, it's a pure filesystem interface.
And there is conceptually only one instance of that: it's basically a
hook in the debugger (or emulation software) to provide some kind of
syscall interface, triggered by a certain instruction ("brk" on AArch64).
This allows some very limited access to the host filesystem - read files,
write files, but no listing, for instance.
It is super convenient to launch the model and load the kernel (or
whatever) directly from your build machine's filesystem - especially if
you do bisects or trial-and-error experiments.

> > ---
> >   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?

Mmh, I am not sure this is a thing, since there is no block device, it's a
filesystem. Which is part of the problem, I believe, and probably also a
good reason to treat it separately.

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

Need to digest that idea ....

Cheers,
Andre

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

Re: [PATCH] efi: fix semihosting EFI payload booting

[Heinrich Schuchardt]

On 5/10/23 19:26, Andre Przywara wrote:
> On Wed, 10 May 2023 17:58:06 +0200
> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Hi,
>
>> 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@arm.com>
>>
>> Could you, please, indicate how to invoke QEMU with semihosting enabled.
>> This information needs to be added to doc/usage/semihosting.rst.
>
> It's already there:
> https://u-boot.readthedocs.io/en/latest/usage/semihosting.html#qemu

This does not tell me which arguments I should pass to qemu-system-aarch64.

>
> It's mostly just "-semihosting" on the QEMU side, but it's not enabled by
> default in the QEMU defconfig, because it *was* needed to be provided in sync
> with the QEMU option. It's a debug feature, so there is no good discovery
> mechanism, really. I think Sean made this work, with this autodetect via
> trap feature, so we might want to enable this option now, at least for the
> filesystem part.

If it is missing in Ubuntu's QEMU, please, indicate how I can build a
QEMU that allows testing. This information needs to go into semihosting.rst.

>
> Regardless I was doing those experiments with the FVP fastmodel, which is

It seems FVP's are not open source
(https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms).

We need some open source solution for testing the suggested changes.

Best regards

Heinrich

> the most original semihosting provider, I think, and always enables
> semihosting.
>
>> Can there be multiple semihosting block devices?
>
> There is no semihosting *block device*, it's a pure filesystem interface.
> And there is conceptually only one instance of that: it's basically a
> hook in the debugger (or emulation software) to provide some kind of
> syscall interface, triggered by a certain instruction ("brk" on AArch64).
> This allows some very limited access to the host filesystem - read files,
> write files, but no listing, for instance.
> It is super convenient to launch the model and load the kernel (or
> whatever) directly from your build machine's filesystem - especially if
> you do bisects or trial-and-error experiments.
>
>>> ---
>>>    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?
>
> Mmh, I am not sure this is a thing, since there is no block device, it's a
> filesystem. Which is part of the problem, I believe, and probably also a
> good reason to treat it separately.
>
>> Then we could create a generic catch all for device path nodes for all
>> uclasses.
>
> Need to digest that idea ....
>
> Cheers,
> Andre
>
>>
>> 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);
>>
>

Re: [PATCH] efi: fix semihosting EFI payload booting

[Andre Przywara]

On Wed, 10 May 2023 23:19:33 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

Hi,

> On 5/10/23 19:26, Andre Przywara wrote:
> > On Wed, 10 May 2023 17:58:06 +0200
> > Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > Hi,
> >  
> >> 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@arm.com>  
> >>
> >> Could you, please, indicate how to invoke QEMU with semihosting enabled.
> >> This information needs to be added to doc/usage/semihosting.rst.  
> >
> > It's already there:
> > https://u-boot.readthedocs.io/en/latest/usage/semihosting.html#qemu  
> 
> This does not tell me which arguments I should pass to qemu-system-aarch64.

I would say that's out of scope for this document, and is explained in
doc/boards/emulation/qemu-arm.rst. A minimum working example is:
$ qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -bios u-boot.bin

> > It's mostly just "-semihosting" on the QEMU side, but it's not
> > enabled by default in the QEMU defconfig, because it *was* needed
> > to be provided in sync with the QEMU option. It's a debug feature,
> > so there is no good discovery mechanism, really. I think Sean made
> > this work, with this autodetect via trap feature, so we might want
> > to enable this option now, at least for the filesystem part.  
> 
> If it is missing in Ubuntu's QEMU, please, indicate how I can build a
> QEMU that allows testing. This information needs to go into
> semihosting.rst.

It's all there and it works on my 20.04 QEMU build out of the box. What
I meant is that -semihosting is a QEMU command line *option*, and a very
optional one, so to speak, as it's more an ARM low level debugger
technology than anything else, and it was just adopted by QEMU. And
until recently you had to make sure that you give that option to QEMU
if you enable semihosting support in U-Boot, otherwise it would crash.
For more details see Sean's FOSDEM talk:
https://fosdem.org/2023/schedule/event/semihosting_uboot/

> > Regardless I was doing those experiments with the FVP fastmodel, which is  
> 
> It seems FVP's are not open source
> (https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms).
> 
> We need some open source solution for testing the suggested changes.

Sure, I just mentioned this because the models are just the reference
implementation and natural habitat for semihosting: because it's
enabled by default there, and it's an ancient ARM technology.

If you want to test it, just use your favourite QEMU command line (or
the one I mention above) and add "-semihosting". Then load your EFI app:
=> load hostfs - $kernel_addr_r app.efi
=> bootefi $kernel_addr_r

Cheers,
Andre

> 
> Best regards
> 
> Heinrich
> 
> > the most original semihosting provider, I think, and always enables
> > semihosting.
> >  
> >> Can there be multiple semihosting block devices?  
> >
> > There is no semihosting *block device*, it's a pure filesystem interface.
> > And there is conceptually only one instance of that: it's basically a
> > hook in the debugger (or emulation software) to provide some kind of
> > syscall interface, triggered by a certain instruction ("brk" on AArch64).
> > This allows some very limited access to the host filesystem - read files,
> > write files, but no listing, for instance.
> > It is super convenient to launch the model and load the kernel (or
> > whatever) directly from your build machine's filesystem - especially if
> > you do bisects or trial-and-error experiments.
> >  
> >>> ---
> >>>    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?  
> >
> > Mmh, I am not sure this is a thing, since there is no block device, it's a
> > filesystem. Which is part of the problem, I believe, and probably also a
> > good reason to treat it separately.
> >  
> >> Then we could create a generic catch all for device path nodes for all
> >> uclasses.  
> >
> > Need to digest that idea ....
> >
> > Cheers,
> > Andre
> >  
> >>
> >> 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);  
> >>  
> >  
> 

Re: [PATCH] efi: fix semihosting EFI payload booting

[Heinrich Schuchardt]

On 5/11/23 02:00, Andre Przywara wrote:
> On Wed, 10 May 2023 23:19:33 +0200
> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Hi,
>
>> On 5/10/23 19:26, Andre Przywara wrote:
>>> On Wed, 10 May 2023 17:58:06 +0200
>>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>
>>> Hi,
>>>
>>>> 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@arm.com>
>>>>
>>>> Could you, please, indicate how to invoke QEMU with semihosting enabled.
>>>> This information needs to be added to doc/usage/semihosting.rst.
>>>
>>> It's already there:
>>> https://u-boot.readthedocs.io/en/latest/usage/semihosting.html#qemu
>>
>> This does not tell me which arguments I should pass to qemu-system-aarch64.
>
> I would say that's out of scope for this document, and is explained in
> doc/boards/emulation/qemu-arm.rst. A minimum working example is:
> $ qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -bios u-boot.bin
>
>>> It's mostly just "-semihosting" on the QEMU side, but it's not
>>> enabled by default in the QEMU defconfig, because it *was* needed
>>> to be provided in sync with the QEMU option. It's a debug feature,
>>> so there is no good discovery mechanism, really. I think Sean made
>>> this work, with this autodetect via trap feature, so we might want
>>> to enable this option now, at least for the filesystem part.
>>
>> If it is missing in Ubuntu's QEMU, please, indicate how I can build a
>> QEMU that allows testing. This information needs to go into
>> semihosting.rst.
>
> It's all there and it works on my 20.04 QEMU build out of the box. What
> I meant is that -semihosting is a QEMU command line *option*, and a very
> optional one, so to speak, as it's more an ARM low level debugger
> technology than anything else, and it was just adopted by QEMU. And
> until recently you had to make sure that you give that option to QEMU
> if you enable semihosting support in U-Boot, otherwise it would crash.
> For more details see Sean's FOSDEM talk:
> https://fosdem.org/2023/schedule/event/semihosting_uboot/
>
>>> Regardless I was doing those experiments with the FVP fastmodel, which is
>>
>> It seems FVP's are not open source
>> (https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms).
>>
>> We need some open source solution for testing the suggested changes.
>
> Sure, I just mentioned this because the models are just the reference
> implementation and natural habitat for semihosting: because it's
> enabled by default there, and it's an ancient ARM technology.
>
> If you want to test it, just use your favourite QEMU command line (or
> the one I mention above) and add "-semihosting". Then load your EFI app:
> => load hostfs - $kernel_addr_r app.efi
> => bootefi $kernel_addr_r
>
> Cheers,
> Andre

I have compiled qemu_arm64_defconfig with CONFIG_SERIAL_PROBE_ALL=y and
invoked qemu-system-aarch64 (1:7.2+dfsg-5ubuntu2) on Ubuntu 23.10 with
-semihosting.

qemu-system-aarch64 -semihosting \
-machine virt,gic-version=max -accel tcg,thread=multi \
-m 1G -smp cores=2 \
-bios denx/u-boot.bin -cpu cortex-a72 -nographic -gdb tcp::1234 \
-netdev user,id=eth0,tftp=tftp -device e1000,netdev=eth0,romfile= \
-drive if=none,file=arm64.img,format=raw,id=mydisk \
-drive if=pflash,format=raw,index=1,file=envstore.img \
-device virtio-rng-pci \
-device ich9-ahci,id=ahci -device ide-hd,drive=mydisk,bus=ahci.0

With and without your patch I get:

=> load hostfs - $kernel_addr_r foo
** No device specified **
Couldn't find partition hostfs -
Can't set block device

Please, describe in sufficient detail how to use semihosting.

Best regards

Heinrich

>
>>
>> Best regards
>>
>> Heinrich
>>
>>> the most original semihosting provider, I think, and always enables
>>> semihosting.
>>>
>>>> Can there be multiple semihosting block devices?
>>>
>>> There is no semihosting *block device*, it's a pure filesystem interface.
>>> And there is conceptually only one instance of that: it's basically a
>>> hook in the debugger (or emulation software) to provide some kind of
>>> syscall interface, triggered by a certain instruction ("brk" on AArch64).
>>> This allows some very limited access to the host filesystem - read files,
>>> write files, but no listing, for instance.
>>> It is super convenient to launch the model and load the kernel (or
>>> whatever) directly from your build machine's filesystem - especially if
>>> you do bisects or trial-and-error experiments.
>>>
>>>>> ---
>>>>>     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?
>>>
>>> Mmh, I am not sure this is a thing, since there is no block device, it's a
>>> filesystem. Which is part of the problem, I believe, and probably also a
>>> good reason to treat it separately.
>>>
>>>> Then we could create a generic catch all for device path nodes for all
>>>> uclasses.
>>>
>>> Need to digest that idea ....
>>>
>>> Cheers,
>>> Andre
>>>
>>>>
>>>> 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);
>>>>
>>>
>>
>

Re: [PATCH] efi: fix semihosting EFI payload booting

[Andre Przywara]

On Thu, 11 May 2023 08:22:30 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> On 5/11/23 02:00, Andre Przywara wrote:
> > On Wed, 10 May 2023 23:19:33 +0200
> > Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > Hi,
> >  
> >> On 5/10/23 19:26, Andre Przywara wrote:  
> >>> On Wed, 10 May 2023 17:58:06 +0200
> >>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>
> >>> Hi,
> >>>  
> >>>> 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@arm.com>  
> >>>>
> >>>> Could you, please, indicate how to invoke QEMU with semihosting enabled.
> >>>> This information needs to be added to doc/usage/semihosting.rst.  
> >>>
> >>> It's already there:
> >>> https://u-boot.readthedocs.io/en/latest/usage/semihosting.html#qemu  
> >>
> >> This does not tell me which arguments I should pass to qemu-system-aarch64.  
> >
> > I would say that's out of scope for this document, and is explained in
> > doc/boards/emulation/qemu-arm.rst. A minimum working example is:
> > $ qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -bios u-boot.bin
> >  
> >>> It's mostly just "-semihosting" on the QEMU side, but it's not
> >>> enabled by default in the QEMU defconfig, because it *was* needed
> >>> to be provided in sync with the QEMU option. It's a debug feature,
> >>> so there is no good discovery mechanism, really. I think Sean made
> >>> this work, with this autodetect via trap feature, so we might want
> >>> to enable this option now, at least for the filesystem part.  
> >>
> >> If it is missing in Ubuntu's QEMU, please, indicate how I can build a
> >> QEMU that allows testing. This information needs to go into
> >> semihosting.rst.  
> >
> > It's all there and it works on my 20.04 QEMU build out of the box. What
> > I meant is that -semihosting is a QEMU command line *option*, and a very
> > optional one, so to speak, as it's more an ARM low level debugger
> > technology than anything else, and it was just adopted by QEMU. And
> > until recently you had to make sure that you give that option to QEMU
> > if you enable semihosting support in U-Boot, otherwise it would crash.
> > For more details see Sean's FOSDEM talk:
> > https://fosdem.org/2023/schedule/event/semihosting_uboot/
> >  
> >>> Regardless I was doing those experiments with the FVP fastmodel, which is  
> >>
> >> It seems FVP's are not open source
> >> (https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms).
> >>
> >> We need some open source solution for testing the suggested changes.  
> >
> > Sure, I just mentioned this because the models are just the reference
> > implementation and natural habitat for semihosting: because it's
> > enabled by default there, and it's an ancient ARM technology.
> >
> > If you want to test it, just use your favourite QEMU command line (or
> > the one I mention above) and add "-semihosting". Then load your EFI app:  
> > => load hostfs - $kernel_addr_r app.efi
> > => bootefi $kernel_addr_r  
> >
> > Cheers,
> > Andre  
> 
> I have compiled qemu_arm64_defconfig with CONFIG_SERIAL_PROBE_ALL=y and

Ah, sorry, I see that the semihosting QEMU doc section fails to
mention that you need to enable CONFIG_SEMIHOSTING. That is just listed
in the FVP section. I will send a patch later.

> invoked qemu-system-aarch64 (1:7.2+dfsg-5ubuntu2) on Ubuntu 23.10 with
> -semihosting.
> 
> qemu-system-aarch64 -semihosting \
> -machine virt,gic-version=max -accel tcg,thread=multi \
> -m 1G -smp cores=2 \
> -bios denx/u-boot.bin -cpu cortex-a72 -nographic -gdb tcp::1234 \
> -netdev user,id=eth0,tftp=tftp -device e1000,netdev=eth0,romfile= \
> -drive if=none,file=arm64.img,format=raw,id=mydisk \
> -drive if=pflash,format=raw,index=1,file=envstore.img \
> -device virtio-rng-pci \
> -device ich9-ahci,id=ahci -device ide-hd,drive=mydisk,bus=ahci.0
> 
> With and without your patch I get:
> 
> => load hostfs - $kernel_addr_r foo  

This should work then, if foo is a file in your local directory.

Hope that helps!

Cheers,
Andre


> ** No device specified **
> Couldn't find partition hostfs -
> Can't set block device
> 
> Please, describe in sufficient detail how to use semihosting.
> 
> Best regards
> 
> Heinrich
> 
> >  
> >>
> >> Best regards
> >>
> >> Heinrich
> >>  
> >>> the most original semihosting provider, I think, and always enables
> >>> semihosting.
> >>>  
> >>>> Can there be multiple semihosting block devices?  
> >>>
> >>> There is no semihosting *block device*, it's a pure filesystem interface.
> >>> And there is conceptually only one instance of that: it's basically a
> >>> hook in the debugger (or emulation software) to provide some kind of
> >>> syscall interface, triggered by a certain instruction ("brk" on AArch64).
> >>> This allows some very limited access to the host filesystem - read files,
> >>> write files, but no listing, for instance.
> >>> It is super convenient to launch the model and load the kernel (or
> >>> whatever) directly from your build machine's filesystem - especially if
> >>> you do bisects or trial-and-error experiments.
> >>>  
> >>>>> ---
> >>>>>     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?  
> >>>
> >>> Mmh, I am not sure this is a thing, since there is no block device, it's a
> >>> filesystem. Which is part of the problem, I believe, and probably also a
> >>> good reason to treat it separately.
> >>>  
> >>>> Then we could create a generic catch all for device path nodes for all
> >>>> uclasses.  
> >>>
> >>> Need to digest that idea ....
> >>>
> >>> Cheers,
> >>> Andre
> >>>  
> >>>>
> >>>> 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);  
> >>>>  
> >>>  
> >>  
> >  
> 

Re: [PATCH] efi: fix semihosting EFI payload booting

[Heinrich Schuchardt]

On 5/11/23 10:59, Andre Przywara wrote:
> On Thu, 11 May 2023 08:22:30 +0200
> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>> On 5/11/23 02:00, Andre Przywara wrote:
>>> On Wed, 10 May 2023 23:19:33 +0200
>>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>
>>> Hi,
>>>
>>>> On 5/10/23 19:26, Andre Przywara wrote:
>>>>> On Wed, 10 May 2023 17:58:06 +0200
>>>>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>> 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@arm.com>
>>>>>>
>>>>>> Could you, please, indicate how to invoke QEMU with semihosting enabled.
>>>>>> This information needs to be added to doc/usage/semihosting.rst.
>>>>>
>>>>> It's already there:
>>>>> https://u-boot.readthedocs.io/en/latest/usage/semihosting.html#qemu
>>>>
>>>> This does not tell me which arguments I should pass to qemu-system-aarch64.
>>>
>>> I would say that's out of scope for this document, and is explained in
>>> doc/boards/emulation/qemu-arm.rst. A minimum working example is:
>>> $ qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -bios u-boot.bin
>>>
>>>>> It's mostly just "-semihosting" on the QEMU side, but it's not
>>>>> enabled by default in the QEMU defconfig, because it *was* needed
>>>>> to be provided in sync with the QEMU option. It's a debug feature,
>>>>> so there is no good discovery mechanism, really. I think Sean made
>>>>> this work, with this autodetect via trap feature, so we might want
>>>>> to enable this option now, at least for the filesystem part.
>>>>
>>>> If it is missing in Ubuntu's QEMU, please, indicate how I can build a
>>>> QEMU that allows testing. This information needs to go into
>>>> semihosting.rst.
>>>
>>> It's all there and it works on my 20.04 QEMU build out of the box. What
>>> I meant is that -semihosting is a QEMU command line *option*, and a very
>>> optional one, so to speak, as it's more an ARM low level debugger
>>> technology than anything else, and it was just adopted by QEMU. And
>>> until recently you had to make sure that you give that option to QEMU
>>> if you enable semihosting support in U-Boot, otherwise it would crash.
>>> For more details see Sean's FOSDEM talk:
>>> https://fosdem.org/2023/schedule/event/semihosting_uboot/
>>>
>>>>> Regardless I was doing those experiments with the FVP fastmodel, which is
>>>>
>>>> It seems FVP's are not open source
>>>> (https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms).
>>>>
>>>> We need some open source solution for testing the suggested changes.
>>>
>>> Sure, I just mentioned this because the models are just the reference
>>> implementation and natural habitat for semihosting: because it's
>>> enabled by default there, and it's an ancient ARM technology.
>>>
>>> If you want to test it, just use your favourite QEMU command line (or
>>> the one I mention above) and add "-semihosting". Then load your EFI app:
>>> => load hostfs - $kernel_addr_r app.efi
>>> => bootefi $kernel_addr_r
>>>
>>> Cheers,
>>> Andre
>>
>> I have compiled qemu_arm64_defconfig with CONFIG_SERIAL_PROBE_ALL=y and
>
> Ah, sorry, I see that the semihosting QEMU doc section fails to
> mention that you need to enable CONFIG_SEMIHOSTING. That is just listed
> in the FVP section. I will send a patch later.
>
>> invoked qemu-system-aarch64 (1:7.2+dfsg-5ubuntu2) on Ubuntu 23.10 with
>> -semihosting.
>>
>> qemu-system-aarch64 -semihosting \
>> -machine virt,gic-version=max -accel tcg,thread=multi \
>> -m 1G -smp cores=2 \
>> -bios denx/u-boot.bin -cpu cortex-a72 -nographic -gdb tcp::1234 \
>> -netdev user,id=eth0,tftp=tftp -device e1000,netdev=eth0,romfile= \
>> -drive if=none,file=arm64.img,format=raw,id=mydisk \
>> -drive if=pflash,format=raw,index=1,file=envstore.img \
>> -device virtio-rng-pci \
>> -device ich9-ahci,id=ahci -device ide-hd,drive=mydisk,bus=ahci.0
>>
>> With and without your patch I get:
>>
>> => load hostfs - $kernel_addr_r foo
>
> This should work then, if foo is a file in your local directory.
>
> Hope that helps!
>
> Cheers,
> Andre
>

Now I am able to debug the code.

I wonder if the file system should be exposed as EFI simple file system
so that EFI applications can access it, e.g. for GRUB to load initrd and
the kernel.

If we don't want to expose it, this change will be enough:

diff --git a/lib/efi_loader/efi_device_path.c
b/lib/efi_loader/efi_device_path.c
index e2e98a39be..058bdc1ee5 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -1203,7 +1203,7 @@ efi_status_t efi_dp_from_name(const char *dev,
const char *devnr,
         } else if (!strcmp(dev, "Uart")) {
                 if (device)
                         *device = efi_dp_from_uart();
-       } else if (!strcmp(dev, "Mem")) {
+       } else if (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs") ) {
                 efi_get_image_parameters(&image_addr, &image_size);

                 if (device)

Best regards

Heinrich

Re: [PATCH] efi: fix semihosting EFI payload booting

[Ilias Apalodimas]

Hi Andre, Heinrich,

Apologies for being late to the party.

On Thu, 11 May 2023 at 18:23, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 5/11/23 10:59, Andre Przywara wrote:
> > On Thu, 11 May 2023 08:22:30 +0200
> > Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> >> On 5/11/23 02:00, Andre Przywara wrote:
> >>> On Wed, 10 May 2023 23:19:33 +0200
> >>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>
> >>> Hi,
> >>>
> >>>> On 5/10/23 19:26, Andre Przywara wrote:
> >>>>> On Wed, 10 May 2023 17:58:06 +0200
> >>>>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>>> 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@arm.com>
> >>>>>>
> >>>>>> Could you, please, indicate how to invoke QEMU with semihosting enabled.
> >>>>>> This information needs to be added to doc/usage/semihosting.rst.
> >>>>>
> >>>>> It's already there:
> >>>>> https://u-boot.readthedocs.io/en/latest/usage/semihosting.html#qemu
> >>>>
> >>>> This does not tell me which arguments I should pass to qemu-system-aarch64.
> >>>
> >>> I would say that's out of scope for this document, and is explained in
> >>> doc/boards/emulation/qemu-arm.rst. A minimum working example is:
> >>> $ qemu-system-aarch64 -M virt -cpu cortex-a57 -nographic -bios u-boot.bin
> >>>
> >>>>> It's mostly just "-semihosting" on the QEMU side, but it's not
> >>>>> enabled by default in the QEMU defconfig, because it *was* needed
> >>>>> to be provided in sync with the QEMU option. It's a debug feature,
> >>>>> so there is no good discovery mechanism, really. I think Sean made
> >>>>> this work, with this autodetect via trap feature, so we might want
> >>>>> to enable this option now, at least for the filesystem part.
> >>>>
> >>>> If it is missing in Ubuntu's QEMU, please, indicate how I can build a
> >>>> QEMU that allows testing. This information needs to go into
> >>>> semihosting.rst.
> >>>
> >>> It's all there and it works on my 20.04 QEMU build out of the box. What
> >>> I meant is that -semihosting is a QEMU command line *option*, and a very
> >>> optional one, so to speak, as it's more an ARM low level debugger
> >>> technology than anything else, and it was just adopted by QEMU. And
> >>> until recently you had to make sure that you give that option to QEMU
> >>> if you enable semihosting support in U-Boot, otherwise it would crash.
> >>> For more details see Sean's FOSDEM talk:
> >>> https://fosdem.org/2023/schedule/event/semihosting_uboot/
> >>>
> >>>>> Regardless I was doing those experiments with the FVP fastmodel, which is
> >>>>
> >>>> It seems FVP's are not open source
> >>>> (https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms).
> >>>>
> >>>> We need some open source solution for testing the suggested changes.
> >>>
> >>> Sure, I just mentioned this because the models are just the reference
> >>> implementation and natural habitat for semihosting: because it's
> >>> enabled by default there, and it's an ancient ARM technology.
> >>>
> >>> If you want to test it, just use your favourite QEMU command line (or
> >>> the one I mention above) and add "-semihosting". Then load your EFI app:
> >>> => load hostfs - $kernel_addr_r app.efi
> >>> => bootefi $kernel_addr_r
> >>>
> >>> Cheers,
> >>> Andre
> >>
> >> I have compiled qemu_arm64_defconfig with CONFIG_SERIAL_PROBE_ALL=y and
> >
> > Ah, sorry, I see that the semihosting QEMU doc section fails to
> > mention that you need to enable CONFIG_SEMIHOSTING. That is just listed
> > in the FVP section. I will send a patch later.
> >
> >> invoked qemu-system-aarch64 (1:7.2+dfsg-5ubuntu2) on Ubuntu 23.10 with
> >> -semihosting.
> >>
> >> qemu-system-aarch64 -semihosting \
> >> -machine virt,gic-version=max -accel tcg,thread=multi \
> >> -m 1G -smp cores=2 \
> >> -bios denx/u-boot.bin -cpu cortex-a72 -nographic -gdb tcp::1234 \
> >> -netdev user,id=eth0,tftp=tftp -device e1000,netdev=eth0,romfile= \
> >> -drive if=none,file=arm64.img,format=raw,id=mydisk \
> >> -drive if=pflash,format=raw,index=1,file=envstore.img \
> >> -device virtio-rng-pci \
> >> -device ich9-ahci,id=ahci -device ide-hd,drive=mydisk,bus=ahci.0
> >>
> >> With and without your patch I get:
> >>
> >> => load hostfs - $kernel_addr_r foo
> >
> > This should work then, if foo is a file in your local directory.
> >
> > Hope that helps!
> >
> > Cheers,
> > Andre
> >
>
> Now I am able to debug the code.
>
> I wonder if the file system should be exposed as EFI simple file system
> so that EFI applications can access it, e.g. for GRUB to load initrd and
> the kernel.
>
> If we don't want to expose it, this change will be enough:
>
> diff --git a/lib/efi_loader/efi_device_path.c
> b/lib/efi_loader/efi_device_path.c
> index e2e98a39be..058bdc1ee5 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -1203,7 +1203,7 @@ efi_status_t efi_dp_from_name(const char *dev,
> const char *devnr,
>          } else if (!strcmp(dev, "Uart")) {
>                  if (device)
>                          *device = efi_dp_from_uart();
> -       } else if (!strcmp(dev, "Mem")) {
> +       } else if (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs") ) {
>                  efi_get_image_parameters(&image_addr, &image_size);
>
>                  if (device)

I think it's a question of use cases here.  Andre do you think that
making the host filesystem accessible through EFI and the simple file
system protocol is important or needed for the low level debugging
semihosting is designed to enable?
If the answer there is no, then I think I prefer Heinrichs patch

Thanks
/Ilias
>
> Best regards
>
> Heinrich
>

Re: [PATCH] efi: fix semihosting EFI payload booting

[Andre Przywara]

On Thu, 11 May 2023 17:23:13 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

Hi Heinrich, Ilias,

> On 5/11/23 10:59, Andre Przywara wrote:
> > On Thu, 11 May 2023 08:22:30 +0200
> > Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >  
> >> On 5/11/23 02:00, Andre Przywara wrote:  
> >>> On Wed, 10 May 2023 23:19:33 +0200
> >>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>
> >>> Hi,
> >>>  
> >>>> On 5/10/23 19:26, Andre Przywara wrote:  
> >>>>> On Wed, 10 May 2023 17:58:06 +0200
> >>>>> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>  
> >>>>>> 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@arm.com>  
> >>>>>>

....

> Now I am able to debug the code.
> 
> I wonder if the file system should be exposed as EFI simple file system
> so that EFI applications can access it, e.g. for GRUB to load initrd and
> the kernel.

Yes, this would be the ultimate goal. There is no file listing
functionality in semihosting, which is a bummer, but it should still work
if you know the file name. On top of the EDK-II shell or grub being able to
load an image, this would also allow to easily specify an initrd via the
kernel command line. This works with EDK-II's semihosting support.

But I see that this requires more work, IIUC we need to support
hostfs explicitly via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
At the moment this seems to assume that there is some underlying block
device, so this might need some refactoring.

As it stands right now, your much simpler patch seems to provide the same
functionality as mine, so I wonder if we should take this as a kind of
quick fix, to allow starting EFI apps loaded via hostfs (including
sandbox, btw).

Eventually, when introducing proper filesystem protocol support, this
patch probably would need to come back.

So I leave this up to you, but am fine with your simpler patch.

Cheers,
Andre

> 
> If we don't want to expose it, this change will be enough:
> 
> diff --git a/lib/efi_loader/efi_device_path.c
> b/lib/efi_loader/efi_device_path.c
> index e2e98a39be..058bdc1ee5 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -1203,7 +1203,7 @@ efi_status_t efi_dp_from_name(const char *dev,
> const char *devnr,
>          } else if (!strcmp(dev, "Uart")) {
>                  if (device)
>                          *device = efi_dp_from_uart();
> -       } else if (!strcmp(dev, "Mem")) {
> +       } else if (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs") ) {
>                  efi_get_image_parameters(&image_addr, &image_size);
> 
>                  if (device)
> 
> Best regards
> 
> Heinrich
> 

Re: [PATCH] efi: fix semihosting EFI payload booting

[Heinrich Schuchardt]

On 5/12/23 16:07, Andre Przywara wrote:
>> I wonder if the file system should be exposed as EFI simple file system
>> so that EFI applications can access it, e.g. for GRUB to load initrd and
>> the kernel.
> Yes, this would be the ultimate goal. There is no file listing
> functionality in semihosting, which is a bummer, but it should still work
> if you know the file name. On top of the EDK-II shell or grub being able to
> load an image, this would also allow to easily specify an initrd via the
> kernel command line. This works with EDK-II's semihosting support.
>
> But I see that this requires more work, IIUC we need to support
> hostfs explicitly via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
> At the moment this seems to assume that there is some underlying block
> device, so this might need some refactoring.
>
> As it stands right now, your much simpler patch seems to provide the same
> functionality as mine, so I wonder if we should take this as a kind of
> quick fix, to allow starting EFI apps loaded via hostfs (including
> sandbox, btw).
>
> Eventually, when introducing proper filesystem protocol support, this
> patch probably would need to come back.
>
> So I leave this up to you, but am fine with your simpler patch.
>
> Cheers,
> Andre
>

GRUB only consumes the block IO protocol so it would not benefit.

Linux consumes the simple file system protocol but passing file names
via the command line has been deprecated by the EFI maintainer.

Best regards

Heinrich