Re: [PATCH v2 36/39] efi: Add a test app

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 07.08.24 16:36, Simon Glass wrote:
> Hi Heinrich,
>
> On Tue, 6 Aug 2024 at 19:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 06.08.24 14:58, 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.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>    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 ab2c1c44364..4de05c6f2d6 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -528,4 +528,14 @@ 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
>>> 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+
>>> +/*
>>> + * 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 sevices\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);
>>
>> Please, have a look at chapter 7.4.6,
>> "EFI_BOOT_SERVICES.ExitBootServices" of the UEFI specification.
>>
>> After ExitBootServices() you cannot return anywhere. Boot services are
>> not available anymore. You can only invoke the UEFI runtime services
>> which include ResetSystem().
>
> Yes, understood, but this is a test, so returning is needed so that
> the test can check the output is correct.
>

ExitBootServices() calls dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL).
You cannot print anything afterwards as all devices are gone.

But you can check if a reboot occurs. This is enough to verify that a
ResetSystem() call placed after ExitBootServices() was executed.

We already have

* lib/efi_selftest/efi_selftest_exitbootservices.c
* lib/efi_selftest/efi_selftest_variables_runtime.c
* lib/efi_selftest/efi_selftest_set_virtual_address_map.c

which test different aspects of ExitBootServices() on the sandbox.

There is no need to duplicate these.

Best regards

Heinrich