[PATCH v6 1/6] efi_loader: menu-driven addition of UEFI boot option

Masahisa Kojima masahisa.kojima at linaro.org
Mon Jun 13 11:44:15 CEST 2022


Hi Akashi-san,

On Mon, 6 Jun 2022 at 09:39, Masahisa Kojima <masahisa.kojima at linaro.org> wrote:
>
> On Wed, 25 May 2022 at 10:39, Takahiro Akashi
> <takahiro.akashi at linaro.org> wrote:
> >
> > On Mon, May 16, 2022 at 08:00:37PM +0900, Masahisa Kojima wrote:
> > > This commit supports the menu-driven UEFI boot option addition.
> > > User can select the block device volume having
> > > efi_simple_file_system_protocol and select the file corresponding
> > > to the Boot#### variable. Then user enter the label of the BOOT####
> > > variable in utf8.
> > >
> > > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > > ---
> > > Changes in v6:
> > > - fix typos
> > > - modify volume name to match U-Boot syntax
> > > - compile in CONFIG_EFI_LOADER=n and CONFIG_CMD_BOOTEFI_BOOTMGR=n
> >
> > Is this correct?
>
> Sorry for the lack of information. I would like to say that build test in
> the following configurations.
> - CONFIG_EFI_LOADER=n,  CONFIG_CMD_BOOTEFI_BOOTMGR=n
> - CONFIG_EFI_LOADER=y, CONFIG_CMD_BOOTEFI_BOOTMGR=n
> >
> > > - simplify u16_strncmp() usage
> > > - support "a\b.efi" file path, use link list to handle filepath
> > > - modify length check condition
> > > - UEFI related menu items only appears with CONFIG_AUTOBOOT_MENU_SHOW=y
> >
> > Why?
> > I think that the feature is useful even without AUTOBOOT.
> > As you know, efidebug is seen as a debugging tool and is not expected
> > to be enabled in production systems.
> >
> > So the feature you're adding is the only available UI for boot manager.
> > What I recommend is
> > - to create a boot manager maintenance as a standalone U-Boot command,
> > - to add an bootmenu entry for invoking the command
>
> I agree.
>
> >
> > > Changes in v5:
> > > - remove forward declarations
> > > - add const qualifier for menu items
> > > - fix the possible unaligned access for directory info access
> > > - split into three commit 1)add boot option 2) delete boot option 3)change boot order
> > >   This commit is 1)add boot option.
> > > - fix file name buffer allocation size, it should be EFI_BOOTMENU_FILE_PATH_MAX * sizeof(u16)
> > > - fix wrong size checking for file selection
> > >
> > > Chanes in v4:
> > > - UEFI boot option maintenance menu is integrated into bootmenu
> > > - display the simplified volume name(e.g. usb0:1, nvme1:2) for the
> > >   volume selection
> > > - instead of extending lib/efi_loader/efi_bootmgr.c, newly create
> > >   lib/efi_loader/efi_bootmenu_maintenance.c and implement boot
> > >   variable maintenance into it.
> > >
> > > Changes in RFC v3:
> > >  not included in v3 series
> > >
> > > Changes in RFC v2:
> > > - enable utf8 user input for boot option name
> > > - create lib/efi_loader/efi_console.c::efi_console_get_u16_string() for
> > >   utf8 user input handling
> > > - use u16_strlcat instead of u16_strcat
> > > - remove the EFI_CALLs, and newly create or expose the following
> > >   xxx_int() functions.
> > >     efi_locate_handle_buffer_int(), efi_open_volume_int(),
> > >     efi_file_open_int(), efi_file_close_int(), efi_file_read_int() and
> > >     efi_file_setpos_int().
> > >   Note that EFI_CALLs still exist for EFI_DEVICE_PATH_TO_TEXT_PROTOCOL
> > >   and EFI_SIMPLE_TEXT_INPUT/OUTPUT_PROTOCOL
> > > - use efi_search_protocol() instead of calling locate_protocol() to get
> > >   the device_path_to_text_protocol interface.
> > > - remove unnecessary puts(ANSI_CLEAR_LINE), this patch is still depends on
> > >   puts(ANSI_CLEAR_CONSOLE)
> > > - skip SetVariable() if the bootorder is not changed
> > >
> > >  cmd/bootmenu.c                            |  73 +-
> > >  include/efi_loader.h                      |  37 +
> > >  lib/efi_loader/Makefile                   |   3 +
> > >  lib/efi_loader/efi_bootmenu_maintenance.c | 906 ++++++++++++++++++++++
> >
> > I would say that this file should be moved under /cmd as the code does not
> > implement any UEFI specification semantics, but simply provides helper
> > functions for bootmenu command.
>
> OK, I will create a new command for UEFI variable maintenance.
>
> >
> > Or I recommend that the boot manager be implemented as a standalone command
> > (as I insisted serveral times before) and the related maintenance feature
> > be invoked as follows:
> >    => efibootmanager -i (i for interactive)
> >
> > >  lib/efi_loader/efi_boottime.c             |  52 +-
> > >  lib/efi_loader/efi_console.c              |  81 ++
> > >  lib/efi_loader/efi_disk.c                 |  11 +
> > >  lib/efi_loader/efi_file.c                 |  75 +-
> > >  8 files changed, 1184 insertions(+), 54 deletions(-)
> > >  create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
> > >
> > > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> > > index 8859eebea5..4b846332b0 100644
> > > --- a/cmd/bootmenu.c
> > > +++ b/cmd/bootmenu.c
> > > @@ -19,6 +19,12 @@
> > >
> > >  /* maximum bootmenu entries */
> > >  #define MAX_COUNT    99
> > > +#if defined(CONFIG_CMD_BOOTEFI_BOOTMGR) && defined(CONFIG_AUTOBOOT_MENU_SHOW)
> > > +#define STATIC_ENTRY 2
> > > +#else
> > > +#define STATIC_ENTRY 1
> > > +#endif
> > > +#define MAX_DYNAMIC_ENTRY (MAX_COUNT - STATIC_ENTRY)
> > >
> > >  /* maximal size of bootmenu env
> > >   *  9 = strlen("bootmenu_")
> > > @@ -38,10 +44,11 @@ enum boot_type {
> > >       BOOTMENU_TYPE_NONE = 0,
> > >       BOOTMENU_TYPE_BOOTMENU,
> > >       BOOTMENU_TYPE_UEFI_BOOT_OPTION,
> > > +     BOOTMENU_TYPE_UEFI_MAINTENANCE,
> > >  };
> > >
> > >  struct bootmenu_entry {
> > > -     unsigned short int num;         /* unique number 0 .. MAX_COUNT */
> > > +     unsigned short int num;         /* unique number 0 .. (MAX_COUNT - 1) */
> > >       char key[3];                    /* key identifier of number */
> > >       u16 *title;                     /* title of entry */
> > >       char *command;                  /* hush command of entry */
> > > @@ -55,7 +62,7 @@ static char *bootmenu_getoption(unsigned short int n)
> > >  {
> > >       char name[MAX_ENV_SIZE];
> > >
> > > -     if (n > MAX_COUNT)
> > > +     if (n > MAX_DYNAMIC_ENTRY)
> > >               return NULL;
> > >
> > >       sprintf(name, "bootmenu_%d", n);
> > > @@ -217,7 +224,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu,
> > >               iter = entry;
> > >               ++i;
> > >
> > > -             if (i == MAX_COUNT - 1)
> > > +             if (i == MAX_DYNAMIC_ENTRY)
> > >                       break;
> > >       }
> > >
> > > @@ -305,7 +312,7 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
> > >
> > >               free(load_option);
> > >
> > > -             if (i == MAX_COUNT - 1)
> > > +             if (i == MAX_DYNAMIC_ENTRY)
> > >                       break;
> > >       }
> > >
> > > @@ -341,14 +348,51 @@ 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) {
> > > +     if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && IS_ENABLED(CONFIG_AUTOBOOT_MENU_SHOW)) {
> > > +             if (i < MAX_DYNAMIC_ENTRY) {
> > >                       ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
> > >                       if (ret < 0 && ret != -ENOENT)
> > >                               goto cleanup;
> > >               }
> > >       }
> > >
> > > +     if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && IS_ENABLED(CONFIG_AUTOBOOT_MENU_SHOW)) {
> > > +             /* Add UEFI Boot Manager Maintenance entry */
> > > +             if (i <= MAX_DYNAMIC_ENTRY) {
> > > +                     entry = malloc(sizeof(struct bootmenu_entry));
> > > +                     if (!entry)
> > > +                             goto cleanup;
> > > +
> > > +                     entry->title = u16_strdup(u"UEFI Boot Manager Maintenance");
> >
> > If 'u16' is a matter, why not define the type of 'title' as u8 and you can always use
> > utf16_to_utf8() for the setting the value to this field.
>
> OK.
>
> >
> > > +                     if (!entry->title) {
> > > +                             free(entry);
> > > +                             goto cleanup;
> > > +                     }
> > > +
> > > +                     entry->command = strdup("");
> > > +                     if (!entry->command) {
> > > +                             free(entry->title);
> > > +                             free(entry);
> > > +                             goto cleanup;
> > > +                     }
> > > +
> > > +                     sprintf(entry->key, "%d", i);
> > > +
> > > +                     entry->num = i;
> > > +                     entry->menu = menu;
> > > +                     entry->type = BOOTMENU_TYPE_UEFI_MAINTENANCE;
> > > +                     entry->next = NULL;
> > > +
> > > +                     if (!iter)
> > > +                             menu->first = entry;
> > > +                     else
> > > +                             iter->next = entry;
> > > +
> > > +                     iter = entry;
> > > +                     i++;
> > > +             }
> > > +     }
> > > +
> > >       /* Add U-Boot console entry at the end */
> > >       if (i <= MAX_COUNT - 1) {
> > >               entry = malloc(sizeof(struct bootmenu_entry));
> > > @@ -520,6 +564,14 @@ static enum bootmenu_ret bootmenu_show(int delay)
> > >               title = u16_strdup(iter->title);
> > >               command = strdup(iter->command);
> > >
> > > +             if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > > +                     if (iter->type == BOOTMENU_TYPE_UEFI_MAINTENANCE) {
> > > +                             efi_bootmenu_show_maintenance_menu();
> > > +                             ret = BOOTMENU_RET_UPDATED;
> > > +                             goto cleanup;
> > > +                     }
> > > +             }
> > > +
> > >               /* last entry is U-Boot console or Quit */
> > >               if (iter->num == iter->menu->count - 1) {
> > >                       ret = BOOTMENU_RET_QUIT;
> > > @@ -610,6 +662,7 @@ int do_bootmenu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > >  {
> > >       char *delay_str = NULL;
> > >       int delay = 10;
> > > +     int ret;
> > >
> > >  #if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0)
> > >       delay = CONFIG_BOOTDELAY;
> > > @@ -624,7 +677,13 @@ int do_bootmenu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > >       if (delay_str)
> > >               delay = (int)simple_strtol(delay_str, NULL, 10);
> > >
> > > -     bootmenu_show(delay);
> > > +     while (1) {
> > > +             ret =  bootmenu_show(delay);
> > > +             delay = -1;
> > > +             if (ret != BOOTMENU_RET_UPDATED)
> > > +                     break;
> > > +     }
> > > +
> > >       return 0;
> > >  }
> > >
> > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > index 733ee03cd7..49f326e585 100644
> > > --- a/include/efi_loader.h
> > > +++ b/include/efi_loader.h
> > > @@ -226,6 +226,9 @@ const char *__efi_nesting_dec(void);
> > >  #define EFI_CACHELINE_SIZE 128
> > >  #endif
> > >
> > > +/* max bootmenu title size for volume selection */
> > > +#define BOOTMENU_DEVICE_NAME_MAX 16
> > > +
> > >  /* Key identifying current memory map */
> > >  extern efi_uintn_t efi_memory_map_key;
> > >
> > > @@ -314,6 +317,9 @@ extern const efi_guid_t efi_guid_firmware_management_protocol;
> > >  extern const efi_guid_t efi_esrt_guid;
> > >  /* GUID of the SMBIOS table */
> > >  extern const efi_guid_t smbios_guid;
> > > +/*GUID of console */
> > > +extern const efi_guid_t efi_guid_text_input_protocol;
> > > +extern const efi_guid_t efi_guid_text_output_protocol;
> > >
> > >  extern char __efi_runtime_start[], __efi_runtime_stop[];
> > >  extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];
> > > @@ -877,6 +883,8 @@ efi_status_t efi_set_load_options(efi_handle_t handle,
> > >                                 void *load_options);
> > >  efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);
> > >
> > > +efi_status_t efi_bootmenu_show_maintenance_menu(void);
> > > +
> > >  /**
> > >   * struct efi_image_regions - A list of memory regions
> > >   *
> > > @@ -1048,4 +1056,33 @@ efi_status_t efi_esrt_populate(void);
> > >  efi_status_t efi_load_capsule_drivers(void);
> > >
> > >  efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, u32 *sz);
> > > +
> > > +efi_status_t efi_locate_handle_buffer_int(enum efi_locate_search_type search_type,
> > > +                                       const efi_guid_t *protocol, void *search_key,
> > > +                                       efi_uintn_t *no_handles, efi_handle_t **buffer);
> > > +
> > > +efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol *this,
> > > +                              struct efi_file_handle **root);
> > > +efi_status_t efi_file_open_int(struct efi_file_handle *this,
> > > +                            struct efi_file_handle **new_handle,
> > > +                            u16 *file_name, u64 open_mode,
> > > +                            u64 attributes);
> > > +efi_status_t efi_file_close_int(struct efi_file_handle *file);
> > > +efi_status_t efi_file_read_int(struct efi_file_handle *this,
> > > +                            efi_uintn_t *buffer_size, void *buffer);
> > > +efi_status_t efi_file_setpos_int(struct efi_file_handle *file, u64 pos);
> > > +
> > > +typedef efi_status_t (*efi_console_filter_func)(struct efi_input_key *key);
> > > +efi_status_t efi_console_get_u16_string
> > > +             (struct efi_simple_text_input_protocol *cin,
> > > +              struct efi_simple_text_output_protocol *cout,
> > > +              u16 *buf, efi_uintn_t count, efi_console_filter_func filer_func,
> > > +              int row, int col);
> > > +
> > > +efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf,
> > > +                                             efi_uintn_t buf_size, u32 *index);
> > > +efi_status_t efi_bootmenu_append_bootorder(u16 index);
> > > +
> > > +efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size);
> > > +
> > >  #endif /* _EFI_LOADER_H */
> > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > > index aaaa25cefe..807e9a4319 100644
> > > --- a/lib/efi_loader/Makefile
> > > +++ b/lib/efi_loader/Makefile
> > > @@ -77,6 +77,9 @@ obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
> > >  obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_riscv.o
> > >  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> > >  obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o
> > > +ifeq ($(CONFIG_CMD_BOOTEFI_BOOTMGR),y)
> > > +obj-$(CONFIG_CMD_BOOTMENU) += efi_bootmenu_maintenance.o
> > > +endif
> > >
> > >  EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> > >  $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
> > > diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c
> > > new file mode 100644
> > > index 0000000000..e5124a8a21
> > > --- /dev/null
> > > +++ b/lib/efi_loader/efi_bootmenu_maintenance.c
> > > @@ -0,0 +1,906 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + *  Menu-driven UEFI Boot Variable maintenance
> > > + *
> > > + *  Copyright (c) 2022 Masahisa Kojima, Linaro Limited
> > > + */
> > > +
> > > +#define LOG_CATEGORY LOGC_EFI
> > > +
> > > +#include <ansi.h>
> > > +#include <common.h>
> > > +#include <charset.h>
> > > +#include <log.h>
> > > +#include <malloc.h>
> > > +#include <menu.h>
> > > +#include <efi_loader.h>
> > > +#include <efi_variable.h>
> > > +#include <asm/unaligned.h>
> > > +
> > > +static struct efi_simple_text_input_protocol *cin;
> > > +static struct efi_simple_text_output_protocol *cout;
> > > +
> > > +#define EFI_BOOTMENU_ENTRY_NUM_MAX 99
> > > +#define EFI_BOOTMENU_FILE_PATH_MAX 512
> > > +#define EFI_BOOTMENU_FILE_PATH_BUF_SIZE (EFI_BOOTMENU_FILE_PATH_MAX * sizeof(u16))
> > > +#define EFI_BOOTMENU_BOOT_NAME_MAX 32
> > > +#define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6
> > > +
> > > +typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
> > > +
> > > +/**
> > > + * struct efi_bootmenu_entry - menu entry structure
> > > + *
> > > + * @num:             menu entry index
> > > + * @title:           title of entry
> > > + * @key:             unique key
> > > + * @bootmgr_menu:    pointer to the menu structure
> > > + * @next:            pointer to the next entry
> > > + * @func:            callback function to be called when this entry is selected
> > > + * @data:            data to be passed to the callback function
> > > + */
> > > +struct efi_bootmenu_entry {
> > > +     u32 num;
> > > +     u16 *title;
> > > +     char key[6];
> > > +     struct efi_bootmenu *bootmgr_menu;
> > > +     struct efi_bootmenu_entry *next;
> > > +     efi_bootmenu_entry_func func;
> > > +     void *data;
> > > +};
> > > +
> > > +/**
> > > + * struct efi_bootmenu - bootmgr menu structure
> > > + *
> > > + * @delay:   delay for autoboot
> > > + * @active:  active menu entry index
> > > + * @count:   total count of menu entry
> > > + * @first:   pointer to the first menu entry
> > > + */
> > > +struct efi_bootmenu {
> > > +     int delay;
> > > +     int active;
> > > +     int count;
> > > +     struct efi_bootmenu_entry *first;
> > > +};
> > > +
> > > +struct efi_bootmenu_item {
> > > +     u16 *title;
> > > +     efi_bootmenu_entry_func func;
> > > +     void *data;
> > > +};
> > > +
> > > +struct efi_bootmenu_boot_selection_data {
> > > +     u16 bootorder_index;
> > > +     void *load_option;
> > > +     int *selected;
> > > +};
> > > +
> > > +struct efi_bootmenu_filepath_info {
> > > +     u16 *name;
> > > +     struct list_head list;
> > > +};
> > > +
> > > +struct efi_bootmenu_boot_option {
> > > +     struct efi_simple_file_system_protocol *current_volume;
> > > +     struct efi_device_path *dp_volume;
> > > +     u16 *current_path;
> > > +     struct list_head filepath_list;
> > > +     u16 *boot_name;
> > > +     bool file_selected;
> > > +};
> > > +
> > > +static const struct efi_device_path END = {
> > > +     .type     = DEVICE_PATH_TYPE_END,
> > > +     .sub_type = DEVICE_PATH_SUB_TYPE_END,
> > > +     .length   = sizeof(END),
> > > +};
> >
> > FYI, 'END' is also defined as a static variable in efi_device_path.c
>
> OK, I will move 'END' to the common include file.
>
> >
> > > +
> > > +struct efi_bootmenu_volume_entry_data {
> > > +     struct efi_bootmenu_boot_option *bo;
> > > +     struct efi_simple_file_system_protocol *v;
> > > +     struct efi_device_path *dp;
> > > +};
> > > +
> > > +struct efi_bootmenu_file_entry_data {
> > > +     struct efi_bootmenu_boot_option *bo;
> > > +     bool is_directory;
> > > +     u16 *file_name;
> > > +};
> > > +
> > > +static void efi_bootmenu_print_entry(void *data)
> > > +{
> > > +     struct efi_bootmenu_entry *entry = data;
> > > +     int reverse = (entry->bootmgr_menu->active == entry->num);
> > > +
> > > +     /* TODO: support scroll or page for many entries */
> > > +
> > > +     /*
> > > +      * Move cursor to line where the entry will be drawn (entry->count)
> > > +      * First 3 lines contain bootmgr menu header + one empty line
> > > +      * For the last "Quit" entry, add one empty line
> > > +      */
> > > +     if (entry->num == (entry->bootmgr_menu->count - 1))
> > > +             printf(ANSI_CURSOR_POSITION, entry->num + 5, 1);
> > > +     else
> > > +             printf(ANSI_CURSOR_POSITION, entry->num + 4, 1);
> > > +
> > > +     puts("     ");
> > > +
> > > +     if (reverse)
> > > +             puts(ANSI_COLOR_REVERSE);
> > > +
> > > +     printf("%ls", entry->title);
> > > +
> > > +     if (reverse)
> > > +             puts(ANSI_COLOR_RESET);
> > > +}
> > > +
> > > +static void efi_bootmenu_display_statusline(struct menu *m)
> > > +{
> > > +     struct efi_bootmenu_entry *entry;
> > > +     struct efi_bootmenu *bootmgr_menu;
> > > +
> > > +     if (menu_default_choice(m, (void *)&entry) < 0)
> > > +             return;
> > > +
> > > +     bootmgr_menu = entry->bootmgr_menu;
> > > +
> > > +     printf(ANSI_CURSOR_POSITION, 1, 1);
> > > +     puts(ANSI_CLEAR_LINE);
> > > +     printf(ANSI_CURSOR_POSITION, 2, 1);
> > > +     puts("  *** U-Boot EFI Boot Manager ***");
> > > +     puts(ANSI_CLEAR_LINE_TO_END);
> > > +     printf(ANSI_CURSOR_POSITION, 3, 1);
> > > +     puts(ANSI_CLEAR_LINE);
> > > +
> > > +     /* First 3 lines are bootmgr_menu header + 2 empty lines between entries */
> > > +     printf(ANSI_CURSOR_POSITION, bootmgr_menu->count + 5, 1);
> > > +     puts(ANSI_CLEAR_LINE);
> > > +     printf(ANSI_CURSOR_POSITION, bootmgr_menu->count + 6, 1);
> > > +     puts("  Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit");
> > > +     puts(ANSI_CLEAR_LINE_TO_END);
> > > +     printf(ANSI_CURSOR_POSITION, bootmgr_menu->count + 7, 1);
> > > +     puts(ANSI_CLEAR_LINE);
> > > +}
> > > +
> > > +static char *efi_bootmenu_choice_entry(void *data)
> > > +{
> > > +     int i;
> > > +     int esc = 0;
> > > +     struct efi_bootmenu_entry *iter;
> > > +     enum bootmenu_key key = KEY_NONE;
> > > +     struct efi_bootmenu *bootmgr_menu = data;
> > > +
> > > +     while (1) {
> > > +             if (bootmgr_menu->delay >= 0) {
> > > +                     /* Autoboot was not stopped */
> > > +                     bootmenu_autoboot_loop((struct bootmenu_data *)bootmgr_menu, &key, &esc);
> > > +             } else {
> > > +                     /* Some key was pressed, so autoboot was stopped */
> > > +                     bootmenu_loop((struct bootmenu_data *)bootmgr_menu, &key, &esc);
> > > +             }
> > > +
> > > +             if (bootmgr_menu->delay == 0)
> > > +                     key = KEY_QUIT;
> > > +
> > > +             switch (key) {
> > > +             case KEY_UP:
> > > +                     if (bootmgr_menu->active > 0)
> > > +                             --bootmgr_menu->active;
> > > +                     /* no menu key selected, regenerate menu */
> > > +                     return NULL;
> > > +             case KEY_DOWN:
> > > +                     if (bootmgr_menu->active < bootmgr_menu->count - 1)
> > > +                             ++bootmgr_menu->active;
> > > +                     /* no menu key selected, regenerate menu */
> > > +                     return NULL;
> > > +             case KEY_SELECT:
> > > +                     iter = bootmgr_menu->first;
> > > +                     for (i = 0; i < bootmgr_menu->active; ++i)
> > > +                             iter = iter->next;
> > > +                     return iter->key;
> > > +             case KEY_QUIT:
> > > +                     /* Quit by choosing the last entry */
> > > +                     iter = bootmgr_menu->first;
> > > +                     while (iter->next)
> > > +                             iter = iter->next;
> > > +                     return iter->key;
> > > +             default:
> > > +                     break;
> > > +             }
> >
> > I don't fully understand how bootmenu works with key strokes,
> > but I think that this kind of key handling is quite generic
> > and put it as a common helper function.
>
> I will consider moving this function to common.
>
> >
> > > +     }
> > > +
> > > +     /* never happens */
> > > +     debug("bootmgr menu: this should not happen");
> > > +     return NULL;
> > > +}
> > > +
> > > +static void efi_bootmenu_destroy(struct efi_bootmenu *bootmgr_menu)
> > > +{
> > > +     struct efi_bootmenu_entry *next;
> > > +     struct efi_bootmenu_entry *iter = bootmgr_menu->first;
> > > +
> > > +     while (iter) {
> > > +             next = iter->next;
> > > +             free(iter);
> > > +             iter = next;
> > > +     }
> > > +     free(bootmgr_menu);
> > > +}
> > > +
> > > +/**
> > > + * efi_bootmenu_process_common() - main handler for uefi bootmgr menu
> > > + *
> > > + * Construct the structures required to show the menu, then handle
> > > + * the user input intracting with u-boot menu functions.
> > > + *
> > > + * @items:   pointer to the structure of each menu entry
> > > + * @count:   the number of menu entry
> > > + * @delay:   delay for autoboot/autoselect
> > > + * Return:   status code
> > > + */
> > > +static efi_status_t efi_bootmenu_process_common(const struct efi_bootmenu_item *items,
> > > +                                             int count, int delay)
> > > +{
> > > +     u32 i;
> > > +     bool exit = false;
> > > +     efi_status_t ret;
> > > +     struct menu *menu;
> > > +     void *choice = NULL;
> > > +     struct efi_bootmenu_entry *entry;
> > > +     struct efi_bootmenu *bootmgr_menu;
> > > +     struct efi_bootmenu_entry *iter = NULL;
> > > +
> > > +     if (count > EFI_BOOTMENU_ENTRY_NUM_MAX)
> > > +             return EFI_OUT_OF_RESOURCES;
> > > +
> > > +     bootmgr_menu = calloc(1, sizeof(struct efi_bootmenu));
> > > +     if (!bootmgr_menu)
> > > +             return EFI_OUT_OF_RESOURCES;
> > > +
> > > +     bootmgr_menu->delay = delay;
> > > +     bootmgr_menu->active = 0;
> > > +     bootmgr_menu->first = NULL;
> > > +
> > > +     for (i = 0; i < count; i++) {
> > > +             entry = calloc(1, sizeof(struct efi_bootmenu_entry));
> > > +             if (!entry) {
> > > +                     ret = EFI_LOAD_ERROR;
> > > +                     goto out;
> > > +             }
> > > +
> > > +             entry->num = i;
> > > +             entry->title = items->title;
> > > +             snprintf(entry->key, sizeof(entry->key), "%04X", i);
> > > +             entry->bootmgr_menu = bootmgr_menu;
> > > +             entry->func = items->func;
> > > +             entry->data = items->data;
> > > +             entry->next = NULL;
> > > +
> > > +             if (!iter)
> > > +                     bootmgr_menu->first = entry;
> > > +             else
> > > +                     iter->next = entry;
> > > +
> > > +             iter = entry;
> > > +             items++;
> > > +     }
> > > +     bootmgr_menu->count = count;
> > > +
> > > +     menu = menu_create(NULL, 0, 1, efi_bootmenu_display_statusline,
> > > +                        efi_bootmenu_print_entry, efi_bootmenu_choice_entry,
> > > +                        bootmgr_menu);
> > > +     if (!menu) {
> > > +             ret = EFI_INVALID_PARAMETER;
> > > +             goto out;
> > > +     }
> > > +
> > > +     for (entry = bootmgr_menu->first; entry; entry = entry->next) {
> > > +             if (!menu_item_add(menu, entry->key, entry)) {
> > > +                     ret = EFI_INVALID_PARAMETER;
> > > +                     goto out;
> > > +             }
> > > +     }
> > > +
> > > +     menu_default_set(menu, bootmgr_menu->first->key);
> > > +
> > > +     while (!exit) {
> > > +             puts(ANSI_CURSOR_HIDE);
> > > +             puts(ANSI_CLEAR_CONSOLE);
> > > +             printf(ANSI_CURSOR_POSITION, 1, 1);
> > > +
> > > +             if (menu_get_choice(menu, &choice)) {
> > > +                     entry = choice;
> > > +                     if (entry->func)
> > > +                             ret = entry->func(entry->data, &exit);
> > > +
> > > +                     /* last entry "Quit" is selected, exit this menu */
> > > +                     if (entry->num == (entry->bootmgr_menu->count - 1)) {
> > > +                             ret = EFI_ABORTED;
> > > +                             break;
> > > +                     }
> > > +             }
> > > +     }
> > > +
> > > +out:
> > > +     menu_destroy(menu);
> > > +     efi_bootmenu_destroy(bootmgr_menu);
> > > +
> > > +     puts(ANSI_CLEAR_CONSOLE);
> > > +     printf(ANSI_CURSOR_POSITION, 1, 1);
> > > +     puts(ANSI_CURSOR_SHOW);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_volume_selected(void *data, bool *exit)
> > > +{
> > > +     struct efi_bootmenu_volume_entry_data *info = data;
> > > +
> > > +     *exit = true;
> > > +
> > > +     if (info) {
> > > +             info->bo->current_volume = info->v;
> > > +             info->bo->dp_volume = info->dp;
> > > +     }
> > > +
> > > +     return EFI_SUCCESS;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_file_selected(void *data, bool *exit)
> > > +{
> > > +     struct efi_bootmenu_file_entry_data *info = data;
> > > +
> > > +     *exit = true;
> > > +
> > > +     if (!info)
> > > +             return EFI_INVALID_PARAMETER;
> > > +
> > > +     if (u16_strcmp(info->file_name, u".") == 0 &&
> > > +         u16_strlen(info->file_name) == 1) {
> > > +             /* stay current path */
> > > +     } else if (u16_strcmp(info->file_name, u"..") == 0 &&
> > > +                u16_strlen(info->file_name) == 2) {
> > > +             struct efi_bootmenu_filepath_info *iter;
> > > +             struct list_head *pos, *n;
> > > +             int is_last;
> > > +
> > > +             memset(info->bo->current_path, 0, EFI_BOOTMENU_FILE_PATH_BUF_SIZE);
> > > +             list_for_each_safe(pos, n, &info->bo->filepath_list) {
> > > +                     iter = list_entry(pos, struct efi_bootmenu_filepath_info, list);
> > > +
> > > +                     is_last = list_is_last(&iter->list, &info->bo->filepath_list);
> > > +                     if (is_last) {
> > > +                             list_del(&iter->list);
> > > +                             free(iter->name);
> > > +                             free(iter);
> > > +                             break;
> > > +                     }
> > > +                     u16_strlcat(info->bo->current_path, iter->name,
> > > +                                 EFI_BOOTMENU_FILE_PATH_MAX);
> > > +                     u16_strlcat(info->bo->current_path, u"\\",
> > > +                                 EFI_BOOTMENU_FILE_PATH_MAX);
> > > +             }
> > > +     } else {
> > > +             size_t new_len;
> > > +             struct efi_bootmenu_filepath_info *filepath;
> > > +
> > > +             new_len = u16_strlen(info->bo->current_path) +
> > > +                                  u16_strlen(info->file_name);
> > > +             if (new_len >= EFI_BOOTMENU_FILE_PATH_MAX) {
> > > +                     /* TODO: show error notification to user */
> > > +                     log_err("file path is too long\n");
> > > +                     return EFI_INVALID_PARAMETER;
> > > +             }
> > > +             u16_strlcat(info->bo->current_path, info->file_name,
> > > +                         EFI_BOOTMENU_FILE_PATH_MAX);
> > > +
> > > +             filepath = calloc(1, sizeof(struct efi_bootmenu_filepath_info));
> > > +             if (!filepath)
> > > +                     return EFI_OUT_OF_RESOURCES;
> > > +
> > > +             filepath->name = u16_strdup(info->file_name);
> > > +             if (!filepath->name) {
> > > +                     free(filepath);
> > > +                     return EFI_OUT_OF_RESOURCES;
> > > +             }
> > > +             list_add_tail(&filepath->list, &info->bo->filepath_list);
> > > +
> > > +             if (info->is_directory) {
> > > +                     /*
> > > +                      * Remainig buffer should have enough space to contain u"\\" and
> > > +                      * at least one character for file name
> > > +                      */
> > > +                     if (new_len + 2 >= EFI_BOOTMENU_FILE_PATH_MAX) {
> > > +                             log_err("directory path is too long\n");
> > > +                             return EFI_INVALID_PARAMETER;
> > > +                     }
> > > +                     u16_strlcat(info->bo->current_path, u"\\",
> > > +                                 EFI_BOOTMENU_FILE_PATH_MAX);
> > > +             } else {
> > > +                     info->bo->file_selected = true;
> > > +             }
> > > +     }
> > > +     return EFI_SUCCESS;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_select_volume(struct efi_bootmenu_boot_option *bo)
> > > +{
> > > +     u32 i;
> > > +     efi_status_t ret;
> > > +     efi_uintn_t count;
> > > +     struct efi_handler *handler;
> > > +     struct efi_device_path *device_path;
> > > +     efi_handle_t *volume_handles = NULL;
> > > +     struct efi_simple_file_system_protocol *v;
> > > +     struct efi_bootmenu_item *menu_item, *iter;
> > > +
> > > +     ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,
> > > +                                        NULL, &count, (efi_handle_t **)&volume_handles);
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     menu_item = calloc(count + 1, sizeof(struct efi_bootmenu_item));
> > > +     if (!menu_item) {
> > > +             ret = EFI_OUT_OF_RESOURCES;
> > > +             goto out1;
> > > +     }
> > > +
> > > +     iter = menu_item;
> > > +     for (i = 0; i < count; i++) {
> > > +             u16 *dev_name, *p;
> > > +             struct efi_block_io *block_io;
> > > +             char buf[BOOTMENU_DEVICE_NAME_MAX];
> > > +             struct efi_bootmenu_volume_entry_data *info;
> > > +
> > > +             ret = efi_search_protocol(volume_handles[i],
> > > +                                       &efi_simple_file_system_protocol_guid, &handler);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     continue;
> > > +             ret = efi_protocol_open(handler, (void **)&v, efi_root, NULL,
> > > +                                     EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     continue;
> > > +
> > > +             ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     continue;
> > > +             ret = efi_protocol_open(handler, (void **)&device_path,
> > > +                                     efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     continue;
> > > +
> > > +             ret = efi_search_protocol(volume_handles[i], &efi_block_io_guid, &handler);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     continue;
> > > +             ret = efi_protocol_open(handler, (void **)&block_io,
> > > +                                     efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     continue;
> > > +
> > > +             info = calloc(1, sizeof(struct efi_bootmenu_volume_entry_data));
> > > +             if (!info) {
> > > +                     ret = EFI_OUT_OF_RESOURCES;
> > > +                     goto out2;
> > > +             }
> > > +
> > > +             efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX);
> > > +             dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16));
> > > +             if (!dev_name) {
> > > +                     free(info);
> > > +                     ret = EFI_OUT_OF_RESOURCES;
> > > +                     goto out2;
> > > +             }
> > > +             p = dev_name;
> > > +             utf8_utf16_strncpy(&p, buf, strlen(buf));
> > > +
> > > +             info->v = v;
> > > +             info->dp = device_path;
> > > +             info->bo = bo;
> > > +             iter->title = dev_name;
> > > +             iter->func = efi_bootmenu_volume_selected;
> > > +             iter->data = info;
> > > +             iter++;
> > > +     }
> > > +
> > > +     iter->title = u16_strdup(u"Quit");
> > > +     iter->func = NULL;
> > > +     iter->data = NULL;
> > > +     count += 1;
> > > +
> > > +     ret = efi_bootmenu_process_common(menu_item, count, -1);
> > > +
> > > +out2:
> > > +     iter = menu_item;
> > > +     for (i = 0; i < count; i++) {
> > > +             struct efi_bootmenu_volume_entry_data *p;
> > > +
> > > +             p = (struct efi_bootmenu_volume_entry_data *)(iter->data);
> > > +             free(iter->title);
> > > +             free(p);
> > > +             iter++;
> > > +     }
> > > +
> > > +     free(menu_item);
> > > +
> > > +out1:
> > > +     efi_free_pool(volume_handles);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_select_file(struct efi_bootmenu_boot_option *bo,
> > > +                                          struct efi_file_handle *root)
> > > +{
> > > +     u32 i;
> > > +     struct efi_file_info *buf;
> > > +     u32 count = 0;
> > > +     efi_uintn_t len;
> > > +     efi_status_t ret;
> > > +     struct efi_file_handle *f;
> > > +     struct efi_bootmenu_item *menu_item, *iter;
> > > +
> > > +     buf = calloc(1, sizeof(struct efi_file_info) + EFI_BOOTMENU_FILE_PATH_BUF_SIZE);
> > > +     if (!buf)
> > > +             return EFI_OUT_OF_RESOURCES;
> > > +
> > > +     while (!bo->file_selected) {
> > > +             count = 0;
> > > +
> > > +             ret = efi_file_open_int(root, &f, bo->current_path, EFI_FILE_MODE_READ, 0);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     return ret;
> > > +
> > > +             /* calculate directory information total count */
> > > +             for (;;) {
> > > +                     len = sizeof(struct efi_file_info) + EFI_BOOTMENU_FILE_PATH_BUF_SIZE;
> > > +                     ret = efi_file_read_int(f, &len, buf);
> > > +                     if (ret != EFI_SUCCESS || len == 0)
> > > +                             break;
> > > +
> > > +                     count++;
> > > +             }
> > > +
> > > +             menu_item = calloc(count + 1, sizeof(struct efi_bootmenu_item));
> > > +             if (!menu_item) {
> > > +                     efi_file_close_int(f);
> > > +                     ret = EFI_OUT_OF_RESOURCES;
> > > +                     goto out;
> > > +             }
> > > +
> > > +             /* read directory and construct menu structure */
> > > +             efi_file_setpos_int(f, 0);
> > > +             iter = menu_item;
> > > +             for (i = 0; i < count; i++) {
> > > +                     u16 *name;
> > > +                     int name_len;
> > > +                     struct efi_bootmenu_file_entry_data *info;
> > > +
> > > +                     len = sizeof(struct efi_file_info) + EFI_BOOTMENU_FILE_PATH_BUF_SIZE;
> > > +                     ret = efi_file_read_int(f, &len, buf);
> > > +                     if (ret != EFI_SUCCESS || len == 0)
> > > +                             goto err;
> > > +
> > > +                     info = calloc(1, sizeof(struct efi_bootmenu_file_entry_data));
> > > +                     if (!info) {
> > > +                             ret = EFI_OUT_OF_RESOURCES;
> > > +                             goto err;
> > > +                     }
> > > +
> > > +                     if (buf->attribute & EFI_FILE_DIRECTORY) {
> > > +                             /* append u'/' at the end of directory name */
> > > +                             name_len = u16_strsize(buf->file_name) + sizeof(u16);
> > > +                             name = calloc(1, name_len);
> > > +                             if (!name) {
> > > +                                     ret = EFI_OUT_OF_RESOURCES;
> > > +                                     goto err;
> > > +                             }
> > > +                             u16_strcpy(name, buf->file_name);
> > > +                             name[u16_strlen(buf->file_name)] = u'/';
> > > +
> > > +                             info->is_directory = true;
> > > +                     } else {
> > > +                             name_len = u16_strsize(buf->file_name);
> > > +                             name = calloc(1, name_len);
> > > +                             if (!name) {
> > > +                                     ret = EFI_OUT_OF_RESOURCES;
> > > +                                     goto err;
> > > +                             }
> > > +                             u16_strcpy(name, buf->file_name);
> > > +                     }
> > > +
> > > +                     info->file_name = u16_strdup(buf->file_name);
> > > +                     info->bo = bo;
> > > +                     iter->title = name;
> > > +                     iter->func = efi_bootmenu_file_selected;
> > > +                     iter->data = info;
> > > +                     iter++;
> > > +             }
> > > +
> > > +             /* add "Quit" entry */
> > > +             iter->title = u"Quit";
> > > +             iter->func = NULL;
> > > +             iter->data = NULL;
> > > +             count += 1;
> > > +
> > > +             ret = efi_bootmenu_process_common(menu_item, count, -1);
> > > +err:
> > > +             efi_file_close_int(f);
> > > +             iter = menu_item;
> > > +             for (i = 0; i < count - 1; i++, iter++) {
> > > +                     free(((struct efi_bootmenu_file_entry_data *)(iter->data))->file_name);
> > > +                     free(iter->title);
> > > +                     free(iter->data);
> > > +             }
> > > +
> > > +             free(menu_item);
> > > +
> > > +             if (ret != EFI_SUCCESS)
> > > +                     break;
> > > +     }
> > > +
> > > +out:
> > > +     free(buf);
> > > +     return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_boot_add_enter_name(struct efi_bootmenu_boot_option *bo)
> > > +{
> > > +     efi_status_t ret;
> > > +
> > > +     printf(ANSI_CURSOR_POSITION, 2, 1);
> > > +     puts("  *** U-Boot EFI Boot Manager Menu ***");
> > > +     printf(ANSI_CURSOR_POSITION, 4, 1);
> > > +     puts("  enter name:");
> > > +
> > > +     printf(ANSI_CURSOR_POSITION, 8, 1);
> > > +     puts("  ENTER to complete, ESC/CTRL+C to quit");
> > > +
> > > +     ret = efi_console_get_u16_string(cin, cout, bo->boot_name,
> > > +                                      EFI_BOOTMENU_BOOT_NAME_MAX, NULL, 4, 15);
> > > +
> > > +     return ret;
> > > +}
> >
> > I prefer to see a menu like:
> >   BOOTxxxx
> >   title:
> >   file:
> >   option(arguments):
> >   set:
> >   cancel:
> > rather than being prompted one by one.
> >
> > Then users can select any of items in arbitrary order to define them
> > or to quit the menu.
> >
> > This way, the same menu can be re-used to *modify* an existing entry.
>
> I agree, I will implement like you suggested.
>
> >
> > > +
> > > +static efi_status_t efi_bootmenu_select_file_handler(struct efi_bootmenu_boot_option *bo)
> > > +{
> > > +     efi_status_t ret;
> > > +     struct efi_file_handle *root;
> > > +
> > > +     bo->file_selected = false;
> > > +
> > > +     while (!bo->file_selected) {
> > > +             bo->current_volume = NULL;
> > > +             memset(bo->current_path, 0, EFI_BOOTMENU_FILE_PATH_BUF_SIZE);
> > > +
> > > +             ret = efi_bootmenu_select_volume(bo);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     return ret;
> > > +
> > > +             if (!bo->current_volume)
> > > +                     return EFI_INVALID_PARAMETER;
> > > +
> > > +             ret = efi_open_volume_int(bo->current_volume, &root);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     return ret;
> > > +
> > > +             ret = efi_bootmenu_select_file(bo, root);
> > > +             if (ret != EFI_SUCCESS)
> > > +                     return ret;
> > > +     }
> > > +
> > > +     ret = efi_bootmenu_boot_add_enter_name(bo);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf,
> > > +                                             efi_uintn_t buf_size, u32 *index)
> > > +{
> > > +     u32 i;
> > > +     efi_status_t ret;
> > > +     efi_uintn_t size;
> > > +
> > > +     if (buf_size < u16_strsize(u"Boot####"))
> > > +             return EFI_BUFFER_TOO_SMALL;
> > > +
> > > +     for (i = 0; i <= 0xFFFF; i++) {
> > > +             size = 0;
> > > +             efi_create_indexed_name(buf, buf_size, "Boot", i);
> > > +             ret = efi_get_variable_int(buf, &efi_global_variable_guid,
> > > +                                        NULL, &size, NULL, NULL);
> > > +             if (ret == EFI_BUFFER_TOO_SMALL)
> > > +                     continue;
> > > +             else
> > > +                     break;
> > > +     }
> > > +
> > > +     if (i > 0xFFFF)
> > > +             return EFI_OUT_OF_RESOURCES;
> > > +
> > > +     *index = i;
> > > +
> > > +     return EFI_SUCCESS;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_set_boot_option(u16 *var_name, struct efi_device_path *dp,
> > > +                                              u16 *label, char *optional_data)
> > > +{
> > > +     void *p = NULL;
> > > +     efi_status_t ret;
> > > +     efi_uintn_t size;
> > > +     struct efi_load_option lo;
> > > +
> > > +     lo.file_path = dp;
> > > +     lo.file_path_length = efi_dp_size(dp) + sizeof(END);
> > > +     lo.attributes = LOAD_OPTION_ACTIVE;
> > > +     lo.optional_data = optional_data;
> > > +     lo.label = label;
> > > +
> > > +     size = efi_serialize_load_option(&lo, (u8 **)&p);
> > > +     if (!size)
> > > +             return EFI_INVALID_PARAMETER;
> > > +
> > > +     ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
> > > +                                EFI_VARIABLE_NON_VOLATILE |
> > > +                                EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > +                                EFI_VARIABLE_RUNTIME_ACCESS,
> > > +                                size, p, false);
> > > +     free(p);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +efi_status_t efi_bootmenu_append_bootorder(u16 index)
> > > +{
> > > +     u16 *bootorder;
> > > +     efi_status_t ret;
> > > +     u16 *new_bootorder = NULL;
> > > +     efi_uintn_t last, size, new_size;
> > > +
> > > +     /* append new boot option */
> > > +     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> > > +     last = size / sizeof(u16);
> > > +     new_size = size + sizeof(u16);
> > > +     new_bootorder = calloc(1, new_size);
> > > +     if (!new_bootorder) {
> > > +             ret = EFI_OUT_OF_RESOURCES;
> > > +             goto out;
> > > +     }
> > > +     memcpy(new_bootorder, bootorder, size);
> > > +     new_bootorder[last] = index;
> > > +
> > > +     ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
> > > +                                EFI_VARIABLE_NON_VOLATILE |
> > > +                                EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > +                                EFI_VARIABLE_RUNTIME_ACCESS,
> > > +                                new_size, new_bootorder, false);
> > > +     if (ret != EFI_SUCCESS)
> > > +             goto out;
> > > +
> > > +out:
> > > +     free(bootorder);
> > > +     free(new_bootorder);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_process_add_boot_option(void *data, bool *exit)
> > > +{
> > > +     u32 index;
> > > +     u16 var_name[9];
> > > +     char *buf = NULL;
> > > +     efi_status_t ret;
> > > +     char *iter = NULL;
> > > +     struct list_head *pos, *n;
> > > +     efi_uintn_t dp_size, fp_size;
> > > +     struct efi_bootmenu_boot_option bo;
> > > +     struct efi_device_path_file_path *fp;
> > > +     struct efi_bootmenu_filepath_info *item;
> > > +
> > > +     ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name),
> > > +                                              &index);
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     bo.current_path = calloc(1, EFI_BOOTMENU_FILE_PATH_BUF_SIZE);
> > > +     if (!bo.current_path)
> > > +             goto out;
> > > +
> > > +     bo.boot_name = calloc(1, EFI_BOOTMENU_BOOT_NAME_MAX * sizeof(u16));
> > > +     if (!bo.boot_name)
> > > +             goto out;
> > > +
> > > +     INIT_LIST_HEAD(&bo.filepath_list);
> > > +
> > > +     ret = efi_bootmenu_select_file_handler(&bo);
> > > +     if (ret != EFI_SUCCESS)
> > > +             goto out;
> > > +
> > > +     dp_size = efi_dp_size(bo.dp_volume);
> > > +     fp_size = sizeof(struct efi_device_path) +
> > > +               ((u16_strlen(bo.current_path) + 1) * sizeof(u16));
> > > +     buf = calloc(1, dp_size + fp_size + sizeof(END));
> > > +     if (!buf)
> > > +             goto out;
> > > +
> > > +     iter = buf;
> > > +     memcpy(iter, bo.dp_volume, dp_size);
> > > +     iter += dp_size;
> > > +
> > > +     fp = (struct efi_device_path_file_path *)iter;
> > > +     fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> > > +     fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
> > > +     fp->dp.length = (u16)fp_size;
> > > +     u16_strcpy(fp->str, bo.current_path);
> > > +     iter += fp_size;
> > > +     *((struct efi_device_path *)iter) = END;
> > > +
> > > +     ret = efi_bootmenu_set_boot_option(var_name, (struct efi_device_path *)buf,
> > > +                                        bo.boot_name, NULL);
> > > +     if (ret != EFI_SUCCESS)
> > > +             goto out;
> > > +
> > > +     efi_bootmenu_append_bootorder((u16)index);
> > > +     if (ret != EFI_SUCCESS)
> > > +             goto out;
> >
> > This if statement doesn't make sense.
>
> OK.
>
> >
> > > +
> > > +out:
> > > +     free(buf);
> > > +     free(bo.boot_name);
> > > +     free(bo.current_path);
> > > +
> > > +     list_for_each_safe(pos, n, &bo.filepath_list) {
> > > +             item = list_entry(pos, struct efi_bootmenu_filepath_info, list);
> > > +             list_del(&item->list);
> > > +             free(item->name);
> > > +             free(item);
> > > +     }
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static efi_status_t efi_bootmenu_init(void)
> > > +{
> > > +     efi_status_t ret;
> > > +     struct efi_handler *handler;
> > > +
> > > +     ret = efi_search_protocol(efi_root, &efi_guid_text_input_protocol, &handler);
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     ret = efi_protocol_open(handler, (void **)&cin, efi_root, NULL,
> > > +                             EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     ret = efi_search_protocol(efi_root, &efi_guid_text_output_protocol, &handler);
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     ret = efi_protocol_open(handler, (void **)&cout, efi_root, NULL,
> > > +                             EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static const struct efi_bootmenu_item maintenance_menu_items[] = {
> > > +     {u"Add Boot Option", efi_bootmenu_process_add_boot_option},
> > > +     {u"Quit", NULL},
> > > +};
> > > +
> > > +efi_status_t efi_bootmenu_show_maintenance_menu(void)
> > > +{
> > > +     efi_status_t ret;
> > > +
> > > +     ret = efi_bootmenu_init();
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     return efi_bootmenu_process_common(maintenance_menu_items,
> > > +                                       ARRAY_SIZE(maintenance_menu_items),
> > > +                                       -1);
> > > +}
> > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > > index 4da64b5d29..1233418e77 100644
> > > --- a/lib/efi_loader/efi_boottime.c
> > > +++ b/lib/efi_loader/efi_boottime.c
> > > @@ -2453,6 +2453,35 @@ static efi_status_t EFIAPI efi_protocols_per_handle(
> > >       return EFI_EXIT(EFI_SUCCESS);
> > >  }
> > >
> > > +efi_status_t efi_locate_handle_buffer_int(enum efi_locate_search_type search_type,
> > > +                                       const efi_guid_t *protocol, void *search_key,
> > > +                                       efi_uintn_t *no_handles, efi_handle_t **buffer)
> > > +{
> > > +     efi_status_t r;
> > > +     efi_uintn_t buffer_size = 0;
> > > +
> > > +     if (!no_handles || !buffer) {
> > > +             r = EFI_INVALID_PARAMETER;
> > > +             goto out;
> > > +     }
> > > +     *no_handles = 0;
> > > +     *buffer = NULL;
> > > +     r = efi_locate_handle(search_type, protocol, search_key, &buffer_size,
> > > +                           *buffer);
> > > +     if (r != EFI_BUFFER_TOO_SMALL)
> > > +             goto out;
> > > +     r = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size,
> > > +                           (void **)buffer);
> > > +     if (r != EFI_SUCCESS)
> > > +             goto out;
> > > +     r = efi_locate_handle(search_type, protocol, search_key, &buffer_size,
> > > +                           *buffer);
> > > +     if (r == EFI_SUCCESS)
> > > +             *no_handles = buffer_size / sizeof(efi_handle_t);
> > > +out:
> > > +     return r;
> > > +}
> > > +
> > >  /**
> > >   * efi_locate_handle_buffer() - locate handles implementing a protocol
> > >   * @search_type: selection criterion
> > > @@ -2474,30 +2503,13 @@ efi_status_t EFIAPI efi_locate_handle_buffer(
> > >                       efi_uintn_t *no_handles, efi_handle_t **buffer)
> > >  {
> > >       efi_status_t r;
> > > -     efi_uintn_t buffer_size = 0;
> > >
> > >       EFI_ENTRY("%d, %pUs, %p, %p, %p", search_type, protocol, search_key,
> > >                 no_handles, buffer);
> > >
> > > -     if (!no_handles || !buffer) {
> > > -             r = EFI_INVALID_PARAMETER;
> > > -             goto out;
> > > -     }
> > > -     *no_handles = 0;
> > > -     *buffer = NULL;
> > > -     r = efi_locate_handle(search_type, protocol, search_key, &buffer_size,
> > > -                           *buffer);
> > > -     if (r != EFI_BUFFER_TOO_SMALL)
> > > -             goto out;
> > > -     r = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size,
> > > -                           (void **)buffer);
> > > -     if (r != EFI_SUCCESS)
> > > -             goto out;
> > > -     r = efi_locate_handle(search_type, protocol, search_key, &buffer_size,
> > > -                           *buffer);
> > > -     if (r == EFI_SUCCESS)
> > > -             *no_handles = buffer_size / sizeof(efi_handle_t);
> > > -out:
> > > +     r = efi_locate_handle_buffer_int(search_type, protocol, search_key,
> > > +                                      no_handles, buffer);
> > > +
> > >       return EFI_EXIT(r);
> > >  }
> > >
> > > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> > > index 60a3fc85ac..cf9622ed1a 100644
> > > --- a/lib/efi_loader/efi_console.c
> > > +++ b/lib/efi_loader/efi_console.c
> > > @@ -5,6 +5,7 @@
> > >   *  Copyright (c) 2016 Alexander Graf
> > >   */
> > >
> > > +#include <ansi.h>
> > >  #include <common.h>
> > >  #include <charset.h>
> > >  #include <malloc.h>
> > > @@ -1312,3 +1313,83 @@ out_of_memory:
> > >       printf("ERROR: Out of memory\n");
> > >       return r;
> > >  }
> > > +
> > > +/**
> > > + * efi_console_get_u16_string() - get user input string
> > > + *
> > > + * @cin:             protocol interface to EFI_SIMPLE_TEXT_INPUT_PROTOCOL
> > > + * @cout:            protocol interface to EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL
> > > + * @buf:             buffer to store user input string in UTF16
> > > + * @size:            buffer size including NULL terminator
> > > + * @filter_func:     callback to filter user input
> > > + * @row:             row number to locate user input form
> > > + * @col:             column number to locate user input form
> > > + * Return:           status code
> > > + */
> > > +efi_status_t efi_console_get_u16_string(struct efi_simple_text_input_protocol *cin,
> > > +                                     struct efi_simple_text_output_protocol *cout,
> > > +                                     u16 *buf, efi_uintn_t size,
> > > +                                     efi_console_filter_func filter_func,
> > > +                                     int row, int col)
> > > +{
> > > +     efi_status_t ret;
> > > +     efi_uintn_t len = 0;
> > > +     struct efi_input_key key;
> > > +
> > > +     printf(ANSI_CURSOR_POSITION, row, col);
> > > +     puts(ANSI_CLEAR_LINE_TO_END);
> > > +     puts(ANSI_CURSOR_SHOW);
> > > +
> > > +     ret = EFI_CALL(cin->reset(cin, false));
> > > +     if (ret != EFI_SUCCESS)
> > > +             return ret;
> > > +
> > > +     for (;;) {
> > > +             do {
> > > +                     ret = EFI_CALL(cin->read_key_stroke(cin, &key));
> > > +                     mdelay(10);
> > > +             } while (ret == EFI_NOT_READY);
> > > +
> > > +             if (key.unicode_char == u'\b') {
> > > +                     if (len > 0)
> > > +                             buf[--len] = u'\0';
> > > +
> > > +                     printf(ANSI_CURSOR_POSITION, row, col);
> > > +                     ret = EFI_CALL(cout->output_string(cout, buf));
> > > +                     if (ret != EFI_SUCCESS)
> > > +                             return ret;
> > > +
> > > +                     puts(ANSI_CLEAR_LINE_TO_END);
> > > +                     continue;
> > > +             } else if (key.unicode_char == u'\r') {
> > > +                     if (len == 0) /* no user input */
> > > +                             continue;
> > > +
> > > +                     buf[len] = u'\0';
> > > +                     return EFI_SUCCESS;
> > > +             } else if (key.unicode_char == 0x3 || key.scan_code == 23) {
> > > +                     return EFI_ABORTED;
> > > +             } else if (key.unicode_char < 0x20) {
> > > +                     /* ignore control codes other than Ctrl+C, '\r' and '\b' */
> > > +                     continue;
> > > +             } else if (key.scan_code != 0) {
> > > +                     /* only accept single ESC press for cancel */
> > > +                     continue;
> > > +             }
> > > +
> > > +             if (filter_func) {
> > > +                     if (filter_func(&key) != EFI_SUCCESS)
> > > +                             continue;
> > > +             }
> > > +
> > > +             if (len >= (size - 1))
> > > +                     continue;
> > > +
> > > +             buf[len] = key.unicode_char;
> > > +             len++;
> > > +             printf(ANSI_CURSOR_POSITION, row, col);
> > > +             ret = EFI_CALL(cout->output_string(cout, buf));
> > > +             if (ret != EFI_SUCCESS)
> > > +                     return ret;
> > > +     }
> > > +}
> > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > > index f5b462fb16..01576c8ed2 100644
> > > --- a/lib/efi_loader/efi_disk.c
> > > +++ b/lib/efi_loader/efi_disk.c
> > > @@ -751,3 +751,14 @@ efi_status_t efi_disk_init(void)
> > >
> > >       return EFI_SUCCESS;
> > >  }
> > > +
> > > +efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size)
> > > +{
> > > +     struct efi_disk_obj *diskobj;
> > > +
> > > +     diskobj = container_of(this, struct efi_disk_obj, ops);
> > > +
> > > +     snprintf(buf, size, "%s %d:%d", diskobj->ifname, diskobj->dev_index, diskobj->part);
> > > +
> > > +     return EFI_SUCCESS;
> > > +}
> >
> > Please note that not all the BLOCK_IO interfaces have corresponding diskobj's,
> > in particular, efi_driver/efi_block_device.c.

When I test with efi_driver/efi_block_device.c,
I could get "efiloader 0:1". So it seems no problem.

Thanks,
Masahisa Kojima

>
> I will check the case that bootmenu starts EFI application
> using efi_driver/efi_block_device.c, then returns to bootmenu.
>
> Thanks,
> Masahisa Kojima
>
> >
> > -Takahiro Akashi
> >
> > > diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> > > index 7a7077e6d0..c96a7f7ca3 100644
> > > --- a/lib/efi_loader/efi_file.c
> > > +++ b/lib/efi_loader/efi_file.c
> > > @@ -246,10 +246,10 @@ error:
> > >       return NULL;
> > >  }
> > >
> > > -static efi_status_t efi_file_open_int(struct efi_file_handle *this,
> > > -                                   struct efi_file_handle **new_handle,
> > > -                                   u16 *file_name, u64 open_mode,
> > > -                                   u64 attributes)
> > > +efi_status_t efi_file_open_int(struct efi_file_handle *this,
> > > +                            struct efi_file_handle **new_handle,
> > > +                            u16 *file_name, u64 open_mode,
> > > +                            u64 attributes)
> > >  {
> > >       struct file_handle *fh = to_fh(this);
> > >       efi_status_t ret;
> > > @@ -369,11 +369,17 @@ static efi_status_t file_close(struct file_handle *fh)
> > >       return EFI_SUCCESS;
> > >  }
> > >
> > > -static efi_status_t EFIAPI efi_file_close(struct efi_file_handle *file)
> > > +efi_status_t efi_file_close_int(struct efi_file_handle *file)
> > >  {
> > >       struct file_handle *fh = to_fh(file);
> > > +
> > > +     return file_close(fh);
> > > +}
> > > +
> > > +static efi_status_t EFIAPI efi_file_close(struct efi_file_handle *file)
> > > +{
> > >       EFI_ENTRY("%p", file);
> > > -     return EFI_EXIT(file_close(fh));
> > > +     return EFI_EXIT(efi_file_close_int(file));
> > >  }
> > >
> > >  static efi_status_t EFIAPI efi_file_delete(struct efi_file_handle *file)
> > > @@ -562,8 +568,8 @@ static efi_status_t dir_read(struct file_handle *fh, u64 *buffer_size,
> > >       return EFI_SUCCESS;
> > >  }
> > >
> > > -static efi_status_t efi_file_read_int(struct efi_file_handle *this,
> > > -                                   efi_uintn_t *buffer_size, void *buffer)
> > > +efi_status_t efi_file_read_int(struct efi_file_handle *this,
> > > +                            efi_uintn_t *buffer_size, void *buffer)
> > >  {
> > >       struct file_handle *fh = to_fh(this);
> > >       efi_status_t ret = EFI_SUCCESS;
> > > @@ -773,24 +779,11 @@ out:
> > >       return EFI_EXIT(ret);
> > >  }
> > >
> > > -/**
> > > - * efi_file_setpos() - set current position in file
> > > - *
> > > - * This function implements the SetPosition service of the EFI file protocol.
> > > - * See the UEFI spec for details.
> > > - *
> > > - * @file:    file handle
> > > - * @pos:     new file position
> > > - * Return:   status code
> > > - */
> > > -static efi_status_t EFIAPI efi_file_setpos(struct efi_file_handle *file,
> > > -                                        u64 pos)
> > > +efi_status_t efi_file_setpos_int(struct efi_file_handle *file, u64 pos)
> > >  {
> > >       struct file_handle *fh = to_fh(file);
> > >       efi_status_t ret = EFI_SUCCESS;
> > >
> > > -     EFI_ENTRY("%p, %llu", file, pos);
> > > -
> > >       if (fh->isdir) {
> > >               if (pos != 0) {
> > >                       ret = EFI_UNSUPPORTED;
> > > @@ -812,6 +805,28 @@ static efi_status_t EFIAPI efi_file_setpos(struct efi_file_handle *file,
> > >       fh->offset = pos;
> > >
> > >  error:
> > > +     return ret;
> > > +}
> > > +
> > > +/**
> > > + * efi_file_setpos() - set current position in file
> > > + *
> > > + * This function implements the SetPosition service of the EFI file protocol.
> > > + * See the UEFI spec for details.
> > > + *
> > > + * @file:    file handle
> > > + * @pos:     new file position
> > > + * Return:   status code
> > > + */
> > > +static efi_status_t EFIAPI efi_file_setpos(struct efi_file_handle *file,
> > > +                                        u64 pos)
> > > +{
> > > +     efi_status_t ret = EFI_SUCCESS;
> > > +
> > > +     EFI_ENTRY("%p, %llu", file, pos);
> > > +
> > > +     ret = efi_file_setpos_int(file, pos);
> > > +
> > >       return EFI_EXIT(ret);
> > >  }
> > >
> > > @@ -1138,17 +1153,23 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp)
> > >       return f;
> > >  }
> > >
> > > +efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol *this,
> > > +                              struct efi_file_handle **root)
> > > +{
> > > +     struct file_system *fs = to_fs(this);
> > > +
> > > +     *root = file_open(fs, NULL, NULL, 0, 0);
> > > +
> > > +     return EFI_SUCCESS;
> > > +}
> > > +
> > >  static efi_status_t EFIAPI
> > >  efi_open_volume(struct efi_simple_file_system_protocol *this,
> > >               struct efi_file_handle **root)
> > >  {
> > > -     struct file_system *fs = to_fs(this);
> > > -
> > >       EFI_ENTRY("%p, %p", this, root);
> > >
> > > -     *root = file_open(fs, NULL, NULL, 0, 0);
> > > -
> > > -     return EFI_EXIT(EFI_SUCCESS);
> > > +     return EFI_EXIT(efi_open_volume_int(this, root));
> > >  }
> > >
> > >  struct efi_simple_file_system_protocol *
> > > --
> > > 2.17.1
> > >


More information about the U-Boot mailing list