[U-Boot] [PATCH 5/5] cmd: run: add "-e" option to run an EFI application

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Wed, Dec 26, 2018 at 10:24:45PM +0100, Alexander Graf wrote:
> 
> 
> On 25.12.18 12:29, AKASHI Takahiro wrote:
> > On Sun, Dec 23, 2018 at 03:19:17AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 18.12.18 06:02, 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      | 31 +++++++++++++++++++++++++++++++
> >>>  include/command.h |  3 +++
> >>>  4 files changed, 40 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> >>> index 2fc52e3056d2..8122793d11c5 100644
> >>> --- a/cmd/bootefi.c
> >>> +++ b/cmd/bootefi.c
> >>> @@ -471,7 +471,7 @@ static efi_status_t bootefi_test_prepare
> >>>  
> >>>  #endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> >>>  
> >>> -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..013dd2e51936 100644
> >>> --- a/common/cli.c
> >>> +++ b/common/cli.c
> >>> @@ -12,8 +12,10 @@
> >>>  #include <cli.h>
> >>>  #include <cli_hush.h>
> >>>  #include <console.h>
> >>> +#include <efi_loader.h>
> >>>  #include <fdtdec.h>
> >>>  #include <malloc.h>
> >>> +#include "../cmd/bootefi.h"
> >>>  
> >>>  DECLARE_GLOBAL_DATA_PTR;
> >>>  
> >>> @@ -125,6 +127,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;
> >>> +			} 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;
> >>
> >> This duplicates the same logic you added to bootefi.c. Better reuse it.
> >> I guess you can just call a function inside bootefi.c from here if you
> >> detect -e:
> >>
> >>   if (!strcmp(argv[1], "-e"))
> >>     return do_bootefi_run(cmdtp, flag, argc, argv);
> >>
> >> and just handle it all inside bootefi.c at that point.
> > 
> > Hmm, OK.
> > In this case, the command syntax of bootmgr will be changed so as to
> > accept "BootXXXX" instead of just an integer (as boot id).
> 
> Not necessarily. You can just move all the code you have here into a new
> function in bootefi.c. The parsing logic really should just be shared.

Ah ha, OK.

-Takahiro Akashi

> 
> Alex