[U-Boot] [PATCH v9 06/18] efi: sandbox: Add a simple 'bootefi test' command

[Alexander Graf <agraf@suse.de>]

On 08.08.18 11:54, Simon Glass wrote:
> This jumps to test code which can call directly into the EFI support. It
> does not need a separate image so it is easy to write tests with it.
> 
> This test can be executed without causing problems to the run-time
> environment (e.g. U-Boot does not need to reboot afterwards).
> 
> For now the test just outputs a message. To try it:
> 
> ./sandbox/u-boot -c "bootefi test"
> U-Boot 2017.09-00204-g696c9855fe (Sep 17 2017 - 16:43:53 -0600)
> 
> DRAM:  128 MiB
> MMC:
> Using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> SCSI:  Net:   No ethernet found.
> IDE:   Bus 0: not available
> Found 0 disks
> WARNING: booting without device tree
> Hello, world!
> Test passed
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

I think I'm missing the point of this exercise. What's the problem with
running the selftest suite? Its set of tests that it runs can be
configured using an environment variable just fine?


Alex

> ---
> 
> Changes in v9: None
> Changes in v8: None
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  cmd/bootefi.c             | 26 ++++++++++++++++++++------
>  include/efi_loader.h      |  3 +++
>  lib/efi_loader/Kconfig    | 10 ++++++++++
>  lib/efi_loader/Makefile   |  1 +
>  lib/efi_loader/efi_test.c | 16 ++++++++++++++++
>  5 files changed, 50 insertions(+), 6 deletions(-)
>  create mode 100644 lib/efi_loader/efi_test.c
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index af13492836d..edb7f786a27 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -409,7 +409,6 @@ exit:
>  	return ret;
>  }
>  
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>  /**
>   * bootefi_test_prepare() - prepare to run an EFI test
>   *
> @@ -465,7 +464,6 @@ static void bootefi_test_finish(struct efi_loaded_image *image,
>  	free(image->load_options);
>  	list_del(&obj->link);
>  }
> -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
>  
>  static int do_bootefi_bootmgr_exec(void)
>  {
> @@ -497,8 +495,10 @@ static int do_bootefi_bootmgr_exec(void)
>  /* Interpreter command to boot an arbitrary EFI image from memory */
>  static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
> -	unsigned long addr;
> +	struct efi_loaded_image image;
> +	struct efi_object obj;
>  	char *saddr;
> +	unsigned long addr;
>  	efi_status_t r;
>  	unsigned long fdt_addr;
>  	void *fdt;
> @@ -545,11 +545,25 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  		memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size);
>  	} else
>  #endif
> +	if (IS_ENABLED(CONFIG_BOOTEFI_TEST) && !strcmp(argv[1], "test")) {
> +		int ret;
> +
> +		if (bootefi_test_prepare(&image, &obj, "\\test",
> +					 (ulong)&efi_test))
> +			return CMD_RET_FAILURE;
> +
> +		ret = efi_test(&image, &systab);
> +		bootefi_test_finish(&image, &obj);
> +		if (ret) {
> +			printf("Test failed: err=%d\n", ret);
> +			return CMD_RET_FAILURE;
> +		}
> +		printf("Test passed\n");
> +
> +		return 0;
> +	}
>  #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>  	if (!strcmp(argv[1], "selftest")) {
> -		struct efi_loaded_image image;
> -		struct efi_object obj;
> -
>  		if (bootefi_test_prepare(&image, &obj, "\\selftest",
>  					 (uintptr_t)&efi_selftest))
>  			return CMD_RET_FAILURE;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 57ca5502726..616dfae7b70 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -457,6 +457,9 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
>  void *efi_bootmgr_load(struct efi_device_path **device_path,
>  		       struct efi_device_path **file_path);
>  
> +/* Perform EFI tests */
> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systab);
> +
>  #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
>  
>  /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index bfd7b19d791..254b18da744 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -23,3 +23,13 @@ config EFI_LOADER_BOUNCE_BUFFER
>  	  Some hardware does not support DMA to full 64bit addresses. For this
>  	  hardware we can create a bounce buffer so that payloads don't have to
>  	  worry about platform details.
> +
> +config BOOTEFI_TEST
> +	bool "Provide a test for the EFI loader"
> +	depends on EFI_LOADER && SANDBOX
> +	default y
> +	help
> +	  Provides a test of the EFI loader functionality accessed via the
> +	  command line ('bootefi test'). This runs within U-Boot so does not
> +	  need a separate EFI application to work. It aims to include coverage
> +	  of all EFI code which can be accessed within sandbox.
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 1ffbf52a898..e6a23d96122 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -27,3 +27,4 @@ obj-$(CONFIG_PARTITIONS) += efi_disk.o
>  obj-$(CONFIG_NET) += efi_net.o
>  obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> +obj-$(CONFIG_BOOTEFI_TEST) += efi_test.o
> diff --git a/lib/efi_loader/efi_test.c b/lib/efi_loader/efi_test.c
> new file mode 100644
> index 00000000000..4b8d49f324b
> --- /dev/null
> +++ b/lib/efi_loader/efi_test.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2017, Google Inc.
> + */
> +
> +#include <common.h>
> +#include <efi_api.h>
> +
> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systable)
> +{
> +	struct efi_simple_text_output_protocol *con_out = systable->con_out;
> +
> +	con_out->output_string(con_out, L"Hello, world!\n");
> +
> +	return 0;
> +}
>