diff mbox series

[v9,02/11] efi_loader: Add a test app

Message ID 20241029192222.362445-3-sjg@chromium.org
State Needs Review / ACK
Delegated to: Tom Rini
Headers show
Series efi: Add a test for EFI bootmeth | expand

Commit Message

Simon Glass Oct. 29, 2024, 7:22 p.m. UTC
Add a simple app to use for testing. This is intended to do whatever it
needs to for testing purposes. For now it just prints a message and
exits boot services.

There was a considerable amount of discussion about whether it is OK to
call exit-boot-services and then return to U-Boot. This is not normally
done in a real application, since exit-boot-services is used to
completely disconnect from U-Boot. However, since this is a test, we
need to check the results of running the app, so returning is necessary.
It works correctly and it provides a nice model of how to test the EFI
code in a simple way.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v9:
- Update license
- Fix 'sevices' typo

Changes in v7:
- Update commit message

 lib/efi_loader/Kconfig   | 10 ++++++
 lib/efi_loader/Makefile  |  1 +
 lib/efi_loader/testapp.c | 68 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 lib/efi_loader/testapp.c

Comments

Tom Rini Oct. 29, 2024, 8:12 p.m. UTC | #1
On Tue, Oct 29, 2024 at 08:22:10PM +0100, Simon Glass wrote:

> Add a simple app to use for testing. This is intended to do whatever it
> needs to for testing purposes. For now it just prints a message and
> exits boot services.
> 
> There was a considerable amount of discussion about whether it is OK to
> call exit-boot-services and then return to U-Boot. This is not normally
> done in a real application, since exit-boot-services is used to
> completely disconnect from U-Boot. However, since this is a test, we
> need to check the results of running the app, so returning is necessary.
> It works correctly and it provides a nice model of how to test the EFI
> code in a simple way.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

This approach has been specifically rejected with an explained
rationale: It breaks how UEFI applications work and you cannot run
further UEFI tests in sandbox without resetting.

Since as you note, you can't reset in a C-based test, rework this to be
a python test where we can safely reset the system and verify that. I
believe Heinrich even noted that a test which checks ExitBootServices()
working as expected would be helpful as we only have a watchdog test
currently.
Simon Glass Oct. 30, 2024, 7:44 a.m. UTC | #2
Hi Tom,

On Tue, 29 Oct 2024 at 21:13, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Oct 29, 2024 at 08:22:10PM +0100, Simon Glass wrote:
>
> > Add a simple app to use for testing. This is intended to do whatever it
> > needs to for testing purposes. For now it just prints a message and
> > exits boot services.
> >
> > There was a considerable amount of discussion about whether it is OK to
> > call exit-boot-services and then return to U-Boot. This is not normally
> > done in a real application, since exit-boot-services is used to
> > completely disconnect from U-Boot. However, since this is a test, we
> > need to check the results of running the app, so returning is necessary.
> > It works correctly and it provides a nice model of how to test the EFI
> > code in a simple way.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> This approach has been specifically rejected with an explained
> rationale: It breaks how UEFI applications work and you cannot run
> further UEFI tests in sandbox without resetting.
>
> Since as you note, you can't reset in a C-based test, rework this to be
> a python test where we can safely reset the system and verify that. I
> believe Heinrich even noted that a test which checks ExitBootServices()
> working as expected would be helpful as we only have a watchdog test
> currently.

Unfortunately I believe this is the wrong direction, as we are unable
to check the state of anything once a reset has happened. Thus I
cannot build on this test, e.g. to see what happened to the devices
and whether they are still bound. It becomes just a 'black box' test.
None of the other bootflow tests work this way, BTW.

Regards,
Simon
Tom Rini Oct. 30, 2024, 3:10 p.m. UTC | #3
On Wed, Oct 30, 2024 at 08:44:04AM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 29 Oct 2024 at 21:13, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Oct 29, 2024 at 08:22:10PM +0100, Simon Glass wrote:
> >
> > > Add a simple app to use for testing. This is intended to do whatever it
> > > needs to for testing purposes. For now it just prints a message and
> > > exits boot services.
> > >
> > > There was a considerable amount of discussion about whether it is OK to
> > > call exit-boot-services and then return to U-Boot. This is not normally
> > > done in a real application, since exit-boot-services is used to
> > > completely disconnect from U-Boot. However, since this is a test, we
> > > need to check the results of running the app, so returning is necessary.
> > > It works correctly and it provides a nice model of how to test the EFI
> > > code in a simple way.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > This approach has been specifically rejected with an explained
> > rationale: It breaks how UEFI applications work and you cannot run
> > further UEFI tests in sandbox without resetting.
> >
> > Since as you note, you can't reset in a C-based test, rework this to be
> > a python test where we can safely reset the system and verify that. I
> > believe Heinrich even noted that a test which checks ExitBootServices()
> > working as expected would be helpful as we only have a watchdog test
> > currently.
> 
> Unfortunately I believe this is the wrong direction, as we are unable
> to check the state of anything once a reset has happened. Thus I
> cannot build on this test, e.g. to see what happened to the devices
> and whether they are still bound. It becomes just a 'black box' test.
> None of the other bootflow tests work this way, BTW.

That's unfortunate in a number of ways then really. It's good when we
can test the state machine is still where we expect it. It's better when
we can test that the functionality works for real. It would probably be
generically useful to have some verification functions that can dump the
state of DM before we finish ourselves up and hand things over to the
OS. We could even make use of this in test/py/tests/test_net_boot.py
when we're booting Linux to make sure it's working as expected.
Heinrich Schuchardt Oct. 30, 2024, 4:55 p.m. UTC | #4
On 10/29/24 20:22, Simon Glass wrote:
> Add a simple app to use for testing. This is intended to do whatever it
> needs to for testing purposes. For now it just prints a message and
> exits boot services.
>
> There was a considerable amount of discussion about whether it is OK to
> call exit-boot-services and then return to U-Boot. This is not normally
> done in a real application, since exit-boot-services is used to
> completely disconnect from U-Boot. However, since this is a test, we
> need to check the results of running the app, so returning is necessary.
> It works correctly and it provides a nice model of how to test the EFI
> code in a simple way.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v9:
> - Update license
> - Fix 'sevices' typo
>
> Changes in v7:
> - Update commit message
>
>   lib/efi_loader/Kconfig   | 10 ++++++
>   lib/efi_loader/Makefile  |  1 +
>   lib/efi_loader/testapp.c | 68 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 79 insertions(+)
>   create mode 100644 lib/efi_loader/testapp.c
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 37572c82f54..44319729bda 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -584,6 +584,16 @@ config BOOTEFI_HELLO_COMPILE
>   	  No additional space will be required in the resulting U-Boot binary
>   	  when this option is enabled.
>
> +config BOOTEFI_TESTAPP_COMPILE
> +	bool "Compile an EFI test app for testing"
> +	default y
> +	help
> +	  This compiles an app designed for testing. It is packed into an image
> +	  by the test.py testing frame in the setup_efi_image() function.
> +
> +	  No additional space will be required in the resulting U-Boot binary
> +	  when this option is enabled.
> +
>   endif
>
>   source "lib/efi/Kconfig"
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 8ec240cb864..7c8b2dd1ad6 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -20,6 +20,7 @@ apps-$(CONFIG_EFI_LOAD_FILE2_INITRD) += initrddump
>   ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
>   apps-y += dtbdump
>   endif
> +apps-$(CONFIG_BOOTEFI_TESTAPP_COMPILE) += testapp
>
>   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>   obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
> diff --git a/lib/efi_loader/testapp.c b/lib/efi_loader/testapp.c
> new file mode 100644
> index 00000000000..a28fc17ce5b
> --- /dev/null
> +++ b/lib/efi_loader/testapp.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hello world EFI application
> + *
> + * Copyright 2024 Google LLC
> + * Written by Simon Glass <sjg@chromium.org>
> + *
> + * This test program is used to test the invocation of an EFI application.
> + * It writes a few messages to the console and then exits boot services
> + */
> +
> +#include <efi_api.h>
> +
> +static const efi_guid_t loaded_image_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID;
> +
> +static struct efi_system_table *systable;
> +static struct efi_boot_services *boottime;
> +static struct efi_simple_text_output_protocol *con_out;
> +
> +/**
> + * efi_main() - entry point of the EFI application.
> + *
> + * @handle:	handle of the loaded image
> + * @systab:	system table
> + * Return:	status code
> + */
> +efi_status_t EFIAPI efi_main(efi_handle_t handle,
> +			     struct efi_system_table *systab)
> +{
> +	struct efi_loaded_image *loaded_image;
> +	efi_status_t ret;
> +	efi_uintn_t map_size;
> +	efi_uintn_t map_key;
> +	efi_uintn_t desc_size;
> +	u32 desc_version;
> +
> +	systable = systab;
> +	boottime = systable->boottime;
> +	con_out = systable->con_out;
> +
> +	/* Get the loaded image protocol */
> +	ret = boottime->open_protocol(handle, &loaded_image_guid,
> +				      (void **)&loaded_image, NULL, NULL,
> +				      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +	if (ret != EFI_SUCCESS) {
> +		con_out->output_string
> +			(con_out, u"Cannot open loaded image protocol\r\n");
> +		goto out;
> +	}
> +
> +	/* UEFI requires CR LF */
> +	con_out->output_string(con_out, u"U-Boot test app for EFI_LOADER\r\n");
> +
> +out:
> +	map_size = 0;
> +	ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
> +				       &desc_version);
> +	con_out->output_string(con_out, u"Exiting boot services\n");
> +
> +	/* exit boot services so that this part of U-Boot can be tested */
> +	boottime->exit_boot_services(handle, map_key);

Hello Simon,

I am not NAKing this patch. But maybe you could rethink if this call
tells us anything new about the correct function of ExitBootServices().

We already have a test of ExitBootServices() in lib/efi_selftest/ and
maybe we should extend that test to check more.

By removing ExitBootServices() we could keep U-Boot in a well defined
state which will allow us to run any number of bootmeth tests in
sequence and keep the EFI sub-system in a consistent state.

Best regards

Heinrich


> +
> +	/* now exit for real */
> +	ret = boottime->exit(handle, ret, 0, NULL);
> +
> +	/* We should never arrive here */
> +	return ret;
> +}
Ilias Apalodimas Oct. 30, 2024, 5:54 p.m. UTC | #5
Hi Heinrich



On Wed, 30 Oct 2024 at 18:55, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/29/24 20:22, Simon Glass wrote:
> > Add a simple app to use for testing. This is intended to do whatever it
> > needs to for testing purposes. For now it just prints a message and
> > exits boot services.
> >
> > There was a considerable amount of discussion about whether it is OK to
> > call exit-boot-services and then return to U-Boot. This is not normally
> > done in a real application, since exit-boot-services is used to
> > completely disconnect from U-Boot. However, since this is a test, we
> > need to check the results of running the app, so returning is necessary.
> > It works correctly and it provides a nice model of how to test the EFI
> > code in a simple way.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v9:
> > - Update license
> > - Fix 'sevices' typo
> >
> > Changes in v7:
> > - Update commit message
> >
> >   lib/efi_loader/Kconfig   | 10 ++++++
> >   lib/efi_loader/Makefile  |  1 +
> >   lib/efi_loader/testapp.c | 68 ++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 79 insertions(+)
> >   create mode 100644 lib/efi_loader/testapp.c
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 37572c82f54..44319729bda 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -584,6 +584,16 @@ config BOOTEFI_HELLO_COMPILE
> >         No additional space will be required in the resulting U-Boot binary
> >         when this option is enabled.
> >
> > +config BOOTEFI_TESTAPP_COMPILE
> > +     bool "Compile an EFI test app for testing"
> > +     default y
> > +     help
> > +       This compiles an app designed for testing. It is packed into an image
> > +       by the test.py testing frame in the setup_efi_image() function.
> > +
> > +       No additional space will be required in the resulting U-Boot binary
> > +       when this option is enabled.
> > +
> >   endif
> >
> >   source "lib/efi/Kconfig"
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index 8ec240cb864..7c8b2dd1ad6 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -20,6 +20,7 @@ apps-$(CONFIG_EFI_LOAD_FILE2_INITRD) += initrddump
> >   ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
> >   apps-y += dtbdump
> >   endif
> > +apps-$(CONFIG_BOOTEFI_TESTAPP_COMPILE) += testapp
> >
> >   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> >   obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
> > diff --git a/lib/efi_loader/testapp.c b/lib/efi_loader/testapp.c
> > new file mode 100644
> > index 00000000000..a28fc17ce5b
> > --- /dev/null
> > +++ b/lib/efi_loader/testapp.c
> > @@ -0,0 +1,68 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Hello world EFI application
> > + *
> > + * Copyright 2024 Google LLC
> > + * Written by Simon Glass <sjg@chromium.org>
> > + *
> > + * This test program is used to test the invocation of an EFI application.
> > + * It writes a few messages to the console and then exits boot services
> > + */
> > +
> > +#include <efi_api.h>
> > +
> > +static const efi_guid_t loaded_image_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID;
> > +
> > +static struct efi_system_table *systable;
> > +static struct efi_boot_services *boottime;
> > +static struct efi_simple_text_output_protocol *con_out;
> > +
> > +/**
> > + * efi_main() - entry point of the EFI application.
> > + *
> > + * @handle:  handle of the loaded image
> > + * @systab:  system table
> > + * Return:   status code
> > + */
> > +efi_status_t EFIAPI efi_main(efi_handle_t handle,
> > +                          struct efi_system_table *systab)
> > +{
> > +     struct efi_loaded_image *loaded_image;
> > +     efi_status_t ret;
> > +     efi_uintn_t map_size;
> > +     efi_uintn_t map_key;
> > +     efi_uintn_t desc_size;
> > +     u32 desc_version;
> > +
> > +     systable = systab;
> > +     boottime = systable->boottime;
> > +     con_out = systable->con_out;
> > +
> > +     /* Get the loaded image protocol */
> > +     ret = boottime->open_protocol(handle, &loaded_image_guid,
> > +                                   (void **)&loaded_image, NULL, NULL,
> > +                                   EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > +     if (ret != EFI_SUCCESS) {
> > +             con_out->output_string
> > +                     (con_out, u"Cannot open loaded image protocol\r\n");
> > +             goto out;
> > +     }
> > +
> > +     /* UEFI requires CR LF */
> > +     con_out->output_string(con_out, u"U-Boot test app for EFI_LOADER\r\n");
> > +
> > +out:
> > +     map_size = 0;
> > +     ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
> > +                                    &desc_version);
> > +     con_out->output_string(con_out, u"Exiting boot services\n");
> > +
> > +     /* exit boot services so that this part of U-Boot can be tested */
> > +     boottime->exit_boot_services(handle, map_key);
>
> Hello Simon,
>
> I am not NAKing this patch. But maybe you could rethink if this call
> tells us anything new about the correct function of ExitBootServices().
>
> We already have a test of ExitBootServices() in lib/efi_selftest/ and
> maybe we should extend that test to check more.
>
> By removing ExitBootServices() we could keep U-Boot in a well defined
> state which will allow us to run any number of bootmeth tests in
> sequence and keep the EFI sub-system in a consistent state.

FWIW I proposed exactly the same in [0].

[0] https://lore.kernel.org/u-boot/CAC_iWjJYqrh6MS50d6MxdStJFGg2pWnf1PHwQ9muYDPmOBP7Lg@mail.gmail.com/

Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
>
> > +
> > +     /* now exit for real */
> > +     ret = boottime->exit(handle, ret, 0, NULL);
> > +
> > +     /* We should never arrive here */
> > +     return ret;
> > +}
>
Simon Glass Oct. 31, 2024, 6:02 p.m. UTC | #6
Hi,

On Wed, 30 Oct 2024 at 18:55, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Heinrich
>
>
>
> On Wed, 30 Oct 2024 at 18:55, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 10/29/24 20:22, Simon Glass wrote:
> > > Add a simple app to use for testing. This is intended to do whatever it
> > > needs to for testing purposes. For now it just prints a message and
> > > exits boot services.
> > >
> > > There was a considerable amount of discussion about whether it is OK to
> > > call exit-boot-services and then return to U-Boot. This is not normally
> > > done in a real application, since exit-boot-services is used to
> > > completely disconnect from U-Boot. However, since this is a test, we
> > > need to check the results of running the app, so returning is necessary.
> > > It works correctly and it provides a nice model of how to test the EFI
> > > code in a simple way.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > > Changes in v9:
> > > - Update license
> > > - Fix 'sevices' typo
> > >
> > > Changes in v7:
> > > - Update commit message
> > >
> > >   lib/efi_loader/Kconfig   | 10 ++++++
> > >   lib/efi_loader/Makefile  |  1 +
> > >   lib/efi_loader/testapp.c | 68 ++++++++++++++++++++++++++++++++++++++++
> > >   3 files changed, 79 insertions(+)
> > >   create mode 100644 lib/efi_loader/testapp.c
> > >
> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > index 37572c82f54..44319729bda 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -584,6 +584,16 @@ config BOOTEFI_HELLO_COMPILE
> > >         No additional space will be required in the resulting U-Boot binary
> > >         when this option is enabled.
> > >
> > > +config BOOTEFI_TESTAPP_COMPILE
> > > +     bool "Compile an EFI test app for testing"
> > > +     default y
> > > +     help
> > > +       This compiles an app designed for testing. It is packed into an image
> > > +       by the test.py testing frame in the setup_efi_image() function.
> > > +
> > > +       No additional space will be required in the resulting U-Boot binary
> > > +       when this option is enabled.
> > > +
> > >   endif
> > >
> > >   source "lib/efi/Kconfig"
> > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > > index 8ec240cb864..7c8b2dd1ad6 100644
> > > --- a/lib/efi_loader/Makefile
> > > +++ b/lib/efi_loader/Makefile
> > > @@ -20,6 +20,7 @@ apps-$(CONFIG_EFI_LOAD_FILE2_INITRD) += initrddump
> > >   ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
> > >   apps-y += dtbdump
> > >   endif
> > > +apps-$(CONFIG_BOOTEFI_TESTAPP_COMPILE) += testapp
> > >
> > >   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> > >   obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
> > > diff --git a/lib/efi_loader/testapp.c b/lib/efi_loader/testapp.c
> > > new file mode 100644
> > > index 00000000000..a28fc17ce5b
> > > --- /dev/null
> > > +++ b/lib/efi_loader/testapp.c
> > > @@ -0,0 +1,68 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * Hello world EFI application
> > > + *
> > > + * Copyright 2024 Google LLC
> > > + * Written by Simon Glass <sjg@chromium.org>
> > > + *
> > > + * This test program is used to test the invocation of an EFI application.
> > > + * It writes a few messages to the console and then exits boot services
> > > + */
> > > +
> > > +#include <efi_api.h>
> > > +
> > > +static const efi_guid_t loaded_image_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID;
> > > +
> > > +static struct efi_system_table *systable;
> > > +static struct efi_boot_services *boottime;
> > > +static struct efi_simple_text_output_protocol *con_out;
> > > +
> > > +/**
> > > + * efi_main() - entry point of the EFI application.
> > > + *
> > > + * @handle:  handle of the loaded image
> > > + * @systab:  system table
> > > + * Return:   status code
> > > + */
> > > +efi_status_t EFIAPI efi_main(efi_handle_t handle,
> > > +                          struct efi_system_table *systab)
> > > +{
> > > +     struct efi_loaded_image *loaded_image;
> > > +     efi_status_t ret;
> > > +     efi_uintn_t map_size;
> > > +     efi_uintn_t map_key;
> > > +     efi_uintn_t desc_size;
> > > +     u32 desc_version;
> > > +
> > > +     systable = systab;
> > > +     boottime = systable->boottime;
> > > +     con_out = systable->con_out;
> > > +
> > > +     /* Get the loaded image protocol */
> > > +     ret = boottime->open_protocol(handle, &loaded_image_guid,
> > > +                                   (void **)&loaded_image, NULL, NULL,
> > > +                                   EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > > +     if (ret != EFI_SUCCESS) {
> > > +             con_out->output_string
> > > +                     (con_out, u"Cannot open loaded image protocol\r\n");
> > > +             goto out;
> > > +     }
> > > +
> > > +     /* UEFI requires CR LF */
> > > +     con_out->output_string(con_out, u"U-Boot test app for EFI_LOADER\r\n");
> > > +
> > > +out:
> > > +     map_size = 0;
> > > +     ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
> > > +                                    &desc_version);
> > > +     con_out->output_string(con_out, u"Exiting boot services\n");
> > > +
> > > +     /* exit boot services so that this part of U-Boot can be tested */
> > > +     boottime->exit_boot_services(handle, map_key);
> >
> > Hello Simon,
> >
> > I am not NAKing this patch. But maybe you could rethink if this call
> > tells us anything new about the correct function of ExitBootServices().
> >
> > We already have a test of ExitBootServices() in lib/efi_selftest/ and
> > maybe we should extend that test to check more.
> >
> > By removing ExitBootServices() we could keep U-Boot in a well defined
> > state which will allow us to run any number of bootmeth tests in
> > sequence and keep the EFI sub-system in a consistent state.

In the interests of trying to make some progress, I will drop that
line and worry about it later, when we can come up with follow-on
tests. It is exactly the 'exit-boot-services' call which was stopping
Ubuntu from booting. But since I don't have any test pieces to check
what happens in that situation, we can leave it for now.

>
> FWIW I proposed exactly the same in [0].
>
> [0] https://lore.kernel.org/u-boot/CAC_iWjJYqrh6MS50d6MxdStJFGg2pWnf1PHwQ9muYDPmOBP7Lg@mail.gmail.com/
>
> Thanks
> /Ilias
> >
> > Best regards
> >
> > Heinrich
> >
> >
> > > +
> > > +     /* now exit for real */
> > > +     ret = boottime->exit(handle, ret, 0, NULL);
> > > +
> > > +     /* We should never arrive here */
> > > +     return ret;
> > > +}
> >

Regards,
Simon
diff mbox series

Patch

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 37572c82f54..44319729bda 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -584,6 +584,16 @@  config BOOTEFI_HELLO_COMPILE
 	  No additional space will be required in the resulting U-Boot binary
 	  when this option is enabled.
 
+config BOOTEFI_TESTAPP_COMPILE
+	bool "Compile an EFI test app for testing"
+	default y
+	help
+	  This compiles an app designed for testing. It is packed into an image
+	  by the test.py testing frame in the setup_efi_image() function.
+
+	  No additional space will be required in the resulting U-Boot binary
+	  when this option is enabled.
+
 endif
 
 source "lib/efi/Kconfig"
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 8ec240cb864..7c8b2dd1ad6 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -20,6 +20,7 @@  apps-$(CONFIG_EFI_LOAD_FILE2_INITRD) += initrddump
 ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
 apps-y += dtbdump
 endif
+apps-$(CONFIG_BOOTEFI_TESTAPP_COMPILE) += testapp
 
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
diff --git a/lib/efi_loader/testapp.c b/lib/efi_loader/testapp.c
new file mode 100644
index 00000000000..a28fc17ce5b
--- /dev/null
+++ b/lib/efi_loader/testapp.c
@@ -0,0 +1,68 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Hello world EFI application
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * This test program is used to test the invocation of an EFI application.
+ * It writes a few messages to the console and then exits boot services
+ */
+
+#include <efi_api.h>
+
+static const efi_guid_t loaded_image_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID;
+
+static struct efi_system_table *systable;
+static struct efi_boot_services *boottime;
+static struct efi_simple_text_output_protocol *con_out;
+
+/**
+ * efi_main() - entry point of the EFI application.
+ *
+ * @handle:	handle of the loaded image
+ * @systab:	system table
+ * Return:	status code
+ */
+efi_status_t EFIAPI efi_main(efi_handle_t handle,
+			     struct efi_system_table *systab)
+{
+	struct efi_loaded_image *loaded_image;
+	efi_status_t ret;
+	efi_uintn_t map_size;
+	efi_uintn_t map_key;
+	efi_uintn_t desc_size;
+	u32 desc_version;
+
+	systable = systab;
+	boottime = systable->boottime;
+	con_out = systable->con_out;
+
+	/* Get the loaded image protocol */
+	ret = boottime->open_protocol(handle, &loaded_image_guid,
+				      (void **)&loaded_image, NULL, NULL,
+				      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+	if (ret != EFI_SUCCESS) {
+		con_out->output_string
+			(con_out, u"Cannot open loaded image protocol\r\n");
+		goto out;
+	}
+
+	/* UEFI requires CR LF */
+	con_out->output_string(con_out, u"U-Boot test app for EFI_LOADER\r\n");
+
+out:
+	map_size = 0;
+	ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
+				       &desc_version);
+	con_out->output_string(con_out, u"Exiting boot services\n");
+
+	/* exit boot services so that this part of U-Boot can be tested */
+	boottime->exit_boot_services(handle, map_key);
+
+	/* now exit for real */
+	ret = boottime->exit(handle, ret, 0, NULL);
+
+	/* We should never arrive here */
+	return ret;
+}