Re: [U-Boot] [PATCH v2 12/14] cmd: run: add "-e" option to run an EFI application

On Mon, Dec 03, 2018 at 12:53:41AM +0100, Alexander Graf wrote:

On 05.11.18 10:06, AKASHI Takahiro wrote:

...

"run -e" allows for executing EFI application with a specific "BootXXXX" variable. If no "BootXXXX" is specified or "BootOrder" is specified, it tries to run an EFI application specified in the order of "bootOrder."

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

cmd/bootefi.c | 2 +- cmd/nvedit.c | 5 +++++ common/cli.c | 30 ++++++++++++++++++++++++++++++ include/command.h | 3 +++ 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 7580008f691b..3cefb4d0ecaa 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -509,7 +509,7 @@ exit: return ret; }

-static int do_bootefi_bootmgr_exec(int boot_id) +int do_bootefi_bootmgr_exec(int boot_id) { struct efi_device_path *device_path, *file_path; void *addr; diff --git a/cmd/nvedit.c b/cmd/nvedit.c index de16c72c23f2..c0facabfc4fe 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1343,8 +1343,13 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", +#if defined(CONFIG_CMD_BOOTEFI)

• "var -e [BootXXXX]\n"
• " - load and run UEFI app based on 'BootXXXX' UEFI variable",

+#else "var [...]\n" " - run the commands in the environment variable(s) 'var'", +#endif var_complete ); #endif diff --git a/common/cli.c b/common/cli.c index 51b8d5f85cbb..e4ec35b5eb00 100644 --- a/common/cli.c +++ b/common/cli.c @@ -14,6 +14,7 @@ #include <console.h> #include <fdtdec.h> #include <malloc.h> +#include "../cmd/efishell.h"

I guess this already shows that something is going wrong ;).

efishell.h here is included solely for efi_init_obj_list() definition and so

Please move the depending functions into lib/efi_loader. That way we can have the efishell command be optional, but keep this logic around regardless.

I can assume that you agree to this function being moved to efi_setup.c.

...

DECLARE_GLOBAL_DATA_PTR;

@@ -125,6 +126,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 2) return CMD_RET_USAGE;

+#ifdef CONFIG_CMD_BOOTEFI

• if (!strcmp(argv[1], "-e")) {

• int boot_id = -1;
  

• char *endp;
  

• if (argc == 3) {
  

• 	if (!strcmp(argv[2], "BootOrder")) {
  

• 		/* boot_id = -1 */;
  

Just write that in code. The compiler will optimize it out.

OK.

...

• 	} else if (!strncmp(argv[2], "Boot", 4)) {
  

• 		boot_id = (int)simple_strtoul(&argv[2][4],
  

• 					      &endp, 0);
  

• 		if ((argv[2] + strlen(argv[2]) != endp) ||
  

• 		    boot_id > 0xffff)
  

• 			return CMD_RET_USAGE;
  

• 	} else {
  

• 		return CMD_RET_USAGE;
  

• 	}
  

• }
  

• if (efi_init_obj_list())
  

• 	return CMD_RET_FAILURE;
  

• if (efi_handle_fdt(NULL))
  

• 	return CMD_RET_FAILURE;
  

• return do_bootefi_bootmgr_exec(boot_id);
  

• }

+#endif

• for (i = 1; i < argc; ++i) { char *arg;

diff --git a/include/command.h b/include/command.h index 5b1577f3b477..5bb675122cce 100644 --- a/include/command.h +++ b/include/command.h @@ -48,6 +48,9 @@ typedef struct cmd_tbl_s cmd_tbl_t; #if defined(CONFIG_CMD_RUN) extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); #endif +#if defined(CONFIG_CMD_BOOTEFI) +int do_bootefi_bootmgr_exec(int boot_id); +#endif

This needs to be in efi_loader.h which then needs to get included from cli.c.

After your comment above, you expect that do_bootefi_bootmgr_exec(), hence do_bootefi_exec() (and efi_handle_fdt()?) as well, be also moved to lib/efi_loader/(efi_bootmgr.c)?

Thanks, -Takahiro Akashi

Alex