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

Hi both,

On Thu, 2 Mar 2023 at 08:44, AKASHI Takahiro takahiro.akashi@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@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@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