[RFC PATCH 1/1] eficonfig: Add a 'scan-only' mode in eficonfig

[Ilias Apalodimas]

Hi both,

On Thu, 2 Mar 2023 at 08:44, AKASHI Takahiro <takahiro.akashi at linaro.org> wrote:
>
> On Thu, Mar 02, 2023 at 11:21:05AM +0900, Masahisa Kojima wrote:
> > Hi Ilias,
> >
> > On Wed, 1 Mar 2023 at 23:44, Ilias Apalodimas
> > <ilias.apalodimas at linaro.org> wrote:
> > >
> > > eficonfig will automatically scan and add Boot#### variables on launch.
> > > It will also perform automatic management of the automatically added
> > > variables (e.g it might decide to delete a boot option if the DP
> > > disappears).  This functionality is useful outside the context of
> > > eficonfig as well.
> > >
> > > So let's add a -a flag to eficonfig that will only perform the
> > > automatic variable management and exit.  That would allow users
> > > to define a bootcmd along the lines of 'eficonfig -a && bootefi bootmgr',
> > > that matches the §3.5.1.1 'Removable Media Boot Behavior' behaviour
> > > described in the EFI spec.
> > >
> > > Open questions:
> > > - Is this ok to add on eficonfig? Similar functionality is described in the EFI
> > >   spec as part of the efibootmgr
> > > - Having the functionality on the command gives us the flexibility to run the
> > >   run the command when needed.  Alternatively we could move it to the efibootmgr
> > >   and hide it behing a Kconfig option
> >
> > In current implementation, "bootmenu" and "eficonfig" invoke this
> > automatic load option
> > management function, but I think "bootmenu" should not have responsibility for
> > load option management.
> > So I'll vote for moving this automatic load option management into efibootmgr,
> > then it is invoked in efi subsystem initialization.
>
> Basically +1, but
> I think that we can implement this kind of action by using 'event' hook
> as "removable media support" itself is implemented.
>

I don't disagree, hence my "open questions" section in there.
Changing eficonfig was the easiest patch I could come up with to show
what I had in mind.

That being said I like Akashi-san's idea of integrating the automatic
scan as an event.  In that case we can even skip hiding it behind a
Kconfig option since this is supposed to be supported as part of the
efi bootmgr?

Thanks!
/Ilias
> -Takahiro Akashi
>
>
> > Regards,
> > Masahisa Kojima
> >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > ---
> > >  cmd/eficonfig.c | 23 ++++++++++++++++++++---
> > >  1 file changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > > index 720f52b48b8c..9b6631816997 100644
> > > --- a/cmd/eficonfig.c
> > > +++ b/cmd/eficonfig.c
> > > @@ -2693,10 +2693,18 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const a
> > >  {
> > >         efi_status_t ret;
> > >         struct efimenu *efi_menu;
> > > +       bool exit_on_scan = false;
> > >
> > > -       if (argc > 1)
> > > +       if (argc > 2)
> > >                 return CMD_RET_USAGE;
> > >
> > > +       if (argc > 1) {
> > > +               if (!strcmp(argv[1], "-a"))
> > > +                       exit_on_scan = true;
> > > +               else
> > > +                       return CMD_RET_FAILURE;
> > > +       }
> > > +
> > >         ret = efi_init_obj_list();
> > >         if (ret != EFI_SUCCESS) {
> > >                 log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > > @@ -2713,6 +2721,9 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const a
> > >         if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
> > >                 return ret;
> > >
> > > +       if (exit_on_scan)
> > > +               return EFI_SUCCESS;
> > > +
> > >         while (1) {
> > >                 efi_menu = eficonfig_create_fixed_menu(maintenance_menu_items,
> > >                                                        ARRAY_SIZE(maintenance_menu_items));
> > > @@ -2734,8 +2745,14 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const a
> > >         return CMD_RET_SUCCESS;
> > >  }
> > >
> > > +static char eficonfig_help_text[] =
> > > +       " - UEFI menu to configure UEFI variables\n"
> > > +       "\n"
> > > +       "eficonfig - Spawn the configuration menu\n"
> > > +       "  -a Scan, configure Boot#### variables and exit\n";
> > > +
> > >  U_BOOT_CMD(
> > > -       eficonfig, 1, 0, do_eficonfig,
> > > +       eficonfig, 2, 0, do_eficonfig,
> > >         "provide menu-driven UEFI variable maintenance interface",
> > > -       ""
> > > +       eficonfig_help_text
> > >  );
> > > --
> > > 2.39.2
> > >