| Message ID | 20181218050257.20142-6-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | efi_loader: run a specific efi application more easily | expand |
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. > + } 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
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
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. Alex
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
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; + } 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 /* common/command.c */ int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
"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(-)