diff mbox series

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

Message ID 20180808095433.230882-7-sjg@chromium.org
State Superseded, archived
Delegated to: Alexander Graf
Headers show
Series efi: Enable sandbox support for EFI loader | expand

Commit Message

Simon Glass Aug. 8, 2018, 9:54 a.m. UTC
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>
---

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

Comments

Alexander Graf Aug. 26, 2018, 4:58 p.m. UTC | #1
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;
> +}
>
Simon Glass Sept. 14, 2018, 3:46 p.m. UTC | #2
Hi Alex,

On 26 August 2018 at 18:58, Alexander Graf <agraf@suse.de> wrote:
>
>
> 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?

I'm not saying you can't run the selftest suite, but that is for a
different purpose. This is just a sanity check. I'll change the name
to 'ping instead'.

Regards,
Simon
diff mbox series

Patch

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;
+}