[v2,12/14] cmd: run: add "-e" option to run an EFI application

Message ID 20181105090653.7409-13-takahiro.akashi@linaro.org
State Superseded
Headers show
Series
  • efi: make efi and bootmgr more usable
Related show

Commit Message

AKASHI Takahiro Nov. 5, 2018, 9:06 a.m.
"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(-)

Comments

Alexander Graf Dec. 2, 2018, 11:53 p.m. | #1
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 ;).

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.

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

> +			} 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.


Alex
AKASHI Takahiro Dec. 3, 2018, 7:57 a.m. | #2
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

Patch

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"
 
 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 */;
+			} 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
 
 /* common/command.c */
 int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int