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

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

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).

Thanks,
-Takahiro Akashi

> > +			} 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 200c7a5e9f4e..9b7b876585d9 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
> 
> I would prefer if nvedit.c includes efi_loader.h and we define it there?
> 
> 
> Alex