[U-Boot] [PATCH v1 15/15] efi_loader: add bootmgr

[Rob Clark]

On Sat, Aug 12, 2017 at 9:10 AM, Alexander Graf <agraf at suse.de> wrote:
>
>
> On 10.08.17 20:29, Rob Clark wrote:
>>
>> Similar to a "real" UEFI implementation, the bootmgr looks at the
>> BootOrder and BootXXXX variables to try to find an EFI payload to load
>> and boot.  This is added as a sub-command of bootefi.
>>
>> The idea is that the distro bootcmd would first try loading a payload
>> via the bootmgr, and then if that fails (ie. first boot or corrupted
>> EFI variables) it would fallback to loading bootaa64.efi.  (Which
>> would then load fallback.efi which would look for \EFI\*\boot.csv and
>> populate BootOrder and BootXXXX based on what it found.)
>>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>   cmd/bootefi.c                     |  48 ++++++++++-
>>   include/config_distro_bootcmd.h   |   5 ++
>>   include/efi_api.h                 |   4 +
>>   include/efi_loader.h              |   6 ++
>>   lib/efi_loader/Makefile           |   2 +-
>>   lib/efi_loader/efi_bootmgr.c      | 169
>> ++++++++++++++++++++++++++++++++++++++
>>   lib/efi_loader/efi_boottime.c     |   6 +-
>>   lib/efi_loader/efi_image_loader.c |   1 +
>>   8 files changed, 235 insertions(+), 6 deletions(-)
>>   create mode 100644 lib/efi_loader/efi_bootmgr.c
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 80f52e9e35..02a0dd159b 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -219,6 +219,36 @@ exit:
>>         return ret;
>>   }
>>   +static int do_bootefi_bootmgr_exec(unsigned long fdt_addr)
>> +{
>> +       struct efi_device_path *device_path, *file_path;
>> +       void *addr;
>> +       efi_status_t r;
>> +
>> +       /* Initialize and populate EFI object list */
>> +       if (!efi_obj_list_initalized)
>> +               efi_init_obj_list();
>> +
>> +       /*
>> +        * gd lives in a fixed register which may get clobbered while we
>> execute
>> +        * the payload. So save it here and restore it on every callback
>> entry
>> +        */
>> +       efi_save_gd();
>> +
>> +       addr = efi_bootmgr_load(&device_path, &file_path);
>> +       if (!addr)
>> +               return 1;
>> +
>> +       printf("## Starting EFI application at %p ...\n", addr);
>> +       r = do_bootefi_exec(addr, (void*)fdt_addr, device_path,
>> file_path);
>> +       printf("## Application terminated, r = %lu\n",
>> +              r & ~EFI_ERROR_MASK);
>> +
>> +       if (r != EFI_SUCCESS)
>> +               return 1;
>> +
>> +       return 0;
>> +}
>>     /* 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[])
>> @@ -237,7 +267,14 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int
>> argc, char * const argv[])
>>                 memcpy((char *)addr, __efi_hello_world_begin, size);
>>         } else
>>   #endif
>> -       {
>> +       if (!strcmp(argv[1], "bootmgr")) {
>> +               unsigned long fdt_addr = 0;
>> +
>> +               if (argc > 2)
>> +                       fdt_addr = simple_strtoul(argv[2], NULL, 16);
>> +
>> +               return do_bootefi_bootmgr_exec(fdt_addr);
>> +       } else {
>>                 saddr = argv[1];
>>                 addr = simple_strtoul(saddr, NULL, 16);
>> @@ -270,7 +307,11 @@ static char bootefi_help_text[] =
>>         "hello\n"
>>         "  - boot a sample Hello World application stored within U-Boot"
>>   #endif
>> -       ;
>> +       "bootmgr [fdt addr]\n"
>> +       "  - load and boot EFI payload based on BootOrder/BootXXXX
>> variables.\n"
>> +       "\n"
>> +       "    If specified, the device tree located at <fdt address>
>> gets\n"
>> +       "    exposed as EFI configuration table.\n";
>>   #endif
>>     U_BOOT_CMD(
>> @@ -308,6 +349,9 @@ void efi_set_bootdev(const char *dev, const char
>> *devnr, const char *path)
>>   #endif
>>         }
>>   +     if (!path)
>> +               return;
>> +
>>         if (strcmp(dev, "Net")) {
>>                 /* Add leading / to fs paths, because they're absolute */
>>                 snprintf(filename, sizeof(filename), "/%s", path);
>> diff --git a/include/config_distro_bootcmd.h
>> b/include/config_distro_bootcmd.h
>> index d8dab8e46a..94ccab02d2 100644
>> --- a/include/config_distro_bootcmd.h
>> +++ b/include/config_distro_bootcmd.h
>> @@ -112,6 +112,11 @@
>>     #define BOOTENV_SHARED_EFI
>> \
>>         "boot_efi_binary="
>> \
>> +               "if fdt addr ${fdt_addr_r}; then "
>> \
>> +                       "bootefi bootmgr ${fdt_addr_r};"
>> \
>
>
> This is too late. At this point you already checked that there indeed is a
> fallback binary. Since the bootmgr target actually knows which device it
> loads itself from, it can occur way before in the boot chain.
>
> Maybe we should just add a new boot_target for bootmgr. That way it
> naturally fits into the distro boot flow. You could then add a
> BOOTENV_DEV_BOOTMGR and simply run it from there.
>
> The only thing missing in that case is the device tree override - hmm...
>
> Oh well, if you need that I'm fine to leave it as hacky as it is here, but
> this boot protocol is definitely not what the UEFI guys had envisioned ;).
>

tbh I'm pretty lost in the distro bootcmd stuff, so if someone wants
to take a stab at a better approach, have at it.

We could possibly do 'bootefi bootmgr' earlier, if we already had the
fdt.  I'm not sure about all the various cases here.  I think the
*main* thing is that we try bootmgr first before falling back to
/efi/boot/bootxyzw.efi.  The rest is just optimization.

BR,
-R