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

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;

+}