Re: [RFC PATCH v3 2/2] bootmenu: add UEFI and disto_boot entries

[...]

...

...

•                 command = calloc(1, command_size);
  

•                 if (!command) {
  

•                         free(entry->title);
  

•                         free(load_option);
  

•                         free(entry);
  

•                         goto cleanup;
  

•                 }
  

•                 snprintf(command, command_size, "bootefi bootindex %X", bootorder[j]);
  

•                 entry->command = command;
  

•                 sprintf(entry->key, "%d", i);
  

•                 entry->num = i;
  

•                 entry->menu = menu;
  

•                 entry->type = BOOT_TYPE_UEFI;
  

•                 entry->bootorder = bootorder[j];
  

•                 entry->next = NULL;
  

•                 if (!iter)
  

•                         menu->first = entry;
  

•                 else
  

•                         iter->next = entry;
  

•                 iter = entry;
  

•                 ++i;
  

•         }
  

•         if (i == MAX_COUNT - 1)
  

•                 break;
  

• }
  

• free(bootorder);
  

+}

+bootmgr:

Why do we need an entire menu entry if the bootorder is not defined? Currently there's no logic in the efibootmgr to look for the standard filenames in the ESP (eg bootarm.efi, bootaa64.efi etc) if no boot option is defined. Instead this is implement in distro_bootcmd.

In this version, newly added command "bootefi bootindex XXXX" is called if the user selects one of the "Boot####" option, and "bootefi bootindex" does not handle "BootNext". That's why I think the menu entry simply call "bootefi bootmgr" is required. Anyway, I will modify to set "BootNext" and call "bootefi bootmgr" if the user selects a "Boot####" option.

I think we need to consider the case that "BootNext" is already set when bootmenu comes up. In this case, bootmenu must automatically try to boot the load option of "BootNext" without any user interaction.

Good point. Especially if we fix setvariable at runtime for specific boards, capsuleupdates will need this

...

I was thinking of something along the lines of:

1. bootmenu comes up
2. We read all the Boot#### variables that are defined and add them on the menu

2a. If the user doesn't explicitly choose a boot option we run 'bootefi bootmgr' 2b. If the user does select a different option we set BootNext and still run 'bootefi bootmgr' 3. If there's not Boot#### option defined, we should in the future add the functionality of searching for bootaa64.efi etc in ESP and still just launch the efi bootmgr

Thank you very much for summarizing. I would like to update as follows.

1. bootmenu comes up
2. If the BootNext is already defined, try to boot with BootNext

without showing bootmenu 3. We read the BootOrder, then read Boot#### with the order specified by BootOrder and add it on the menu (UEFI spec requires to honor the priorities set in BootOrder variable) 3a. If the user doesn't explicitly choose a boot option we run 'bootefi bootmgr' 3b. If the user does select a different option we set BootNext and still run 'bootefi bootmgr' 4. If there's not Boot#### option defined, we should in the future add the functionality of searching for bootaa64.efi etc in ESP and still just launch the efi bootmgr 4a. If "bootefi bootmgr" returns, run U-Boot "bootcmd" environment variable.

Yep, that looks correct

...

...

• /* Add UEFI Boot Manager entry if available */
  

• if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
  

[...]

...

...

•                 free(entry);
  

•                 goto cleanup;
  

•         }
  

Any idea of how we'll tackle that? We could export the efibootmgr functions that deal with this and use them on the edit menu ?

I guess you are referring the RFC patch[*1] I sent before? If yes, yes I will reuse them.

Yes

Or do you mean the "efidebug" command?

[*1] https://lore.kernel.org/u-boot/20220225013257.15674-5-masahisa.kojima@linaro...

...

...

•         sprintf(entry->key, "%d", i);
  

•         entry->num = i;
  

•         entry->menu = menu;
  

•         entry->type = BOOT_TYPE_NONE;
  

•         entry->next = NULL;
  

•         if (!iter)
  

•                 menu->first = entry;
  

•         else
  

•                 iter->next = entry;
  

•         iter = entry;
  

•         ++i;
  }
  
  /* Add U-Boot console entry at the end */
  

@@ -353,7 +602,7 @@ static struct bootmenu_data *bootmenu_create(int delay) if (!entry) goto cleanup;

•         entry->title = strdup("U-Boot console");
  

•         entry->title = u16_strdup(u"U-Boot console");
  

Can we add an option to prohibit the user from going back to the console?

Yes, I will add this option.

[...]

Thanks /Ilias