[RFC PATCH v3 1/2] efi_loader: introduce "bootefi bootindex" command

Masahisa Kojima masahisa.kojima at linaro.org
Thu Mar 10 01:21:17 CET 2022


Hi Akashi-san, Ilias,

On Wed, 9 Mar 2022 at 22:50, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Kojima-san
>
> On Wed, Mar 09, 2022 at 11:35:57AM +0900, Takahiro Akashi wrote:
> > On Wed, Mar 09, 2022 at 09:47:25AM +0900, Masahisa Kojima wrote:
> > > On Tue, 8 Mar 2022 at 23:17, Takahiro Akashi <takahiro.akashi at linaro.org> wrote:
> > > >
> > > > On Tue, Mar 08, 2022 at 11:07:44PM +0900, Masahisa Kojima wrote:
> > > > > This commit introduces the new command "bootefi bootindex".
> > > > > With this command, user can select which "Boot####" option
> > > > > to load and execute.
> > > >
> > > > You can do the same thing with:
> > > > $ efidebug boot next 1 (for BOOT0001)
> > > > $ bootefi bootmgr
> > >
> > > Thank you for the information, it is good to know.
> > > My only concern is that user needs to enable "efidebug" command
> > > for this case, since efidebug implies that it is for debug purpose.
> >
> > Yeah, that is the point where I and ex-maintainer have never agreed.
> > So we have no standard UI for UEFI subsystem on U-Boot until now.
> >
> > Just FYI, we can do the same thing in yet another way :)
> >   => env set -e -nv -bs -rt BootNext =0x0001
> >
> > Similarly, BootOrder as well.
> >
> > -Takahiro Akashi
>
> I also think it's safe to drop this patch.  It's indeed an easier way to
> load a specific boot index, however the use of bootnext is clearly defined
> in the spec.  I don't think we need to increase the EFI code more as long
> as it's clear to anyone that setting bootnext (note here it has to be set
> on each reboot) would have the same result.

Thank you for your comment.
I will drop this patch and will use BootNext for the same purpose.

Thanks,
Masahisa Kojima

>
> Cheers
> /Ilias
> >
> > > Thanks,
> > > Masahisa Kojima
> > >
> > > >
> > > > -Takahiro Akashi
> > > >
> > > >
> > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > > > > ---
> > > > > Changes in v3:
> > > > > - newly created
> > > > >
> > > > >  cmd/bootefi.c                | 42 ++++++++++++++++++++++++++++++++++++
> > > > >  include/efi_loader.h         |  1 +
> > > > >  lib/efi_loader/efi_bootmgr.c |  7 +++---
> > > > >  3 files changed, 46 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > > > index 46eebd5ee2..df86438fec 100644
> > > > > --- a/cmd/bootefi.c
> > > > > +++ b/cmd/bootefi.c
> > > > > @@ -416,6 +416,30 @@ static int do_efibootmgr(void)
> > > > >       return CMD_RET_SUCCESS;
> > > > >  }
> > > > >
> > > > > +/**
> > > > > + * do_efibootindex() - load and execute the specified Boot#### option
> > > > > + *
> > > > > + * Return:   status code
> > > > > + */
> > > > > +static int do_efibootindex(u16 boot_index)
> > > > > +{
> > > > > +     efi_handle_t handle;
> > > > > +     efi_status_t ret;
> > > > > +     void *load_options;
> > > > > +
> > > > > +     ret = efi_try_load_entry(boot_index, &handle, &load_options);
> > > > > +     if (ret != EFI_SUCCESS) {
> > > > > +             log_notice("EFI boot manager: failed to load Boot%04X\n", boot_index);
> > > > > +             return CMD_RET_FAILURE;
> > > > > +     }
> > > > > +
> > > > > +     ret = do_bootefi_exec(handle, load_options);
> > > > > +
> > > > > +     if (ret != EFI_SUCCESS)
> > > > > +             return CMD_RET_FAILURE;
> > > > > +
> > > > > +     return CMD_RET_SUCCESS;
> > > > > +}
> > > > >  /**
> > > > >   * do_bootefi_image() - execute EFI binary
> > > > >   *
> > > > > @@ -654,6 +678,22 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > >               return CMD_RET_FAILURE;
> > > > >       }
> > > > >
> > > > > +     if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > > > > +             if (!strcmp(argv[1], "bootindex")) {
> > > > > +                     char *endp;
> > > > > +                     int boot_index;
> > > > > +
> > > > > +                     if (argc < 3)
> > > > > +                             return CMD_RET_USAGE;
> > > > > +
> > > > > +                     boot_index = (int)hextoul(argv[2], &endp);
> > > > > +                     if (*endp != '\0' || boot_index > 0xffff)
> > > > > +                             return CMD_RET_USAGE;
> > > > > +
> > > > > +                     return do_efibootindex((u16)boot_index);
> > > > > +             }
> > > > > +     }
> > > > > +
> > > > >       if (argc > 2) {
> > > > >               uintptr_t fdt_addr;
> > > > >
> > > > > @@ -702,6 +742,8 @@ static char bootefi_help_text[] =
> > > > >       "\n"
> > > > >       "    If specified, the device tree located at <fdt address> gets\n"
> > > > >       "    exposed as EFI configuration table.\n"
> > > > > +     "bootefi bootindex <boot option>\n"
> > > > > +     "  - load and boot EFI payload based on the specified BootXXXX variable.\n"
> > > > >  #endif
> > > > >       ;
> > > > >  #endif
> > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > > > index 80a5f1ec01..e5972f5fee 100644
> > > > > --- a/include/efi_loader.h
> > > > > +++ b/include/efi_loader.h
> > > > > @@ -861,6 +861,7 @@ efi_status_t efi_set_load_options(efi_handle_t handle,
> > > > >                                 efi_uintn_t load_options_size,
> > > > >                                 void *load_options);
> > > > >  efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);
> > > > > +efi_status_t efi_try_load_entry(u16 n, efi_handle_t *handle, void **load_options);
> > > > >
> > > > >  /**
> > > > >   * struct efi_image_regions - A list of memory regions
> > > > > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > > > > index 8c04ecbdc8..a3060b5c62 100644
> > > > > --- a/lib/efi_loader/efi_bootmgr.c
> > > > > +++ b/lib/efi_loader/efi_bootmgr.c
> > > > > @@ -42,8 +42,7 @@ static const struct efi_runtime_services *rs;
> > > > >   * @load_options:    load options set on the loaded image protocol
> > > > >   * Return:           status code
> > > > >   */
> > > > > -static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> > > > > -                                void **load_options)
> > > > > +efi_status_t efi_try_load_entry(u16 n, efi_handle_t *handle, void **load_options)
> > > > >  {
> > > > >       struct efi_load_option lo;
> > > > >       u16 varname[] = u"Boot0000";
> > > > > @@ -165,7 +164,7 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
> > > > >               /* load BootNext */
> > > > >               if (ret == EFI_SUCCESS) {
> > > > >                       if (size == sizeof(u16)) {
> > > > > -                             ret = try_load_entry(bootnext, handle,
> > > > > +                             ret = efi_try_load_entry(bootnext, handle,
> > > > >                                                    load_options);
> > > > >                               if (ret == EFI_SUCCESS)
> > > > >                                       return ret;
> > > > > @@ -189,7 +188,7 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
> > > > >       for (i = 0; i < num; i++) {
> > > > >               log_debug("%s trying to load Boot%04X\n", __func__,
> > > > >                         bootorder[i]);
> > > > > -             ret = try_load_entry(bootorder[i], handle, load_options);
> > > > > +             ret = efi_try_load_entry(bootorder[i], handle, load_options);
> > > > >               if (ret == EFI_SUCCESS)
> > > > >                       break;
> > > > >       }
> > > > > --
> > > > > 2.17.1
> > > > >


More information about the U-Boot mailing list