Hi Heinrich,
On Mon, 2 May 2022 at 06:44, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
>
> On 4/28/22 10:09, Masahisa Kojima wrote:
> > This commit adds the UEFI related menu entries
> > into the bootmenu.
> >
> > User can select which UEFI "Boot####" option to execute
> > from bootmenu, then bootmenu sets the "BootNext" UEFI
> > variable and invoke efi bootmgr. The efi bootmgr
> > will handle the "BootNext" UEFI variable.
> >
> > If the "BootNext" UEFI variable is preset and efi bootmgr is enabled,
> > bootmenu invokes efi bootmgr to handle "BootNext" as first priority.
> >
> > The UEFI boot entry has the "UEFI BOOTXXXX" prefix as below.
>
> This prefix provides no value.
>
> >
> > *** U-Boot Boot Menu ***
> >
> > UEFI BOOT0000 : debian
> > UEFI BOOT0001 : ubuntu
> >
> > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> > ---
> > Changes in v5:
> > - split into the separate patch
> > - add function description comment
> > - remove non-volatile attribute for BootNext variable to minimize
> > the access to the non-volatile storage
> >
> > cmd/bootmenu.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 154 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> > index 15ad621c9f..da688e6213 100644
> > --- a/cmd/bootmenu.c
> > +++ b/cmd/bootmenu.c
> > @@ -7,6 +7,8 @@
> > #include <common.h>
> > #include <command.h>
> > #include <ansi.h>
> > +#include <efi_loader.h>
> > +#include <efi_variable.h>
> > #include <env.h>
> > #include <log.h>
> > #include <menu.h>
> > @@ -28,6 +30,7 @@
> > enum boot_type {
> > BOOTMENU_TYPE_NONE = 0,
> > BOOTMENU_TYPE_BOOTMENU,
> > + BOOTMENU_TYPE_UEFI_BOOT_OPTION,
> > };
> >
> > struct bootmenu_entry {
> > @@ -85,6 +88,8 @@ static void bootmenu_print_entry(void *data)
> >
> > if (entry->type == BOOTMENU_TYPE_BOOTMENU)
> > printf("bootmenu_%02d : %ls", entry->bootorder,
> > entry->title);
> > + else if (entry->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION)
> > + printf("UEFI BOOT%04X : %ls", entry->bootorder, entry->title);
>
> Please, remove this hunk.
Thank you for requesting to merge this patch into u-boot/master.
You have removed the "UEFI BOOTXXXX" prefix from
the bootmenu title, but the commit message still says that there is
"UEFI BOOTXXXX" prefix for UEFI entries.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> > else
> > printf("%ls", entry->title);
> >
> > @@ -371,6 +376,95 @@ static int prepare_bootmenu_entry(struct bootmenu_data
> > *menu,
> > return 1;
> > }
> >
> > +/**
> > + * prepare_uefi_bootorder_entry() - generate the uefi bootmenu entries
> > + *
> > + * This function read the "BootOrder" UEFI variable
> > + * and generate the bootmenu entries in the order of "BootOrder".
> > + *
> > + * @menu: pointer to the bootmenu structure
> > + * @current: pointer to the last bootmenu entry list
> > + * @index: pointer to the index of the last bootmenu entry,
> > + * the number of uefi entry is added by this function
> > + * Return: 1 on success, negative value on error
> > + */
> > +static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
> > + struct bootmenu_entry **current,
> > + unsigned short int *index)
> > +{
> > + u16 *bootorder;
> > + efi_status_t ret;
> > + unsigned short j;
> > + efi_uintn_t num, size;
> > + void *load_option;
> > + struct efi_load_option lo;
> > + u16 varname[] = u"Boot####";
> > + unsigned short int i = *index;
> > + struct bootmenu_entry *entry = NULL;
> > + struct bootmenu_entry *iter = *current;
> > +
> > + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > &size);
> > + if (!bootorder)
> > + return -ENOENT;
> > +
> > + num = size / sizeof(u16);
> > + for (j = 0; j < num; j++) {
> > + entry = malloc(sizeof(struct bootmenu_entry));
> > + if (!entry)
> > + return -ENOMEM;
> > +
> > + efi_create_indexed_name(varname, sizeof(varname),
> > + "Boot", bootorder[j]);
> > + load_option = efi_get_var(varname, &efi_global_variable_guid,
> > &size);
> > + if (!load_option)
> > + continue;
> > +
> > + ret = efi_deserialize_load_option(&lo, load_option, &size);
> > + if (ret != EFI_SUCCESS) {
> > + log_warning("Invalid load option for %ls\n", varname);
> > + free(load_option);
> > + free(entry);
> > + continue;
> > + }
> > +
> > + if (lo.attributes & LOAD_OPTION_ACTIVE) {
> > + entry->title = u16_strdup(lo.label);
> > + if (!entry->title) {
> > + free(load_option);
> > + free(entry);
> > + free(bootorder);
> > + return -ENOMEM;
> > + }
> > + entry->command = strdup("bootefi bootmgr");
> > + sprintf(entry->key, "%d", i);
> > + entry->num = i;
> > + entry->menu = menu;
> > + entry->type = BOOTMENU_TYPE_UEFI_BOOT_OPTION;
> > + entry->bootorder = bootorder[j];
> > + entry->next = NULL;
> > +
> > + if (!iter)
> > + menu->first = entry;
> > + else
> > + iter->next = entry;
> > +
> > + iter = entry;
> > + i++;
> > + }
> > +
> > + free(load_option);
> > +
> > + if (i == MAX_COUNT - 1)
> > + break;
> > + }
> > +
> > + free(bootorder);
> > + *index = i;
> > + *current = iter;
> > +
> > + return 1;
> > +}
> > +
> > static struct bootmenu_data *bootmenu_create(int delay)
> > {
> > int ret;
> > @@ -396,6 +490,14 @@ static struct bootmenu_data *bootmenu_create(int delay)
> > if (ret < 0)
> > goto cleanup;
> >
> > + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > + if (i < MAX_COUNT - 1) {
> > + ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
> > + if (ret < 0 && ret != -ENOENT)
> > + goto cleanup;
> > + }
> > + }
> > +
> > /* Add U-Boot console entry at the end */
> > if (i <= MAX_COUNT - 1) {
> > entry = malloc(sizeof(struct bootmenu_entry));
> > @@ -473,6 +575,31 @@ static void menu_display_statusline(struct menu *m)
> > puts(ANSI_CLEAR_LINE);
> > }
> >
> > +static void handle_uefi_bootnext(void)
> > +{
> > + u16 bootnext;
> > + efi_status_t ret;
> > + efi_uintn_t size;
> > +
> > + /* Initialize EFI drivers */
> > + ret = efi_init_obj_list();
> > + if (ret != EFI_SUCCESS) {
> > + log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > + ret & ~EFI_ERROR_MASK);
> > +
> > + return;
> > + }
> > +
> > + /* If UEFI BootNext variable is set, boot the BootNext load option */
> > + size = sizeof(u16);
> > + ret = efi_get_variable_int(u"BootNext",
> > + &efi_global_variable_guid,
> > + NULL, &size, &bootnext, NULL);
> > + if (ret == EFI_SUCCESS)
> > + /* BootNext does exist here, try to boot */
> > + run_command("bootefi bootmgr", 0);
> > +}
> > +
> > static void bootmenu_show(int delay)
> > {
> > int init = 0;
> > @@ -482,8 +609,12 @@ static void bootmenu_show(int delay)
> > struct menu *menu;
> > struct bootmenu_data *bootmenu;
> > struct bootmenu_entry *iter;
> > + efi_status_t efi_ret = EFI_SUCCESS;
> > char *option, *sep;
> >
> > + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
> > + handle_uefi_bootnext();
> > +
> > /* If delay is 0 do not create menu, just run first entry */
> > if (delay == 0) {
> > option = bootmenu_getoption(0);
> > @@ -532,6 +663,27 @@ static void bootmenu_show(int delay)
> > command = strdup(iter->command);
> > }
> >
> > + /*
> > + * If the selected entry is UEFI BOOT####, set the BootNext variable.
> > + * Then uefi bootmgr is invoked by the preset command in
> > iter->command.
> > + */
> > + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > + if (iter->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION) {
> > + /*
> > + * UEFI specification requires BootNext variable
> > needs non-volatile
> > + * attribute, but this BootNext is only used inside
> > of U-Boot and
> > + * removed by efi bootmgr once BootNext is processed.
> > + * So this BootNext can be volatile.
> > + */
> > + efi_ret = efi_set_variable_int(u"BootNext",
> > &efi_global_variable_guid,
> > +
> > EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +
> > EFI_VARIABLE_RUNTIME_ACCESS,
> > + sizeof(u16),
> > &iter->bootorder, false);
> > + if (efi_ret != EFI_SUCCESS)
> > + goto cleanup;
> > + }
> > + }
> > +
> > cleanup:
> > menu_destroy(menu);
> > bootmenu_destroy(bootmenu);
> > @@ -545,7 +697,8 @@ cleanup:
> > if (title && command) {
> > debug("Starting entry '%ls'\n", title);
> > free(title);
> > - run_command(command, 0);
> > + if (efi_ret == EFI_SUCCESS)
> > + run_command(command, 0);
> > free(command);
> > }
> >
>
