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

Heinrich Schuchardt
2019-02-27T06:25:52Z

On 2/27/19 7:12 AM, AKASHI Takahiro wrote:
> On Tue, Feb 26, 2019 at 08:20:10PM +0100, Heinrich Schuchardt wrote:
>> On 1/15/19 3:54 AM, 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."
>>>
>>
>> If we cannot specify the device tree what would be the use of this for
>> ARM processors?
>
> I don't get your point. What's the matter with device tree?

To boot an ARM board on Linux or BSD you need a device tree.

So run -e Boot0001 will not allow you to boot into Linux because it does
not specify a device tree.

>
>
>> Why do you add the option to run and not to bootefi?
>>
>> You already introduced the capability to set BootNext. Why isn't that
>> enough?
>
> Simple.
>
> => run -e Boot00>
> versus
>
> => efidebug boot next 1
> => bootefi bootmgr

In patch 4/5 you already introduced 'bootefi bootmgr $fdt_addr_r 0001'

So there is no need to go through efidebug.

I think we should avoid alternative commands.

>
> First of all, efidebug is only recognized as a *debugging* tool.
> I believe that the former syntax is more intuitive, and it looks
> a natural extension to "run" command semantics akin to "env -e".
>
> As a result, we don't have to know about bootefi for normal operations.

But you have to know about 'run' which you might not need otherwise.

Best regards

Heinrich

>
> Thanks,
> -Takahiro Akashi
>
>
>> Best regards
>>
>> Heinrich
>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>> cmd/bootefi.c | 31 +++++++++++++++++++++++++++++++
>>> cmd/nvedit.c | 9 ++++++++-
>>> common/cli.c | 10 ++++++++++
>>> include/command.h | 3 +++
>>> 4 files changed, 52 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index 241fd0f987ab..ebe149dffa1f 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -492,6 +492,37 @@ static int do_bootefi_bootmgr_exec(int boot_id)
>>> return CMD_RET_SUCCESS;
>>> }
>>>
>>> +/* Called by "run" command */
>>> +int do_bootefi_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> +{
>>> + int boot_id = -1;
>>> + char *endp;
>>> +
>>> + if (argc > 2)
>>> + return CMD_RET_USAGE;
>>> +
>>> + if (argc == 2) {
>>> + if (!strcmp(argv[1], "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);
>>> +}
>>> +
>>> /* Interpreter command to boot an arbitrary EFI image from memory */
>>> static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> {
>>> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
>>> index de16c72c23f2..ce746bbf1b3e 100644
>>> --- a/cmd/nvedit.c
>>> +++ b/cmd/nvedit.c
>>> @@ -1344,7 +1344,14 @@ U_BOOT_CMD_COMPLETE(
>>> run, CONFIG_SYS_MAXARGS, 1, do_run,
>>> "run commands in an environment variable",
>>> "var [...]\n"
>>> - " - run the commands in the environment variable(s) 'var'",
>>> + " - run the commands in the environment variable(s) 'var'"
>>> +#if defined(CONFIG_CMD_BOOTEFI)
>>> + "\n"
>>> + "run -e [BootXXXX]\n"
>>> + " - load and run UEFI app based on 'BootXXXX' UEFI variable",
>>> +#else
>>> + ,
>>> +#endif
>>> var_complete
>>> );
>>> #endif
>>> diff --git a/common/cli.c b/common/cli.c
>>> index 51b8d5f85cbb..fbb09d5049a4 100644
>>> --- a/common/cli.c
>>> +++ b/common/cli.c
>>> @@ -12,6 +12,7 @@
>>> #include <cli.h>
>>> #include <cli_hush.h>
>>> #include <console.h>
>>> +#include <efi_loader.h>
>>> #include <fdtdec.h>
>>> #include <malloc.h>
>>>
>>> @@ -125,6 +126,15 @@ 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")) {
>>> + argc--;
>>> + argv++;
>>> +
>>> + return do_bootefi_run(cmdtp, flag, argc, argv);
>>> + }
>>> +#endif
>>> +
>>> for (i = 1; i < argc; ++i) {
>>> char *arg;
>>>
>>> diff --git a/include/command.h b/include/command.h
>>> index 200c7a5e9f4e..feddef300ccc 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)
>>> +extern int do_bootefi_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>>> +#endif
>>>
>>> /* common/command.c */
>>> int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
>>>
>>
>