On Mon, Oct 28, 2024 at 05:47:08PM +0100, Heinrich Schuchardt wrote:
> On 10/28/24 16:17, Tom Rini wrote:
> > On Mon, Oct 28, 2024 at 06:59:10AM +0100, Heinrich Schuchardt wrote:
> > > On 10/22/24 14:00, 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 <s...@chromium.org>
> > > > ---
> > > >
> > > > (no changes since v7)
> > > >
> > > > 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 69b2c9360d8..6ced29da719 100644
> > > > --- a/lib/efi_loader/Kconfig
> > > > +++ b/lib/efi_loader/Kconfig
> > > > @@ -565,6 +565,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 00d18966f9e..87131ab911d 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..feb444c92e9
> > > > --- /dev/null
> > > > +++ b/lib/efi_loader/testapp.c
> > > > @@ -0,0 +1,68 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > >
> > > This is not a valid SPDX identifier. Please, use GPL-2.0-or-later.
> >
> > We should switch to the proper one here, but there are numerous examples
> > of this today.
> >
> > > > +/*
> > > > + * Hello world EFI application
> > > > + *
> > > > + * Copyright 2024 Google LLC
> > > > + * Written by Simon Glass <s...@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 sevices\n");
> > >
> > > %s/sevices/services/g
> > >
> > > > +
> > > > + /* 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);
> > >
> > > As written before boot services are not available after
> > > ExitBootServices(). Please, call the ResetSystem() service.
> >
> > Right, but the point is to not do that and instead test what happens. I
> > think Ilias had said we need to make some big loud print on console that
> > you must reset the system for it to be usable afterwards, would that
> > be enough?
> >
>
> There is no point in testing what happens as this call is not allowable.
>
> Do we really need to zero out the bootservices table for people who do
> not read the spec to find out the hard way?Then perhaps this is a case where "test causes U-Boot to reset, check for that as the result" is what the test needs to do, as it's not an arbitrary reset? -- Tom
signature.asc
Description: PGP signature
