Re: [PATCH v6 09/12] efi_loader: Add a test app

Hi Ilias,

On Fri, 27 Sept 2024 at 07:51, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

Hi Simon,

On Fri, 27 Sept 2024 at 01:04, Simon Glass sjg@chromium.org 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 6f6fa8d629d..41083e7c137 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -564,6 +564,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+ +/*

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

What Heinrich keeps repeating here, is that after you call EBS, boot services must not be called again.

...

•   /* now exit for real */
  

•   ret = boottime->exit(handle, ret, 0, NULL);
  

IOW this is wrong. It might happen to work because not much has run and the memory will still be there. But you certainly should not rely on that

Well, not much has run because it is running the test program. So we know it will be OK.

One of the challenges we have is that we need to design things for testing, i.e. to make testing easy and practical. The call to exit() happily returns back to sandbox and all is well. It serves the purpose of the test, which is all the app is wanting to do.

So I have to ask, what problem are you seeing?

Thanks

/Ilias

...

•   /* We should never arrive here */
  

•   return ret;
  

+}

2.43.0

Regards, SImon