diff mbox series

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

Message ID 20180219154827.215990-19-sjg@chromium.org
State Superseded
Delegated to: Alexander Graf
Headers show
Series efi: Enable basic sandbox support for EFI loader | expand

Commit Message

Simon Glass Feb. 19, 2018, 3:48 p.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
environemnt (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
Hello, world!
Test passed

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Rebase to master

Changes in v2:
- Rebase to master

 cmd/bootefi.c             | 24 +++++++++++++++++++-----
 include/efi_loader.h      |  3 +++
 lib/efi_loader/Kconfig    | 10 ++++++++++
 lib/efi_loader/Makefile   |  1 +
 lib/efi_loader/efi_test.c | 17 +++++++++++++++++
 5 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 lib/efi_loader/efi_test.c

Comments

Heinrich Schuchardt Feb. 19, 2018, 7:40 p.m. UTC | #1
On 02/19/2018 04:48 PM, 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
> environemnt (e.g. U-Boot does not need to reboot afterwards).
> 
> For now the test just outputs a message. To try it:

Please, explain what the use of the command shall be.
We already have bootefi selftest.
We shouldn't invent the wheel twice.

Best regards

Heinrich

> 
> ./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
> Hello, world!
> Test passed
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v3:
> - Rebase to master
> 
> Changes in v2:
> - Rebase to master
> 
>   cmd/bootefi.c             | 24 +++++++++++++++++++-----
>   include/efi_loader.h      |  3 +++
>   lib/efi_loader/Kconfig    | 10 ++++++++++
>   lib/efi_loader/Makefile   |  1 +
>   lib/efi_loader/efi_test.c | 17 +++++++++++++++++
>   5 files changed, 50 insertions(+), 5 deletions(-)
>   create mode 100644 lib/efi_loader/efi_test.c
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 2a5d66e798..d85d17a9b9 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -297,7 +297,6 @@ exit:
>   	return ret;
>   }
>   
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>   /**
>    * bootefi_test_prepare() - prepare to run an EFI test
>    *
> @@ -353,7 +352,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(unsigned long fdt_addr)
>   {
> @@ -388,6 +386,8 @@ static int do_bootefi_bootmgr_exec(unsigned long fdt_addr)
>   /* 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[])
>   {
> +	struct efi_loaded_image image;
> +	struct efi_object obj;
>   	char *saddr, *sfdt;
>   	unsigned long addr, fdt_addr = 0;
>   	efi_status_t r;
> @@ -406,11 +406,25 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   		memcpy((char *)addr, __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, "\\test",
>   					 (uintptr_t)&efi_selftest))
>   			return CMD_RET_FAILURE;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 831883287f..8bb2398c29 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -390,6 +390,9 @@ efi_status_t EFIAPI efi_set_variable(s16 *variable_name,
>   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 97658f1665..219fc3a51d 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -22,3 +22,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 2722265ee3..44ea145d5b 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>   obj-$(CONFIG_PARTITIONS) += efi_disk.o
>   obj-$(CONFIG_NET) += efi_net.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 0000000000..3fdce78b05
> --- /dev/null
> +++ b/lib/efi_loader/efi_test.c
> @@ -0,0 +1,17 @@
> +/*
> + * Copyright (c) 2017, Google Inc. All rights reserved.
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#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 May 16, 2018, 3:40 p.m. UTC | #2
Hi Heinrich,

On 19 February 2018 at 12:40, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 02/19/2018 04:48 PM, 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
>> environemnt (e.g. U-Boot does not need to reboot afterwards).
>>
>> For now the test just outputs a message. To try it:
>
>
> Please, explain what the use of the command shall be.
> We already have bootefi selftest.
> We shouldn't invent the wheel twice.

These are different wheels, With 'selftest' we build an EFI
application and run it as a binary. With 'test' we just call into the
U-Boot code. IMO the latter is much easier to debug and test since it
is a single application - e.g. it can be run entirely within gdb.

Regards,
Simon
diff mbox series

Patch

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 2a5d66e798..d85d17a9b9 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -297,7 +297,6 @@  exit:
 	return ret;
 }
 
-#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
 /**
  * bootefi_test_prepare() - prepare to run an EFI test
  *
@@ -353,7 +352,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(unsigned long fdt_addr)
 {
@@ -388,6 +386,8 @@  static int do_bootefi_bootmgr_exec(unsigned long fdt_addr)
 /* 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[])
 {
+	struct efi_loaded_image image;
+	struct efi_object obj;
 	char *saddr, *sfdt;
 	unsigned long addr, fdt_addr = 0;
 	efi_status_t r;
@@ -406,11 +406,25 @@  static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		memcpy((char *)addr, __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, "\\test",
 					 (uintptr_t)&efi_selftest))
 			return CMD_RET_FAILURE;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 831883287f..8bb2398c29 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -390,6 +390,9 @@  efi_status_t EFIAPI efi_set_variable(s16 *variable_name,
 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 97658f1665..219fc3a51d 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -22,3 +22,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 2722265ee3..44ea145d5b 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -23,3 +23,4 @@  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
 obj-$(CONFIG_PARTITIONS) += efi_disk.o
 obj-$(CONFIG_NET) += efi_net.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 0000000000..3fdce78b05
--- /dev/null
+++ b/lib/efi_loader/efi_test.c
@@ -0,0 +1,17 @@ 
+/*
+ * Copyright (c) 2017, Google Inc. All rights reserved.
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#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;
+}