[PATCH v2 00/12] cmd: bootefi: refactor the code for bootmgr

[PATCH v2 00/12] cmd: bootefi: refactor the code for bootmgr

[AKASHI Takahiro]

This patch set is motivated by the discussion[1] regarding
CONFIG_BOOTEFI_BOOTMGR option.

At the end, bootefi.c will be decomposed into two parts, one for
providing the command itself and one for implementing helper functions.
EFI_LOADER will now be available without CONFIG_CMDLINE or specifically
CONFIG_CMD_BOOTEFI if invoked via bootmeth/bootstd.

Then, EFI_LOADER library side will be further split into two options
for fine-grain control:
CONFIG_EFI_BINARY_EXEC: execute UEFI binaries which are to be explicitly
    loaded by U-Boot's load commands/functions or other methods
    (like a jtag debugger?)
    It supports bootmeth_efi as well as "bootefi <addr>|hello"(/"bootm"?).

CONFIG_EFI_BOOTMGR: provide EFI boot manger functionality
    It supports bootmeth_efi_mgr as well as "bootefi bootmgr".

As such, We will no longer need CONFIG_EFI_BINARY_EXEC if we want to only
make use of the UEFI boot manger for booting a next stage OS.

Prerequisite
============
This patch set is based on top of Tom's "next" branch.

Patches
=======
Patch#1-#12: I hope that those commits show step-by-step refactoring
    without introducing degradation.

Tests
=====
* run UT efi_selftest on sandbox locally
* run test_efi_bootmgr on sandbox locally

Unfortunately, I could not submit a pull request for CI test.

Changes
=======
v2 (Nov 21, 2023)
* rebased onto Tom's next branch
* remove already merged commits
* revise commit messages
* add patch #5 which was split from ex-patch#5
RFC (Oct 26, 2023)

[1] https://lists.denx.de/pipermail/u-boot/2023-October/534598.html

AKASHI Takahiro (12):
  cmd: bootefi: unfold do_bootefi_image()
  cmd: bootefi: re-organize do_bootefi()
  cmd: bootefi: carve out EFI boot manager interface
  cmd: bootefi: carve out binary execution interface
  cmd: bootefi: localize global device paths for efi_selftest
  cmd: bootefi: move library interfaces under lib/efi_loader
  cmd: efidebug: ease efi configuration dependency
  bootmeth: use efi_loader interfaces instead of bootefi command
  efi_loader: split unrelated code from efi_bootmgr.c
  efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR
  net: tftp: remove explicit efi configuration dependency
  fs: remove explicit efi configuration dependency

 boot/Kconfig                     |   4 +-
 boot/Makefile                    |   2 +-
 boot/bootm_os.c                  |  31 +-
 boot/bootmeth_efi.c              |   8 +-
 boot/bootmeth_efi_mgr.c          |   2 +-
 cmd/Kconfig                      |  21 +-
 cmd/bootefi.c                    | 670 +++++--------------------------
 cmd/efidebug.c                   |   4 +-
 fs/fs.c                          |   7 +-
 include/efi_loader.h             |  34 +-
 lib/efi_loader/Kconfig           |  11 +-
 lib/efi_loader/Makefile          |   2 +-
 lib/efi_loader/efi_bootmgr.c     |  37 ++
 lib/efi_loader/efi_device_path.c |   3 +-
 lib/efi_loader/efi_helper.c      | 498 ++++++++++++++++++++++-
 net/tftp.c                       |  10 +-
 test/boot/bootflow.c             |   2 +-
 17 files changed, 700 insertions(+), 646 deletions(-)

-- 
2.34.1

[PATCH v2 01/12] cmd: bootefi: unfold do_bootefi_image()

[AKASHI Takahiro]

Unfold do_bootefi_image() into do_bootefi() in order to make it easier
to re-organize do_bootefi() in the next commit.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/bootefi.c | 101 ++++++++++++++++++--------------------------------
 1 file changed, 37 insertions(+), 64 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 4d74969ad621..190ccba260e0 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -437,58 +437,6 @@ static int do_efibootmgr(void)
 	return CMD_RET_SUCCESS;
 }
 
-/**
- * do_bootefi_image() - execute EFI binary
- *
- * Set up memory image for the binary to be loaded, prepare device path, and
- * then call do_bootefi_exec() to execute it.
- *
- * @image_opt:	string with image start address
- * @size_opt:	string with image size or NULL
- * Return:	status code
- */
-static int do_bootefi_image(const char *image_opt, const char *size_opt)
-{
-	void *image_buf;
-	unsigned long addr, size;
-	efi_status_t ret;
-
-#ifdef CONFIG_CMD_BOOTEFI_HELLO
-	if (!strcmp(image_opt, "hello")) {
-		image_buf = __efi_helloworld_begin;
-		size = __efi_helloworld_end - __efi_helloworld_begin;
-		efi_clear_bootdev();
-	} else
-#endif
-	{
-		addr = strtoul(image_opt, NULL, 16);
-		/* Check that a numeric value was passed */
-		if (!addr)
-			return CMD_RET_USAGE;
-		image_buf = map_sysmem(addr, 0);
-
-		if (size_opt) {
-			size = strtoul(size_opt, NULL, 16);
-			if (!size)
-				return CMD_RET_USAGE;
-			efi_clear_bootdev();
-		} else {
-			if (image_buf != image_addr) {
-				log_err("No UEFI binary known at %s\n",
-					image_opt);
-				return CMD_RET_FAILURE;
-			}
-			size = image_size;
-		}
-	}
-	ret = efi_run_image(image_buf, size);
-
-	if (ret != EFI_SUCCESS)
-		return CMD_RET_FAILURE;
-
-	return CMD_RET_SUCCESS;
-}
-
 /**
  * efi_run_image() - run loaded UEFI image
  *
@@ -660,8 +608,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 		      char *const argv[])
 {
 	efi_status_t ret;
-	char *img_addr, *img_size, *str_copy, *pos;
-	void *fdt;
+	char *p;
+	void *fdt, *image_buf;
+	unsigned long addr, size;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
@@ -696,18 +645,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (!strcmp(argv[1], "selftest"))
 		return do_efi_selftest();
 #endif
-	str_copy = strdup(argv[1]);
-	if (!str_copy) {
-		log_err("Out of memory\n");
-		return CMD_RET_FAILURE;
+
+#ifdef CONFIG_CMD_BOOTEFI_HELLO
+	if (!strcmp(argv[1], "hello")) {
+		image_buf = __efi_helloworld_begin;
+		size = __efi_helloworld_end - __efi_helloworld_begin;
+		efi_clear_bootdev();
+	} else
+#endif
+	{
+		addr = strtoul(argv[1], NULL, 16);
+		/* Check that a numeric value was passed */
+		if (!addr)
+			return CMD_RET_USAGE;
+		image_buf = map_sysmem(addr, 0);
+
+		p  = strchr(argv[1], ':');
+		if (p) {
+			size = strtoul(++p, NULL, 16);
+			if (!size)
+				return CMD_RET_USAGE;
+			efi_clear_bootdev();
+		} else {
+			if (image_buf != image_addr) {
+				log_err("No UEFI binary known at %s\n",
+					argv[1]);
+				return CMD_RET_FAILURE;
+			}
+			size = image_size;
+		}
 	}
-	pos = str_copy;
-	img_addr = strsep(&pos, ":");
-	img_size = strsep(&pos, ":");
-	ret = do_bootefi_image(img_addr, img_size);
-	free(str_copy);
+	ret = efi_run_image(image_buf, size);
 
-	return ret;
+	if (ret != EFI_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	return CMD_RET_SUCCESS;
 }
 
 U_BOOT_LONGHELP(bootefi,
-- 
2.34.1

[PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[AKASHI Takahiro]

Replicate some code and re-organize do_bootefi() into three cases, which
will be carved out as independent functions in the next two commits.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/Kconfig          | 15 ++++++--
 cmd/bootefi.c        | 82 ++++++++++++++++++++++++++++++--------------
 include/efi_loader.h |  2 --
 3 files changed, 69 insertions(+), 30 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 6f636155e5b6..4cf9a210c4a1 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -362,9 +362,19 @@ config CMD_BOOTEFI
 	help
 	  Boot an EFI image from memory.
 
+if CMD_BOOTEFI
+config CMD_BOOTEFI_BINARY
+	bool "Allow booting an EFI binary directly"
+	depends on BOOTEFI_BOOTMGR
+	default y
+	help
+	  Select this option to enable direct execution of binary at 'bootefi'.
+	  This subcommand will allow you to load the UEFI binary using
+	  other U-Boot commands or external methods and then run it.
+
 config CMD_BOOTEFI_BOOTMGR
 	bool "UEFI Boot Manager command"
-	depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
+	depends on BOOTEFI_BOOTMGR
 	default y
 	help
 	  Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
@@ -373,7 +383,7 @@ config CMD_BOOTEFI_BOOTMGR
 
 config CMD_BOOTEFI_HELLO_COMPILE
 	bool "Compile a standard EFI hello world binary for testing"
-	depends on CMD_BOOTEFI && !CPU_V7M
+	depends on !CPU_V7M
 	default y
 	help
 	  This compiles a standard EFI hello world application with U-Boot so
@@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
 	  up EFI support on a new architecture.
 
 source lib/efi_selftest/Kconfig
+endif
 
 config CMD_BOOTMENU
 	bool "bootmenu"
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 190ccba260e0..e9e5ab67a1f5 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -503,7 +503,6 @@ out:
 	return (ret != EFI_SUCCESS) ? ret : ret2;
 }
 
-#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
 static efi_status_t bootefi_run_prepare(const char *load_options_path,
 		struct efi_device_path *device_path,
 		struct efi_device_path *image_path,
@@ -593,7 +592,6 @@ static int do_efi_selftest(void)
 
 	return ret != EFI_SUCCESS;
 }
-#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
 
 /**
  * do_bootefi() - execute `bootefi` command
@@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
-	/* Initialize EFI drivers */
-	ret = efi_init_obj_list();
-	if (ret != EFI_SUCCESS) {
-		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		return CMD_RET_FAILURE;
-	}
-
 	if (argc > 2) {
 		uintptr_t fdt_addr;
 
@@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 	} else {
 		fdt = EFI_FDT_USE_INTERNAL;
 	}
-	ret = efi_install_fdt(fdt);
-	if (ret == EFI_INVALID_PARAMETER)
-		return CMD_RET_USAGE;
-	else if (ret != EFI_SUCCESS)
-		return CMD_RET_FAILURE;
 
-	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
-		if (!strcmp(argv[1], "bootmgr"))
-			return do_efibootmgr();
+	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
+	    !strcmp(argv[1], "bootmgr")) {
+		/* Initialize EFI drivers */
+		ret = efi_init_obj_list();
+		if (ret != EFI_SUCCESS) {
+			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+				ret & ~EFI_ERROR_MASK);
+			return CMD_RET_FAILURE;
+		}
+
+		ret = efi_install_fdt(fdt);
+		if (ret == EFI_INVALID_PARAMETER)
+			return CMD_RET_USAGE;
+		else if (ret != EFI_SUCCESS)
+			return CMD_RET_FAILURE;
+
+		return do_efibootmgr();
 	}
-#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
-	if (!strcmp(argv[1], "selftest"))
+
+	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
+	    !strcmp(argv[1], "selftest")) {
+		/* Initialize EFI drivers */
+		ret = efi_init_obj_list();
+		if (ret != EFI_SUCCESS) {
+			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+				ret & ~EFI_ERROR_MASK);
+			return CMD_RET_FAILURE;
+		}
+
+		ret = efi_install_fdt(fdt);
+		if (ret == EFI_INVALID_PARAMETER)
+			return CMD_RET_USAGE;
+		else if (ret != EFI_SUCCESS)
+			return CMD_RET_FAILURE;
+
 		return do_efi_selftest();
-#endif
+	}
 
-#ifdef CONFIG_CMD_BOOTEFI_HELLO
-	if (!strcmp(argv[1], "hello")) {
+	if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
+		return CMD_RET_SUCCESS;
+
+	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
+	    !strcmp(argv[1], "hello")) {
 		image_buf = __efi_helloworld_begin;
 		size = __efi_helloworld_end - __efi_helloworld_begin;
 		efi_clear_bootdev();
-	} else
-#endif
-	{
+	} else {
 		addr = strtoul(argv[1], NULL, 16);
 		/* Check that a numeric value was passed */
 		if (!addr)
@@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 			size = image_size;
 		}
 	}
+
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
+	if (ret != EFI_SUCCESS) {
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		return CMD_RET_FAILURE;
+	}
+
+	ret = efi_install_fdt(fdt);
+	if (ret == EFI_INVALID_PARAMETER)
+		return CMD_RET_USAGE;
+	else if (ret != EFI_SUCCESS)
+		return CMD_RET_FAILURE;
+
 	ret = efi_run_image(image_buf, size);
 
 	if (ret != EFI_SUCCESS)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 664dae28f882..44436d346286 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
 
 efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
 
-#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
 /*
  * Entry point for the tests of the EFI API.
  * It is called by 'bootefi selftest'
  */
 efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
 				 struct efi_system_table *systab);
-#endif
 
 efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
 				     const efi_guid_t *vendor, u32 *attributes,
-- 
2.34.1

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[Heinrich Schuchardt]

On 11/21/23 02:29, AKASHI Takahiro wrote:
> Replicate some code and re-organize do_bootefi() into three cases, which
> will be carved out as independent functions in the next two commits.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   cmd/Kconfig          | 15 ++++++--
>   cmd/bootefi.c        | 82 ++++++++++++++++++++++++++++++--------------
>   include/efi_loader.h |  2 --
>   3 files changed, 69 insertions(+), 30 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 6f636155e5b6..4cf9a210c4a1 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -362,9 +362,19 @@ config CMD_BOOTEFI
>   	help
>   	  Boot an EFI image from memory.
>
> +if CMD_BOOTEFI
> +config CMD_BOOTEFI_BINARY
> +	bool "Allow booting an EFI binary directly"
> +	depends on BOOTEFI_BOOTMGR

Why should booting a known binary depend on the boot manager?

> +	default y
> +	help
> +	  Select this option to enable direct execution of binary at 'bootefi'.
> +	  This subcommand will allow you to load the UEFI binary using
> +	  other U-Boot commands or external methods and then run it.
> +
>   config CMD_BOOTEFI_BOOTMGR
>   	bool "UEFI Boot Manager command"
> -	depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
> +	depends on BOOTEFI_BOOTMGR
>   	default y
>   	help
>   	  Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
> @@ -373,7 +383,7 @@ config CMD_BOOTEFI_BOOTMGR
>
>   config CMD_BOOTEFI_HELLO_COMPILE
>   	bool "Compile a standard EFI hello world binary for testing"
> -	depends on CMD_BOOTEFI && !CPU_V7M
> +	depends on !CPU_V7M

Why do we have this dependency?
EFI_LOADER cannot be selected for SYS_CPU=armv7m.

>   	default y
>   	help
>   	  This compiles a standard EFI hello world application with U-Boot so
> @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
>   	  up EFI support on a new architecture.
>
>   source lib/efi_selftest/Kconfig
> +endif
>
>   config CMD_BOOTMENU
>   	bool "bootmenu"
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 190ccba260e0..e9e5ab67a1f5 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -503,7 +503,6 @@ out:
>   	return (ret != EFI_SUCCESS) ? ret : ret2;
>   }
>
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>   static efi_status_t bootefi_run_prepare(const char *load_options_path,
>   		struct efi_device_path *device_path,
>   		struct efi_device_path *image_path,
> @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
>
>   	return ret != EFI_SUCCESS;
>   }
> -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
>
>   /**
>    * do_bootefi() - execute `bootefi` command
> @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   	if (argc < 2)
>   		return CMD_RET_USAGE;
>
> -	/* Initialize EFI drivers */
> -	ret = efi_init_obj_list();
> -	if (ret != EFI_SUCCESS) {
> -		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> -			ret & ~EFI_ERROR_MASK);
> -		return CMD_RET_FAILURE;
> -	}
> -
>   	if (argc > 2) {
>   		uintptr_t fdt_addr;
>
> @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   	} else {
>   		fdt = EFI_FDT_USE_INTERNAL;
>   	}
> -	ret = efi_install_fdt(fdt);
> -	if (ret == EFI_INVALID_PARAMETER)
> -		return CMD_RET_USAGE;
> -	else if (ret != EFI_SUCCESS)
> -		return CMD_RET_FAILURE;
>
> -	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> -		if (!strcmp(argv[1], "bootmgr"))
> -			return do_efibootmgr();
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> +	    !strcmp(argv[1], "bootmgr")) {


https://docs.u-boot.org/en/latest/develop/commands.html
suggests to use U_BOOT_CMD_MKENT() to define sub-commands.

> +		/* Initialize EFI drivers */
> +		ret = efi_init_obj_list();

We should not duplicate this call for each sub-command.

> +		if (ret != EFI_SUCCESS) {
> +			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +				ret & ~EFI_ERROR_MASK);
> +			return CMD_RET_FAILURE;
> +		}
> +
> +		ret = efi_install_fdt(fdt);
> +		if (ret == EFI_INVALID_PARAMETER)
> +			return CMD_RET_USAGE;
> +		else if (ret != EFI_SUCCESS)
> +			return CMD_RET_FAILURE;

These lines could be moved into do_efibootmgr.

Should we move the translations of the return codes into efi_install_fdt?

Best regards

Heinrich

> +
> +		return do_efibootmgr();
>   	}
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> -	if (!strcmp(argv[1], "selftest"))
> +
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> +	    !strcmp(argv[1], "selftest")) {
> +		/* Initialize EFI drivers */
> +		ret = efi_init_obj_list();
> +		if (ret != EFI_SUCCESS) {
> +			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +				ret & ~EFI_ERROR_MASK);
> +			return CMD_RET_FAILURE;
> +		}
> +
> +		ret = efi_install_fdt(fdt);
> +		if (ret == EFI_INVALID_PARAMETER)
> +			return CMD_RET_USAGE;
> +		else if (ret != EFI_SUCCESS)
> +			return CMD_RET_FAILURE;
> +
>   		return do_efi_selftest();
> -#endif
> +	}
>
> -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> -	if (!strcmp(argv[1], "hello")) {
> +	if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> +		return CMD_RET_SUCCESS;
> +
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> +	    !strcmp(argv[1], "hello")) {
>   		image_buf = __efi_helloworld_begin;
>   		size = __efi_helloworld_end - __efi_helloworld_begin;
>   		efi_clear_bootdev();
> -	} else
> -#endif
> -	{
> +	} else {
>   		addr = strtoul(argv[1], NULL, 16);
>   		/* Check that a numeric value was passed */
>   		if (!addr)
> @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   			size = image_size;
>   		}
>   	}
> +
> +	/* Initialize EFI drivers */
> +	ret = efi_init_obj_list();
> +	if (ret != EFI_SUCCESS) {
> +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +			ret & ~EFI_ERROR_MASK);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	ret = efi_install_fdt(fdt);
> +	if (ret == EFI_INVALID_PARAMETER)
> +		return CMD_RET_USAGE;
> +	else if (ret != EFI_SUCCESS)
> +		return CMD_RET_FAILURE;
> +
>   	ret = efi_run_image(image_buf, size);
>
>   	if (ret != EFI_SUCCESS)
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 664dae28f882..44436d346286 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
>
>   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
>
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>   /*
>    * Entry point for the tests of the EFI API.
>    * It is called by 'bootefi selftest'
>    */
>   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>   				 struct efi_system_table *systab);
> -#endif
>
>   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>   				     const efi_guid_t *vendor, u32 *attributes,

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[AKASHI Takahiro]

Hi Heinrich,

On Tue, Nov 21, 2023 at 04:31:40AM +0100, Heinrich Schuchardt wrote:
> On 11/21/23 02:29, AKASHI Takahiro wrote:
> > Replicate some code and re-organize do_bootefi() into three cases, which
> > will be carved out as independent functions in the next two commits.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   cmd/Kconfig          | 15 ++++++--
> >   cmd/bootefi.c        | 82 ++++++++++++++++++++++++++++++--------------
> >   include/efi_loader.h |  2 --
> >   3 files changed, 69 insertions(+), 30 deletions(-)
> > 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 6f636155e5b6..4cf9a210c4a1 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -362,9 +362,19 @@ config CMD_BOOTEFI
> >   	help
> >   	  Boot an EFI image from memory.
> > 
> > +if CMD_BOOTEFI
> > +config CMD_BOOTEFI_BINARY
> > +	bool "Allow booting an EFI binary directly"
> > +	depends on BOOTEFI_BOOTMGR
> 
> Why should booting a known binary depend on the boot manager?

Because I tried to maintain the meaning of CONFIG_BOOTEFI_BOOTMGR
at this point of refactoring.
This configuration will eventually be changed to
config CMD_BOOTEFI_BINARY
        bool "Allow booting an EFI binary directly"
        depends on EFI_BINARY_EXEC
        default y
in patch#9.

> > +	default y
> > +	help
> > +	  Select this option to enable direct execution of binary at 'bootefi'.
> > +	  This subcommand will allow you to load the UEFI binary using
> > +	  other U-Boot commands or external methods and then run it.
> > +
> >   config CMD_BOOTEFI_BOOTMGR
> >   	bool "UEFI Boot Manager command"
> > -	depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
> > +	depends on BOOTEFI_BOOTMGR
> >   	default y
> >   	help
> >   	  Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
> > @@ -373,7 +383,7 @@ config CMD_BOOTEFI_BOOTMGR
> > 
> >   config CMD_BOOTEFI_HELLO_COMPILE
> >   	bool "Compile a standard EFI hello world binary for testing"
> > -	depends on CMD_BOOTEFI && !CPU_V7M
> > +	depends on !CPU_V7M
> 
> Why do we have this dependency?

CPU_V7M?
It was introduced in your commit:
---
commit 0ea8741ff65e
Author: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date:   Sun Dec 30 10:11:14 2018 +0100

    efi_loader: CMD_BOOTEFI_HELLO_COMPILE in configs
---

> EFI_LOADER cannot be selected for SYS_CPU=armv7m.

If not needed, you can delete it, but it is out of scope
of this patch series.

> >   	default y
> >   	help
> >   	  This compiles a standard EFI hello world application with U-Boot so
> > @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
> >   	  up EFI support on a new architecture.
> > 
> >   source lib/efi_selftest/Kconfig
> > +endif
> > 
> >   config CMD_BOOTMENU
> >   	bool "bootmenu"
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 190ccba260e0..e9e5ab67a1f5 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -503,7 +503,6 @@ out:
> >   	return (ret != EFI_SUCCESS) ? ret : ret2;
> >   }
> > 
> > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> >   static efi_status_t bootefi_run_prepare(const char *load_options_path,
> >   		struct efi_device_path *device_path,
> >   		struct efi_device_path *image_path,
> > @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
> > 
> >   	return ret != EFI_SUCCESS;
> >   }
> > -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> > 
> >   /**
> >    * do_bootefi() - execute `bootefi` command
> > @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> >   	if (argc < 2)
> >   		return CMD_RET_USAGE;
> > 
> > -	/* Initialize EFI drivers */
> > -	ret = efi_init_obj_list();
> > -	if (ret != EFI_SUCCESS) {
> > -		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > -			ret & ~EFI_ERROR_MASK);
> > -		return CMD_RET_FAILURE;
> > -	}
> > -
> >   	if (argc > 2) {
> >   		uintptr_t fdt_addr;
> > 
> > @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> >   	} else {
> >   		fdt = EFI_FDT_USE_INTERNAL;
> >   	}
> > -	ret = efi_install_fdt(fdt);
> > -	if (ret == EFI_INVALID_PARAMETER)
> > -		return CMD_RET_USAGE;
> > -	else if (ret != EFI_SUCCESS)
> > -		return CMD_RET_FAILURE;
> > 
> > -	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > -		if (!strcmp(argv[1], "bootmgr"))
> > -			return do_efibootmgr();
> > +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> > +	    !strcmp(argv[1], "bootmgr")) {
> 
> 
> https://docs.u-boot.org/en/latest/develop/commands.html
> suggests to use U_BOOT_CMD_MKENT() to define sub-commands.

As you know, these "if (!strcmp(argv[1], ...)" code exist since
the early days when efi_selftest and bootmgr sub-commands were
introduced in bootefi.

In my personal preference, I would move bootmgr to a new independent
command, efi_selftest to efidebug, leaving only binary-execution
syntax in bootefi.
(So no sub-command.)

> 
> > +		/* Initialize EFI drivers */
> > +		ret = efi_init_obj_list();
> 
> We should not duplicate this call for each sub-command.

Please also take a look at the succeeding commits.
A call to efi_init_obj_list() will be included in independent
library functions, either efi_bootmgr_run(), efi_binary_run()
or do_bootefi() (for efi_selftest) so that a caller of these
functions doesn't have to know/care much about detailed APIs.

> 
> > +		if (ret != EFI_SUCCESS) {
> > +			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > +				ret & ~EFI_ERROR_MASK);
> > +			return CMD_RET_FAILURE;
> > +		}
> > +
> > +		ret = efi_install_fdt(fdt);
> > +		if (ret == EFI_INVALID_PARAMETER)
> > +			return CMD_RET_USAGE;
> > +		else if (ret != EFI_SUCCESS)
> > +			return CMD_RET_FAILURE;
> 
> These lines could be moved into do_efibootmgr.

It will be done in patch#3 when carving out bootmgr specific code.

> Should we move the translations of the return codes into efi_install_fdt?

No, I don't think so. efi_install_fdt() can be called not only from
the command (bootefi) but also from other library code (at least,
efi_bootmgr_run() and efi_binary_run()).

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > +
> > +		return do_efibootmgr();
> >   	}
> > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > -	if (!strcmp(argv[1], "selftest"))
> > +
> > +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> > +	    !strcmp(argv[1], "selftest")) {
> > +		/* Initialize EFI drivers */
> > +		ret = efi_init_obj_list();
> > +		if (ret != EFI_SUCCESS) {
> > +			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > +				ret & ~EFI_ERROR_MASK);
> > +			return CMD_RET_FAILURE;
> > +		}
> > +
> > +		ret = efi_install_fdt(fdt);
> > +		if (ret == EFI_INVALID_PARAMETER)
> > +			return CMD_RET_USAGE;
> > +		else if (ret != EFI_SUCCESS)
> > +			return CMD_RET_FAILURE;
> > +
> >   		return do_efi_selftest();
> > -#endif
> > +	}
> > 
> > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > -	if (!strcmp(argv[1], "hello")) {
> > +	if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> > +		return CMD_RET_SUCCESS;
> > +
> > +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> > +	    !strcmp(argv[1], "hello")) {
> >   		image_buf = __efi_helloworld_begin;
> >   		size = __efi_helloworld_end - __efi_helloworld_begin;
> >   		efi_clear_bootdev();
> > -	} else
> > -#endif
> > -	{
> > +	} else {
> >   		addr = strtoul(argv[1], NULL, 16);
> >   		/* Check that a numeric value was passed */
> >   		if (!addr)
> > @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> >   			size = image_size;
> >   		}
> >   	}
> > +
> > +	/* Initialize EFI drivers */
> > +	ret = efi_init_obj_list();
> > +	if (ret != EFI_SUCCESS) {
> > +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > +			ret & ~EFI_ERROR_MASK);
> > +		return CMD_RET_FAILURE;
> > +	}
> > +
> > +	ret = efi_install_fdt(fdt);
> > +	if (ret == EFI_INVALID_PARAMETER)
> > +		return CMD_RET_USAGE;
> > +	else if (ret != EFI_SUCCESS)
> > +		return CMD_RET_FAILURE;
> > +
> >   	ret = efi_run_image(image_buf, size);
> > 
> >   	if (ret != EFI_SUCCESS)
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 664dae28f882..44436d346286 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
> > 
> >   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
> > 
> > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> >   /*
> >    * Entry point for the tests of the EFI API.
> >    * It is called by 'bootefi selftest'
> >    */
> >   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
> >   				 struct efi_system_table *systab);
> > -#endif
> > 
> >   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >   				     const efi_guid_t *vendor, u32 *attributes,
> 

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[Ilias Apalodimas]

Hello Akashi-san,
Thanks for taking a shot at the cleanup

On Tue, 21 Nov 2023 at 06:53, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Hi Heinrich,
>
> On Tue, Nov 21, 2023 at 04:31:40AM +0100, Heinrich Schuchardt wrote:
> > On 11/21/23 02:29, AKASHI Takahiro wrote:
> > > Replicate some code and re-organize do_bootefi() into three cases, which
> > > will be carved out as independent functions in the next two commits.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > >   cmd/Kconfig          | 15 ++++++--
> > >   cmd/bootefi.c        | 82 ++++++++++++++++++++++++++++++--------------
> > >   include/efi_loader.h |  2 --
> > >   3 files changed, 69 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index 6f636155e5b6..4cf9a210c4a1 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -362,9 +362,19 @@ config CMD_BOOTEFI
> > >     help
> > >       Boot an EFI image from memory.
> > >
> > > +if CMD_BOOTEFI
> > > +config CMD_BOOTEFI_BINARY
> > > +   bool "Allow booting an EFI binary directly"
> > > +   depends on BOOTEFI_BOOTMGR
> >
> > Why should booting a known binary depend on the boot manager?
>
> Because I tried to maintain the meaning of CONFIG_BOOTEFI_BOOTMGR
> at this point of refactoring.
> This configuration will eventually be changed to
> config CMD_BOOTEFI_BINARY
>         bool "Allow booting an EFI binary directly"
>         depends on EFI_BINARY_EXEC
>         default y
> in patch#9.
>
> > > +   default y
> > > +   help
> > > +     Select this option to enable direct execution of binary at 'bootefi'.
> > > +     This subcommand will allow you to load the UEFI binary using
> > > +     other U-Boot commands or external methods and then run it.
> > > +
> > >   config CMD_BOOTEFI_BOOTMGR
> > >     bool "UEFI Boot Manager command"
> > > -   depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
> > > +   depends on BOOTEFI_BOOTMGR
> > >     default y
> > >     help
> > >       Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
> > > @@ -373,7 +383,7 @@ config CMD_BOOTEFI_BOOTMGR
> > >
> > >   config CMD_BOOTEFI_HELLO_COMPILE
> > >     bool "Compile a standard EFI hello world binary for testing"
> > > -   depends on CMD_BOOTEFI && !CPU_V7M
> > > +   depends on !CPU_V7M
> >
> > Why do we have this dependency?
>
> CPU_V7M?
> It was introduced in your commit:
> ---
> commit 0ea8741ff65e
> Author: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date:   Sun Dec 30 10:11:14 2018 +0100
>
>     efi_loader: CMD_BOOTEFI_HELLO_COMPILE in configs
> ---
>
> > EFI_LOADER cannot be selected for SYS_CPU=armv7m.
>
> If not needed, you can delete it, but it is out of scope
> of this patch series.
>
> > >     default y
> > >     help
> > >       This compiles a standard EFI hello world application with U-Boot so
> > > @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
> > >       up EFI support on a new architecture.
> > >
> > >   source lib/efi_selftest/Kconfig
> > > +endif
> > >
> > >   config CMD_BOOTMENU
> > >     bool "bootmenu"
> > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > index 190ccba260e0..e9e5ab67a1f5 100644
> > > --- a/cmd/bootefi.c
> > > +++ b/cmd/bootefi.c
> > > @@ -503,7 +503,6 @@ out:
> > >     return (ret != EFI_SUCCESS) ? ret : ret2;
> > >   }
> > >
> > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > >   static efi_status_t bootefi_run_prepare(const char *load_options_path,
> > >             struct efi_device_path *device_path,
> > >             struct efi_device_path *image_path,
> > > @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
> > >
> > >     return ret != EFI_SUCCESS;
> > >   }
> > > -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> > >
> > >   /**
> > >    * do_bootefi() - execute `bootefi` command
> > > @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > >     if (argc < 2)
> > >             return CMD_RET_USAGE;
> > >
> > > -   /* Initialize EFI drivers */
> > > -   ret = efi_init_obj_list();
> > > -   if (ret != EFI_SUCCESS) {
> > > -           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > -                   ret & ~EFI_ERROR_MASK);
> > > -           return CMD_RET_FAILURE;
> > > -   }
> > > -
> > >     if (argc > 2) {
> > >             uintptr_t fdt_addr;
> > >
> > > @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > >     } else {
> > >             fdt = EFI_FDT_USE_INTERNAL;
> > >     }
> > > -   ret = efi_install_fdt(fdt);
> > > -   if (ret == EFI_INVALID_PARAMETER)
> > > -           return CMD_RET_USAGE;
> > > -   else if (ret != EFI_SUCCESS)
> > > -           return CMD_RET_FAILURE;
> > >
> > > -   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > > -           if (!strcmp(argv[1], "bootmgr"))
> > > -                   return do_efibootmgr();
> > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> > > +       !strcmp(argv[1], "bootmgr")) {
> >
> >
> > https://docs.u-boot.org/en/latest/develop/commands.html
> > suggests to use U_BOOT_CMD_MKENT() to define sub-commands.
>
> As you know, these "if (!strcmp(argv[1], ...)" code exist since
> the early days when efi_selftest and bootmgr sub-commands were
> introduced in bootefi.
>
> In my personal preference, I would move bootmgr to a new independent
> command, efi_selftest to efidebug, leaving only binary-execution
> syntax in bootefi.
> (So no sub-command.)

And that's a good idea, does anything prevent us from doing that ? The
code works reliably as-is so if we are thinking about refactoring it,
take a deeper dive and let's do all of it.

An idea would be to start with patches that move 'bootefi hello' and
'bootefi selftest' to the efidebug command?

>
> >
> > > +           /* Initialize EFI drivers */
> > > +           ret = efi_init_obj_list();
> >
> > We should not duplicate this call for each sub-command.
>
> Please also take a look at the succeeding commits.
> A call to efi_init_obj_list() will be included in independent
> library functions, either efi_bootmgr_run(), efi_binary_run()
> or do_bootefi() (for efi_selftest) so that a caller of these
> functions doesn't have to know/care much about detailed APIs.

I am with Heinrich on this. Despite the further refactoring in
subsequent patches, we could just initialize the EFI subsystem at the
beginning. It will eventually be done by some part of u-boot and we
don't clean up the initialization on any failure, so why not move it
on top of the function right after the argument parsing?

>
> >
> > > +           if (ret != EFI_SUCCESS) {
> > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > +                           ret & ~EFI_ERROR_MASK);
> > > +                   return CMD_RET_FAILURE;
> > > +           }
> > > +
> > > +           ret = efi_install_fdt(fdt);
> > > +           if (ret == EFI_INVALID_PARAMETER)
> > > +                   return CMD_RET_USAGE;
> > > +           else if (ret != EFI_SUCCESS)
> > > +                   return CMD_RET_FAILURE;
> >
> > These lines could be moved into do_efibootmgr.
>
> It will be done in patch#3 when carving out bootmgr specific code.
>
> > Should we move the translations of the return codes into efi_install_fdt?
>
> No, I don't think so. efi_install_fdt() can be called not only from
> the command (bootefi) but also from other library code (at least,
> efi_bootmgr_run() and efi_binary_run()).
>
> -Takahiro Akashi
>
> > Best regards
> >
> > Heinrich
> >
> > > +
> > > +           return do_efibootmgr();
> > >     }
> > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > -   if (!strcmp(argv[1], "selftest"))
> > > +
> > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> > > +       !strcmp(argv[1], "selftest")) {
> > > +           /* Initialize EFI drivers */
> > > +           ret = efi_init_obj_list();
> > > +           if (ret != EFI_SUCCESS) {
> > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > +                           ret & ~EFI_ERROR_MASK);
> > > +                   return CMD_RET_FAILURE;
> > > +           }
> > > +
> > > +           ret = efi_install_fdt(fdt);
> > > +           if (ret == EFI_INVALID_PARAMETER)
> > > +                   return CMD_RET_USAGE;
> > > +           else if (ret != EFI_SUCCESS)
> > > +                   return CMD_RET_FAILURE;
> > > +
> > >             return do_efi_selftest();
> > > -#endif
> > > +   }
> > >
> > > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > -   if (!strcmp(argv[1], "hello")) {
> > > +   if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> > > +           return CMD_RET_SUCCESS;
> > > +
> > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> > > +       !strcmp(argv[1], "hello")) {
> > >             image_buf = __efi_helloworld_begin;
> > >             size = __efi_helloworld_end - __efi_helloworld_begin;
> > >             efi_clear_bootdev();
> > > -   } else
> > > -#endif
> > > -   {
> > > +   } else {
> > >             addr = strtoul(argv[1], NULL, 16);
> > >             /* Check that a numeric value was passed */
> > >             if (!addr)
> > > @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > >                     size = image_size;
> > >             }
> > >     }
> > > +
> > > +   /* Initialize EFI drivers */
> > > +   ret = efi_init_obj_list();
> > > +   if (ret != EFI_SUCCESS) {
> > > +           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > +                   ret & ~EFI_ERROR_MASK);
> > > +           return CMD_RET_FAILURE;
> > > +   }
> > > +
> > > +   ret = efi_install_fdt(fdt);
> > > +   if (ret == EFI_INVALID_PARAMETER)
> > > +           return CMD_RET_USAGE;
> > > +   else if (ret != EFI_SUCCESS)
> > > +           return CMD_RET_FAILURE;
> > > +
> > >     ret = efi_run_image(image_buf, size);
> > >
> > >     if (ret != EFI_SUCCESS)
> > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > index 664dae28f882..44436d346286 100644
> > > --- a/include/efi_loader.h
> > > +++ b/include/efi_loader.h
> > > @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
> > >
> > >   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
> > >
> > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > >   /*
> > >    * Entry point for the tests of the EFI API.
> > >    * It is called by 'bootefi selftest'
> > >    */
> > >   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
> > >                              struct efi_system_table *systab);
> > > -#endif
> > >
> > >   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > >                                  const efi_guid_t *vendor, u32 *attributes,
> >

Thanks
/Ilias

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[AKASHI Takahiro]

On Tue, Dec 05, 2023 at 10:54:23AM +0200, Ilias Apalodimas wrote:
> Hello Akashi-san,
> Thanks for taking a shot at the cleanup
> 
> On Tue, 21 Nov 2023 at 06:53, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Hi Heinrich,
> >
> > On Tue, Nov 21, 2023 at 04:31:40AM +0100, Heinrich Schuchardt wrote:
> > > On 11/21/23 02:29, AKASHI Takahiro wrote:
> > > > Replicate some code and re-organize do_bootefi() into three cases, which
> > > > will be carved out as independent functions in the next two commits.
> > > >
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > >   cmd/Kconfig          | 15 ++++++--
> > > >   cmd/bootefi.c        | 82 ++++++++++++++++++++++++++++++--------------
> > > >   include/efi_loader.h |  2 --
> > > >   3 files changed, 69 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > index 6f636155e5b6..4cf9a210c4a1 100644
> > > > --- a/cmd/Kconfig
> > > > +++ b/cmd/Kconfig
> > > > @@ -362,9 +362,19 @@ config CMD_BOOTEFI
> > > >     help
> > > >       Boot an EFI image from memory.
> > > >
> > > > +if CMD_BOOTEFI
> > > > +config CMD_BOOTEFI_BINARY
> > > > +   bool "Allow booting an EFI binary directly"
> > > > +   depends on BOOTEFI_BOOTMGR
> > >
> > > Why should booting a known binary depend on the boot manager?
> >
> > Because I tried to maintain the meaning of CONFIG_BOOTEFI_BOOTMGR
> > at this point of refactoring.
> > This configuration will eventually be changed to
> > config CMD_BOOTEFI_BINARY
> >         bool "Allow booting an EFI binary directly"
> >         depends on EFI_BINARY_EXEC
> >         default y
> > in patch#9.
> >
> > > > +   default y
> > > > +   help
> > > > +     Select this option to enable direct execution of binary at 'bootefi'.
> > > > +     This subcommand will allow you to load the UEFI binary using
> > > > +     other U-Boot commands or external methods and then run it.
> > > > +
> > > >   config CMD_BOOTEFI_BOOTMGR
> > > >     bool "UEFI Boot Manager command"
> > > > -   depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
> > > > +   depends on BOOTEFI_BOOTMGR
> > > >     default y
> > > >     help
> > > >       Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
> > > > @@ -373,7 +383,7 @@ config CMD_BOOTEFI_BOOTMGR
> > > >
> > > >   config CMD_BOOTEFI_HELLO_COMPILE
> > > >     bool "Compile a standard EFI hello world binary for testing"
> > > > -   depends on CMD_BOOTEFI && !CPU_V7M
> > > > +   depends on !CPU_V7M
> > >
> > > Why do we have this dependency?
> >
> > CPU_V7M?
> > It was introduced in your commit:
> > ---
> > commit 0ea8741ff65e
> > Author: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Date:   Sun Dec 30 10:11:14 2018 +0100
> >
> >     efi_loader: CMD_BOOTEFI_HELLO_COMPILE in configs
> > ---
> >
> > > EFI_LOADER cannot be selected for SYS_CPU=armv7m.
> >
> > If not needed, you can delete it, but it is out of scope
> > of this patch series.
> >
> > > >     default y
> > > >     help
> > > >       This compiles a standard EFI hello world application with U-Boot so
> > > > @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
> > > >       up EFI support on a new architecture.
> > > >
> > > >   source lib/efi_selftest/Kconfig
> > > > +endif
> > > >
> > > >   config CMD_BOOTMENU
> > > >     bool "bootmenu"
> > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > > index 190ccba260e0..e9e5ab67a1f5 100644
> > > > --- a/cmd/bootefi.c
> > > > +++ b/cmd/bootefi.c
> > > > @@ -503,7 +503,6 @@ out:
> > > >     return (ret != EFI_SUCCESS) ? ret : ret2;
> > > >   }
> > > >
> > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > >   static efi_status_t bootefi_run_prepare(const char *load_options_path,
> > > >             struct efi_device_path *device_path,
> > > >             struct efi_device_path *image_path,
> > > > @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
> > > >
> > > >     return ret != EFI_SUCCESS;
> > > >   }
> > > > -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> > > >
> > > >   /**
> > > >    * do_bootefi() - execute `bootefi` command
> > > > @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > >     if (argc < 2)
> > > >             return CMD_RET_USAGE;
> > > >
> > > > -   /* Initialize EFI drivers */
> > > > -   ret = efi_init_obj_list();
> > > > -   if (ret != EFI_SUCCESS) {
> > > > -           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > -                   ret & ~EFI_ERROR_MASK);
> > > > -           return CMD_RET_FAILURE;
> > > > -   }
> > > > -
> > > >     if (argc > 2) {
> > > >             uintptr_t fdt_addr;
> > > >
> > > > @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > >     } else {
> > > >             fdt = EFI_FDT_USE_INTERNAL;
> > > >     }
> > > > -   ret = efi_install_fdt(fdt);
> > > > -   if (ret == EFI_INVALID_PARAMETER)
> > > > -           return CMD_RET_USAGE;
> > > > -   else if (ret != EFI_SUCCESS)
> > > > -           return CMD_RET_FAILURE;
> > > >
> > > > -   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > > > -           if (!strcmp(argv[1], "bootmgr"))
> > > > -                   return do_efibootmgr();
> > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> > > > +       !strcmp(argv[1], "bootmgr")) {
> > >
> > >
> > > https://docs.u-boot.org/en/latest/develop/commands.html
> > > suggests to use U_BOOT_CMD_MKENT() to define sub-commands.
> >
> > As you know, these "if (!strcmp(argv[1], ...)" code exist since
> > the early days when efi_selftest and bootmgr sub-commands were
> > introduced in bootefi.
> >
> > In my personal preference, I would move bootmgr to a new independent
> > command, efi_selftest to efidebug, leaving only binary-execution
> > syntax in bootefi.
> > (So no sub-command.)
> 
> And that's a good idea, does anything prevent us from doing that ? The
> code works reliably as-is so if we are thinking about refactoring it,
> take a deeper dive and let's do all of it.
> 
> An idea would be to start with patches that move 'bootefi hello' and
> 'bootefi selftest' to the efidebug command?
> 
> >
> > >
> > > > +           /* Initialize EFI drivers */
> > > > +           ret = efi_init_obj_list();
> > >
> > > We should not duplicate this call for each sub-command.
> >
> > Please also take a look at the succeeding commits.
> > A call to efi_init_obj_list() will be included in independent
> > library functions, either efi_bootmgr_run(), efi_binary_run()
> > or do_bootefi() (for efi_selftest) so that a caller of these
> > functions doesn't have to know/care much about detailed APIs.
> 
> I am with Heinrich on this. Despite the further refactoring in
> subsequent patches, we could just initialize the EFI subsystem at the
> beginning. It will eventually be done by some part of u-boot and we

Yes if you want to always call efi_init_obj_list() in, say, board_init_r().
But we didn't take this approach by design because we wanted to initialize
the subsystem only if needed.

> don't clean up the initialization on any failure, so why not move it
> on top of the function right after the argument parsing?

I'm not sure what you mean here, but please remember that either
efi_binary_run() or efi_bootmgr_run() is more or less a utility
function which is convenient in order for other part of u-boot to utilize
efi subsystem easily. Since there is no guarantee that efi_init_obj_list()
is called before, it would be better to always call efi_init_obj_list()
in those functions.

-Takahiro Akashi


> >
> > >
> > > > +           if (ret != EFI_SUCCESS) {
> > > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > +                           ret & ~EFI_ERROR_MASK);
> > > > +                   return CMD_RET_FAILURE;
> > > > +           }
> > > > +
> > > > +           ret = efi_install_fdt(fdt);
> > > > +           if (ret == EFI_INVALID_PARAMETER)
> > > > +                   return CMD_RET_USAGE;
> > > > +           else if (ret != EFI_SUCCESS)
> > > > +                   return CMD_RET_FAILURE;
> > >
> > > These lines could be moved into do_efibootmgr.
> >
> > It will be done in patch#3 when carving out bootmgr specific code.
> >
> > > Should we move the translations of the return codes into efi_install_fdt?
> >
> > No, I don't think so. efi_install_fdt() can be called not only from
> > the command (bootefi) but also from other library code (at least,
> > efi_bootmgr_run() and efi_binary_run()).
> >
> > -Takahiro Akashi
> >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > > +
> > > > +           return do_efibootmgr();
> > > >     }
> > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > > -   if (!strcmp(argv[1], "selftest"))
> > > > +
> > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> > > > +       !strcmp(argv[1], "selftest")) {
> > > > +           /* Initialize EFI drivers */
> > > > +           ret = efi_init_obj_list();
> > > > +           if (ret != EFI_SUCCESS) {
> > > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > +                           ret & ~EFI_ERROR_MASK);
> > > > +                   return CMD_RET_FAILURE;
> > > > +           }
> > > > +
> > > > +           ret = efi_install_fdt(fdt);
> > > > +           if (ret == EFI_INVALID_PARAMETER)
> > > > +                   return CMD_RET_USAGE;
> > > > +           else if (ret != EFI_SUCCESS)
> > > > +                   return CMD_RET_FAILURE;
> > > > +
> > > >             return do_efi_selftest();
> > > > -#endif
> > > > +   }
> > > >
> > > > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > > -   if (!strcmp(argv[1], "hello")) {
> > > > +   if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> > > > +           return CMD_RET_SUCCESS;
> > > > +
> > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> > > > +       !strcmp(argv[1], "hello")) {
> > > >             image_buf = __efi_helloworld_begin;
> > > >             size = __efi_helloworld_end - __efi_helloworld_begin;
> > > >             efi_clear_bootdev();
> > > > -   } else
> > > > -#endif
> > > > -   {
> > > > +   } else {
> > > >             addr = strtoul(argv[1], NULL, 16);
> > > >             /* Check that a numeric value was passed */
> > > >             if (!addr)
> > > > @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > >                     size = image_size;
> > > >             }
> > > >     }
> > > > +
> > > > +   /* Initialize EFI drivers */
> > > > +   ret = efi_init_obj_list();
> > > > +   if (ret != EFI_SUCCESS) {
> > > > +           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > +                   ret & ~EFI_ERROR_MASK);
> > > > +           return CMD_RET_FAILURE;
> > > > +   }
> > > > +
> > > > +   ret = efi_install_fdt(fdt);
> > > > +   if (ret == EFI_INVALID_PARAMETER)
> > > > +           return CMD_RET_USAGE;
> > > > +   else if (ret != EFI_SUCCESS)
> > > > +           return CMD_RET_FAILURE;
> > > > +
> > > >     ret = efi_run_image(image_buf, size);
> > > >
> > > >     if (ret != EFI_SUCCESS)
> > > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > > index 664dae28f882..44436d346286 100644
> > > > --- a/include/efi_loader.h
> > > > +++ b/include/efi_loader.h
> > > > @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
> > > >
> > > >   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
> > > >
> > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > >   /*
> > > >    * Entry point for the tests of the EFI API.
> > > >    * It is called by 'bootefi selftest'
> > > >    */
> > > >   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
> > > >                              struct efi_system_table *systab);
> > > > -#endif
> > > >
> > > >   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > > >                                  const efi_guid_t *vendor, u32 *attributes,
> > >
> 
> Thanks
> /Ilias

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[Ilias Apalodimas]

Akashi-san,

[...]

> > > > >     help
> > > > >       This compiles a standard EFI hello world application with U-Boot so
> > > > > @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
> > > > >       up EFI support on a new architecture.
> > > > >
> > > > >   source lib/efi_selftest/Kconfig
> > > > > +endif
> > > > >
> > > > >   config CMD_BOOTMENU
> > > > >     bool "bootmenu"
> > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > > > index 190ccba260e0..e9e5ab67a1f5 100644
> > > > > --- a/cmd/bootefi.c
> > > > > +++ b/cmd/bootefi.c
> > > > > @@ -503,7 +503,6 @@ out:
> > > > >     return (ret != EFI_SUCCESS) ? ret : ret2;
> > > > >   }
> > > > >
> > > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > > >   static efi_status_t bootefi_run_prepare(const char *load_options_path,
> > > > >             struct efi_device_path *device_path,
> > > > >             struct efi_device_path *image_path,
> > > > > @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
> > > > >
> > > > >     return ret != EFI_SUCCESS;
> > > > >   }
> > > > > -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> > > > >
> > > > >   /**
> > > > >    * do_bootefi() - execute `bootefi` command
> > > > > @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > >     if (argc < 2)
> > > > >             return CMD_RET_USAGE;
> > > > >
> > > > > -   /* Initialize EFI drivers */
> > > > > -   ret = efi_init_obj_list();
> > > > > -   if (ret != EFI_SUCCESS) {
> > > > > -           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > -                   ret & ~EFI_ERROR_MASK);
> > > > > -           return CMD_RET_FAILURE;
> > > > > -   }
> > > > > -
> > > > >     if (argc > 2) {
> > > > >             uintptr_t fdt_addr;
> > > > >
> > > > > @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > >     } else {
> > > > >             fdt = EFI_FDT_USE_INTERNAL;
> > > > >     }
> > > > > -   ret = efi_install_fdt(fdt);
> > > > > -   if (ret == EFI_INVALID_PARAMETER)
> > > > > -           return CMD_RET_USAGE;
> > > > > -   else if (ret != EFI_SUCCESS)
> > > > > -           return CMD_RET_FAILURE;
> > > > >
> > > > > -   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > > > > -           if (!strcmp(argv[1], "bootmgr"))
> > > > > -                   return do_efibootmgr();
> > > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> > > > > +       !strcmp(argv[1], "bootmgr")) {
> > > >
> > > >
> > > > https://docs.u-boot.org/en/latest/develop/commands.html
> > > > suggests to use U_BOOT_CMD_MKENT() to define sub-commands.
> > >
> > > As you know, these "if (!strcmp(argv[1], ...)" code exist since
> > > the early days when efi_selftest and bootmgr sub-commands were
> > > introduced in bootefi.
> > >
> > > In my personal preference, I would move bootmgr to a new independent
> > > command, efi_selftest to efidebug, leaving only binary-execution
> > > syntax in bootefi.
> > > (So no sub-command.)
> >
> > And that's a good idea, does anything prevent us from doing that ? The
> > code works reliably as-is so if we are thinking about refactoring it,
> > take a deeper dive and let's do all of it.
> >
> > An idea would be to start with patches that move 'bootefi hello' and
> > 'bootefi selftest' to the efidebug command?
> >
> > >
> > > >
> > > > > +           /* Initialize EFI drivers */
> > > > > +           ret = efi_init_obj_list();
> > > >
> > > > We should not duplicate this call for each sub-command.
> > >
> > > Please also take a look at the succeeding commits.
> > > A call to efi_init_obj_list() will be included in independent
> > > library functions, either efi_bootmgr_run(), efi_binary_run()
> > > or do_bootefi() (for efi_selftest) so that a caller of these
> > > functions doesn't have to know/care much about detailed APIs.
> >
> > I am with Heinrich on this. Despite the further refactoring in
> > subsequent patches, we could just initialize the EFI subsystem at the
> > beginning. It will eventually be done by some part of u-boot and we
>
> Yes if you want to always call efi_init_obj_list() in, say, board_init_r().
> But we didn't take this approach by design because we wanted to initialize
> the subsystem only if needed.

Not in board_init_r(). Perhaps we can have that as a Kconfig option

>
> > don't clean up the initialization on any failure, so why not move it
> > on top of the function right after the argument parsing?
>
> I'm not sure what you mean here, but please remember that either
> efi_binary_run() or efi_bootmgr_run() is more or less a utility
> function which is convenient in order for other part of u-boot to utilize
> efi subsystem easily. Since there is no guarantee that efi_init_obj_list()
> is called before, it would be better to always call efi_init_obj_list()
> in those functions.

Yes, but I prefer calling it once.
Instead of duplicating the init on
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && ...
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) && ...and then at the end
simply move it to the top of the function.  You are trying to call efi
bootmgr, no one will care if the EFI subsystem comes up regardless of
ifs and potential failures.

Thanks
/Ilias



>
> -Takahiro Akashi
>
>
> > >
> > > >
> > > > > +           if (ret != EFI_SUCCESS) {
> > > > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > +                           ret & ~EFI_ERROR_MASK);
> > > > > +                   return CMD_RET_FAILURE;
> > > > > +           }
> > > > > +
> > > > > +           ret = efi_install_fdt(fdt);
> > > > > +           if (ret == EFI_INVALID_PARAMETER)
> > > > > +                   return CMD_RET_USAGE;
> > > > > +           else if (ret != EFI_SUCCESS)
> > > > > +                   return CMD_RET_FAILURE;
> > > >
> > > > These lines could be moved into do_efibootmgr.
> > >
> > > It will be done in patch#3 when carving out bootmgr specific code.
> > >
> > > > Should we move the translations of the return codes into efi_install_fdt?
> > >
> > > No, I don't think so. efi_install_fdt() can be called not only from
> > > the command (bootefi) but also from other library code (at least,
> > > efi_bootmgr_run() and efi_binary_run()).
> > >
> > > -Takahiro Akashi
> > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > > +
> > > > > +           return do_efibootmgr();
> > > > >     }
> > > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > > > -   if (!strcmp(argv[1], "selftest"))
> > > > > +
> > > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> > > > > +       !strcmp(argv[1], "selftest")) {
> > > > > +           /* Initialize EFI drivers */
> > > > > +           ret = efi_init_obj_list();
> > > > > +           if (ret != EFI_SUCCESS) {
> > > > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > +                           ret & ~EFI_ERROR_MASK);
> > > > > +                   return CMD_RET_FAILURE;
> > > > > +           }
> > > > > +
> > > > > +           ret = efi_install_fdt(fdt);
> > > > > +           if (ret == EFI_INVALID_PARAMETER)
> > > > > +                   return CMD_RET_USAGE;
> > > > > +           else if (ret != EFI_SUCCESS)
> > > > > +                   return CMD_RET_FAILURE;
> > > > > +
> > > > >             return do_efi_selftest();
> > > > > -#endif
> > > > > +   }
> > > > >
> > > > > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > > > -   if (!strcmp(argv[1], "hello")) {
> > > > > +   if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> > > > > +           return CMD_RET_SUCCESS;
> > > > > +
> > > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> > > > > +       !strcmp(argv[1], "hello")) {
> > > > >             image_buf = __efi_helloworld_begin;
> > > > >             size = __efi_helloworld_end - __efi_helloworld_begin;
> > > > >             efi_clear_bootdev();
> > > > > -   } else
> > > > > -#endif
> > > > > -   {
> > > > > +   } else {
> > > > >             addr = strtoul(argv[1], NULL, 16);
> > > > >             /* Check that a numeric value was passed */
> > > > >             if (!addr)
> > > > > @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > >                     size = image_size;
> > > > >             }
> > > > >     }
> > > > > +
> > > > > +   /* Initialize EFI drivers */
> > > > > +   ret = efi_init_obj_list();
> > > > > +   if (ret != EFI_SUCCESS) {
> > > > > +           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > +                   ret & ~EFI_ERROR_MASK);
> > > > > +           return CMD_RET_FAILURE;
> > > > > +   }
> > > > > +
> > > > > +   ret = efi_install_fdt(fdt);
> > > > > +   if (ret == EFI_INVALID_PARAMETER)
> > > > > +           return CMD_RET_USAGE;
> > > > > +   else if (ret != EFI_SUCCESS)
> > > > > +           return CMD_RET_FAILURE;
> > > > > +
> > > > >     ret = efi_run_image(image_buf, size);
> > > > >
> > > > >     if (ret != EFI_SUCCESS)
> > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > > > index 664dae28f882..44436d346286 100644
> > > > > --- a/include/efi_loader.h
> > > > > +++ b/include/efi_loader.h
> > > > > @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
> > > > >
> > > > >   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
> > > > >
> > > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > > >   /*
> > > > >    * Entry point for the tests of the EFI API.
> > > > >    * It is called by 'bootefi selftest'
> > > > >    */
> > > > >   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
> > > > >                              struct efi_system_table *systab);
> > > > > -#endif
> > > > >
> > > > >   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > > > >                                  const efi_guid_t *vendor, u32 *attributes,
> > > >
> >
> > Thanks
> > /Ilias

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[AKASHI Takahiro]

Hi Ilias,

On Fri, Dec 08, 2023 at 08:33:11AM +0200, Ilias Apalodimas wrote:
> Akashi-san,
> 
> [...]
> 
> > > > > >     help
> > > > > >       This compiles a standard EFI hello world application with U-Boot so
> > > > > > @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
> > > > > >       up EFI support on a new architecture.
> > > > > >
> > > > > >   source lib/efi_selftest/Kconfig
> > > > > > +endif
> > > > > >
> > > > > >   config CMD_BOOTMENU
> > > > > >     bool "bootmenu"
> > > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > > > > index 190ccba260e0..e9e5ab67a1f5 100644
> > > > > > --- a/cmd/bootefi.c
> > > > > > +++ b/cmd/bootefi.c
> > > > > > @@ -503,7 +503,6 @@ out:
> > > > > >     return (ret != EFI_SUCCESS) ? ret : ret2;
> > > > > >   }
> > > > > >
> > > > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > > > >   static efi_status_t bootefi_run_prepare(const char *load_options_path,
> > > > > >             struct efi_device_path *device_path,
> > > > > >             struct efi_device_path *image_path,
> > > > > > @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
> > > > > >
> > > > > >     return ret != EFI_SUCCESS;
> > > > > >   }
> > > > > > -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> > > > > >
> > > > > >   /**
> > > > > >    * do_bootefi() - execute `bootefi` command
> > > > > > @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > > >     if (argc < 2)
> > > > > >             return CMD_RET_USAGE;
> > > > > >
> > > > > > -   /* Initialize EFI drivers */
> > > > > > -   ret = efi_init_obj_list();
> > > > > > -   if (ret != EFI_SUCCESS) {
> > > > > > -           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > > -                   ret & ~EFI_ERROR_MASK);
> > > > > > -           return CMD_RET_FAILURE;
> > > > > > -   }
> > > > > > -
> > > > > >     if (argc > 2) {
> > > > > >             uintptr_t fdt_addr;
> > > > > >
> > > > > > @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > > >     } else {
> > > > > >             fdt = EFI_FDT_USE_INTERNAL;
> > > > > >     }
> > > > > > -   ret = efi_install_fdt(fdt);
> > > > > > -   if (ret == EFI_INVALID_PARAMETER)
> > > > > > -           return CMD_RET_USAGE;
> > > > > > -   else if (ret != EFI_SUCCESS)
> > > > > > -           return CMD_RET_FAILURE;
> > > > > >
> > > > > > -   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > > > > > -           if (!strcmp(argv[1], "bootmgr"))
> > > > > > -                   return do_efibootmgr();
> > > > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> > > > > > +       !strcmp(argv[1], "bootmgr")) {
> > > > >
> > > > >
> > > > > https://docs.u-boot.org/en/latest/develop/commands.html
> > > > > suggests to use U_BOOT_CMD_MKENT() to define sub-commands.
> > > >
> > > > As you know, these "if (!strcmp(argv[1], ...)" code exist since
> > > > the early days when efi_selftest and bootmgr sub-commands were
> > > > introduced in bootefi.
> > > >
> > > > In my personal preference, I would move bootmgr to a new independent
> > > > command, efi_selftest to efidebug, leaving only binary-execution
> > > > syntax in bootefi.
> > > > (So no sub-command.)
> > >
> > > And that's a good idea, does anything prevent us from doing that ? The
> > > code works reliably as-is so if we are thinking about refactoring it,
> > > take a deeper dive and let's do all of it.
> > >
> > > An idea would be to start with patches that move 'bootefi hello' and
> > > 'bootefi selftest' to the efidebug command?
> > >
> > > >
> > > > >
> > > > > > +           /* Initialize EFI drivers */
> > > > > > +           ret = efi_init_obj_list();
> > > > >
> > > > > We should not duplicate this call for each sub-command.
> > > >
> > > > Please also take a look at the succeeding commits.
> > > > A call to efi_init_obj_list() will be included in independent
> > > > library functions, either efi_bootmgr_run(), efi_binary_run()
> > > > or do_bootefi() (for efi_selftest) so that a caller of these
> > > > functions doesn't have to know/care much about detailed APIs.
> > >
> > > I am with Heinrich on this. Despite the further refactoring in
> > > subsequent patches, we could just initialize the EFI subsystem at the
> > > beginning. It will eventually be done by some part of u-boot and we
> >
> > Yes if you want to always call efi_init_obj_list() in, say, board_init_r().
> > But we didn't take this approach by design because we wanted to initialize
> > the subsystem only if needed.
> 
> Not in board_init_r(). Perhaps we can have that as a Kconfig option

Then where do you want to call the function?

> >
> > > don't clean up the initialization on any failure, so why not move it
> > > on top of the function right after the argument parsing?
> >
> > I'm not sure what you mean here, but please remember that either
> > efi_binary_run() or efi_bootmgr_run() is more or less a utility
> > function which is convenient in order for other part of u-boot to utilize
> > efi subsystem easily. Since there is no guarantee that efi_init_obj_list()
> > is called before, it would be better to always call efi_init_obj_list()
> > in those functions.
> 
> Yes, but I prefer calling it once.
> Instead of duplicating the init on
> if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && ...
> if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) && ...and then at the end
> simply move it to the top of the function.  You are trying to call efi
> bootmgr, no one will care if the EFI subsystem comes up regardless of
> ifs and potential failures.

You are in the middle of refactoring here.

I would like you to see the *final* code after refactoring.
Then you will understand why I did so (duplicating efi_init_obj_list()
at each sub command parts).

-Takahiro Akashi

> Thanks
> /Ilias
> 
> 
> 
> >
> > -Takahiro Akashi
> >
> >
> > > >
> > > > >
> > > > > > +           if (ret != EFI_SUCCESS) {
> > > > > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > > +                           ret & ~EFI_ERROR_MASK);
> > > > > > +                   return CMD_RET_FAILURE;
> > > > > > +           }
> > > > > > +
> > > > > > +           ret = efi_install_fdt(fdt);
> > > > > > +           if (ret == EFI_INVALID_PARAMETER)
> > > > > > +                   return CMD_RET_USAGE;
> > > > > > +           else if (ret != EFI_SUCCESS)
> > > > > > +                   return CMD_RET_FAILURE;
> > > > >
> > > > > These lines could be moved into do_efibootmgr.
> > > >
> > > > It will be done in patch#3 when carving out bootmgr specific code.
> > > >
> > > > > Should we move the translations of the return codes into efi_install_fdt?
> > > >
> > > > No, I don't think so. efi_install_fdt() can be called not only from
> > > > the command (bootefi) but also from other library code (at least,
> > > > efi_bootmgr_run() and efi_binary_run()).
> > > >
> > > > -Takahiro Akashi
> > > >
> > > > > Best regards
> > > > >
> > > > > Heinrich
> > > > >
> > > > > > +
> > > > > > +           return do_efibootmgr();
> > > > > >     }
> > > > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > > > > -   if (!strcmp(argv[1], "selftest"))
> > > > > > +
> > > > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> > > > > > +       !strcmp(argv[1], "selftest")) {
> > > > > > +           /* Initialize EFI drivers */
> > > > > > +           ret = efi_init_obj_list();
> > > > > > +           if (ret != EFI_SUCCESS) {
> > > > > > +                   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > > +                           ret & ~EFI_ERROR_MASK);
> > > > > > +                   return CMD_RET_FAILURE;
> > > > > > +           }
> > > > > > +
> > > > > > +           ret = efi_install_fdt(fdt);
> > > > > > +           if (ret == EFI_INVALID_PARAMETER)
> > > > > > +                   return CMD_RET_USAGE;
> > > > > > +           else if (ret != EFI_SUCCESS)
> > > > > > +                   return CMD_RET_FAILURE;
> > > > > > +
> > > > > >             return do_efi_selftest();
> > > > > > -#endif
> > > > > > +   }
> > > > > >
> > > > > > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > > > > -   if (!strcmp(argv[1], "hello")) {
> > > > > > +   if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> > > > > > +           return CMD_RET_SUCCESS;
> > > > > > +
> > > > > > +   if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> > > > > > +       !strcmp(argv[1], "hello")) {
> > > > > >             image_buf = __efi_helloworld_begin;
> > > > > >             size = __efi_helloworld_end - __efi_helloworld_begin;
> > > > > >             efi_clear_bootdev();
> > > > > > -   } else
> > > > > > -#endif
> > > > > > -   {
> > > > > > +   } else {
> > > > > >             addr = strtoul(argv[1], NULL, 16);
> > > > > >             /* Check that a numeric value was passed */
> > > > > >             if (!addr)
> > > > > > @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > > >                     size = image_size;
> > > > > >             }
> > > > > >     }
> > > > > > +
> > > > > > +   /* Initialize EFI drivers */
> > > > > > +   ret = efi_init_obj_list();
> > > > > > +   if (ret != EFI_SUCCESS) {
> > > > > > +           log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > > > > +                   ret & ~EFI_ERROR_MASK);
> > > > > > +           return CMD_RET_FAILURE;
> > > > > > +   }
> > > > > > +
> > > > > > +   ret = efi_install_fdt(fdt);
> > > > > > +   if (ret == EFI_INVALID_PARAMETER)
> > > > > > +           return CMD_RET_USAGE;
> > > > > > +   else if (ret != EFI_SUCCESS)
> > > > > > +           return CMD_RET_FAILURE;
> > > > > > +
> > > > > >     ret = efi_run_image(image_buf, size);
> > > > > >
> > > > > >     if (ret != EFI_SUCCESS)
> > > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > > > > index 664dae28f882..44436d346286 100644
> > > > > > --- a/include/efi_loader.h
> > > > > > +++ b/include/efi_loader.h
> > > > > > @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
> > > > > >
> > > > > >   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
> > > > > >
> > > > > > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > > > > >   /*
> > > > > >    * Entry point for the tests of the EFI API.
> > > > > >    * It is called by 'bootefi selftest'
> > > > > >    */
> > > > > >   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
> > > > > >                              struct efi_system_table *systab);
> > > > > > -#endif
> > > > > >
> > > > > >   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > > > > >                                  const efi_guid_t *vendor, u32 *attributes,
> > > > >
> > >
> > > Thanks
> > > /Ilias

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[Heinrich Schuchardt]

On 11/21/23 02:29, AKASHI Takahiro wrote:
> Replicate some code and re-organize do_bootefi() into three cases, which
> will be carved out as independent functions in the next two commits.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   cmd/Kconfig          | 15 ++++++--
>   cmd/bootefi.c        | 82 ++++++++++++++++++++++++++++++--------------
>   include/efi_loader.h |  2 --
>   3 files changed, 69 insertions(+), 30 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 6f636155e5b6..4cf9a210c4a1 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -362,9 +362,19 @@ config CMD_BOOTEFI
>   	help
>   	  Boot an EFI image from memory.
>
> +if CMD_BOOTEFI
> +config CMD_BOOTEFI_BINARY
> +	bool "Allow booting an EFI binary directly"
> +	depends on BOOTEFI_BOOTMGR
> +	default y
> +	help
> +	  Select this option to enable direct execution of binary at 'bootefi'.
> +	  This subcommand will allow you to load the UEFI binary using
> +	  other U-Boot commands or external methods and then run it.
> +
>   config CMD_BOOTEFI_BOOTMGR
>   	bool "UEFI Boot Manager command"
> -	depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
> +	depends on BOOTEFI_BOOTMGR
>   	default y
>   	help
>   	  Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
> @@ -373,7 +383,7 @@ config CMD_BOOTEFI_BOOTMGR
>
>   config CMD_BOOTEFI_HELLO_COMPILE
>   	bool "Compile a standard EFI hello world binary for testing"
> -	depends on CMD_BOOTEFI && !CPU_V7M
> +	depends on !CPU_V7M

CMD_BOOTEFI_HELLO_COMPILE makes no sense without a command to run the
hello command. The CPU_V7M dependency is not needed as described in

[PATCH 1/1] cmd: CONFIG_CMD_BOOTEFI_HELLO_COMPILE dependencies
https://lore.kernel.org/u-boot/20231217114457.70128-1-heinrich.schuchardt@canonical.com/T/#u

Best regards

Heinrich

>   	default y
>   	help
>   	  This compiles a standard EFI hello world application with U-Boot so
> @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
>   	  up EFI support on a new architecture.
>
>   source lib/efi_selftest/Kconfig
> +endif
>
>   config CMD_BOOTMENU
>   	bool "bootmenu"
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 190ccba260e0..e9e5ab67a1f5 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -503,7 +503,6 @@ out:
>   	return (ret != EFI_SUCCESS) ? ret : ret2;
>   }
>
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>   static efi_status_t bootefi_run_prepare(const char *load_options_path,
>   		struct efi_device_path *device_path,
>   		struct efi_device_path *image_path,
> @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
>
>   	return ret != EFI_SUCCESS;
>   }
> -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
>
>   /**
>    * do_bootefi() - execute `bootefi` command
> @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   	if (argc < 2)
>   		return CMD_RET_USAGE;
>
> -	/* Initialize EFI drivers */
> -	ret = efi_init_obj_list();
> -	if (ret != EFI_SUCCESS) {
> -		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> -			ret & ~EFI_ERROR_MASK);
> -		return CMD_RET_FAILURE;
> -	}
> -
>   	if (argc > 2) {
>   		uintptr_t fdt_addr;
>
> @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   	} else {
>   		fdt = EFI_FDT_USE_INTERNAL;
>   	}
> -	ret = efi_install_fdt(fdt);
> -	if (ret == EFI_INVALID_PARAMETER)
> -		return CMD_RET_USAGE;
> -	else if (ret != EFI_SUCCESS)
> -		return CMD_RET_FAILURE;
>
> -	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> -		if (!strcmp(argv[1], "bootmgr"))
> -			return do_efibootmgr();
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> +	    !strcmp(argv[1], "bootmgr")) {
> +		/* Initialize EFI drivers */
> +		ret = efi_init_obj_list();
> +		if (ret != EFI_SUCCESS) {
> +			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +				ret & ~EFI_ERROR_MASK);
> +			return CMD_RET_FAILURE;
> +		}
> +
> +		ret = efi_install_fdt(fdt);
> +		if (ret == EFI_INVALID_PARAMETER)
> +			return CMD_RET_USAGE;
> +		else if (ret != EFI_SUCCESS)
> +			return CMD_RET_FAILURE;
> +
> +		return do_efibootmgr();
>   	}
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> -	if (!strcmp(argv[1], "selftest"))
> +
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> +	    !strcmp(argv[1], "selftest")) {
> +		/* Initialize EFI drivers */
> +		ret = efi_init_obj_list();
> +		if (ret != EFI_SUCCESS) {
> +			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +				ret & ~EFI_ERROR_MASK);
> +			return CMD_RET_FAILURE;
> +		}
> +
> +		ret = efi_install_fdt(fdt);
> +		if (ret == EFI_INVALID_PARAMETER)
> +			return CMD_RET_USAGE;
> +		else if (ret != EFI_SUCCESS)
> +			return CMD_RET_FAILURE;
> +
>   		return do_efi_selftest();
> -#endif
> +	}
>
> -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> -	if (!strcmp(argv[1], "hello")) {
> +	if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> +		return CMD_RET_SUCCESS;
> +
> +	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> +	    !strcmp(argv[1], "hello")) {
>   		image_buf = __efi_helloworld_begin;
>   		size = __efi_helloworld_end - __efi_helloworld_begin;
>   		efi_clear_bootdev();
> -	} else
> -#endif
> -	{
> +	} else {
>   		addr = strtoul(argv[1], NULL, 16);
>   		/* Check that a numeric value was passed */
>   		if (!addr)
> @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   			size = image_size;
>   		}
>   	}
> +
> +	/* Initialize EFI drivers */
> +	ret = efi_init_obj_list();
> +	if (ret != EFI_SUCCESS) {
> +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +			ret & ~EFI_ERROR_MASK);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	ret = efi_install_fdt(fdt);
> +	if (ret == EFI_INVALID_PARAMETER)
> +		return CMD_RET_USAGE;
> +	else if (ret != EFI_SUCCESS)
> +		return CMD_RET_FAILURE;
> +
>   	ret = efi_run_image(image_buf, size);
>
>   	if (ret != EFI_SUCCESS)
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 664dae28f882..44436d346286 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
>
>   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
>
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>   /*
>    * Entry point for the tests of the EFI API.
>    * It is called by 'bootefi selftest'
>    */
>   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>   				 struct efi_system_table *systab);
> -#endif
>
>   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>   				     const efi_guid_t *vendor, u32 *attributes,

Re: [PATCH v2 02/12] cmd: bootefi: re-organize do_bootefi()

[Heinrich Schuchardt]

On 12/17/23 13:00, Heinrich Schuchardt wrote:
> On 11/21/23 02:29, AKASHI Takahiro wrote:
>> Replicate some code and re-organize do_bootefi() into three cases, which
>> will be carved out as independent functions in the next two commits.
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> ---
>>   cmd/Kconfig          | 15 ++++++--
>>   cmd/bootefi.c        | 82 ++++++++++++++++++++++++++++++--------------
>>   include/efi_loader.h |  2 --
>>   3 files changed, 69 insertions(+), 30 deletions(-)
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 6f636155e5b6..4cf9a210c4a1 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -362,9 +362,19 @@ config CMD_BOOTEFI
>>       help
>>         Boot an EFI image from memory.
>>
>> +if CMD_BOOTEFI
>> +config CMD_BOOTEFI_BINARY
>> +    bool "Allow booting an EFI binary directly"
>> +    depends on BOOTEFI_BOOTMGR
>> +    default y
>> +    help
>> +      Select this option to enable direct execution of binary at 
>> 'bootefi'.
>> +      This subcommand will allow you to load the UEFI binary using
>> +      other U-Boot commands or external methods and then run it.
>> +
>>   config CMD_BOOTEFI_BOOTMGR
>>       bool "UEFI Boot Manager command"
>> -    depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
>> +    depends on BOOTEFI_BOOTMGR
>>       default y
>>       help
>>         Select this option to enable the 'bootmgr' subcommand of 
>> 'bootefi'.
>> @@ -373,7 +383,7 @@ config CMD_BOOTEFI_BOOTMGR
>>
>>   config CMD_BOOTEFI_HELLO_COMPILE
>>       bool "Compile a standard EFI hello world binary for testing"
>> -    depends on CMD_BOOTEFI && !CPU_V7M
>> +    depends on !CPU_V7M
> 
> CMD_BOOTEFI_HELLO_COMPILE makes no sense without a command to run the
> hello command. The CPU_V7M dependency is not needed as described in

Missed that you pulled that CMD_BOOTEFI into the if above.

> 
> [PATCH 1/1] cmd: CONFIG_CMD_BOOTEFI_HELLO_COMPILE dependencies
> https://lore.kernel.org/u-boot/20231217114457.70128-1-heinrich.schuchardt@canonical.com/T/#u
> 
> Best regards
> 
> Heinrich
> 
>>       default y
>>       help
>>         This compiles a standard EFI hello world application with 
>> U-Boot so
>> @@ -395,6 +405,7 @@ config CMD_BOOTEFI_HELLO
>>         up EFI support on a new architecture.
>>
>>   source lib/efi_selftest/Kconfig
>> +endif
>>
>>   config CMD_BOOTMENU
>>       bool "bootmenu"
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 190ccba260e0..e9e5ab67a1f5 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -503,7 +503,6 @@ out:
>>       return (ret != EFI_SUCCESS) ? ret : ret2;
>>   }
>>
>> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>>   static efi_status_t bootefi_run_prepare(const char *load_options_path,
>>           struct efi_device_path *device_path,
>>           struct efi_device_path *image_path,
>> @@ -593,7 +592,6 @@ static int do_efi_selftest(void)
>>
>>       return ret != EFI_SUCCESS;
>>   }
>> -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
>>
>>   /**
>>    * do_bootefi() - execute `bootefi` command
>> @@ -615,14 +613,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int 
>> flag, int argc,
>>       if (argc < 2)
>>           return CMD_RET_USAGE;
>>
>> -    /* Initialize EFI drivers */
>> -    ret = efi_init_obj_list();
>> -    if (ret != EFI_SUCCESS) {
>> -        log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>> -            ret & ~EFI_ERROR_MASK);
>> -        return CMD_RET_FAILURE;
>> -    }
>> -
>>       if (argc > 2) {
>>           uintptr_t fdt_addr;
>>
>> @@ -631,29 +621,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int 
>> flag, int argc,
>>       } else {
>>           fdt = EFI_FDT_USE_INTERNAL;
>>       }
>> -    ret = efi_install_fdt(fdt);
>> -    if (ret == EFI_INVALID_PARAMETER)
>> -        return CMD_RET_USAGE;
>> -    else if (ret != EFI_SUCCESS)
>> -        return CMD_RET_FAILURE;
>>
>> -    if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
>> -        if (!strcmp(argv[1], "bootmgr"))
>> -            return do_efibootmgr();
>> +    if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
>> +        !strcmp(argv[1], "bootmgr")) {
>> +        /* Initialize EFI drivers */
>> +        ret = efi_init_obj_list();
>> +        if (ret != EFI_SUCCESS) {
>> +            log_err("Error: Cannot initialize UEFI sub-system, r = 
>> %lu\n",
>> +                ret & ~EFI_ERROR_MASK);
>> +            return CMD_RET_FAILURE;
>> +        }
>> +
>> +        ret = efi_install_fdt(fdt);
>> +        if (ret == EFI_INVALID_PARAMETER)
>> +            return CMD_RET_USAGE;
>> +        else if (ret != EFI_SUCCESS)
>> +            return CMD_RET_FAILURE;
>> +
>> +        return do_efibootmgr();
>>       }
>> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>> -    if (!strcmp(argv[1], "selftest"))
>> +
>> +    if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
>> +        !strcmp(argv[1], "selftest")) {
>> +        /* Initialize EFI drivers */
>> +        ret = efi_init_obj_list();
>> +        if (ret != EFI_SUCCESS) {
>> +            log_err("Error: Cannot initialize UEFI sub-system, r = 
>> %lu\n",
>> +                ret & ~EFI_ERROR_MASK);
>> +            return CMD_RET_FAILURE;
>> +        }
>> +
>> +        ret = efi_install_fdt(fdt);
>> +        if (ret == EFI_INVALID_PARAMETER)
>> +            return CMD_RET_USAGE;
>> +        else if (ret != EFI_SUCCESS)
>> +            return CMD_RET_FAILURE;
>> +
>>           return do_efi_selftest();
>> -#endif
>> +    }
>>
>> -#ifdef CONFIG_CMD_BOOTEFI_HELLO
>> -    if (!strcmp(argv[1], "hello")) {
>> +    if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
>> +        return CMD_RET_SUCCESS;
>> +
>> +    if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
>> +        !strcmp(argv[1], "hello")) {
>>           image_buf = __efi_helloworld_begin;
>>           size = __efi_helloworld_end - __efi_helloworld_begin;
>>           efi_clear_bootdev();
>> -    } else
>> -#endif
>> -    {
>> +    } else {
>>           addr = strtoul(argv[1], NULL, 16);
>>           /* Check that a numeric value was passed */
>>           if (!addr)
>> @@ -675,6 +690,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int 
>> flag, int argc,
>>               size = image_size;
>>           }
>>       }
>> +
>> +    /* Initialize EFI drivers */
>> +    ret = efi_init_obj_list();
>> +    if (ret != EFI_SUCCESS) {
>> +        log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>> +            ret & ~EFI_ERROR_MASK);
>> +        return CMD_RET_FAILURE;
>> +    }
>> +
>> +    ret = efi_install_fdt(fdt);
>> +    if (ret == EFI_INVALID_PARAMETER)
>> +        return CMD_RET_USAGE;
>> +    else if (ret != EFI_SUCCESS)
>> +        return CMD_RET_FAILURE;
>> +
>>       ret = efi_run_image(image_buf, size);
>>
>>       if (ret != EFI_SUCCESS)
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 664dae28f882..44436d346286 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -879,14 +879,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
>>CMD_BOOTEFI 
>>   efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
>>
>> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>>   /*
>>    * Entry point for the tests of the EFI API.
>>    * It is called by 'bootefi selftest'
>>    */
>>   efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>                    struct efi_system_table *systab);
>> -#endif
>>
>>   efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>>                        const efi_guid_t *vendor, u32 *attributes,
> 

[PATCH v2 03/12] cmd: bootefi: carve out EFI boot manager interface

[AKASHI Takahiro]

Carve EFI boot manager related code out of do_bootefi_image() in order
to move boot manager specific code into library directory in the later
commit.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/bootefi.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index e9e5ab67a1f5..87910c42333a 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -413,28 +413,40 @@ out:
 }
 
 /**
- * do_efibootmgr() - execute EFI boot manager
+ * efi_bootmgr_run() - execute EFI boot manager
+ * fdt:	Flat device tree
+ *
+ * Invoke EFI boot manager and execute a binary depending on
+ * boot options. If @fdt is not NULL, it will be passed to
+ * the executed binary.
  *
  * Return:	status code
  */
-static int do_efibootmgr(void)
+static efi_status_t efi_bootmgr_run(void *fdt)
 {
 	efi_handle_t handle;
-	efi_status_t ret;
 	void *load_options;
+	efi_status_t ret;
 
-	ret = efi_bootmgr_load(&handle, &load_options);
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
 	if (ret != EFI_SUCCESS) {
-		log_notice("EFI boot manager: Cannot load any image\n");
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
 		return CMD_RET_FAILURE;
 	}
 
-	ret = do_bootefi_exec(handle, load_options);
-
+	ret = efi_install_fdt(fdt);
 	if (ret != EFI_SUCCESS)
-		return CMD_RET_FAILURE;
+		return ret;
 
-	return CMD_RET_SUCCESS;
+	ret = efi_bootmgr_load(&handle, &load_options);
+	if (ret != EFI_SUCCESS) {
+		log_notice("EFI boot manager: Cannot load any image\n");
+		return ret;
+	}
+
+	return do_bootefi_exec(handle, load_options);
 }
 
 /**
@@ -624,21 +636,14 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
 	    !strcmp(argv[1], "bootmgr")) {
-		/* Initialize EFI drivers */
-		ret = efi_init_obj_list();
-		if (ret != EFI_SUCCESS) {
-			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
-				ret & ~EFI_ERROR_MASK);
-			return CMD_RET_FAILURE;
-		}
+		ret = efi_bootmgr_run(fdt);
 
-		ret = efi_install_fdt(fdt);
 		if (ret == EFI_INVALID_PARAMETER)
 			return CMD_RET_USAGE;
-		else if (ret != EFI_SUCCESS)
+		else if (ret)
 			return CMD_RET_FAILURE;
 
-		return do_efibootmgr();
+		return CMD_RET_SUCCESS;
 	}
 
 	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
-- 
2.34.1

Re: [PATCH v2 03/12] cmd: bootefi: carve out EFI boot manager interface

[Heinrich Schuchardt]

On 11/21/23 02:29, AKASHI Takahiro wrote:
> Carve EFI boot manager related code out of do_bootefi_image() in order
> to move boot manager specific code into library directory in the later
> commit.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   cmd/bootefi.c | 43 ++++++++++++++++++++++++-------------------
>   1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index e9e5ab67a1f5..87910c42333a 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -413,28 +413,40 @@ out:
>   }
>
>   /**
> - * do_efibootmgr() - execute EFI boot manager
> + * efi_bootmgr_run() - execute EFI boot manager
> + * fdt:	Flat device tree
> + *
> + * Invoke EFI boot manager and execute a binary depending on
> + * boot options. If @fdt is not NULL, it will be passed to
> + * the executed binary.

How about the fallback to the control device-tree?
How about booting with ACPI?

Best regards

Heinrich

>    *
>    * Return:	status code
>    */
> -static int do_efibootmgr(void)
> +static efi_status_t efi_bootmgr_run(void *fdt)
>   {
>   	efi_handle_t handle;
> -	efi_status_t ret;
>   	void *load_options;
> +	efi_status_t ret;
>
> -	ret = efi_bootmgr_load(&handle, &load_options);
> +	/* Initialize EFI drivers */
> +	ret = efi_init_obj_list();
>   	if (ret != EFI_SUCCESS) {
> -		log_notice("EFI boot manager: Cannot load any image\n");
> +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +			ret & ~EFI_ERROR_MASK);
>   		return CMD_RET_FAILURE;
>   	}
>
> -	ret = do_bootefi_exec(handle, load_options);
> -
> +	ret = efi_install_fdt(fdt);
>   	if (ret != EFI_SUCCESS)
> -		return CMD_RET_FAILURE;
> +		return ret;
>
> -	return CMD_RET_SUCCESS;
> +	ret = efi_bootmgr_load(&handle, &load_options);
> +	if (ret != EFI_SUCCESS) {
> +		log_notice("EFI boot manager: Cannot load any image\n");
> +		return ret;
> +	}
> +
> +	return do_bootefi_exec(handle, load_options);
>   }
>
>   /**
> @@ -624,21 +636,14 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>
>   	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
>   	    !strcmp(argv[1], "bootmgr")) {
> -		/* Initialize EFI drivers */
> -		ret = efi_init_obj_list();
> -		if (ret != EFI_SUCCESS) {
> -			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> -				ret & ~EFI_ERROR_MASK);
> -			return CMD_RET_FAILURE;
> -		}
> +		ret = efi_bootmgr_run(fdt);
>
> -		ret = efi_install_fdt(fdt);
>   		if (ret == EFI_INVALID_PARAMETER)
>   			return CMD_RET_USAGE;
> -		else if (ret != EFI_SUCCESS)
> +		else if (ret)
>   			return CMD_RET_FAILURE;
>
> -		return do_efibootmgr();
> +		return CMD_RET_SUCCESS;
>   	}
>
>   	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&

Re: [PATCH v2 03/12] cmd: bootefi: carve out EFI boot manager interface

[AKASHI Takahiro]

On Tue, Nov 21, 2023 at 04:38:12AM +0100, Heinrich Schuchardt wrote:
> On 11/21/23 02:29, AKASHI Takahiro wrote:
> > Carve EFI boot manager related code out of do_bootefi_image() in order
> > to move boot manager specific code into library directory in the later
> > commit.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   cmd/bootefi.c | 43 ++++++++++++++++++++++++-------------------
> >   1 file changed, 24 insertions(+), 19 deletions(-)
> > 
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index e9e5ab67a1f5..87910c42333a 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -413,28 +413,40 @@ out:
> >   }
> > 
> >   /**
> > - * do_efibootmgr() - execute EFI boot manager
> > + * efi_bootmgr_run() - execute EFI boot manager
> > + * fdt:	Flat device tree
> > + *
> > + * Invoke EFI boot manager and execute a binary depending on
> > + * boot options. If @fdt is not NULL, it will be passed to
> > + * the executed binary.
> 
> How about the fallback to the control device-tree?

efi_install_fdt() will take care of that if fdt == EFI_FDT_INTERNAL_USE.
I didn't change any semantics.
I will add some description here to clarify it.

> How about booting with ACPI?

Not sure what is your concern, but
I didn't change any semantics in bootmgr use-case.

-Takahiro Akashi

> 
> Best regards
> 
> Heinrich
> 
> >    *
> >    * Return:	status code
> >    */
> > -static int do_efibootmgr(void)
> > +static efi_status_t efi_bootmgr_run(void *fdt)
> >   {
> >   	efi_handle_t handle;
> > -	efi_status_t ret;
> >   	void *load_options;
> > +	efi_status_t ret;
> > 
> > -	ret = efi_bootmgr_load(&handle, &load_options);
> > +	/* Initialize EFI drivers */
> > +	ret = efi_init_obj_list();
> >   	if (ret != EFI_SUCCESS) {
> > -		log_notice("EFI boot manager: Cannot load any image\n");
> > +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > +			ret & ~EFI_ERROR_MASK);
> >   		return CMD_RET_FAILURE;
> >   	}
> > 
> > -	ret = do_bootefi_exec(handle, load_options);
> > -
> > +	ret = efi_install_fdt(fdt);
> >   	if (ret != EFI_SUCCESS)
> > -		return CMD_RET_FAILURE;
> > +		return ret;
> > 
> > -	return CMD_RET_SUCCESS;
> > +	ret = efi_bootmgr_load(&handle, &load_options);
> > +	if (ret != EFI_SUCCESS) {
> > +		log_notice("EFI boot manager: Cannot load any image\n");
> > +		return ret;
> > +	}
> > +
> > +	return do_bootefi_exec(handle, load_options);
> >   }
> > 
> >   /**
> > @@ -624,21 +636,14 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > 
> >   	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> >   	    !strcmp(argv[1], "bootmgr")) {
> > -		/* Initialize EFI drivers */
> > -		ret = efi_init_obj_list();
> > -		if (ret != EFI_SUCCESS) {
> > -			log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > -				ret & ~EFI_ERROR_MASK);
> > -			return CMD_RET_FAILURE;
> > -		}
> > +		ret = efi_bootmgr_run(fdt);
> > 
> > -		ret = efi_install_fdt(fdt);
> >   		if (ret == EFI_INVALID_PARAMETER)
> >   			return CMD_RET_USAGE;
> > -		else if (ret != EFI_SUCCESS)
> > +		else if (ret)
> >   			return CMD_RET_FAILURE;
> > 
> > -		return do_efibootmgr();
> > +		return CMD_RET_SUCCESS;
> >   	}
> > 
> >   	if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> 

[PATCH v2 04/12] cmd: bootefi: carve out binary execution interface

[AKASHI Takahiro]

Carve binary execution code out of do_bootefi_image() in order to move
binary-execution specific code into library directory in the later
commit.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/bootefi.c | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 87910c42333a..957e2618aca2 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -515,6 +515,36 @@ out:
 	return (ret != EFI_SUCCESS) ? ret : ret2;
 }
 
+/**
+ * efi_binary_run() - run loaded UEFI image
+ *
+ * @image:	memory address of the UEFI image
+ * @size:	size of the UEFI image
+ *
+ * Execute an EFI binary image loaded at @image.
+ * @size may be zero if the binary is loaded with U-Boot load command.
+ *
+ * Return:	status code
+ */
+static efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
+{
+	efi_status_t ret;
+
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
+	if (ret != EFI_SUCCESS) {
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		return ret;
+	}
+
+	ret = efi_install_fdt(fdt);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	return efi_run_image(image, size);
+}
+
 static efi_status_t bootefi_run_prepare(const char *load_options_path,
 		struct efi_device_path *device_path,
 		struct efi_device_path *image_path,
@@ -696,23 +726,11 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 		}
 	}
 
-	/* Initialize EFI drivers */
-	ret = efi_init_obj_list();
-	if (ret != EFI_SUCCESS) {
-		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		return CMD_RET_FAILURE;
-	}
+	ret = efi_binary_run(image_buf, size, fdt);
 
-	ret = efi_install_fdt(fdt);
 	if (ret == EFI_INVALID_PARAMETER)
 		return CMD_RET_USAGE;
-	else if (ret != EFI_SUCCESS)
-		return CMD_RET_FAILURE;
-
-	ret = efi_run_image(image_buf, size);
-
-	if (ret != EFI_SUCCESS)
+	else if (ret)
 		return CMD_RET_FAILURE;
 
 	return CMD_RET_SUCCESS;
-- 
2.34.1

[PATCH v2 05/12] cmd: bootefi: localize global device paths for efi_selftest

[AKASHI Takahiro]

Device paths allocated in bootefi_test_prepare() will be immediately
consumed by do_efi_selftest() and there is no need to keep them for later
use. Introduce test-specific varialbles to make it easier to move other
bootmgr functions into library directory in the next commit.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/bootefi.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 957e2618aca2..7930c99def44 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -29,6 +29,8 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static struct efi_device_path *test_image_path;
+static struct efi_device_path *test_device_path;
 static struct efi_device_path *bootefi_image_path;
 static struct efi_device_path *bootefi_device_path;
 static void *image_addr;
@@ -586,23 +588,26 @@ static efi_status_t bootefi_test_prepare
 	efi_status_t ret;
 
 	/* Construct a dummy device path */
-	bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
-	if (!bootefi_device_path)
+	test_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
+	if (!test_device_path)
 		return EFI_OUT_OF_RESOURCES;
 
-	bootefi_image_path = efi_dp_from_file(NULL, path);
-	if (!bootefi_image_path) {
+	test_image_path = efi_dp_from_file(NULL, path);
+	if (!test_image_path) {
 		ret = EFI_OUT_OF_RESOURCES;
 		goto failure;
 	}
 
-	ret = bootefi_run_prepare(load_options_path, bootefi_device_path,
-				  bootefi_image_path, image_objp,
+	ret = bootefi_run_prepare(load_options_path, test_device_path,
+				  test_image_path, image_objp,
 				  loaded_image_infop);
 	if (ret == EFI_SUCCESS)
 		return ret;
 
 failure:
+	efi_free_pool(test_device_path);
+	efi_free_pool(test_image_path);
+	/* TODO: not sure calling clear function is necessary */
 	efi_clear_bootdev();
 	return ret;
 }
@@ -627,6 +632,8 @@ static int do_efi_selftest(void)
 	ret = EFI_CALL(efi_selftest(&image_obj->header, &systab));
 	efi_restore_gd();
 	free(loaded_image_info->load_options);
+	efi_free_pool(test_device_path);
+	efi_free_pool(test_image_path);
 	if (ret != EFI_SUCCESS)
 		efi_delete_handle(&image_obj->header);
 	else
-- 
2.34.1

[PATCH v2 06/12] cmd: bootefi: move library interfaces under lib/efi_loader

[AKASHI Takahiro]

In the prior commits, interfaces for executing EFI binary and boot manager
were carved out. Move them under efi_loader directory so that they can
be called from other places without depending on bootefi command.

Only efi_selftest-related code will be left in bootefi.c.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/bootefi.c                | 539 +----------------------------------
 include/efi_loader.h         |  10 +
 lib/efi_loader/efi_bootmgr.c | 529 ++++++++++++++++++++++++++++++++++
 3 files changed, 550 insertions(+), 528 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 7930c99def44..9cf9027bf409 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -7,545 +7,22 @@
 
 #define LOG_CATEGORY LOGC_EFI
 
-#include <common.h>
-#include <bootm.h>
-#include <charset.h>
 #include <command.h>
-#include <dm.h>
+#include <efi.h>
 #include <efi_loader.h>
-#include <efi_selftest.h>
-#include <env.h>
-#include <errno.h>
-#include <image.h>
+#include <exports.h>
 #include <log.h>
 #include <malloc.h>
-#include <asm/global_data.h>
-#include <linux/libfdt.h>
-#include <linux/libfdt_env.h>
 #include <mapmem.h>
-#include <memalign.h>
+#include <vsprintf.h>
 #include <asm-generic/sections.h>
-#include <linux/linkage.h>
+#include <asm/global_data.h>
+#include <linux/string.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 static struct efi_device_path *test_image_path;
 static struct efi_device_path *test_device_path;
-static struct efi_device_path *bootefi_image_path;
-static struct efi_device_path *bootefi_device_path;
-static void *image_addr;
-static size_t image_size;
-
-/**
- * efi_get_image_parameters() - return image parameters
- *
- * @img_addr:		address of loaded image in memory
- * @img_size:		size of loaded image
- */
-void efi_get_image_parameters(void **img_addr, size_t *img_size)
-{
-	*img_addr = image_addr;
-	*img_size = image_size;
-}
-
-/**
- * efi_clear_bootdev() - clear boot device
- */
-static void efi_clear_bootdev(void)
-{
-	efi_free_pool(bootefi_device_path);
-	efi_free_pool(bootefi_image_path);
-	bootefi_device_path = NULL;
-	bootefi_image_path = NULL;
-	image_addr = NULL;
-	image_size = 0;
-}
-
-/**
- * efi_set_bootdev() - set boot device
- *
- * This function is called when a file is loaded, e.g. via the 'load' command.
- * We use the path to this file to inform the UEFI binary about the boot device.
- *
- * @dev:		device, e.g. "MMC"
- * @devnr:		number of the device, e.g. "1:2"
- * @path:		path to file loaded
- * @buffer:		buffer with file loaded
- * @buffer_size:	size of file loaded
- */
-void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
-		     void *buffer, size_t buffer_size)
-{
-	struct efi_device_path *device, *image;
-	efi_status_t ret;
-
-	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
-		  devnr, path, buffer, buffer_size);
-
-	/* Forget overwritten image */
-	if (buffer + buffer_size >= image_addr &&
-	    image_addr + image_size >= buffer)
-		efi_clear_bootdev();
-
-	/* Remember only PE-COFF and FIT images */
-	if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
-		if (IS_ENABLED(CONFIG_FIT) &&
-		    !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
-			/*
-			 * FIT images of type EFI_OS are started via command
-			 * bootm. We should not use their boot device with the
-			 * bootefi command.
-			 */
-			buffer = 0;
-			buffer_size = 0;
-		} else {
-			log_debug("- not remembering image\n");
-			return;
-		}
-	}
-
-	/* efi_set_bootdev() is typically called repeatedly, recover memory */
-	efi_clear_bootdev();
-
-	image_addr = buffer;
-	image_size = buffer_size;
-
-	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
-	if (ret == EFI_SUCCESS) {
-		bootefi_device_path = device;
-		if (image) {
-			/* FIXME: image should not contain device */
-			struct efi_device_path *image_tmp = image;
-
-			efi_dp_split_file_path(image, &device, &image);
-			efi_free_pool(image_tmp);
-		}
-		bootefi_image_path = image;
-		log_debug("- boot device %pD\n", device);
-		if (image)
-			log_debug("- image %pD\n", image);
-	} else {
-		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
-		efi_clear_bootdev();
-	}
-}
-
-/**
- * efi_env_set_load_options() - set load options from environment variable
- *
- * @handle:		the image handle
- * @env_var:		name of the environment variable
- * @load_options:	pointer to load options (output)
- * Return:		status code
- */
-static efi_status_t efi_env_set_load_options(efi_handle_t handle,
-					     const char *env_var,
-					     u16 **load_options)
-{
-	const char *env = env_get(env_var);
-	size_t size;
-	u16 *pos;
-	efi_status_t ret;
-
-	*load_options = NULL;
-	if (!env)
-		return EFI_SUCCESS;
-	size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
-	pos = calloc(size, 1);
-	if (!pos)
-		return EFI_OUT_OF_RESOURCES;
-	*load_options = pos;
-	utf8_utf16_strcpy(&pos, env);
-	ret = efi_set_load_options(handle, size, *load_options);
-	if (ret != EFI_SUCCESS) {
-		free(*load_options);
-		*load_options = NULL;
-	}
-	return ret;
-}
-
-#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
-
-/**
- * copy_fdt() - Copy the device tree to a new location available to EFI
- *
- * The FDT is copied to a suitable location within the EFI memory map.
- * Additional 12 KiB are added to the space in case the device tree needs to be
- * expanded later with fdt_open_into().
- *
- * @fdtp:	On entry a pointer to the flattened device tree.
- *		On exit a pointer to the copy of the flattened device tree.
- *		FDT start
- * Return:	status code
- */
-static efi_status_t copy_fdt(void **fdtp)
-{
-	unsigned long fdt_ram_start = -1L, fdt_pages;
-	efi_status_t ret = 0;
-	void *fdt, *new_fdt;
-	u64 new_fdt_addr;
-	uint fdt_size;
-	int i;
-
-	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
-		u64 ram_start = gd->bd->bi_dram[i].start;
-		u64 ram_size = gd->bd->bi_dram[i].size;
-
-		if (!ram_size)
-			continue;
-
-		if (ram_start < fdt_ram_start)
-			fdt_ram_start = ram_start;
-	}
-
-	/*
-	 * Give us at least 12 KiB of breathing room in case the device tree
-	 * needs to be expanded later.
-	 */
-	fdt = *fdtp;
-	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
-	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
-
-	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
-				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-				 &new_fdt_addr);
-	if (ret != EFI_SUCCESS) {
-		log_err("ERROR: Failed to reserve space for FDT\n");
-		goto done;
-	}
-	new_fdt = (void *)(uintptr_t)new_fdt_addr;
-	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
-	fdt_set_totalsize(new_fdt, fdt_size);
-
-	*fdtp = (void *)(uintptr_t)new_fdt_addr;
-done:
-	return ret;
-}
-
-/**
- * get_config_table() - get configuration table
- *
- * @guid:	GUID of the configuration table
- * Return:	pointer to configuration table or NULL
- */
-static void *get_config_table(const efi_guid_t *guid)
-{
-	size_t i;
-
-	for (i = 0; i < systab.nr_tables; i++) {
-		if (!guidcmp(guid, &systab.tables[i].guid))
-			return systab.tables[i].table;
-	}
-	return NULL;
-}
-
-#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
-
-/**
- * efi_install_fdt() - install device tree
- *
- * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
- * address will will be installed as configuration table, otherwise the device
- * tree located at the address indicated by environment variable fdt_addr or as
- * fallback fdtcontroladdr will be used.
- *
- * On architectures using ACPI tables device trees shall not be installed as
- * configuration table.
- *
- * @fdt:	address of device tree or EFI_FDT_USE_INTERNAL to use the
- *		the hardware device tree as indicated by environment variable
- *		fdt_addr or as fallback the internal device tree as indicated by
- *		the environment variable fdtcontroladdr
- * Return:	status code
- */
-efi_status_t efi_install_fdt(void *fdt)
-{
-	/*
-	 * The EBBR spec requires that we have either an FDT or an ACPI table
-	 * but not both.
-	 */
-#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
-	if (fdt) {
-		log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
-		return EFI_SUCCESS;
-	}
-#else
-	struct bootm_headers img = { 0 };
-	efi_status_t ret;
-
-	if (fdt == EFI_FDT_USE_INTERNAL) {
-		const char *fdt_opt;
-		uintptr_t fdt_addr;
-
-		/* Look for device tree that is already installed */
-		if (get_config_table(&efi_guid_fdt))
-			return EFI_SUCCESS;
-		/* Check if there is a hardware device tree */
-		fdt_opt = env_get("fdt_addr");
-		/* Use our own device tree as fallback */
-		if (!fdt_opt) {
-			fdt_opt = env_get("fdtcontroladdr");
-			if (!fdt_opt) {
-				log_err("ERROR: need device tree\n");
-				return EFI_NOT_FOUND;
-			}
-		}
-		fdt_addr = hextoul(fdt_opt, NULL);
-		if (!fdt_addr) {
-			log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
-			return EFI_LOAD_ERROR;
-		}
-		fdt = map_sysmem(fdt_addr, 0);
-	}
-
-	/* Install device tree */
-	if (fdt_check_header(fdt)) {
-		log_err("ERROR: invalid device tree\n");
-		return EFI_LOAD_ERROR;
-	}
-
-	/* Prepare device tree for payload */
-	ret = copy_fdt(&fdt);
-	if (ret) {
-		log_err("ERROR: out of memory\n");
-		return EFI_OUT_OF_RESOURCES;
-	}
-
-	if (image_setup_libfdt(&img, fdt, 0, NULL)) {
-		log_err("ERROR: failed to process device tree\n");
-		return EFI_LOAD_ERROR;
-	}
-
-	/* Create memory reservations as indicated by the device tree */
-	efi_carve_out_dt_rsv(fdt);
-
-	efi_try_purge_kaslr_seed(fdt);
-
-	if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
-		ret = efi_tcg2_measure_dtb(fdt);
-		if (ret == EFI_SECURITY_VIOLATION) {
-			log_err("ERROR: failed to measure DTB\n");
-			return ret;
-		}
-	}
-
-	/* Install device tree as UEFI table */
-	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
-	if (ret != EFI_SUCCESS) {
-		log_err("ERROR: failed to install device tree\n");
-		return ret;
-	}
-#endif /* GENERATE_ACPI_TABLE */
-
-	return EFI_SUCCESS;
-}
-
-/**
- * do_bootefi_exec() - execute EFI binary
- *
- * The image indicated by @handle is started. When it returns the allocated
- * memory for the @load_options is freed.
- *
- * @handle:		handle of loaded image
- * @load_options:	load options
- * Return:		status code
- *
- * Load the EFI binary into a newly assigned memory unwinding the relocation
- * information, install the loaded image protocol, and call the binary.
- */
-static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
-{
-	efi_status_t ret;
-	efi_uintn_t exit_data_size = 0;
-	u16 *exit_data = NULL;
-	struct efi_event *evt;
-
-	/* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
-	switch_to_non_secure_mode();
-
-	/*
-	 * The UEFI standard requires that the watchdog timer is set to five
-	 * minutes when invoking an EFI boot option.
-	 *
-	 * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
-	 * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
-	 */
-	ret = efi_set_watchdog(300);
-	if (ret != EFI_SUCCESS) {
-		log_err("ERROR: Failed to set watchdog timer\n");
-		goto out;
-	}
-
-	/* Call our payload! */
-	ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
-	if (ret != EFI_SUCCESS) {
-		log_err("## Application failed, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		if (exit_data) {
-			log_err("## %ls\n", exit_data);
-			efi_free_pool(exit_data);
-		}
-	}
-
-	efi_restore_gd();
-
-out:
-	free(load_options);
-
-	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
-		if (efi_initrd_deregister() != EFI_SUCCESS)
-			log_err("Failed to remove loadfile2 for initrd\n");
-	}
-
-	/* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
-	list_for_each_entry(evt, &efi_events, link) {
-		if (evt->group &&
-		    !guidcmp(evt->group,
-			     &efi_guid_event_group_return_to_efibootmgr)) {
-			efi_signal_event(evt);
-			EFI_CALL(systab.boottime->close_event(evt));
-			break;
-		}
-	}
-
-	/* Control is returned to U-Boot, disable EFI watchdog */
-	efi_set_watchdog(0);
-
-	return ret;
-}
-
-/**
- * efi_bootmgr_run() - execute EFI boot manager
- * fdt:	Flat device tree
- *
- * Invoke EFI boot manager and execute a binary depending on
- * boot options. If @fdt is not NULL, it will be passed to
- * the executed binary.
- *
- * Return:	status code
- */
-static efi_status_t efi_bootmgr_run(void *fdt)
-{
-	efi_handle_t handle;
-	void *load_options;
-	efi_status_t ret;
-
-	/* Initialize EFI drivers */
-	ret = efi_init_obj_list();
-	if (ret != EFI_SUCCESS) {
-		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		return CMD_RET_FAILURE;
-	}
-
-	ret = efi_install_fdt(fdt);
-	if (ret != EFI_SUCCESS)
-		return ret;
-
-	ret = efi_bootmgr_load(&handle, &load_options);
-	if (ret != EFI_SUCCESS) {
-		log_notice("EFI boot manager: Cannot load any image\n");
-		return ret;
-	}
-
-	return do_bootefi_exec(handle, load_options);
-}
-
-/**
- * efi_run_image() - run loaded UEFI image
- *
- * @source_buffer:	memory address of the UEFI image
- * @source_size:	size of the UEFI image
- * Return:		status code
- */
-efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
-{
-	efi_handle_t mem_handle = NULL, handle;
-	struct efi_device_path *file_path = NULL;
-	struct efi_device_path *msg_path;
-	efi_status_t ret, ret2;
-	u16 *load_options;
-
-	if (!bootefi_device_path || !bootefi_image_path) {
-		log_debug("Not loaded from disk\n");
-		/*
-		 * Special case for efi payload not loaded from disk,
-		 * such as 'bootefi hello' or for example payload
-		 * loaded directly into memory via JTAG, etc:
-		 */
-		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
-					    (uintptr_t)source_buffer,
-					    source_size);
-		/*
-		 * Make sure that device for device_path exist
-		 * in load_image(). Otherwise, shell and grub will fail.
-		 */
-		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
-							       &efi_guid_device_path,
-							       file_path, NULL);
-		if (ret != EFI_SUCCESS)
-			goto out;
-		msg_path = file_path;
-	} else {
-		file_path = efi_dp_append(bootefi_device_path,
-					  bootefi_image_path);
-		msg_path = bootefi_image_path;
-		log_debug("Loaded from disk\n");
-	}
-
-	log_info("Booting %pD\n", msg_path);
-
-	ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
-				      source_size, &handle));
-	if (ret != EFI_SUCCESS) {
-		log_err("Loading image failed\n");
-		goto out;
-	}
-
-	/* Transfer environment variable as load options */
-	ret = efi_env_set_load_options(handle, "bootargs", &load_options);
-	if (ret != EFI_SUCCESS)
-		goto out;
-
-	ret = do_bootefi_exec(handle, load_options);
-
-out:
-	ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
-							  &efi_guid_device_path,
-							  file_path, NULL);
-	efi_free_pool(file_path);
-	return (ret != EFI_SUCCESS) ? ret : ret2;
-}
-
-/**
- * efi_binary_run() - run loaded UEFI image
- *
- * @image:	memory address of the UEFI image
- * @size:	size of the UEFI image
- *
- * Execute an EFI binary image loaded at @image.
- * @size may be zero if the binary is loaded with U-Boot load command.
- *
- * Return:	status code
- */
-static efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
-{
-	efi_status_t ret;
-
-	/* Initialize EFI drivers */
-	ret = efi_init_obj_list();
-	if (ret != EFI_SUCCESS) {
-		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		return ret;
-	}
-
-	ret = efi_install_fdt(fdt);
-	if (ret != EFI_SUCCESS)
-		return ret;
-
-	return efi_run_image(image, size);
-}
 
 static efi_status_t bootefi_run_prepare(const char *load_options_path,
 		struct efi_device_path *device_path,
@@ -658,6 +135,8 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 	char *p;
 	void *fdt, *image_buf;
 	unsigned long addr, size;
+	void *image_addr;
+	size_t image_size;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
@@ -709,6 +188,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 	    !strcmp(argv[1], "hello")) {
 		image_buf = __efi_helloworld_begin;
 		size = __efi_helloworld_end - __efi_helloworld_begin;
+		/* TODO: not sure calling clear function is necessary */
 		efi_clear_bootdev();
 	} else {
 		addr = strtoul(argv[1], NULL, 16);
@@ -724,6 +204,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
 				return CMD_RET_USAGE;
 			efi_clear_bootdev();
 		} else {
+			/* Image should be already loaded */
+			efi_get_image_parameters(&image_addr, &image_size);
+
 			if (image_buf != image_addr) {
 				log_err("No UEFI binary known at %s\n",
 					argv[1]);
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 44436d346286..34e7fbbf1840 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -90,6 +90,8 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len);
  * back to u-boot world
  */
 void efi_restore_gd(void);
+/* Call this to unset the current device name */
+void efi_clear_bootdev(void);
 /* Call this to set the current device name */
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
 		     void *buffer, size_t buffer_size);
@@ -114,6 +116,7 @@ static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len)
 
 /* No loader configured, stub out EFI_ENTRY */
 static inline void efi_restore_gd(void) { }
+static inline void efi_clear_bootdev(void) { }
 static inline void efi_set_bootdev(const char *dev, const char *devnr,
 				   const char *path, void *buffer,
 				   size_t buffer_size) { }
@@ -527,14 +530,21 @@ efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
 efi_status_t efi_bootmgr_update_media_device_boot_option(void);
 /* Delete selected boot option */
 efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
+/* Invoke EFI boot manager */
+efi_status_t efi_bootmgr_run(void *fdt);
 /* search the boot option index in BootOrder */
 bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index);
 /* Set up console modes */
 void efi_setup_console_size(void);
+/* Set up load options from environment variable */
+efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
+				      u16 **load_options);
 /* Install device tree */
 efi_status_t efi_install_fdt(void *fdt);
 /* Run loaded UEFI image */
 efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
+/* Run loaded UEFI image with given fdt */
+efi_status_t efi_binary_run(void *image, size_t size, void *fdt);
 /* Initialize variable services */
 efi_status_t efi_init_variables(void);
 /* Notify ExitBootServices() is called */
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 48153bd5ffbb..e910f1bb2a23 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -3,6 +3,8 @@
  *  EFI boot manager
  *
  *  Copyright (c) 2017 Rob Clark
+ *  For the code moved from cmd/bootefi.c
+ *  Copyright (c) 2016 Alexander Graf
  */
 
 #define LOG_CATEGORY LOGC_EFI
@@ -20,6 +22,17 @@
 #include <efi_variable.h>
 #include <asm/unaligned.h>
 
+/* TODO: temporarily added here; clean up later */
+#include <bootm.h>
+#include <efi_selftest.h>
+#include <env.h>
+#include <mapmem.h>
+#include <asm/global_data.h>
+#include <linux/libfdt.h>
+#include <linux/libfdt_env.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
 static const struct efi_boot_services *bs;
 static const struct efi_runtime_services *rs;
 
@@ -1115,3 +1128,519 @@ out:
 		return EFI_SUCCESS;
 	return ret;
 }
+
+static struct efi_device_path *bootefi_image_path;
+static struct efi_device_path *bootefi_device_path;
+static void *image_addr;
+static size_t image_size;
+
+/**
+ * efi_get_image_parameters() - return image parameters
+ *
+ * @img_addr:		address of loaded image in memory
+ * @img_size:		size of loaded image
+ */
+void efi_get_image_parameters(void **img_addr, size_t *img_size)
+{
+	*img_addr = image_addr;
+	*img_size = image_size;
+}
+
+/**
+ * efi_clear_bootdev() - clear boot device
+ */
+void efi_clear_bootdev(void)
+{
+	efi_free_pool(bootefi_device_path);
+	efi_free_pool(bootefi_image_path);
+	bootefi_device_path = NULL;
+	bootefi_image_path = NULL;
+	image_addr = NULL;
+	image_size = 0;
+}
+
+/**
+ * efi_set_bootdev() - set boot device
+ *
+ * This function is called when a file is loaded, e.g. via the 'load' command.
+ * We use the path to this file to inform the UEFI binary about the boot device.
+ *
+ * @dev:		device, e.g. "MMC"
+ * @devnr:		number of the device, e.g. "1:2"
+ * @path:		path to file loaded
+ * @buffer:		buffer with file loaded
+ * @buffer_size:	size of file loaded
+ */
+void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
+		     void *buffer, size_t buffer_size)
+{
+	struct efi_device_path *device, *image;
+	efi_status_t ret;
+
+	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
+		  devnr, path, buffer, buffer_size);
+
+	/* Forget overwritten image */
+	if (buffer + buffer_size >= image_addr &&
+	    image_addr + image_size >= buffer)
+		efi_clear_bootdev();
+
+	/* Remember only PE-COFF and FIT images */
+	if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
+		if (IS_ENABLED(CONFIG_FIT) &&
+		    !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
+			/*
+			 * FIT images of type EFI_OS are started via command
+			 * bootm. We should not use their boot device with the
+			 * bootefi command.
+			 */
+			buffer = 0;
+			buffer_size = 0;
+		} else {
+			log_debug("- not remembering image\n");
+			return;
+		}
+	}
+
+	/* efi_set_bootdev() is typically called repeatedly, recover memory */
+	efi_clear_bootdev();
+
+	image_addr = buffer;
+	image_size = buffer_size;
+
+	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
+	if (ret == EFI_SUCCESS) {
+		bootefi_device_path = device;
+		if (image) {
+			/* FIXME: image should not contain device */
+			struct efi_device_path *image_tmp = image;
+
+			efi_dp_split_file_path(image, &device, &image);
+			efi_free_pool(image_tmp);
+		}
+		bootefi_image_path = image;
+		log_debug("- boot device %pD\n", device);
+		if (image)
+			log_debug("- image %pD\n", image);
+	} else {
+		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
+		efi_clear_bootdev();
+	}
+}
+
+/**
+ * efi_env_set_load_options() - set load options from environment variable
+ *
+ * @handle:		the image handle
+ * @env_var:		name of the environment variable
+ * @load_options:	pointer to load options (output)
+ * Return:		status code
+ */
+efi_status_t efi_env_set_load_options(efi_handle_t handle,
+				      const char *env_var,
+				      u16 **load_options)
+{
+	const char *env = env_get(env_var);
+	size_t size;
+	u16 *pos;
+	efi_status_t ret;
+
+	*load_options = NULL;
+	if (!env)
+		return EFI_SUCCESS;
+	size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
+	pos = calloc(size, 1);
+	if (!pos)
+		return EFI_OUT_OF_RESOURCES;
+	*load_options = pos;
+	utf8_utf16_strcpy(&pos, env);
+	ret = efi_set_load_options(handle, size, *load_options);
+	if (ret != EFI_SUCCESS) {
+		free(*load_options);
+		*load_options = NULL;
+	}
+	return ret;
+}
+
+#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+
+/**
+ * copy_fdt() - Copy the device tree to a new location available to EFI
+ *
+ * The FDT is copied to a suitable location within the EFI memory map.
+ * Additional 12 KiB are added to the space in case the device tree needs to be
+ * expanded later with fdt_open_into().
+ *
+ * @fdtp:	On entry a pointer to the flattened device tree.
+ *		On exit a pointer to the copy of the flattened device tree.
+ *		FDT start
+ * Return:	status code
+ */
+static efi_status_t copy_fdt(void **fdtp)
+{
+	unsigned long fdt_ram_start = -1L, fdt_pages;
+	efi_status_t ret = 0;
+	void *fdt, *new_fdt;
+	u64 new_fdt_addr;
+	uint fdt_size;
+	int i;
+
+	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+		u64 ram_start = gd->bd->bi_dram[i].start;
+		u64 ram_size = gd->bd->bi_dram[i].size;
+
+		if (!ram_size)
+			continue;
+
+		if (ram_start < fdt_ram_start)
+			fdt_ram_start = ram_start;
+	}
+
+	/*
+	 * Give us at least 12 KiB of breathing room in case the device tree
+	 * needs to be expanded later.
+	 */
+	fdt = *fdtp;
+	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
+	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
+
+	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
+				 &new_fdt_addr);
+	if (ret != EFI_SUCCESS) {
+		log_err("ERROR: Failed to reserve space for FDT\n");
+		goto done;
+	}
+	new_fdt = (void *)(uintptr_t)new_fdt_addr;
+	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
+	fdt_set_totalsize(new_fdt, fdt_size);
+
+	*fdtp = (void *)(uintptr_t)new_fdt_addr;
+done:
+	return ret;
+}
+
+/**
+ * get_config_table() - get configuration table
+ *
+ * @guid:	GUID of the configuration table
+ * Return:	pointer to configuration table or NULL
+ */
+static void *get_config_table(const efi_guid_t *guid)
+{
+	size_t i;
+
+	for (i = 0; i < systab.nr_tables; i++) {
+		if (!guidcmp(guid, &systab.tables[i].guid))
+			return systab.tables[i].table;
+	}
+	return NULL;
+}
+
+#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
+
+/**
+ * efi_install_fdt() - install device tree
+ *
+ * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
+ * address will be installed as configuration table, otherwise the device
+ * tree located at the address indicated by environment variable fdt_addr or as
+ * fallback fdtcontroladdr will be used.
+ *
+ * On architectures using ACPI tables device trees shall not be installed as
+ * configuration table.
+ *
+ * @fdt:	address of device tree or EFI_FDT_USE_INTERNAL to use
+ *		the hardware device tree as indicated by environment variable
+ *		fdt_addr or as fallback the internal device tree as indicated by
+ *		the environment variable fdtcontroladdr
+ * Return:	status code
+ */
+efi_status_t efi_install_fdt(void *fdt)
+{
+	/*
+	 * The EBBR spec requires that we have either an FDT or an ACPI table
+	 * but not both.
+	 */
+#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+	if (fdt) {
+		log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
+		return EFI_SUCCESS;
+	}
+#else
+	struct bootm_headers img = { 0 };
+	efi_status_t ret;
+
+	if (fdt == EFI_FDT_USE_INTERNAL) {
+		const char *fdt_opt;
+		uintptr_t fdt_addr;
+
+		/* Look for device tree that is already installed */
+		if (get_config_table(&efi_guid_fdt))
+			return EFI_SUCCESS;
+		/* Check if there is a hardware device tree */
+		fdt_opt = env_get("fdt_addr");
+		/* Use our own device tree as fallback */
+		if (!fdt_opt) {
+			fdt_opt = env_get("fdtcontroladdr");
+			if (!fdt_opt) {
+				log_err("ERROR: need device tree\n");
+				return EFI_NOT_FOUND;
+			}
+		}
+		fdt_addr = hextoul(fdt_opt, NULL);
+		if (!fdt_addr) {
+			log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
+			return EFI_LOAD_ERROR;
+		}
+		fdt = map_sysmem(fdt_addr, 0);
+	}
+
+	/* Install device tree */
+	if (fdt_check_header(fdt)) {
+		log_err("ERROR: invalid device tree\n");
+		return EFI_LOAD_ERROR;
+	}
+
+	/* Prepare device tree for payload */
+	ret = copy_fdt(&fdt);
+	if (ret) {
+		log_err("ERROR: out of memory\n");
+		return EFI_OUT_OF_RESOURCES;
+	}
+
+	if (image_setup_libfdt(&img, fdt, 0, NULL)) {
+		log_err("ERROR: failed to process device tree\n");
+		return EFI_LOAD_ERROR;
+	}
+
+	/* Create memory reservations as indicated by the device tree */
+	efi_carve_out_dt_rsv(fdt);
+
+	efi_try_purge_kaslr_seed(fdt);
+
+	if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
+		ret = efi_tcg2_measure_dtb(fdt);
+		if (ret == EFI_SECURITY_VIOLATION) {
+			log_err("ERROR: failed to measure DTB\n");
+			return ret;
+		}
+	}
+
+	/* Install device tree as UEFI table */
+	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
+	if (ret != EFI_SUCCESS) {
+		log_err("ERROR: failed to install device tree\n");
+		return ret;
+	}
+#endif /* GENERATE_ACPI_TABLE */
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * do_bootefi_exec() - execute EFI binary
+ *
+ * The image indicated by @handle is started. When it returns the allocated
+ * memory for the @load_options is freed.
+ *
+ * @handle:		handle of loaded image
+ * @load_options:	load options
+ * Return:		status code
+ *
+ * Load the EFI binary into a newly assigned memory unwinding the relocation
+ * information, install the loaded image protocol, and call the binary.
+ */
+static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
+{
+	efi_status_t ret;
+	efi_uintn_t exit_data_size = 0;
+	u16 *exit_data = NULL;
+	struct efi_event *evt;
+
+	/* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
+	switch_to_non_secure_mode();
+
+	/*
+	 * The UEFI standard requires that the watchdog timer is set to five
+	 * minutes when invoking an EFI boot option.
+	 *
+	 * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
+	 * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
+	 */
+	ret = efi_set_watchdog(300);
+	if (ret != EFI_SUCCESS) {
+		log_err("ERROR: Failed to set watchdog timer\n");
+		goto out;
+	}
+
+	/* Call our payload! */
+	ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
+	if (ret != EFI_SUCCESS) {
+		log_err("## Application failed, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		if (exit_data) {
+			log_err("## %ls\n", exit_data);
+			efi_free_pool(exit_data);
+		}
+	}
+
+	efi_restore_gd();
+
+out:
+	free(load_options);
+
+	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
+		if (efi_initrd_deregister() != EFI_SUCCESS)
+			log_err("Failed to remove loadfile2 for initrd\n");
+	}
+
+	/* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
+	list_for_each_entry(evt, &efi_events, link) {
+		if (evt->group &&
+		    !guidcmp(evt->group,
+			     &efi_guid_event_group_return_to_efibootmgr)) {
+			efi_signal_event(evt);
+			EFI_CALL(systab.boottime->close_event(evt));
+			break;
+		}
+	}
+
+	/* Control is returned to U-Boot, disable EFI watchdog */
+	efi_set_watchdog(0);
+
+	return ret;
+}
+
+/**
+ * efi_bootmgr_run() - execute EFI boot manager
+ * fdt:	Flat device tree
+ *
+ * Invoke EFI boot manager and execute a binary depending on
+ * boot options. If @fdt is not NULL, it will be passed to
+ * the executed binary.
+ *
+ * Return:	status code
+ */
+efi_status_t efi_bootmgr_run(void *fdt)
+{
+	efi_handle_t handle;
+	void *load_options;
+	efi_status_t ret;
+
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
+	if (ret != EFI_SUCCESS) {
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		return CMD_RET_FAILURE;
+	}
+
+	ret = efi_install_fdt(fdt);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	ret = efi_bootmgr_load(&handle, &load_options);
+	if (ret != EFI_SUCCESS) {
+		log_notice("EFI boot manager: Cannot load any image\n");
+		return ret;
+	}
+
+	return do_bootefi_exec(handle, load_options);
+}
+
+/**
+ * efi_run_image() - run loaded UEFI image
+ *
+ * @source_buffer:	memory address of the UEFI image
+ * @source_size:	size of the UEFI image
+ * Return:		status code
+ */
+efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
+{
+	efi_handle_t mem_handle = NULL, handle;
+	struct efi_device_path *file_path = NULL;
+	struct efi_device_path *msg_path;
+	efi_status_t ret, ret2;
+	u16 *load_options;
+
+	if (!bootefi_device_path || !bootefi_image_path) {
+		log_debug("Not loaded from disk\n");
+		/*
+		 * Special case for efi payload not loaded from disk,
+		 * such as 'bootefi hello' or for example payload
+		 * loaded directly into memory via JTAG, etc:
+		 */
+		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
+					    (uintptr_t)source_buffer,
+					    source_size);
+		/*
+		 * Make sure that device for device_path exist
+		 * in load_image(). Otherwise, shell and grub will fail.
+		 */
+		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
+							       &efi_guid_device_path,
+							       file_path, NULL);
+		if (ret != EFI_SUCCESS)
+			goto out;
+		msg_path = file_path;
+	} else {
+		file_path = efi_dp_append(bootefi_device_path,
+					  bootefi_image_path);
+		msg_path = bootefi_image_path;
+		log_debug("Loaded from disk\n");
+	}
+
+	log_info("Booting %pD\n", msg_path);
+
+	ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
+				      source_size, &handle));
+	if (ret != EFI_SUCCESS) {
+		log_err("Loading image failed\n");
+		goto out;
+	}
+
+	/* Transfer environment variable as load options */
+	ret = efi_env_set_load_options(handle, "bootargs", &load_options);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = do_bootefi_exec(handle, load_options);
+
+out:
+	ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
+							  &efi_guid_device_path,
+							  file_path, NULL);
+	efi_free_pool(file_path);
+	return (ret != EFI_SUCCESS) ? ret : ret2;
+}
+
+/**
+ * efi_binary_run() - run loaded UEFI image
+ *
+ * @image:	memory address of the UEFI image
+ * @size:	size of the UEFI image
+ *
+ * Execute an EFI binary image loaded at @image.
+ * @size may be zero if the binary is loaded with U-Boot load command.
+ *
+ * Return:	status code
+ */
+efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
+{
+	efi_status_t ret;
+
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
+	if (ret != EFI_SUCCESS) {
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		return -1;
+	}
+
+	ret = efi_install_fdt(fdt);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	return efi_run_image(image, size);
+}
-- 
2.34.1

Re: [PATCH v2 06/12] cmd: bootefi: move library interfaces under lib/efi_loader

[Heinrich Schuchardt]

On 11/21/23 02:29, AKASHI Takahiro wrote:
> In the prior commits, interfaces for executing EFI binary and boot manager
> were carved out. Move them under efi_loader directory so that they can
> be called from other places without depending on bootefi command.
>
> Only efi_selftest-related code will be left in bootefi.c.
>
> Signed-off-by: AKASHI Takahiro<takahiro.akashi@linaro.org>

Hello Takahiro,

the patch in not applicable anymore to origin/next due to:

1de1a0348755 ("boot: Drop size parameter from image_setup_libfdt()")

We will have to rebase the series once reviewed.

Best regards

Heinrich

Re: [PATCH v2 06/12] cmd: bootefi: move library interfaces under lib/efi_loader

[Heinrich Schuchardt]

On 11/21/23 02:29, AKASHI Takahiro wrote:
> In the prior commits, interfaces for executing EFI binary and boot manager
> were carved out. Move them under efi_loader directory so that they can
> be called from other places without depending on bootefi command.
>
> Only efi_selftest-related code will be left in bootefi.c.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   cmd/bootefi.c                | 539 +----------------------------------
>   include/efi_loader.h         |  10 +
>   lib/efi_loader/efi_bootmgr.c | 529 ++++++++++++++++++++++++++++++++++
>   3 files changed, 550 insertions(+), 528 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 7930c99def44..9cf9027bf409 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -7,545 +7,22 @@
>
>   #define LOG_CATEGORY LOGC_EFI
>
> -#include <common.h>
> -#include <bootm.h>
> -#include <charset.h>
>   #include <command.h>
> -#include <dm.h>
> +#include <efi.h>
>   #include <efi_loader.h>
> -#include <efi_selftest.h>
> -#include <env.h>
> -#include <errno.h>
> -#include <image.h>
> +#include <exports.h>
>   #include <log.h>
>   #include <malloc.h>
> -#include <asm/global_data.h>
> -#include <linux/libfdt.h>
> -#include <linux/libfdt_env.h>
>   #include <mapmem.h>
> -#include <memalign.h>
> +#include <vsprintf.h>
>   #include <asm-generic/sections.h>
> -#include <linux/linkage.h>
> +#include <asm/global_data.h>
> +#include <linux/string.h>
>
>   DECLARE_GLOBAL_DATA_PTR;
>
>   static struct efi_device_path *test_image_path;
>   static struct efi_device_path *test_device_path;
> -static struct efi_device_path *bootefi_image_path;
> -static struct efi_device_path *bootefi_device_path;
> -static void *image_addr;
> -static size_t image_size;
> -
> -/**
> - * efi_get_image_parameters() - return image parameters
> - *
> - * @img_addr:		address of loaded image in memory
> - * @img_size:		size of loaded image
> - */
> -void efi_get_image_parameters(void **img_addr, size_t *img_size)
> -{
> -	*img_addr = image_addr;
> -	*img_size = image_size;
> -}
> -
> -/**
> - * efi_clear_bootdev() - clear boot device
> - */
> -static void efi_clear_bootdev(void)
> -{
> -	efi_free_pool(bootefi_device_path);
> -	efi_free_pool(bootefi_image_path);
> -	bootefi_device_path = NULL;
> -	bootefi_image_path = NULL;
> -	image_addr = NULL;
> -	image_size = 0;
> -}
> -
> -/**
> - * efi_set_bootdev() - set boot device
> - *
> - * This function is called when a file is loaded, e.g. via the 'load' command.
> - * We use the path to this file to inform the UEFI binary about the boot device.
> - *
> - * @dev:		device, e.g. "MMC"
> - * @devnr:		number of the device, e.g. "1:2"
> - * @path:		path to file loaded
> - * @buffer:		buffer with file loaded
> - * @buffer_size:	size of file loaded
> - */
> -void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
> -		     void *buffer, size_t buffer_size)
> -{
> -	struct efi_device_path *device, *image;
> -	efi_status_t ret;
> -
> -	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
> -		  devnr, path, buffer, buffer_size);
> -
> -	/* Forget overwritten image */
> -	if (buffer + buffer_size >= image_addr &&
> -	    image_addr + image_size >= buffer)
> -		efi_clear_bootdev();
> -
> -	/* Remember only PE-COFF and FIT images */
> -	if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
> -		if (IS_ENABLED(CONFIG_FIT) &&
> -		    !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
> -			/*
> -			 * FIT images of type EFI_OS are started via command
> -			 * bootm. We should not use their boot device with the
> -			 * bootefi command.
> -			 */
> -			buffer = 0;
> -			buffer_size = 0;
> -		} else {
> -			log_debug("- not remembering image\n");
> -			return;
> -		}
> -	}
> -
> -	/* efi_set_bootdev() is typically called repeatedly, recover memory */
> -	efi_clear_bootdev();
> -
> -	image_addr = buffer;
> -	image_size = buffer_size;
> -
> -	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
> -	if (ret == EFI_SUCCESS) {
> -		bootefi_device_path = device;
> -		if (image) {
> -			/* FIXME: image should not contain device */
> -			struct efi_device_path *image_tmp = image;
> -
> -			efi_dp_split_file_path(image, &device, &image);
> -			efi_free_pool(image_tmp);
> -		}
> -		bootefi_image_path = image;
> -		log_debug("- boot device %pD\n", device);
> -		if (image)
> -			log_debug("- image %pD\n", image);
> -	} else {
> -		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
> -		efi_clear_bootdev();
> -	}
> -}
> -
> -/**
> - * efi_env_set_load_options() - set load options from environment variable
> - *
> - * @handle:		the image handle
> - * @env_var:		name of the environment variable
> - * @load_options:	pointer to load options (output)
> - * Return:		status code
> - */
> -static efi_status_t efi_env_set_load_options(efi_handle_t handle,
> -					     const char *env_var,
> -					     u16 **load_options)
> -{
> -	const char *env = env_get(env_var);
> -	size_t size;
> -	u16 *pos;
> -	efi_status_t ret;
> -
> -	*load_options = NULL;
> -	if (!env)
> -		return EFI_SUCCESS;
> -	size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
> -	pos = calloc(size, 1);
> -	if (!pos)
> -		return EFI_OUT_OF_RESOURCES;
> -	*load_options = pos;
> -	utf8_utf16_strcpy(&pos, env);
> -	ret = efi_set_load_options(handle, size, *load_options);
> -	if (ret != EFI_SUCCESS) {
> -		free(*load_options);
> -		*load_options = NULL;
> -	}
> -	return ret;
> -}
> -
> -#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
> -
> -/**
> - * copy_fdt() - Copy the device tree to a new location available to EFI
> - *
> - * The FDT is copied to a suitable location within the EFI memory map.
> - * Additional 12 KiB are added to the space in case the device tree needs to be
> - * expanded later with fdt_open_into().
> - *
> - * @fdtp:	On entry a pointer to the flattened device tree.
> - *		On exit a pointer to the copy of the flattened device tree.
> - *		FDT start
> - * Return:	status code
> - */
> -static efi_status_t copy_fdt(void **fdtp)
> -{
> -	unsigned long fdt_ram_start = -1L, fdt_pages;
> -	efi_status_t ret = 0;
> -	void *fdt, *new_fdt;
> -	u64 new_fdt_addr;
> -	uint fdt_size;
> -	int i;
> -
> -	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> -		u64 ram_start = gd->bd->bi_dram[i].start;
> -		u64 ram_size = gd->bd->bi_dram[i].size;
> -
> -		if (!ram_size)
> -			continue;
> -
> -		if (ram_start < fdt_ram_start)
> -			fdt_ram_start = ram_start;
> -	}
> -
> -	/*
> -	 * Give us at least 12 KiB of breathing room in case the device tree
> -	 * needs to be expanded later.
> -	 */
> -	fdt = *fdtp;
> -	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
> -	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
> -
> -	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> -				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
> -				 &new_fdt_addr);
> -	if (ret != EFI_SUCCESS) {
> -		log_err("ERROR: Failed to reserve space for FDT\n");
> -		goto done;
> -	}
> -	new_fdt = (void *)(uintptr_t)new_fdt_addr;
> -	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
> -	fdt_set_totalsize(new_fdt, fdt_size);
> -
> -	*fdtp = (void *)(uintptr_t)new_fdt_addr;
> -done:
> -	return ret;
> -}
> -
> -/**
> - * get_config_table() - get configuration table
> - *
> - * @guid:	GUID of the configuration table
> - * Return:	pointer to configuration table or NULL
> - */
> -static void *get_config_table(const efi_guid_t *guid)
> -{
> -	size_t i;
> -
> -	for (i = 0; i < systab.nr_tables; i++) {
> -		if (!guidcmp(guid, &systab.tables[i].guid))
> -			return systab.tables[i].table;
> -	}
> -	return NULL;
> -}
> -
> -#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
> -
> -/**
> - * efi_install_fdt() - install device tree
> - *
> - * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
> - * address will will be installed as configuration table, otherwise the device
> - * tree located at the address indicated by environment variable fdt_addr or as
> - * fallback fdtcontroladdr will be used.
> - *
> - * On architectures using ACPI tables device trees shall not be installed as
> - * configuration table.
> - *
> - * @fdt:	address of device tree or EFI_FDT_USE_INTERNAL to use the
> - *		the hardware device tree as indicated by environment variable
> - *		fdt_addr or as fallback the internal device tree as indicated by
> - *		the environment variable fdtcontroladdr
> - * Return:	status code
> - */
> -efi_status_t efi_install_fdt(void *fdt)
> -{
> -	/*
> -	 * The EBBR spec requires that we have either an FDT or an ACPI table
> -	 * but not both.
> -	 */
> -#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
> -	if (fdt) {
> -		log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
> -		return EFI_SUCCESS;
> -	}
> -#else
> -	struct bootm_headers img = { 0 };
> -	efi_status_t ret;
> -
> -	if (fdt == EFI_FDT_USE_INTERNAL) {
> -		const char *fdt_opt;
> -		uintptr_t fdt_addr;
> -
> -		/* Look for device tree that is already installed */
> -		if (get_config_table(&efi_guid_fdt))
> -			return EFI_SUCCESS;
> -		/* Check if there is a hardware device tree */
> -		fdt_opt = env_get("fdt_addr");
> -		/* Use our own device tree as fallback */
> -		if (!fdt_opt) {
> -			fdt_opt = env_get("fdtcontroladdr");
> -			if (!fdt_opt) {
> -				log_err("ERROR: need device tree\n");
> -				return EFI_NOT_FOUND;
> -			}
> -		}
> -		fdt_addr = hextoul(fdt_opt, NULL);
> -		if (!fdt_addr) {
> -			log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
> -			return EFI_LOAD_ERROR;
> -		}
> -		fdt = map_sysmem(fdt_addr, 0);
> -	}
> -
> -	/* Install device tree */
> -	if (fdt_check_header(fdt)) {
> -		log_err("ERROR: invalid device tree\n");
> -		return EFI_LOAD_ERROR;
> -	}
> -
> -	/* Prepare device tree for payload */
> -	ret = copy_fdt(&fdt);
> -	if (ret) {
> -		log_err("ERROR: out of memory\n");
> -		return EFI_OUT_OF_RESOURCES;
> -	}
> -
> -	if (image_setup_libfdt(&img, fdt, 0, NULL)) {
> -		log_err("ERROR: failed to process device tree\n");
> -		return EFI_LOAD_ERROR;
> -	}
> -
> -	/* Create memory reservations as indicated by the device tree */
> -	efi_carve_out_dt_rsv(fdt);
> -
> -	efi_try_purge_kaslr_seed(fdt);
> -
> -	if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
> -		ret = efi_tcg2_measure_dtb(fdt);
> -		if (ret == EFI_SECURITY_VIOLATION) {
> -			log_err("ERROR: failed to measure DTB\n");
> -			return ret;
> -		}
> -	}
> -
> -	/* Install device tree as UEFI table */
> -	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
> -	if (ret != EFI_SUCCESS) {
> -		log_err("ERROR: failed to install device tree\n");
> -		return ret;
> -	}
> -#endif /* GENERATE_ACPI_TABLE */
> -
> -	return EFI_SUCCESS;
> -}
> -
> -/**
> - * do_bootefi_exec() - execute EFI binary
> - *
> - * The image indicated by @handle is started. When it returns the allocated
> - * memory for the @load_options is freed.
> - *
> - * @handle:		handle of loaded image
> - * @load_options:	load options
> - * Return:		status code
> - *
> - * Load the EFI binary into a newly assigned memory unwinding the relocation
> - * information, install the loaded image protocol, and call the binary.
> - */
> -static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
> -{
> -	efi_status_t ret;
> -	efi_uintn_t exit_data_size = 0;
> -	u16 *exit_data = NULL;
> -	struct efi_event *evt;
> -
> -	/* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
> -	switch_to_non_secure_mode();
> -
> -	/*
> -	 * The UEFI standard requires that the watchdog timer is set to five
> -	 * minutes when invoking an EFI boot option.
> -	 *
> -	 * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
> -	 * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
> -	 */
> -	ret = efi_set_watchdog(300);
> -	if (ret != EFI_SUCCESS) {
> -		log_err("ERROR: Failed to set watchdog timer\n");
> -		goto out;
> -	}
> -
> -	/* Call our payload! */
> -	ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
> -	if (ret != EFI_SUCCESS) {
> -		log_err("## Application failed, r = %lu\n",
> -			ret & ~EFI_ERROR_MASK);
> -		if (exit_data) {
> -			log_err("## %ls\n", exit_data);
> -			efi_free_pool(exit_data);
> -		}
> -	}
> -
> -	efi_restore_gd();
> -
> -out:
> -	free(load_options);
> -
> -	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> -		if (efi_initrd_deregister() != EFI_SUCCESS)
> -			log_err("Failed to remove loadfile2 for initrd\n");
> -	}
> -
> -	/* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
> -	list_for_each_entry(evt, &efi_events, link) {
> -		if (evt->group &&
> -		    !guidcmp(evt->group,
> -			     &efi_guid_event_group_return_to_efibootmgr)) {
> -			efi_signal_event(evt);
> -			EFI_CALL(systab.boottime->close_event(evt));
> -			break;
> -		}
> -	}
> -
> -	/* Control is returned to U-Boot, disable EFI watchdog */
> -	efi_set_watchdog(0);
> -
> -	return ret;
> -}
> -
> -/**
> - * efi_bootmgr_run() - execute EFI boot manager
> - * fdt:	Flat device tree
> - *
> - * Invoke EFI boot manager and execute a binary depending on
> - * boot options. If @fdt is not NULL, it will be passed to
> - * the executed binary.
> - *
> - * Return:	status code
> - */
> -static efi_status_t efi_bootmgr_run(void *fdt)
> -{
> -	efi_handle_t handle;
> -	void *load_options;
> -	efi_status_t ret;
> -
> -	/* Initialize EFI drivers */
> -	ret = efi_init_obj_list();
> -	if (ret != EFI_SUCCESS) {
> -		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> -			ret & ~EFI_ERROR_MASK);
> -		return CMD_RET_FAILURE;
> -	}
> -
> -	ret = efi_install_fdt(fdt);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> -
> -	ret = efi_bootmgr_load(&handle, &load_options);
> -	if (ret != EFI_SUCCESS) {
> -		log_notice("EFI boot manager: Cannot load any image\n");
> -		return ret;
> -	}
> -
> -	return do_bootefi_exec(handle, load_options);
> -}
> -
> -/**
> - * efi_run_image() - run loaded UEFI image
> - *
> - * @source_buffer:	memory address of the UEFI image
> - * @source_size:	size of the UEFI image
> - * Return:		status code
> - */
> -efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
> -{
> -	efi_handle_t mem_handle = NULL, handle;
> -	struct efi_device_path *file_path = NULL;
> -	struct efi_device_path *msg_path;
> -	efi_status_t ret, ret2;
> -	u16 *load_options;
> -
> -	if (!bootefi_device_path || !bootefi_image_path) {
> -		log_debug("Not loaded from disk\n");
> -		/*
> -		 * Special case for efi payload not loaded from disk,
> -		 * such as 'bootefi hello' or for example payload
> -		 * loaded directly into memory via JTAG, etc:
> -		 */
> -		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> -					    (uintptr_t)source_buffer,
> -					    source_size);
> -		/*
> -		 * Make sure that device for device_path exist
> -		 * in load_image(). Otherwise, shell and grub will fail.
> -		 */
> -		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
> -							       &efi_guid_device_path,
> -							       file_path, NULL);
> -		if (ret != EFI_SUCCESS)
> -			goto out;
> -		msg_path = file_path;
> -	} else {
> -		file_path = efi_dp_append(bootefi_device_path,
> -					  bootefi_image_path);
> -		msg_path = bootefi_image_path;
> -		log_debug("Loaded from disk\n");
> -	}
> -
> -	log_info("Booting %pD\n", msg_path);
> -
> -	ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
> -				      source_size, &handle));
> -	if (ret != EFI_SUCCESS) {
> -		log_err("Loading image failed\n");
> -		goto out;
> -	}
> -
> -	/* Transfer environment variable as load options */
> -	ret = efi_env_set_load_options(handle, "bootargs", &load_options);
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -
> -	ret = do_bootefi_exec(handle, load_options);
> -
> -out:
> -	ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
> -							  &efi_guid_device_path,
> -							  file_path, NULL);
> -	efi_free_pool(file_path);
> -	return (ret != EFI_SUCCESS) ? ret : ret2;
> -}
> -
> -/**
> - * efi_binary_run() - run loaded UEFI image
> - *
> - * @image:	memory address of the UEFI image
> - * @size:	size of the UEFI image
> - *
> - * Execute an EFI binary image loaded at @image.
> - * @size may be zero if the binary is loaded with U-Boot load command.
> - *
> - * Return:	status code
> - */
> -static efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
> -{
> -	efi_status_t ret;
> -
> -	/* Initialize EFI drivers */
> -	ret = efi_init_obj_list();
> -	if (ret != EFI_SUCCESS) {
> -		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> -			ret & ~EFI_ERROR_MASK);
> -		return ret;
> -	}
> -
> -	ret = efi_install_fdt(fdt);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> -
> -	return efi_run_image(image, size);
> -}
>
>   static efi_status_t bootefi_run_prepare(const char *load_options_path,
>   		struct efi_device_path *device_path,
> @@ -658,6 +135,8 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   	char *p;
>   	void *fdt, *image_buf;
>   	unsigned long addr, size;
> +	void *image_addr;
> +	size_t image_size;
>
>   	if (argc < 2)
>   		return CMD_RET_USAGE;
> @@ -709,6 +188,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   	    !strcmp(argv[1], "hello")) {
>   		image_buf = __efi_helloworld_begin;
>   		size = __efi_helloworld_end - __efi_helloworld_begin;
> +		/* TODO: not sure calling clear function is necessary */
>   		efi_clear_bootdev();
>   	} else {
>   		addr = strtoul(argv[1], NULL, 16);
> @@ -724,6 +204,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
>   				return CMD_RET_USAGE;
>   			efi_clear_bootdev();
>   		} else {
> +			/* Image should be already loaded */
> +			efi_get_image_parameters(&image_addr, &image_size);
> +
>   			if (image_buf != image_addr) {
>   				log_err("No UEFI binary known at %s\n",
>   					argv[1]);
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 44436d346286..34e7fbbf1840 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -90,6 +90,8 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len);
>    * back to u-boot world
>    */
>   void efi_restore_gd(void);
> +/* Call this to unset the current device name */
> +void efi_clear_bootdev(void);
>   /* Call this to set the current device name */
>   void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
>   		     void *buffer, size_t buffer_size);
> @@ -114,6 +116,7 @@ static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len)
>
>   /* No loader configured, stub out EFI_ENTRY */
>   static inline void efi_restore_gd(void) { }
> +static inline void efi_clear_bootdev(void) { }
>   static inline void efi_set_bootdev(const char *dev, const char *devnr,
>   				   const char *path, void *buffer,
>   				   size_t buffer_size) { }
> @@ -527,14 +530,21 @@ efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
>   efi_status_t efi_bootmgr_update_media_device_boot_option(void);
>   /* Delete selected boot option */
>   efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
> +/* Invoke EFI boot manager */
> +efi_status_t efi_bootmgr_run(void *fdt);
>   /* search the boot option index in BootOrder */
>   bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index);
>   /* Set up console modes */
>   void efi_setup_console_size(void);
> +/* Set up load options from environment variable */
> +efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
> +				      u16 **load_options);
>   /* Install device tree */
>   efi_status_t efi_install_fdt(void *fdt);
>   /* Run loaded UEFI image */
>   efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
> +/* Run loaded UEFI image with given fdt */
> +efi_status_t efi_binary_run(void *image, size_t size, void *fdt);
>   /* Initialize variable services */
>   efi_status_t efi_init_variables(void);
>   /* Notify ExitBootServices() is called */
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 48153bd5ffbb..e910f1bb2a23 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -3,6 +3,8 @@
>    *  EFI boot manager
>    *
>    *  Copyright (c) 2017 Rob Clark
> + *  For the code moved from cmd/bootefi.c
> + *  Copyright (c) 2016 Alexander Graf
>    */
>
>   #define LOG_CATEGORY LOGC_EFI
> @@ -20,6 +22,17 @@
>   #include <efi_variable.h>
>   #include <asm/unaligned.h>
>
> +/* TODO: temporarily added here; clean up later */
> +#include <bootm.h>
> +#include <efi_selftest.h>
> +#include <env.h>
> +#include <mapmem.h>
> +#include <asm/global_data.h>
> +#include <linux/libfdt.h>
> +#include <linux/libfdt_env.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
>   static const struct efi_boot_services *bs;
>   static const struct efi_runtime_services *rs;
>
> @@ -1115,3 +1128,519 @@ out:
>   		return EFI_SUCCESS;
>   	return ret;
>   }
> +
> +static struct efi_device_path *bootefi_image_path;
> +static struct efi_device_path *bootefi_device_path;
> +static void *image_addr;
> +static size_t image_size;
> +
> +/**
> + * efi_get_image_parameters() - return image parameters
> + *
> + * @img_addr:		address of loaded image in memory
> + * @img_size:		size of loaded image
> + */
> +void efi_get_image_parameters(void **img_addr, size_t *img_size)
> +{
> +	*img_addr = image_addr;
> +	*img_size = image_size;
> +}
> +
> +/**
> + * efi_clear_bootdev() - clear boot device
> + */
> +void efi_clear_bootdev(void)
> +{
> +	efi_free_pool(bootefi_device_path);
> +	efi_free_pool(bootefi_image_path);
> +	bootefi_device_path = NULL;
> +	bootefi_image_path = NULL;
> +	image_addr = NULL;
> +	image_size = 0;
> +}
> +
> +/**
> + * efi_set_bootdev() - set boot device
> + *
> + * This function is called when a file is loaded, e.g. via the 'load' command.
> + * We use the path to this file to inform the UEFI binary about the boot device.
> + *
> + * @dev:		device, e.g. "MMC"
> + * @devnr:		number of the device, e.g. "1:2"
> + * @path:		path to file loaded
> + * @buffer:		buffer with file loaded
> + * @buffer_size:	size of file loaded
> + */
> +void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
> +		     void *buffer, size_t buffer_size)
> +{
> +	struct efi_device_path *device, *image;
> +	efi_status_t ret;
> +
> +	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
> +		  devnr, path, buffer, buffer_size);
> +
> +	/* Forget overwritten image */
> +	if (buffer + buffer_size >= image_addr &&
> +	    image_addr + image_size >= buffer)
> +		efi_clear_bootdev();
> +
> +	/* Remember only PE-COFF and FIT images */
> +	if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
> +		if (IS_ENABLED(CONFIG_FIT) &&
> +		    !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
> +			/*
> +			 * FIT images of type EFI_OS are started via command
> +			 * bootm. We should not use their boot device with the
> +			 * bootefi command.
> +			 */
> +			buffer = 0;
> +			buffer_size = 0;
> +		} else {
> +			log_debug("- not remembering image\n");
> +			return;
> +		}
> +	}
> +
> +	/* efi_set_bootdev() is typically called repeatedly, recover memory */
> +	efi_clear_bootdev();
> +
> +	image_addr = buffer;
> +	image_size = buffer_size;
> +
> +	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
> +	if (ret == EFI_SUCCESS) {
> +		bootefi_device_path = device;
> +		if (image) {
> +			/* FIXME: image should not contain device */
> +			struct efi_device_path *image_tmp = image;
> +
> +			efi_dp_split_file_path(image, &device, &image);
> +			efi_free_pool(image_tmp);
> +		}
> +		bootefi_image_path = image;
> +		log_debug("- boot device %pD\n", device);
> +		if (image)
> +			log_debug("- image %pD\n", image);
> +	} else {
> +		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
> +		efi_clear_bootdev();
> +	}
> +}
> +
> +/**
> + * efi_env_set_load_options() - set load options from environment variable
> + *
> + * @handle:		the image handle
> + * @env_var:		name of the environment variable
> + * @load_options:	pointer to load options (output)
> + * Return:		status code
> + */
> +efi_status_t efi_env_set_load_options(efi_handle_t handle,
> +				      const char *env_var,
> +				      u16 **load_options)
> +{
> +	const char *env = env_get(env_var);
> +	size_t size;
> +	u16 *pos;
> +	efi_status_t ret;
> +
> +	*load_options = NULL;
> +	if (!env)
> +		return EFI_SUCCESS;
> +	size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
> +	pos = calloc(size, 1);
> +	if (!pos)
> +		return EFI_OUT_OF_RESOURCES;
> +	*load_options = pos;
> +	utf8_utf16_strcpy(&pos, env);
> +	ret = efi_set_load_options(handle, size, *load_options);
> +	if (ret != EFI_SUCCESS) {
> +		free(*load_options);
> +		*load_options = NULL;
> +	}
> +	return ret;
> +}
> +
> +#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
> +
> +/**
> + * copy_fdt() - Copy the device tree to a new location available to EFI
> + *
> + * The FDT is copied to a suitable location within the EFI memory map.
> + * Additional 12 KiB are added to the space in case the device tree needs to be
> + * expanded later with fdt_open_into().
> + *
> + * @fdtp:	On entry a pointer to the flattened device tree.
> + *		On exit a pointer to the copy of the flattened device tree.
> + *		FDT start
> + * Return:	status code
> + */
> +static efi_status_t copy_fdt(void **fdtp)
> +{
> +	unsigned long fdt_ram_start = -1L, fdt_pages;
> +	efi_status_t ret = 0;
> +	void *fdt, *new_fdt;
> +	u64 new_fdt_addr;
> +	uint fdt_size;
> +	int i;
> +
> +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> +		u64 ram_start = gd->bd->bi_dram[i].start;
> +		u64 ram_size = gd->bd->bi_dram[i].size;
> +
> +		if (!ram_size)
> +			continue;
> +
> +		if (ram_start < fdt_ram_start)
> +			fdt_ram_start = ram_start;
> +	}
> +
> +	/*
> +	 * Give us at least 12 KiB of breathing room in case the device tree
> +	 * needs to be expanded later.
> +	 */
> +	fdt = *fdtp;
> +	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
> +	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
> +
> +	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> +				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
> +				 &new_fdt_addr);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("ERROR: Failed to reserve space for FDT\n");
> +		goto done;
> +	}
> +	new_fdt = (void *)(uintptr_t)new_fdt_addr;
> +	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
> +	fdt_set_totalsize(new_fdt, fdt_size);
> +
> +	*fdtp = (void *)(uintptr_t)new_fdt_addr;
> +done:
> +	return ret;
> +}
> +
> +/**
> + * get_config_table() - get configuration table
> + *
> + * @guid:	GUID of the configuration table
> + * Return:	pointer to configuration table or NULL
> + */
> +static void *get_config_table(const efi_guid_t *guid)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < systab.nr_tables; i++) {
> +		if (!guidcmp(guid, &systab.tables[i].guid))
> +			return systab.tables[i].table;
> +	}
> +	return NULL;
> +}
> +
> +#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
> +
> +/**
> + * efi_install_fdt() - install device tree
> + *
> + * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
> + * address will be installed as configuration table, otherwise the device
> + * tree located at the address indicated by environment variable fdt_addr or as
> + * fallback fdtcontroladdr will be used.
> + *
> + * On architectures using ACPI tables device trees shall not be installed as
> + * configuration table.
> + *
> + * @fdt:	address of device tree or EFI_FDT_USE_INTERNAL to use
> + *		the hardware device tree as indicated by environment variable
> + *		fdt_addr or as fallback the internal device tree as indicated by
> + *		the environment variable fdtcontroladdr
> + * Return:	status code
> + */
> +efi_status_t efi_install_fdt(void *fdt)
> +{
> +	/*
> +	 * The EBBR spec requires that we have either an FDT or an ACPI table
> +	 * but not both.
> +	 */
> +#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
> +	if (fdt) {
> +		log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
> +		return EFI_SUCCESS;
> +	}
> +#else
> +	struct bootm_headers img = { 0 };
> +	efi_status_t ret;
> +
> +	if (fdt == EFI_FDT_USE_INTERNAL) {
> +		const char *fdt_opt;
> +		uintptr_t fdt_addr;
> +
> +		/* Look for device tree that is already installed */
> +		if (get_config_table(&efi_guid_fdt))
> +			return EFI_SUCCESS;
> +		/* Check if there is a hardware device tree */
> +		fdt_opt = env_get("fdt_addr");
> +		/* Use our own device tree as fallback */
> +		if (!fdt_opt) {
> +			fdt_opt = env_get("fdtcontroladdr");
> +			if (!fdt_opt) {
> +				log_err("ERROR: need device tree\n");
> +				return EFI_NOT_FOUND;
> +			}
> +		}
> +		fdt_addr = hextoul(fdt_opt, NULL);
> +		if (!fdt_addr) {
> +			log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
> +			return EFI_LOAD_ERROR;
> +		}
> +		fdt = map_sysmem(fdt_addr, 0);
> +	}
> +
> +	/* Install device tree */
> +	if (fdt_check_header(fdt)) {
> +		log_err("ERROR: invalid device tree\n");
> +		return EFI_LOAD_ERROR;
> +	}
> +
> +	/* Prepare device tree for payload */
> +	ret = copy_fdt(&fdt);
> +	if (ret) {
> +		log_err("ERROR: out of memory\n");
> +		return EFI_OUT_OF_RESOURCES;
> +	}
> +
> +	if (image_setup_libfdt(&img, fdt, 0, NULL)) {
> +		log_err("ERROR: failed to process device tree\n");
> +		return EFI_LOAD_ERROR;
> +	}
> +
> +	/* Create memory reservations as indicated by the device tree */
> +	efi_carve_out_dt_rsv(fdt);
> +
> +	efi_try_purge_kaslr_seed(fdt);
> +
> +	if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
> +		ret = efi_tcg2_measure_dtb(fdt);
> +		if (ret == EFI_SECURITY_VIOLATION) {
> +			log_err("ERROR: failed to measure DTB\n");
> +			return ret;
> +		}
> +	}
> +
> +	/* Install device tree as UEFI table */
> +	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("ERROR: failed to install device tree\n");
> +		return ret;
> +	}
> +#endif /* GENERATE_ACPI_TABLE */
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * do_bootefi_exec() - execute EFI binary
> + *
> + * The image indicated by @handle is started. When it returns the allocated
> + * memory for the @load_options is freed.
> + *
> + * @handle:		handle of loaded image
> + * @load_options:	load options
> + * Return:		status code
> + *
> + * Load the EFI binary into a newly assigned memory unwinding the relocation
> + * information, install the loaded image protocol, and call the binary.
> + */
> +static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
> +{
> +	efi_status_t ret;
> +	efi_uintn_t exit_data_size = 0;
> +	u16 *exit_data = NULL;
> +	struct efi_event *evt;
> +
> +	/* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
> +	switch_to_non_secure_mode();
> +
> +	/*
> +	 * The UEFI standard requires that the watchdog timer is set to five
> +	 * minutes when invoking an EFI boot option.
> +	 *
> +	 * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
> +	 * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
> +	 */
> +	ret = efi_set_watchdog(300);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("ERROR: Failed to set watchdog timer\n");
> +		goto out;
> +	}
> +
> +	/* Call our payload! */
> +	ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
> +	if (ret != EFI_SUCCESS) {
> +		log_err("## Application failed, r = %lu\n",
> +			ret & ~EFI_ERROR_MASK);
> +		if (exit_data) {
> +			log_err("## %ls\n", exit_data);
> +			efi_free_pool(exit_data);
> +		}
> +	}
> +
> +	efi_restore_gd();
> +
> +out:
> +	free(load_options);
> +
> +	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> +		if (efi_initrd_deregister() != EFI_SUCCESS)
> +			log_err("Failed to remove loadfile2 for initrd\n");
> +	}
> +
> +	/* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
> +	list_for_each_entry(evt, &efi_events, link) {
> +		if (evt->group &&
> +		    !guidcmp(evt->group,
> +			     &efi_guid_event_group_return_to_efibootmgr)) {
> +			efi_signal_event(evt);
> +			EFI_CALL(systab.boottime->close_event(evt));
> +			break;
> +		}
> +	}
> +
> +	/* Control is returned to U-Boot, disable EFI watchdog */
> +	efi_set_watchdog(0);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_bootmgr_run() - execute EFI boot manager
> + * fdt:	Flat device tree

'make htmldocs' fails as this documentation does not comply to the Sphix
style.

./lib/efi_loader/efi_bootmgr.c:1526: warning: Function parameter or
member 'fdt' not described in 'efi_bootmgr_run'

> + *
> + * Invoke EFI boot manager and execute a binary depending on
> + * boot options. If @fdt is not NULL, it will be passed to
> + * the executed binary.
> + *
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_run(void *fdt)
> +{
> +	efi_handle_t handle;
> +	void *load_options;
> +	efi_status_t ret;
> +
> +	/* Initialize EFI drivers */
> +	ret = efi_init_obj_list();
> +	if (ret != EFI_SUCCESS) {
> +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +			ret & ~EFI_ERROR_MASK);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	ret = efi_install_fdt(fdt);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	ret = efi_bootmgr_load(&handle, &load_options);
> +	if (ret != EFI_SUCCESS) {
> +		log_notice("EFI boot manager: Cannot load any image\n");
> +		return ret;
> +	}
> +
> +	return do_bootefi_exec(handle, load_options);
> +}
> +
> +/**
> + * efi_run_image() - run loaded UEFI image
> + *
> + * @source_buffer:	memory address of the UEFI image
> + * @source_size:	size of the UEFI image
> + * Return:		status code
> + */
> +efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
> +{
> +	efi_handle_t mem_handle = NULL, handle;
> +	struct efi_device_path *file_path = NULL;
> +	struct efi_device_path *msg_path;
> +	efi_status_t ret, ret2;
> +	u16 *load_options;
> +
> +	if (!bootefi_device_path || !bootefi_image_path) {
> +		log_debug("Not loaded from disk\n");
> +		/*
> +		 * Special case for efi payload not loaded from disk,
> +		 * such as 'bootefi hello' or for example payload
> +		 * loaded directly into memory via JTAG, etc:
> +		 */
> +		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> +					    (uintptr_t)source_buffer,
> +					    source_size);
> +		/*
> +		 * Make sure that device for device_path exist
> +		 * in load_image(). Otherwise, shell and grub will fail.
> +		 */
> +		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
> +							       &efi_guid_device_path,
> +							       file_path, NULL);
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +		msg_path = file_path;
> +	} else {
> +		file_path = efi_dp_append(bootefi_device_path,
> +					  bootefi_image_path);
> +		msg_path = bootefi_image_path;
> +		log_debug("Loaded from disk\n");
> +	}
> +
> +	log_info("Booting %pD\n", msg_path);
> +
> +	ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
> +				      source_size, &handle));
> +	if (ret != EFI_SUCCESS) {
> +		log_err("Loading image failed\n");
> +		goto out;
> +	}
> +
> +	/* Transfer environment variable as load options */
> +	ret = efi_env_set_load_options(handle, "bootargs", &load_options);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = do_bootefi_exec(handle, load_options);
> +
> +out:
> +	ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
> +							  &efi_guid_device_path,
> +							  file_path, NULL);
> +	efi_free_pool(file_path);
> +	return (ret != EFI_SUCCESS) ? ret : ret2;
> +}
> +
> +/**
> + * efi_binary_run() - run loaded UEFI image
> + *
> + * @image:	memory address of the UEFI image
> + * @size:	size of the UEFI image

'make htmldocs' fails as parameter fdt is not described.

Best regards

Heinrich

> + *
> + * Execute an EFI binary image loaded at @image.
> + * @size may be zero if the binary is loaded with U-Boot load command.
> + *
> + * Return:	status code
> + */
> +efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
> +{
> +	efi_status_t ret;
> +
> +	/* Initialize EFI drivers */
> +	ret = efi_init_obj_list();
> +	if (ret != EFI_SUCCESS) {
> +		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> +			ret & ~EFI_ERROR_MASK);
> +		return -1;
> +	}
> +
> +	ret = efi_install_fdt(fdt);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	return efi_run_image(image, size);
> +}

[PATCH v2 07/12] cmd: efidebug: ease efi configuration dependency

[AKASHI Takahiro]

Now it is clear that the command actually depends on interfaces,
not "bootefi bootmgr" command.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/efidebug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 78ef16f4cb5c..e10fbf891a42 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -1410,7 +1410,7 @@ static __maybe_unused int do_efi_test_bootmgr(struct cmd_tbl *cmdtp, int flag,
 }
 
 static struct cmd_tbl cmd_efidebug_test_sub[] = {
-#ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
+#ifdef CONFIG_BOOTEFI_BOOTMGR
 	U_BOOT_CMD_MKENT(bootmgr, CONFIG_SYS_MAXARGS, 1, do_efi_test_bootmgr,
 			 "", ""),
 #endif
@@ -1604,7 +1604,7 @@ U_BOOT_LONGHELP(efidebug,
 	"  - show UEFI memory map\n"
 	"efidebug tables\n"
 	"  - show UEFI configuration tables\n"
-#ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
+#ifdef CONFIG_BOOTEFI_BOOTMGR
 	"efidebug test bootmgr\n"
 	"  - run simple bootmgr for test\n"
 #endif
-- 
2.34.1

[PATCH v2 08/12] bootmeth: use efi_loader interfaces instead of bootefi command

[AKASHI Takahiro]

Now that efi_loader subsystem provides interfaces that are equivalent
with bootefi command, we can replace command invocations with APIs.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 boot/Kconfig            |  4 ++--
 boot/Makefile           |  2 +-
 boot/bootm_os.c         | 31 +++++++------------------------
 boot/bootmeth_efi.c     |  8 +-------
 boot/bootmeth_efi_mgr.c |  2 +-
 cmd/Kconfig             |  2 +-
 test/boot/bootflow.c    |  2 +-
 7 files changed, 14 insertions(+), 37 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index ef71883a5026..e879c63b84e3 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -511,7 +511,7 @@ config BOOTMETH_EXTLINUX_PXE
 
 config BOOTMETH_EFILOADER
 	bool "Bootdev support for EFI boot"
-	depends on CMD_BOOTEFI
+	depends on BOOTEFI_BOOTMGR
 	default y
 	help
 	  Enables support for EFI boot using bootdevs. This makes the
@@ -546,7 +546,7 @@ config BOOTMETH_DISTRO
 	select BOOTMETH_SCRIPT if CMDLINE # E.g. Armbian uses scripts
 	select BOOTMETH_EXTLINUX  # E.g. Debian uses these
 	select BOOTMETH_EXTLINUX_PXE if CMD_PXE && CMD_NET && DM_ETH
-	select BOOTMETH_EFILOADER if CMD_BOOTEFI # E.g. Ubuntu uses this
+	select BOOTMETH_EFILOADER if BOOTEFI_BOOTMGR # E.g. Ubuntu uses this
 
 config SPL_BOOTMETH_VBE
 	bool "Bootdev support for Verified Boot for Embedded (SPL)"
diff --git a/boot/Makefile b/boot/Makefile
index 3fd048bb41ab..4eaa5eab4b77 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
 ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL
-obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
+obj-$(CONFIG_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
 obj-$(CONFIG_$(SPL_TPL_)EXPO) += bootflow_menu.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o
 obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o
diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index 30296eb27d7d..c2f39cdf3f36 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -487,7 +487,6 @@ static int do_bootm_efi(int flag, int argc, char *const argv[],
 			struct bootm_headers *images)
 {
 	int ret;
-	efi_status_t efi_ret;
 	void *image_buf;
 
 	if (flag != BOOTM_STATE_OS_GO)
@@ -498,37 +497,21 @@ static int do_bootm_efi(int flag, int argc, char *const argv[],
 	if (ret)
 		return ret;
 
-	/* Initialize EFI drivers */
-	efi_ret = efi_init_obj_list();
-	if (efi_ret != EFI_SUCCESS) {
-		printf("## Failed to initialize UEFI sub-system: r = %lu\n",
-		       efi_ret & ~EFI_ERROR_MASK);
-		return 1;
-	}
+	/* We expect to return */
+	images->os.type = IH_TYPE_STANDALONE;
 
-	/* Install device tree */
-	efi_ret = efi_install_fdt(images->ft_len
-				  ? images->ft_addr : EFI_FDT_USE_INTERNAL);
-	if (efi_ret != EFI_SUCCESS) {
-		printf("## Failed to install device tree: r = %lu\n",
-		       efi_ret & ~EFI_ERROR_MASK);
-		return 1;
-	}
+	image_buf = map_sysmem(images->ep, images->os.image_len);
 
 	/* Run EFI image */
 	printf("## Transferring control to EFI (at address %08lx) ...\n",
 	       images->ep);
 	bootstage_mark(BOOTSTAGE_ID_RUN_OS);
 
-	/* We expect to return */
-	images->os.type = IH_TYPE_STANDALONE;
-
-	image_buf = map_sysmem(images->ep, images->os.image_len);
+	ret = efi_binary_run(image_buf, images->os.image_len,
+			     images->ft_len
+			     ? images->ft_addr : EFI_FDT_USE_INTERNAL);
 
-	efi_ret = efi_run_image(image_buf, images->os.image_len);
-	if (efi_ret != EFI_SUCCESS)
-		return 1;
-	return 0;
+	return ret;
 }
 #endif
 
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index ae936c8daa18..2a9f29f9db5a 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -412,7 +412,6 @@ static int distro_efi_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow)
 {
 	ulong kernel, fdt;
-	char cmd[50];
 	int ret;
 
 	kernel = env_get_hex("kernel_addr_r", 0);
@@ -441,12 +440,7 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow)
 		fdt = env_get_hex("fdt_addr_r", 0);
 	}
 
-	/*
-	 * At some point we can add a real interface to bootefi so we can call
-	 * this directly. For now, go through the CLI, like distro boot.
-	 */
-	snprintf(cmd, sizeof(cmd), "bootefi %lx %lx", kernel, fdt);
-	if (run_command(cmd, 0))
+	if (efi_binary_run(map_sysmem(kernel, 0), 0, map_sysmem(fdt, 0)))
 		return log_msg_ret("run", -EINVAL);
 
 	return 0;
diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
index e6c42d41fb80..5f6f68e423e6 100644
--- a/boot/bootmeth_efi_mgr.c
+++ b/boot/bootmeth_efi_mgr.c
@@ -85,7 +85,7 @@ static int efi_mgr_boot(struct udevice *dev, struct bootflow *bflow)
 	int ret;
 
 	/* Booting is handled by the 'bootefi bootmgr' command */
-	ret = run_command("bootefi bootmgr", 0);
+	ret = efi_bootmgr_run(EFI_FDT_USE_INTERNAL);
 
 	return 0;
 }
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 4cf9a210c4a1..dc4ea48f78bc 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -273,7 +273,7 @@ config CMD_BOOTMETH
 
 config BOOTM_EFI
 	bool "Support booting UEFI FIT images"
-	depends on CMD_BOOTEFI && CMD_BOOTM && FIT
+	depends on BOOTEFI_BOOTMGR && CMD_BOOTM && FIT
 	default y
 	help
 	  Support booting UEFI FIT images via the bootm command.
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index f640db8a2418..fa1bebdd02da 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -374,7 +374,7 @@ static int bootflow_system(struct unit_test_state *uts)
 {
 	struct udevice *bootstd, *dev;
 
-	if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
+	if (!IS_ENABLED(CONFIG_BOOTEFI_BOOTMGR))
 		return -EAGAIN;
 	ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
 	ut_assertok(device_bind(bootstd, DM_DRIVER_GET(bootmeth_efi_mgr),
-- 
2.34.1

[PATCH v2 09/12] efi_loader: split unrelated code from efi_bootmgr.c

[AKASHI Takahiro]

Some code moved from cmd/bootefi.c is actually necessary only for "bootefi
<addr>" command (starting an image manually loaded by a user using U-Boot
load commands or other methods (like JTAG debugger).

The code will never been opted out as unused code by a compiler which
doesn't know how EFI boot manager is implemented. So introduce a new
configuration, CONFIG_EFI_BINARY_EXEC, to enforce theem opted out
explicitly.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 boot/Kconfig                     |   4 +-
 cmd/Kconfig                      |   6 +-
 include/efi_loader.h             |  28 +-
 lib/efi_loader/Kconfig           |   9 +
 lib/efi_loader/efi_bootmgr.c     | 492 ------------------------------
 lib/efi_loader/efi_device_path.c |   3 +-
 lib/efi_loader/efi_helper.c      | 498 ++++++++++++++++++++++++++++++-
 7 files changed, 528 insertions(+), 512 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index e879c63b84e3..dddc53766bb9 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -511,7 +511,7 @@ config BOOTMETH_EXTLINUX_PXE
 
 config BOOTMETH_EFILOADER
 	bool "Bootdev support for EFI boot"
-	depends on BOOTEFI_BOOTMGR
+	depends on EFI_BINARY_EXEC
 	default y
 	help
 	  Enables support for EFI boot using bootdevs. This makes the
@@ -546,7 +546,7 @@ config BOOTMETH_DISTRO
 	select BOOTMETH_SCRIPT if CMDLINE # E.g. Armbian uses scripts
 	select BOOTMETH_EXTLINUX  # E.g. Debian uses these
 	select BOOTMETH_EXTLINUX_PXE if CMD_PXE && CMD_NET && DM_ETH
-	select BOOTMETH_EFILOADER if BOOTEFI_BOOTMGR # E.g. Ubuntu uses this
+	select BOOTMETH_EFILOADER if EFI_BINARY_EXEC # E.g. Ubuntu uses this
 
 config SPL_BOOTMETH_VBE
 	bool "Bootdev support for Verified Boot for Embedded (SPL)"
diff --git a/cmd/Kconfig b/cmd/Kconfig
index dc4ea48f78bc..189ea6003293 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -273,7 +273,7 @@ config CMD_BOOTMETH
 
 config BOOTM_EFI
 	bool "Support booting UEFI FIT images"
-	depends on BOOTEFI_BOOTMGR && CMD_BOOTM && FIT
+	depends on EFI_BINARY_EXEC && CMD_BOOTM && FIT
 	default y
 	help
 	  Support booting UEFI FIT images via the bootm command.
@@ -365,7 +365,7 @@ config CMD_BOOTEFI
 if CMD_BOOTEFI
 config CMD_BOOTEFI_BINARY
 	bool "Allow booting an EFI binary directly"
-	depends on BOOTEFI_BOOTMGR
+	depends on EFI_BINARY_EXEC
 	default y
 	help
 	  Select this option to enable direct execution of binary at 'bootefi'.
@@ -396,7 +396,7 @@ config CMD_BOOTEFI_HELLO_COMPILE
 
 config CMD_BOOTEFI_HELLO
 	bool "Allow booting a standard EFI hello world for testing"
-	depends on CMD_BOOTEFI_HELLO_COMPILE
+	depends on CMD_BOOTEFI_BINARY && CMD_BOOTEFI_HELLO_COMPILE
 	default y if CMD_BOOTEFI_SELFTEST
 	help
 	  This adds a standard EFI hello world application to U-Boot so that
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 34e7fbbf1840..484c9fad239f 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -90,11 +90,7 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len);
  * back to u-boot world
  */
 void efi_restore_gd(void);
-/* Call this to unset the current device name */
-void efi_clear_bootdev(void);
-/* Call this to set the current device name */
-void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
-		     void *buffer, size_t buffer_size);
+
 /* Called by networking code to memorize the dhcp ack package */
 void efi_net_set_dhcp_ack(void *pkt, int len);
 /* Print information about all loaded images */
@@ -116,10 +112,6 @@ static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len)
 
 /* No loader configured, stub out EFI_ENTRY */
 static inline void efi_restore_gd(void) { }
-static inline void efi_clear_bootdev(void) { }
-static inline void efi_set_bootdev(const char *dev, const char *devnr,
-				   const char *path, void *buffer,
-				   size_t buffer_size) { }
 static inline void efi_net_set_dhcp_ack(void *pkt, int len) { }
 static inline void efi_print_image_infos(void *pc) { }
 static inline efi_status_t efi_launch_capsules(void)
@@ -129,6 +121,20 @@ static inline efi_status_t efi_launch_capsules(void)
 
 #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
 
+#if CONFIG_IS_ENABLED(EFI_BINARY_EXEC)
+/* Call this to unset the current device name */
+void efi_clear_bootdev(void);
+/* Call this to set the current device name */
+void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
+		     void *buffer, size_t buffer_size);
+#else
+static inline void efi_clear_bootdev(void) { }
+
+static inline void efi_set_bootdev(const char *dev, const char *devnr,
+				   const char *path, void *buffer,
+				   size_t buffer_size) { }
+#endif
+
 /* Maximum number of configuration tables */
 #define EFI_MAX_CONFIGURATION_TABLES 16
 
@@ -541,8 +547,8 @@ efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
 				      u16 **load_options);
 /* Install device tree */
 efi_status_t efi_install_fdt(void *fdt);
-/* Run loaded UEFI image */
-efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
+/* Execute loaded UEFI image */
+efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options);
 /* Run loaded UEFI image with given fdt */
 efi_status_t efi_binary_run(void *image, size_t size, void *fdt);
 /* Initialize variable services */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 2913d1c9dfe7..ddb1a012a761 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -32,6 +32,15 @@ config EFI_LOADER
 
 if EFI_LOADER
 
+config EFI_BINARY_EXEC
+	bool "Execute UEFI binary"
+	default y
+	help
+	  Select this option if you want to execute the UEFI binary after
+	  loading it with U-Boot load commands or other methods.
+	  You may enable CMD_BOOTEFI_BINARY so that you can use bootefi
+	  command to do that.
+
 config BOOTEFI_BOOTMGR
 	bool "UEFI Boot Manager"
 	default y
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index e910f1bb2a23..3d70c643f895 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -3,8 +3,6 @@
  *  EFI boot manager
  *
  *  Copyright (c) 2017 Rob Clark
- *  For the code moved from cmd/bootefi.c
- *  Copyright (c) 2016 Alexander Graf
  */
 
 #define LOG_CATEGORY LOGC_EFI
@@ -22,17 +20,6 @@
 #include <efi_variable.h>
 #include <asm/unaligned.h>
 
-/* TODO: temporarily added here; clean up later */
-#include <bootm.h>
-#include <efi_selftest.h>
-#include <env.h>
-#include <mapmem.h>
-#include <asm/global_data.h>
-#include <linux/libfdt.h>
-#include <linux/libfdt_env.h>
-
-DECLARE_GLOBAL_DATA_PTR;
-
 static const struct efi_boot_services *bs;
 static const struct efi_runtime_services *rs;
 
@@ -1129,389 +1116,6 @@ out:
 	return ret;
 }
 
-static struct efi_device_path *bootefi_image_path;
-static struct efi_device_path *bootefi_device_path;
-static void *image_addr;
-static size_t image_size;
-
-/**
- * efi_get_image_parameters() - return image parameters
- *
- * @img_addr:		address of loaded image in memory
- * @img_size:		size of loaded image
- */
-void efi_get_image_parameters(void **img_addr, size_t *img_size)
-{
-	*img_addr = image_addr;
-	*img_size = image_size;
-}
-
-/**
- * efi_clear_bootdev() - clear boot device
- */
-void efi_clear_bootdev(void)
-{
-	efi_free_pool(bootefi_device_path);
-	efi_free_pool(bootefi_image_path);
-	bootefi_device_path = NULL;
-	bootefi_image_path = NULL;
-	image_addr = NULL;
-	image_size = 0;
-}
-
-/**
- * efi_set_bootdev() - set boot device
- *
- * This function is called when a file is loaded, e.g. via the 'load' command.
- * We use the path to this file to inform the UEFI binary about the boot device.
- *
- * @dev:		device, e.g. "MMC"
- * @devnr:		number of the device, e.g. "1:2"
- * @path:		path to file loaded
- * @buffer:		buffer with file loaded
- * @buffer_size:	size of file loaded
- */
-void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
-		     void *buffer, size_t buffer_size)
-{
-	struct efi_device_path *device, *image;
-	efi_status_t ret;
-
-	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
-		  devnr, path, buffer, buffer_size);
-
-	/* Forget overwritten image */
-	if (buffer + buffer_size >= image_addr &&
-	    image_addr + image_size >= buffer)
-		efi_clear_bootdev();
-
-	/* Remember only PE-COFF and FIT images */
-	if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
-		if (IS_ENABLED(CONFIG_FIT) &&
-		    !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
-			/*
-			 * FIT images of type EFI_OS are started via command
-			 * bootm. We should not use their boot device with the
-			 * bootefi command.
-			 */
-			buffer = 0;
-			buffer_size = 0;
-		} else {
-			log_debug("- not remembering image\n");
-			return;
-		}
-	}
-
-	/* efi_set_bootdev() is typically called repeatedly, recover memory */
-	efi_clear_bootdev();
-
-	image_addr = buffer;
-	image_size = buffer_size;
-
-	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
-	if (ret == EFI_SUCCESS) {
-		bootefi_device_path = device;
-		if (image) {
-			/* FIXME: image should not contain device */
-			struct efi_device_path *image_tmp = image;
-
-			efi_dp_split_file_path(image, &device, &image);
-			efi_free_pool(image_tmp);
-		}
-		bootefi_image_path = image;
-		log_debug("- boot device %pD\n", device);
-		if (image)
-			log_debug("- image %pD\n", image);
-	} else {
-		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
-		efi_clear_bootdev();
-	}
-}
-
-/**
- * efi_env_set_load_options() - set load options from environment variable
- *
- * @handle:		the image handle
- * @env_var:		name of the environment variable
- * @load_options:	pointer to load options (output)
- * Return:		status code
- */
-efi_status_t efi_env_set_load_options(efi_handle_t handle,
-				      const char *env_var,
-				      u16 **load_options)
-{
-	const char *env = env_get(env_var);
-	size_t size;
-	u16 *pos;
-	efi_status_t ret;
-
-	*load_options = NULL;
-	if (!env)
-		return EFI_SUCCESS;
-	size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
-	pos = calloc(size, 1);
-	if (!pos)
-		return EFI_OUT_OF_RESOURCES;
-	*load_options = pos;
-	utf8_utf16_strcpy(&pos, env);
-	ret = efi_set_load_options(handle, size, *load_options);
-	if (ret != EFI_SUCCESS) {
-		free(*load_options);
-		*load_options = NULL;
-	}
-	return ret;
-}
-
-#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
-
-/**
- * copy_fdt() - Copy the device tree to a new location available to EFI
- *
- * The FDT is copied to a suitable location within the EFI memory map.
- * Additional 12 KiB are added to the space in case the device tree needs to be
- * expanded later with fdt_open_into().
- *
- * @fdtp:	On entry a pointer to the flattened device tree.
- *		On exit a pointer to the copy of the flattened device tree.
- *		FDT start
- * Return:	status code
- */
-static efi_status_t copy_fdt(void **fdtp)
-{
-	unsigned long fdt_ram_start = -1L, fdt_pages;
-	efi_status_t ret = 0;
-	void *fdt, *new_fdt;
-	u64 new_fdt_addr;
-	uint fdt_size;
-	int i;
-
-	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
-		u64 ram_start = gd->bd->bi_dram[i].start;
-		u64 ram_size = gd->bd->bi_dram[i].size;
-
-		if (!ram_size)
-			continue;
-
-		if (ram_start < fdt_ram_start)
-			fdt_ram_start = ram_start;
-	}
-
-	/*
-	 * Give us at least 12 KiB of breathing room in case the device tree
-	 * needs to be expanded later.
-	 */
-	fdt = *fdtp;
-	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
-	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
-
-	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
-				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-				 &new_fdt_addr);
-	if (ret != EFI_SUCCESS) {
-		log_err("ERROR: Failed to reserve space for FDT\n");
-		goto done;
-	}
-	new_fdt = (void *)(uintptr_t)new_fdt_addr;
-	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
-	fdt_set_totalsize(new_fdt, fdt_size);
-
-	*fdtp = (void *)(uintptr_t)new_fdt_addr;
-done:
-	return ret;
-}
-
-/**
- * get_config_table() - get configuration table
- *
- * @guid:	GUID of the configuration table
- * Return:	pointer to configuration table or NULL
- */
-static void *get_config_table(const efi_guid_t *guid)
-{
-	size_t i;
-
-	for (i = 0; i < systab.nr_tables; i++) {
-		if (!guidcmp(guid, &systab.tables[i].guid))
-			return systab.tables[i].table;
-	}
-	return NULL;
-}
-
-#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
-
-/**
- * efi_install_fdt() - install device tree
- *
- * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
- * address will be installed as configuration table, otherwise the device
- * tree located at the address indicated by environment variable fdt_addr or as
- * fallback fdtcontroladdr will be used.
- *
- * On architectures using ACPI tables device trees shall not be installed as
- * configuration table.
- *
- * @fdt:	address of device tree or EFI_FDT_USE_INTERNAL to use
- *		the hardware device tree as indicated by environment variable
- *		fdt_addr or as fallback the internal device tree as indicated by
- *		the environment variable fdtcontroladdr
- * Return:	status code
- */
-efi_status_t efi_install_fdt(void *fdt)
-{
-	/*
-	 * The EBBR spec requires that we have either an FDT or an ACPI table
-	 * but not both.
-	 */
-#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
-	if (fdt) {
-		log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
-		return EFI_SUCCESS;
-	}
-#else
-	struct bootm_headers img = { 0 };
-	efi_status_t ret;
-
-	if (fdt == EFI_FDT_USE_INTERNAL) {
-		const char *fdt_opt;
-		uintptr_t fdt_addr;
-
-		/* Look for device tree that is already installed */
-		if (get_config_table(&efi_guid_fdt))
-			return EFI_SUCCESS;
-		/* Check if there is a hardware device tree */
-		fdt_opt = env_get("fdt_addr");
-		/* Use our own device tree as fallback */
-		if (!fdt_opt) {
-			fdt_opt = env_get("fdtcontroladdr");
-			if (!fdt_opt) {
-				log_err("ERROR: need device tree\n");
-				return EFI_NOT_FOUND;
-			}
-		}
-		fdt_addr = hextoul(fdt_opt, NULL);
-		if (!fdt_addr) {
-			log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
-			return EFI_LOAD_ERROR;
-		}
-		fdt = map_sysmem(fdt_addr, 0);
-	}
-
-	/* Install device tree */
-	if (fdt_check_header(fdt)) {
-		log_err("ERROR: invalid device tree\n");
-		return EFI_LOAD_ERROR;
-	}
-
-	/* Prepare device tree for payload */
-	ret = copy_fdt(&fdt);
-	if (ret) {
-		log_err("ERROR: out of memory\n");
-		return EFI_OUT_OF_RESOURCES;
-	}
-
-	if (image_setup_libfdt(&img, fdt, 0, NULL)) {
-		log_err("ERROR: failed to process device tree\n");
-		return EFI_LOAD_ERROR;
-	}
-
-	/* Create memory reservations as indicated by the device tree */
-	efi_carve_out_dt_rsv(fdt);
-
-	efi_try_purge_kaslr_seed(fdt);
-
-	if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
-		ret = efi_tcg2_measure_dtb(fdt);
-		if (ret == EFI_SECURITY_VIOLATION) {
-			log_err("ERROR: failed to measure DTB\n");
-			return ret;
-		}
-	}
-
-	/* Install device tree as UEFI table */
-	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
-	if (ret != EFI_SUCCESS) {
-		log_err("ERROR: failed to install device tree\n");
-		return ret;
-	}
-#endif /* GENERATE_ACPI_TABLE */
-
-	return EFI_SUCCESS;
-}
-
-/**
- * do_bootefi_exec() - execute EFI binary
- *
- * The image indicated by @handle is started. When it returns the allocated
- * memory for the @load_options is freed.
- *
- * @handle:		handle of loaded image
- * @load_options:	load options
- * Return:		status code
- *
- * Load the EFI binary into a newly assigned memory unwinding the relocation
- * information, install the loaded image protocol, and call the binary.
- */
-static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
-{
-	efi_status_t ret;
-	efi_uintn_t exit_data_size = 0;
-	u16 *exit_data = NULL;
-	struct efi_event *evt;
-
-	/* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
-	switch_to_non_secure_mode();
-
-	/*
-	 * The UEFI standard requires that the watchdog timer is set to five
-	 * minutes when invoking an EFI boot option.
-	 *
-	 * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
-	 * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
-	 */
-	ret = efi_set_watchdog(300);
-	if (ret != EFI_SUCCESS) {
-		log_err("ERROR: Failed to set watchdog timer\n");
-		goto out;
-	}
-
-	/* Call our payload! */
-	ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
-	if (ret != EFI_SUCCESS) {
-		log_err("## Application failed, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		if (exit_data) {
-			log_err("## %ls\n", exit_data);
-			efi_free_pool(exit_data);
-		}
-	}
-
-	efi_restore_gd();
-
-out:
-	free(load_options);
-
-	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
-		if (efi_initrd_deregister() != EFI_SUCCESS)
-			log_err("Failed to remove loadfile2 for initrd\n");
-	}
-
-	/* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
-	list_for_each_entry(evt, &efi_events, link) {
-		if (evt->group &&
-		    !guidcmp(evt->group,
-			     &efi_guid_event_group_return_to_efibootmgr)) {
-			efi_signal_event(evt);
-			EFI_CALL(systab.boottime->close_event(evt));
-			break;
-		}
-	}
-
-	/* Control is returned to U-Boot, disable EFI watchdog */
-	efi_set_watchdog(0);
-
-	return ret;
-}
-
 /**
  * efi_bootmgr_run() - execute EFI boot manager
  * fdt:	Flat device tree
@@ -1548,99 +1152,3 @@ efi_status_t efi_bootmgr_run(void *fdt)
 
 	return do_bootefi_exec(handle, load_options);
 }
-
-/**
- * efi_run_image() - run loaded UEFI image
- *
- * @source_buffer:	memory address of the UEFI image
- * @source_size:	size of the UEFI image
- * Return:		status code
- */
-efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
-{
-	efi_handle_t mem_handle = NULL, handle;
-	struct efi_device_path *file_path = NULL;
-	struct efi_device_path *msg_path;
-	efi_status_t ret, ret2;
-	u16 *load_options;
-
-	if (!bootefi_device_path || !bootefi_image_path) {
-		log_debug("Not loaded from disk\n");
-		/*
-		 * Special case for efi payload not loaded from disk,
-		 * such as 'bootefi hello' or for example payload
-		 * loaded directly into memory via JTAG, etc:
-		 */
-		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
-					    (uintptr_t)source_buffer,
-					    source_size);
-		/*
-		 * Make sure that device for device_path exist
-		 * in load_image(). Otherwise, shell and grub will fail.
-		 */
-		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
-							       &efi_guid_device_path,
-							       file_path, NULL);
-		if (ret != EFI_SUCCESS)
-			goto out;
-		msg_path = file_path;
-	} else {
-		file_path = efi_dp_append(bootefi_device_path,
-					  bootefi_image_path);
-		msg_path = bootefi_image_path;
-		log_debug("Loaded from disk\n");
-	}
-
-	log_info("Booting %pD\n", msg_path);
-
-	ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
-				      source_size, &handle));
-	if (ret != EFI_SUCCESS) {
-		log_err("Loading image failed\n");
-		goto out;
-	}
-
-	/* Transfer environment variable as load options */
-	ret = efi_env_set_load_options(handle, "bootargs", &load_options);
-	if (ret != EFI_SUCCESS)
-		goto out;
-
-	ret = do_bootefi_exec(handle, load_options);
-
-out:
-	ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
-							  &efi_guid_device_path,
-							  file_path, NULL);
-	efi_free_pool(file_path);
-	return (ret != EFI_SUCCESS) ? ret : ret2;
-}
-
-/**
- * efi_binary_run() - run loaded UEFI image
- *
- * @image:	memory address of the UEFI image
- * @size:	size of the UEFI image
- *
- * Execute an EFI binary image loaded at @image.
- * @size may be zero if the binary is loaded with U-Boot load command.
- *
- * Return:	status code
- */
-efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
-{
-	efi_status_t ret;
-
-	/* Initialize EFI drivers */
-	ret = efi_init_obj_list();
-	if (ret != EFI_SUCCESS) {
-		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
-			ret & ~EFI_ERROR_MASK);
-		return -1;
-	}
-
-	ret = efi_install_fdt(fdt);
-	if (ret != EFI_SUCCESS)
-		return ret;
-
-	return efi_run_image(image, size);
-}
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index ed7214f3a347..786d8a70e2ad 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -1090,7 +1090,8 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
 	if (path && !file)
 		return EFI_INVALID_PARAMETER;
 
-	if (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs"))  {
+	if (IS_ENABLED(CONFIG_EFI_BINARY_EXEC) &&
+	    (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs")))  {
 		/* loadm command and semihosting */
 		efi_get_image_parameters(&image_addr, &image_size);
 
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
index cdfd16ea7742..74d2c54f57b2 100644
--- a/lib/efi_loader/efi_helper.c
+++ b/lib/efi_loader/efi_helper.c
@@ -1,17 +1,28 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
  * Copyright (c) 2020, Linaro Limited
+ * For the code moved from cmd/bootefi.c
+ * Copyright (c) 2016 Alexander Graf
  */
 
 #define LOG_CATEGORY LOGC_EFI
+#include <bootm.h>
 #include <common.h>
-#include <env.h>
-#include <malloc.h>
 #include <dm.h>
-#include <fs.h>
 #include <efi_load_initrd.h>
 #include <efi_loader.h>
 #include <efi_variable.h>
+#include <env.h>
+#include <fs.h>
+#include <log.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <vsprintf.h>
+#include <asm/global_data.h>
+#include <linux/libfdt.h>
+#include <linux/libfdt_env.h>
+
+DECLARE_GLOBAL_DATA_PTR;
 
 #if defined(CONFIG_CMD_EFIDEBUG) || defined(CONFIG_EFI_LOAD_FILE2_INITRD)
 /* GUID used by Linux to identify the LoadFile2 protocol with the initrd */
@@ -282,3 +293,484 @@ bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *inde
 
 	return false;
 }
+
+#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+
+/**
+ * copy_fdt() - Copy the device tree to a new location available to EFI
+ *
+ * The FDT is copied to a suitable location within the EFI memory map.
+ * Additional 12 KiB are added to the space in case the device tree needs to be
+ * expanded later with fdt_open_into().
+ *
+ * @fdtp:	On entry a pointer to the flattened device tree.
+ *		On exit a pointer to the copy of the flattened device tree.
+ *		FDT start
+ * Return:	status code
+ */
+static efi_status_t copy_fdt(void **fdtp)
+{
+	unsigned long fdt_ram_start = -1L, fdt_pages;
+	efi_status_t ret = 0;
+	void *fdt, *new_fdt;
+	u64 new_fdt_addr;
+	uint fdt_size;
+	int i;
+
+	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+		u64 ram_start = gd->bd->bi_dram[i].start;
+		u64 ram_size = gd->bd->bi_dram[i].size;
+
+		if (!ram_size)
+			continue;
+
+		if (ram_start < fdt_ram_start)
+			fdt_ram_start = ram_start;
+	}
+
+	/*
+	 * Give us at least 12 KiB of breathing room in case the device tree
+	 * needs to be expanded later.
+	 */
+	fdt = *fdtp;
+	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
+	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
+
+	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
+				 &new_fdt_addr);
+	if (ret != EFI_SUCCESS) {
+		log_err("ERROR: Failed to reserve space for FDT\n");
+		goto done;
+	}
+	new_fdt = (void *)(uintptr_t)new_fdt_addr;
+	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
+	fdt_set_totalsize(new_fdt, fdt_size);
+
+	*fdtp = (void *)(uintptr_t)new_fdt_addr;
+done:
+	return ret;
+}
+
+/**
+ * get_config_table() - get configuration table
+ *
+ * @guid:	GUID of the configuration table
+ * Return:	pointer to configuration table or NULL
+ */
+static void *get_config_table(const efi_guid_t *guid)
+{
+	size_t i;
+
+	for (i = 0; i < systab.nr_tables; i++) {
+		if (!guidcmp(guid, &systab.tables[i].guid))
+			return systab.tables[i].table;
+	}
+	return NULL;
+}
+
+#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
+
+/**
+ * efi_install_fdt() - install device tree
+ *
+ * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
+ * address will be installed as configuration table, otherwise the device
+ * tree located at the address indicated by environment variable fdt_addr or as
+ * fallback fdtcontroladdr will be used.
+ *
+ * On architectures using ACPI tables device trees shall not be installed as
+ * configuration table.
+ *
+ * @fdt:	address of device tree or EFI_FDT_USE_INTERNAL to use
+ *		the hardware device tree as indicated by environment variable
+ *		fdt_addr or as fallback the internal device tree as indicated by
+ *		the environment variable fdtcontroladdr
+ * Return:	status code
+ */
+efi_status_t efi_install_fdt(void *fdt)
+{
+	/*
+	 * The EBBR spec requires that we have either an FDT or an ACPI table
+	 * but not both.
+	 */
+#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+	if (fdt) {
+		log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
+		return EFI_SUCCESS;
+	}
+#else
+	struct bootm_headers img = { 0 };
+	efi_status_t ret;
+
+	if (fdt == EFI_FDT_USE_INTERNAL) {
+		const char *fdt_opt;
+		uintptr_t fdt_addr;
+
+		/* Look for device tree that is already installed */
+		if (get_config_table(&efi_guid_fdt))
+			return EFI_SUCCESS;
+		/* Check if there is a hardware device tree */
+		fdt_opt = env_get("fdt_addr");
+		/* Use our own device tree as fallback */
+		if (!fdt_opt) {
+			fdt_opt = env_get("fdtcontroladdr");
+			if (!fdt_opt) {
+				log_err("ERROR: need device tree\n");
+				return EFI_NOT_FOUND;
+			}
+		}
+		fdt_addr = hextoul(fdt_opt, NULL);
+		if (!fdt_addr) {
+			log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
+			return EFI_LOAD_ERROR;
+		}
+		fdt = map_sysmem(fdt_addr, 0);
+	}
+
+	/* Install device tree */
+	if (fdt_check_header(fdt)) {
+		log_err("ERROR: invalid device tree\n");
+		return EFI_LOAD_ERROR;
+	}
+
+	/* Prepare device tree for payload */
+	ret = copy_fdt(&fdt);
+	if (ret) {
+		log_err("ERROR: out of memory\n");
+		return EFI_OUT_OF_RESOURCES;
+	}
+
+	if (image_setup_libfdt(&img, fdt, 0, NULL)) {
+		log_err("ERROR: failed to process device tree\n");
+		return EFI_LOAD_ERROR;
+	}
+
+	/* Create memory reservations as indicated by the device tree */
+	efi_carve_out_dt_rsv(fdt);
+
+	efi_try_purge_kaslr_seed(fdt);
+
+	if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
+		ret = efi_tcg2_measure_dtb(fdt);
+		if (ret == EFI_SECURITY_VIOLATION) {
+			log_err("ERROR: failed to measure DTB\n");
+			return ret;
+		}
+	}
+
+	/* Install device tree as UEFI table */
+	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
+	if (ret != EFI_SUCCESS) {
+		log_err("ERROR: failed to install device tree\n");
+		return ret;
+	}
+#endif /* GENERATE_ACPI_TABLE */
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * do_bootefi_exec() - execute EFI binary
+ *
+ * The image indicated by @handle is started. When it returns the allocated
+ * memory for the @load_options is freed.
+ *
+ * @handle:		handle of loaded image
+ * @load_options:	load options
+ * Return:		status code
+ *
+ * Load the EFI binary into a newly assigned memory unwinding the relocation
+ * information, install the loaded image protocol, and call the binary.
+ */
+efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
+{
+	efi_status_t ret;
+	efi_uintn_t exit_data_size = 0;
+	u16 *exit_data = NULL;
+	struct efi_event *evt;
+
+	/* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
+	switch_to_non_secure_mode();
+
+	/*
+	 * The UEFI standard requires that the watchdog timer is set to five
+	 * minutes when invoking an EFI boot option.
+	 *
+	 * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
+	 * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
+	 */
+	ret = efi_set_watchdog(300);
+	if (ret != EFI_SUCCESS) {
+		log_err("ERROR: Failed to set watchdog timer\n");
+		goto out;
+	}
+
+	/* Call our payload! */
+	ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
+	if (ret != EFI_SUCCESS) {
+		log_err("## Application failed, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		if (exit_data) {
+			log_err("## %ls\n", exit_data);
+			efi_free_pool(exit_data);
+		}
+	}
+
+	efi_restore_gd();
+
+out:
+	free(load_options);
+
+	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
+		if (efi_initrd_deregister() != EFI_SUCCESS)
+			log_err("Failed to remove loadfile2 for initrd\n");
+	}
+
+	/* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
+	list_for_each_entry(evt, &efi_events, link) {
+		if (evt->group &&
+		    !guidcmp(evt->group,
+			     &efi_guid_event_group_return_to_efibootmgr)) {
+			efi_signal_event(evt);
+			EFI_CALL(systab.boottime->close_event(evt));
+			break;
+		}
+	}
+
+	/* Control is returned to U-Boot, disable EFI watchdog */
+	efi_set_watchdog(0);
+
+	return ret;
+}
+
+#if CONFIG_IS_ENABLED(EFI_BINARY_EXEC)
+static struct efi_device_path *bootefi_image_path;
+static struct efi_device_path *bootefi_device_path;
+static void *image_addr;
+static size_t image_size;
+
+/**
+ * efi_get_image_parameters() - return image parameters
+ *
+ * @img_addr:		address of loaded image in memory
+ * @img_size:		size of loaded image
+ */
+void efi_get_image_parameters(void **img_addr, size_t *img_size)
+{
+	*img_addr = image_addr;
+	*img_size = image_size;
+}
+
+/**
+ * efi_clear_bootdev() - clear boot device
+ */
+void efi_clear_bootdev(void)
+{
+	efi_free_pool(bootefi_device_path);
+	efi_free_pool(bootefi_image_path);
+	bootefi_device_path = NULL;
+	bootefi_image_path = NULL;
+	image_addr = NULL;
+	image_size = 0;
+}
+
+/**
+ * efi_set_bootdev() - set boot device
+ *
+ * This function is called when a file is loaded, e.g. via the 'load' command.
+ * We use the path to this file to inform the UEFI binary about the boot device.
+ *
+ * @dev:		device, e.g. "MMC"
+ * @devnr:		number of the device, e.g. "1:2"
+ * @path:		path to file loaded
+ * @buffer:		buffer with file loaded
+ * @buffer_size:	size of file loaded
+ */
+void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
+		     void *buffer, size_t buffer_size)
+{
+	struct efi_device_path *device, *image;
+	efi_status_t ret;
+
+	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
+		  devnr, path, buffer, buffer_size);
+
+	/* Forget overwritten image */
+	if (buffer + buffer_size >= image_addr &&
+	    image_addr + image_size >= buffer)
+		efi_clear_bootdev();
+
+	/* Remember only PE-COFF and FIT images */
+	if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
+		if (IS_ENABLED(CONFIG_FIT) &&
+		    !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
+			/*
+			 * FIT images of type EFI_OS are started via command
+			 * bootm. We should not use their boot device with the
+			 * bootefi command.
+			 */
+			buffer = 0;
+			buffer_size = 0;
+		} else {
+			log_debug("- not remembering image\n");
+			return;
+		}
+	}
+
+	/* efi_set_bootdev() is typically called repeatedly, recover memory */
+	efi_clear_bootdev();
+
+	image_addr = buffer;
+	image_size = buffer_size;
+
+	ret = efi_dp_from_name(dev, devnr, path, &device, &image);
+	if (ret == EFI_SUCCESS) {
+		bootefi_device_path = device;
+		if (image) {
+			/* FIXME: image should not contain device */
+			struct efi_device_path *image_tmp = image;
+
+			efi_dp_split_file_path(image, &device, &image);
+			efi_free_pool(image_tmp);
+		}
+		bootefi_image_path = image;
+		log_debug("- boot device %pD\n", device);
+		if (image)
+			log_debug("- image %pD\n", image);
+	} else {
+		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
+		efi_clear_bootdev();
+	}
+}
+
+/**
+ * efi_env_set_load_options() - set load options from environment variable
+ *
+ * @handle:		the image handle
+ * @env_var:		name of the environment variable
+ * @load_options:	pointer to load options (output)
+ * Return:		status code
+ */
+efi_status_t efi_env_set_load_options(efi_handle_t handle,
+				      const char *env_var,
+				      u16 **load_options)
+{
+	const char *env = env_get(env_var);
+	size_t size;
+	u16 *pos;
+	efi_status_t ret;
+
+	*load_options = NULL;
+	if (!env)
+		return EFI_SUCCESS;
+	size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
+	pos = calloc(size, 1);
+	if (!pos)
+		return EFI_OUT_OF_RESOURCES;
+	*load_options = pos;
+	utf8_utf16_strcpy(&pos, env);
+	ret = efi_set_load_options(handle, size, *load_options);
+	if (ret != EFI_SUCCESS) {
+		free(*load_options);
+		*load_options = NULL;
+	}
+	return ret;
+}
+
+/**
+ * efi_run_image() - run loaded UEFI image
+ *
+ * @source_buffer:	memory address of the UEFI image
+ * @source_size:	size of the UEFI image
+ * Return:		status code
+ */
+static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
+{
+	efi_handle_t mem_handle = NULL, handle;
+	struct efi_device_path *file_path = NULL;
+	struct efi_device_path *msg_path;
+	efi_status_t ret, ret2;
+	u16 *load_options;
+
+	if (!bootefi_device_path || !bootefi_image_path) {
+		log_debug("Not loaded from disk\n");
+		/*
+		 * Special case for efi payload not loaded from disk,
+		 * such as 'bootefi hello' or for example payload
+		 * loaded directly into memory via JTAG, etc:
+		 */
+		file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
+					    (uintptr_t)source_buffer,
+					    source_size);
+		/*
+		 * Make sure that device for device_path exist
+		 * in load_image(). Otherwise, shell and grub will fail.
+		 */
+		ret = efi_install_multiple_protocol_interfaces(&mem_handle,
+							       &efi_guid_device_path,
+							       file_path, NULL);
+		if (ret != EFI_SUCCESS)
+			goto out;
+		msg_path = file_path;
+	} else {
+		file_path = efi_dp_append(bootefi_device_path,
+					  bootefi_image_path);
+		msg_path = bootefi_image_path;
+		log_debug("Loaded from disk\n");
+	}
+
+	log_info("Booting %pD\n", msg_path);
+
+	ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
+				      source_size, &handle));
+	if (ret != EFI_SUCCESS) {
+		log_err("Loading image failed\n");
+		goto out;
+	}
+
+	/* Transfer environment variable as load options */
+	ret = efi_env_set_load_options(handle, "bootargs", &load_options);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = do_bootefi_exec(handle, load_options);
+
+out:
+	ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
+							  &efi_guid_device_path,
+							  file_path, NULL);
+	efi_free_pool(file_path);
+	return (ret != EFI_SUCCESS) ? ret : ret2;
+}
+
+/**
+ * efi_binary_run() - run loaded UEFI image
+ *
+ * @image:	memory address of the UEFI image
+ * @size:	size of the UEFI image
+ *
+ * Execute an EFI binary image loaded at @image.
+ * @size may be zero if the binary is loaded with U-Boot load command.
+ *
+ * Return:	status code
+ */
+efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
+{
+	efi_status_t ret;
+
+	/* Initialize EFI drivers */
+	ret = efi_init_obj_list();
+	if (ret != EFI_SUCCESS) {
+		log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+			ret & ~EFI_ERROR_MASK);
+		return ret;
+	}
+
+	ret = efi_install_fdt(fdt);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	return efi_run_image(image, size);
+}
+#endif /* CONFIG_EFI_BINARY_EXEC */
-- 
2.34.1

[PATCH v2 10/12] efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR

[AKASHI Takahiro]

At this point, EFI boot manager interfaces is fully independent from
bootefi command. So just rename the configuration parameter.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 boot/Makefile           | 2 +-
 cmd/Kconfig             | 4 ++--
 cmd/efidebug.c          | 4 ++--
 lib/efi_loader/Kconfig  | 2 +-
 lib/efi_loader/Makefile | 2 +-
 test/boot/bootflow.c    | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/boot/Makefile b/boot/Makefile
index 4eaa5eab4b77..48d74c67d680 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
 ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL
-obj-$(CONFIG_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
+obj-$(CONFIG_EFI_BOOTMGR) += bootmeth_efi_mgr.o
 obj-$(CONFIG_$(SPL_TPL_)EXPO) += bootflow_menu.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o
 obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 189ea6003293..897e99ae68de 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -374,7 +374,7 @@ config CMD_BOOTEFI_BINARY
 
 config CMD_BOOTEFI_BOOTMGR
 	bool "UEFI Boot Manager command"
-	depends on BOOTEFI_BOOTMGR
+	depends on EFI_BOOTMGR
 	default y
 	help
 	  Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
@@ -2124,7 +2124,7 @@ config CMD_EFIDEBUG
 config CMD_EFICONFIG
 	bool "eficonfig - provide menu-driven uefi variables maintenance interface"
 	default y if !HAS_BOARD_SIZE_LIMIT
-	depends on BOOTEFI_BOOTMGR
+	depends on EFI_BOOTMGR
 	select MENU
 	help
 	  Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index e10fbf891a42..b4954258eeba 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -1410,7 +1410,7 @@ static __maybe_unused int do_efi_test_bootmgr(struct cmd_tbl *cmdtp, int flag,
 }
 
 static struct cmd_tbl cmd_efidebug_test_sub[] = {
-#ifdef CONFIG_BOOTEFI_BOOTMGR
+#ifdef CONFIG_EFI_BOOTMGR
 	U_BOOT_CMD_MKENT(bootmgr, CONFIG_SYS_MAXARGS, 1, do_efi_test_bootmgr,
 			 "", ""),
 #endif
@@ -1604,7 +1604,7 @@ U_BOOT_LONGHELP(efidebug,
 	"  - show UEFI memory map\n"
 	"efidebug tables\n"
 	"  - show UEFI configuration tables\n"
-#ifdef CONFIG_BOOTEFI_BOOTMGR
+#ifdef CONFIG_EFI_BOOTMGR
 	"efidebug test bootmgr\n"
 	"  - run simple bootmgr for test\n"
 #endif
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index ddb1a012a761..61e056dedb5b 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -41,7 +41,7 @@ config EFI_BINARY_EXEC
 	  You may enable CMD_BOOTEFI_BINARY so that you can use bootefi
 	  command to do that.
 
-config BOOTEFI_BOOTMGR
+config EFI_BOOTMGR
 	bool "UEFI Boot Manager"
 	default y
 	select BOOTMETH_GLOBAL if BOOTSTD
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 0a2cb6e3c476..f882474bba6f 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -42,7 +42,7 @@ targets += initrddump.o
 endif
 
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
-obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o
+obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
 obj-y += efi_boottime.o
 obj-y += efi_helper.o
 obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index fa1bebdd02da..b670dfd5e5b5 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -374,7 +374,7 @@ static int bootflow_system(struct unit_test_state *uts)
 {
 	struct udevice *bootstd, *dev;
 
-	if (!IS_ENABLED(CONFIG_BOOTEFI_BOOTMGR))
+	if (!IS_ENABLED(CONFIG_EFI_BOOTMGR))
 		return -EAGAIN;
 	ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
 	ut_assertok(device_bind(bootstd, DM_DRIVER_GET(bootmeth_efi_mgr),
-- 
2.34.1

[PATCH v2 11/12] net: tftp: remove explicit efi configuration dependency

[AKASHI Takahiro]

Now it is clear that the feature actually depends on efi interfaces,
not "bootefi" command. efi_set_bootdev() will automatically be nullified
if necessary efi component is disabled.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 net/tftp.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 88e71e67de35..2e335413492b 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -302,12 +302,10 @@ static void tftp_complete(void)
 			time_start * 1000, "/s");
 	}
 	puts("\ndone\n");
-	if (IS_ENABLED(CONFIG_CMD_BOOTEFI)) {
-		if (!tftp_put_active)
-			efi_set_bootdev("Net", "", tftp_filename,
-					map_sysmem(tftp_load_addr, 0),
-					net_boot_file_size);
-	}
+	if (!tftp_put_active)
+		efi_set_bootdev("Net", "", tftp_filename,
+				map_sysmem(tftp_load_addr, 0),
+				net_boot_file_size);
 	net_set_state(NETLOOP_SUCCESS);
 }
 
-- 
2.34.1

[PATCH v2 12/12] fs: remove explicit efi configuration dependency

[AKASHI Takahiro]

Now it is clear that the feature actually depends on efi interfaces,
not "bootefi" command. efi_set_bootdev() will automatically be nullified
if necessary efi component is disabled.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fs.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index 4cb4310c9cc2..70cdb594c4c8 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
 		return 1;
 	}
 
-	if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
-		efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
-				(argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
-				len_read);
+	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
+			(argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
+			len_read);
 
 	printf("%llu bytes read in %lu ms", len_read, time);
 	if (time > 0) {
-- 
2.34.1

Re: [PATCH v2 00/12] cmd: bootefi: refactor the code for bootmgr

[AKASHI Takahiro]

Hi Heinrich, Ilias

On Tue, Nov 21, 2023 at 10:29:38AM +0900, AKASHI Takahiro wrote:
> This patch set is motivated by the discussion[1] regarding
> CONFIG_BOOTEFI_BOOTMGR option.
> 
> At the end, bootefi.c will be decomposed into two parts, one for
> providing the command itself and one for implementing helper functions.
> EFI_LOADER will now be available without CONFIG_CMDLINE or specifically
> CONFIG_CMD_BOOTEFI if invoked via bootmeth/bootstd.
> 
> Then, EFI_LOADER library side will be further split into two options
> for fine-grain control:
> CONFIG_EFI_BINARY_EXEC: execute UEFI binaries which are to be explicitly
>     loaded by U-Boot's load commands/functions or other methods
>     (like a jtag debugger?)
>     It supports bootmeth_efi as well as "bootefi <addr>|hello"(/"bootm"?).
> 
> CONFIG_EFI_BOOTMGR: provide EFI boot manger functionality
>     It supports bootmeth_efi_mgr as well as "bootefi bootmgr".
> 
> As such, We will no longer need CONFIG_EFI_BINARY_EXEC if we want to only
> make use of the UEFI boot manger for booting a next stage OS.

Any other comments?
I think the changes are no doubt trivial.

-Takahiro Akashi


> Prerequisite
> ============
> This patch set is based on top of Tom's "next" branch.
> 
> Patches
> =======
> Patch#1-#12: I hope that those commits show step-by-step refactoring
>     without introducing degradation.
> 
> Tests
> =====
> * run UT efi_selftest on sandbox locally
> * run test_efi_bootmgr on sandbox locally
> 
> Unfortunately, I could not submit a pull request for CI test.
> 
> Changes
> =======
> v2 (Nov 21, 2023)
> * rebased onto Tom's next branch
> * remove already merged commits
> * revise commit messages
> * add patch #5 which was split from ex-patch#5
> RFC (Oct 26, 2023)
> 
> [1] https://lists.denx.de/pipermail/u-boot/2023-October/534598.html
> 
> AKASHI Takahiro (12):
>   cmd: bootefi: unfold do_bootefi_image()
>   cmd: bootefi: re-organize do_bootefi()
>   cmd: bootefi: carve out EFI boot manager interface
>   cmd: bootefi: carve out binary execution interface
>   cmd: bootefi: localize global device paths for efi_selftest
>   cmd: bootefi: move library interfaces under lib/efi_loader
>   cmd: efidebug: ease efi configuration dependency
>   bootmeth: use efi_loader interfaces instead of bootefi command
>   efi_loader: split unrelated code from efi_bootmgr.c
>   efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR
>   net: tftp: remove explicit efi configuration dependency
>   fs: remove explicit efi configuration dependency
> 
>  boot/Kconfig                     |   4 +-
>  boot/Makefile                    |   2 +-
>  boot/bootm_os.c                  |  31 +-
>  boot/bootmeth_efi.c              |   8 +-
>  boot/bootmeth_efi_mgr.c          |   2 +-
>  cmd/Kconfig                      |  21 +-
>  cmd/bootefi.c                    | 670 +++++--------------------------
>  cmd/efidebug.c                   |   4 +-
>  fs/fs.c                          |   7 +-
>  include/efi_loader.h             |  34 +-
>  lib/efi_loader/Kconfig           |  11 +-
>  lib/efi_loader/Makefile          |   2 +-
>  lib/efi_loader/efi_bootmgr.c     |  37 ++
>  lib/efi_loader/efi_device_path.c |   3 +-
>  lib/efi_loader/efi_helper.c      | 498 ++++++++++++++++++++++-
>  net/tftp.c                       |  10 +-
>  test/boot/bootflow.c             |   2 +-
>  17 files changed, 700 insertions(+), 646 deletions(-)
> 
> -- 
> 2.34.1
> 

Re: [PATCH v2 00/12] cmd: bootefi: refactor the code for bootmgr

[Ilias Apalodimas]

Akashi-san

On Mon, 4 Dec 2023 at 03:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> Hi Heinrich, Ilias
>
> On Tue, Nov 21, 2023 at 10:29:38AM +0900, AKASHI Takahiro wrote:
> > This patch set is motivated by the discussion[1] regarding
> > CONFIG_BOOTEFI_BOOTMGR option.
> >
> > At the end, bootefi.c will be decomposed into two parts, one for
> > providing the command itself and one for implementing helper functions.
> > EFI_LOADER will now be available without CONFIG_CMDLINE or specifically
> > CONFIG_CMD_BOOTEFI if invoked via bootmeth/bootstd.
> >
> > Then, EFI_LOADER library side will be further split into two options
> > for fine-grain control:
> > CONFIG_EFI_BINARY_EXEC: execute UEFI binaries which are to be explicitly
> >     loaded by U-Boot's load commands/functions or other methods
> >     (like a jtag debugger?)
> >     It supports bootmeth_efi as well as "bootefi <addr>|hello"(/"bootm"?).
> >
> > CONFIG_EFI_BOOTMGR: provide EFI boot manger functionality
> >     It supports bootmeth_efi_mgr as well as "bootefi bootmgr".
> >
> > As such, We will no longer need CONFIG_EFI_BINARY_EXEC if we want to only
> > make use of the UEFI boot manger for booting a next stage OS.
>
> Any other comments?
> I think the changes are no doubt trivial.

I had other patches down the queue, I'll have a look within the week!

Thanks
/Ilias
>
> -Takahiro Akashi
>
>
> > Prerequisite
> > ============
> > This patch set is based on top of Tom's "next" branch.
> >
> > Patches
> > =======
> > Patch#1-#12: I hope that those commits show step-by-step refactoring
> >     without introducing degradation.
> >
> > Tests
> > =====
> > * run UT efi_selftest on sandbox locally
> > * run test_efi_bootmgr on sandbox locally
> >
> > Unfortunately, I could not submit a pull request for CI test.
> >
> > Changes
> > =======
> > v2 (Nov 21, 2023)
> > * rebased onto Tom's next branch
> > * remove already merged commits
> > * revise commit messages
> > * add patch #5 which was split from ex-patch#5
> > RFC (Oct 26, 2023)
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2023-October/534598.html
> >
> > AKASHI Takahiro (12):
> >   cmd: bootefi: unfold do_bootefi_image()
> >   cmd: bootefi: re-organize do_bootefi()
> >   cmd: bootefi: carve out EFI boot manager interface
> >   cmd: bootefi: carve out binary execution interface
> >   cmd: bootefi: localize global device paths for efi_selftest
> >   cmd: bootefi: move library interfaces under lib/efi_loader
> >   cmd: efidebug: ease efi configuration dependency
> >   bootmeth: use efi_loader interfaces instead of bootefi command
> >   efi_loader: split unrelated code from efi_bootmgr.c
> >   efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR
> >   net: tftp: remove explicit efi configuration dependency
> >   fs: remove explicit efi configuration dependency
> >
> >  boot/Kconfig                     |   4 +-
> >  boot/Makefile                    |   2 +-
> >  boot/bootm_os.c                  |  31 +-
> >  boot/bootmeth_efi.c              |   8 +-
> >  boot/bootmeth_efi_mgr.c          |   2 +-
> >  cmd/Kconfig                      |  21 +-
> >  cmd/bootefi.c                    | 670 +++++--------------------------
> >  cmd/efidebug.c                   |   4 +-
> >  fs/fs.c                          |   7 +-
> >  include/efi_loader.h             |  34 +-
> >  lib/efi_loader/Kconfig           |  11 +-
> >  lib/efi_loader/Makefile          |   2 +-
> >  lib/efi_loader/efi_bootmgr.c     |  37 ++
> >  lib/efi_loader/efi_device_path.c |   3 +-
> >  lib/efi_loader/efi_helper.c      | 498 ++++++++++++++++++++++-
> >  net/tftp.c                       |  10 +-
> >  test/boot/bootflow.c             |   2 +-
> >  17 files changed, 700 insertions(+), 646 deletions(-)
> >
> > --
> > 2.34.1
> >