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

Alexander Graf
2018-12-26T21:24:45Z

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