On Sun, 10 Jul 2022 at 12:10, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 6/19/22 06:55, Masahisa Kojima wrote:

...

This commit exposes the END device path node.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de

...

---

No change in v8

Newly created in v7

include/efi_loader.h | 3 +++ lib/efi_loader/efi_device_path.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index f6651e2c60..c6df29993c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -798,6 +798,9 @@ ssize_t efi_dp_check_length(const struct efi_device_path *dp, (((_dp)->type == DEVICE_PATH_TYPE_##_type) && \ ((_dp)->sub_type == DEVICE_PATH_SUB_TYPE_##_subtype))

+/* template END node: */ +extern const struct efi_device_path END;

• /* Indicate supported runtime services */ efi_status_t efi_init_runtime_supported(void);

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 50a988c561..4798cec622 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -30,7 +30,7 @@ const efi_guid_t efi_guid_virtio_dev = U_BOOT_VIRTIO_DEV_GUID; #endif

/* template END node: */ -static const struct efi_device_path END = { +const struct efi_device_path END = { .type = DEVICE_PATH_TYPE_END, .sub_type = DEVICE_PATH_SUB_TYPE_END, .length = sizeof(END),

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

On 6/19/22 06:56, Masahisa Kojima wrote:

This commit add the "eficonfig" command. The "eficonfig" command implements the menu-driven UEFI boot option maintenance feature. This commit implements the addition of new boot option. User can select the block device volume having efi_simple_file_system_protocol and select the file corresponding to the Boot#### variable. User can also enter the description and optional_data of the BOOT#### variable in utf8.

This commit adds "include/efi_config.h", it contains the common definition to be used from other menus such as UEFI Secure Boot key management.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

Changes in v8:

• command name is change from "efimenu" to "eficonfig"
• function and struct prefixes is changed to "eficonfig"
• fix menu header string

Changes in v7:

• add "efimenu" command and uefi variable maintenance code moved into cmd/efimenu.c

• create include/efimenu.h to define the common definition for the other menu such as UEFI Secure Boot key management

• update boot option edit UI, user can select description, file, and optional_data to edit in the same menu like following.

  ** Edit Boot Option **

  Description: debian
  File: virtio 0:1/EFI\debian\grubaa64.efi
  Optional Data: test
  Save
  Quit
  
  

• remove exit parameter from efimenu_process_common()

• menu title type is changed from u16 to char

• efimenu_process_common() add menu title string

• reduce printf/puts function call for displaying the menu

• efi_console_get_u16_string() accept 0 length to allow optional_data is empty

• efi_console_get_u16_string() the "size" parameter name is changes to "count"

• efimenu is now designed to maintain the UEFI variables, remove autoboot related code

• remove one empty line before "Quit" entry

• efimenu_init() processes only the first time

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
• 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

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/Kconfig | 7 + cmd/Makefile | 1 + cmd/eficonfig.c | 1270 +++++++++++++++++++++++++++++++++ include/efi_config.h | 91 +++ include/efi_loader.h | 40 ++ lib/efi_loader/efi_boottime.c | 52 +- lib/efi_loader/efi_console.c | 78 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- 9 files changed, 1578 insertions(+), 47 deletions(-) create mode 100644 cmd/eficonfig.c create mode 100644 include/efi_config.h

diff --git a/cmd/Kconfig b/cmd/Kconfig index 09193b61b9..bb7f1d0463 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1870,6 +1870,13 @@ config CMD_EFIDEBUG particularly for managing boot parameters as well as examining various EFI status for debugging.

+config CMD_EFICONFIG

• bool "eficonfig - provide menu-driven uefi variables maintenance interface"
• depends on CMD_BOOTEFI_BOOTMGR
• help

•  Enable the 'eficonfig' command which provides the menu-driven UEFI
  

•  variable maintenance interface.
  

• config CMD_EXCEPTION bool "exception - raise exception" depends on ARM || RISCV || SANDBOX || X86

diff --git a/cmd/Makefile b/cmd/Makefile index 5e43a1e022..0afa687e94 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -63,6 +63,7 @@ obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI) += efi.o obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o +obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_CMD_EROFS) += erofs.o obj-$(CONFIG_HUSH_PARSER) += exit.o diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c new file mode 100644 index 0000000000..20747db115 --- /dev/null +++ b/cmd/eficonfig.c @@ -0,0 +1,1270 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Menu-driven UEFI Variable maintenance
• • Copyright (c) 2022 Masahisa Kojima, Linaro Limited
• */

+#include <ansi.h> +#include <common.h> +#include <charset.h> +#include <efi_loader.h> +#include <efi_config.h> +#include <efi_variable.h> +#include <log.h> +#include <malloc.h> +#include <menu.h> +#include <watchdog.h> +#include <asm/unaligned.h> +#include <linux/delay.h>

+static struct efi_simple_text_input_protocol *cin; +static struct efi_simple_text_output_protocol *cout;

+#define EFICONFIG_DESCRIPTION_MAX 32 +#define EFICONFIG_OPTIONAL_DATA_MAX 64 +#define EFICONFIG_EDIT_BOOT_OPTION_MENU_ENTRY 5

+#define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \

• EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \

•  0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde)
  

+const efi_guid_t efi_guid_bootmenu_auto_generated =

• EFICONFIG_AUTO_GENERATED_ENTRY_GUID;
  

+struct eficonfig_boot_selection_data {

• u16 bootorder_index;
• int *selected;

+};

+struct eficonfig_filepath_info {

• u16 *name;
• struct list_head list;

+};

+/**

• • struct eficonfig_boot_option - structure to be used for uefi boot option update
• • @file_info: user selected file info
• • @boot_index: index of the uefi BootOrder variable
• • @description: pointer to the description string
• • @optional_data: pointer to the optional_data
• • @edit_completed: flag indicates edit complete
• */

+struct eficonfig_boot_option {

• struct eficonfig_select_file_info file_info;
• unsigned int boot_index;
• u16 *description;
• u16 *optional_data;
• bool edit_completed;

+};

+struct eficonfig_volume_entry_data {

• struct eficonfig_select_file_info *file_info;
• struct efi_simple_file_system_protocol *v;
• struct efi_device_path *dp;

+};

+struct eficonfig_file_entry_data {

• struct eficonfig_select_file_info *file_info;
• bool is_directory;
• u16 *file_name;

+};

+/**

• • eficonfig_print_msg() - print message
• • display the message to the user, user proceeds the screen
• • with ENTER key press.
• • @items: pointer to the structure of each menu entry
• • @count: the number of menu entry
• • @menu_header: pointer to the menu header string
• • Return: status code
• */

+void eficonfig_print_msg(char *msg)

None of you patches adds this function to an include. So it should be static.

+{

• char c;
• puts(ANSI_CURSOR_HIDE);
• puts(ANSI_CLEAR_CONSOLE);
• printf(ANSI_CURSOR_POSITION, 3, 4);
• printf(msg);
• /* Flush input */
• while (tstc())

• getchar();
  

• printf("\n\n Press ENTER to continue");

We want to minimize the size of the U-Boot binary.

Isn't the following single function call good enough?

printf(ANSI_CURSOR_HIDE ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION msg "\n\n Press ENTER to continue", 3, 4);

• while (1) {

• while (!tstc()) {
  

• 	WATCHDOG_RESET();
  

If there is no user to hit a key, why should be stop the watchdog reset?

• 	mdelay(10);
  

There is no benefit of calling mdelay here.

• }
  

• c = getchar();
  

• if (c == '\r')
  

• 	break;
  

• }

+}

We should replace the whole loop by:

getchar();

Please, provide Sphinx style function descriptions for all functions.

+static void eficonfig_print_entry(void *data) +{

• struct eficonfig_entry *entry = data;
• int reverse = (entry->efi_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(menu header) + one empty line
  

• */
  

• printf(ANSI_CURSOR_POSITION " ", entry->num + 4, 1);
• if (reverse)

• puts(ANSI_COLOR_REVERSE);
  

• printf("%s", entry->title);
• if (reverse)

• puts(ANSI_COLOR_RESET);
  

+}

+static void eficonfig_display_statusline(struct menu *m) +{

• struct eficonfig_entry *entry;
• if (menu_default_choice(m, (void *)&entry) < 0)

• return;
  

• printf(ANSI_CURSOR_POSITION

•      "\n%s\n"
  

•       ANSI_CURSOR_POSITION
  

•       "\n"
  

•       "  Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit",
  

•       0, 1, entry->efi_menu->menu_header, entry->efi_menu->count + 5, 1);
  

+}

+static char *eficonfig_choice_entry(void *data) +{

• int i;
• int esc = 0;
• struct eficonfig_entry *iter;
• enum bootmenu_key key = KEY_NONE;
• struct efimenu *efi_menu = data;
• while (1) {

• bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
  

• switch (key) {
  

• case KEY_UP:
  

• 	if (efi_menu->active > 0)
  

• 		--efi_menu->active;
  

• 	/* no menu key selected, regenerate menu */
  

• 	return NULL;
  

• case KEY_DOWN:
  

• 	if (efi_menu->active < efi_menu->count - 1)
  

• 		++efi_menu->active;
  

• 	/* no menu key selected, regenerate menu */
  

• 	return NULL;
  

• case KEY_SELECT:
  

• 	iter = efi_menu->first;
  

• 	for (i = 0; i < efi_menu->active; ++i)
  

• 		iter = iter->next;
  

• 	return iter->key;
  

• case KEY_QUIT:
  

• 	/* Quit by choosing the last entry */
  

• 	iter = efi_menu->first;
  

• 	while (iter->next)
  

• 		iter = iter->next;
  

• 	return iter->key;
  

• default:
  

• 	break;
  

• }
  

• }
• /* never happens */
• debug("eficonfig: this should not happen");

This comment and message is wrong. key = KEY_NONE is an expected value for key.

• return NULL;

+}

+static void eficonfig_destroy(struct efimenu *efi_menu) +{

• struct eficonfig_entry *next;
• struct eficonfig_entry *iter = efi_menu->first;
• while (iter) {

• next = iter->next;
  

• free(iter);
  

• iter = next;
  

• }
• free(efi_menu->menu_header);
• free(efi_menu);

+}

+/**

• • eficonfig_process_quit() - callback function for "Quit" entry
• • @data: pointer to the data
• • Return: status code
• */

+efi_status_t eficonfig_process_quit(void *data) +{

• return EFI_ABORTED;

+}

+/**

• • eficonfig_process_common() - main handler for uefi menu

%s/uefi/UEFI/

• • 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
• • @menu_header: pointer to the menu header string
• • Return: status code
• */

+efi_status_t eficonfig_process_common(const struct eficonfig_item *items, int count,

• 		      char *menu_header)
  

+{

• u32 i;
• efi_status_t ret;
• struct menu *menu;
• void *choice = NULL;
• struct eficonfig_entry *entry;
• struct efimenu *efi_menu;
• struct eficonfig_entry *iter = NULL;
• if (count > EFICONFIG_ENTRY_NUM_MAX)

• return EFI_OUT_OF_RESOURCES;
  

• efi_menu = calloc(1, sizeof(struct efimenu));
• if (!efi_menu)

• return EFI_OUT_OF_RESOURCES;
  

• efi_menu->delay = -1;
• efi_menu->active = 0;
• efi_menu->first = NULL;
• if (menu_header) {

• efi_menu->menu_header = strdup(menu_header);
  

• if (!efi_menu->menu_header) {
  

• 	free(efi_menu);
  

• 	return EFI_OUT_OF_RESOURCES;
  

• }
  

• }
• for (i = 0; i < count; i++) {

• entry = calloc(1, sizeof(struct eficonfig_entry));
  

• if (!entry) {
  

• 	ret = EFI_LOAD_ERROR;
  

• 	goto out;
  

• }
  

• entry->num = i;
  

• entry->title = items->title;
  

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

• entry->efi_menu = efi_menu;
  

• entry->func = items->func;
  

• entry->data = items->data;
  

• entry->next = NULL;
  

• if (!iter)
  

• 	efi_menu->first = entry;
  

• else
  

• 	iter->next = entry;
  

• iter = entry;
  

• items++;
  

• }
• efi_menu->count = count;
• menu = menu_create(NULL, 0, 1, eficonfig_display_statusline,

• 	   eficonfig_print_entry, eficonfig_choice_entry,
  

• 	   efi_menu);
  

• if (!menu) {

• ret = EFI_INVALID_PARAMETER;
  

• goto out;
  

• }
• for (entry = efi_menu->first; entry; entry = entry->next) {

• if (!menu_item_add(menu, entry->key, entry)) {
  

• 	ret = EFI_INVALID_PARAMETER;
  

• 	goto out;
  

• }
  

• }
• menu_default_set(menu, efi_menu->first->key);
• puts(ANSI_CURSOR_HIDE);
• puts(ANSI_CLEAR_CONSOLE);
• printf(ANSI_CURSOR_POSITION, 1, 1);

Please, use a single function call.

• if (menu_get_choice(menu, &choice)) {

• entry = choice;
  

• if (entry->func)
  

• 	ret = entry->func(entry->data);
  

• }

+out:

• menu_destroy(menu);
• eficonfig_destroy(efi_menu);
• puts(ANSI_CLEAR_CONSOLE);
• printf(ANSI_CURSOR_POSITION, 1, 1);
• puts(ANSI_CURSOR_SHOW);

ditto

• return ret;

+}

+static efi_status_t eficonfig_volume_selected(void *data) +{

• struct eficonfig_volume_entry_data *info = data;
• if (info) {

• info->file_info->current_volume = info->v;
  

• info->file_info->dp_volume = info->dp;
  

• }
• return EFI_SUCCESS;

+}

+static efi_status_t eficonfig_file_selected(void *data) +{

• struct eficonfig_file_entry_data *info = data;
• 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 eficonfig_filepath_info *iter;
  

• struct list_head *pos, *n;
  

• int is_last;
  

• memset(info->file_info->current_path, 0, EFICONFIG_FILE_PATH_BUF_SIZE);
  

• list_for_each_safe(pos, n, &info->file_info->filepath_list) {
  

• 	iter = list_entry(pos, struct eficonfig_filepath_info, list);
  

• 	is_last = list_is_last(&iter->list, &info->file_info->filepath_list);
  

• 	if (is_last) {
  

• 		list_del(&iter->list);
  

• 		free(iter->name);
  

• 		free(iter);
  

• 		break;
  

• 	}
  

• 	u16_strlcat(info->file_info->current_path, iter->name,
  

• 		    EFICONFIG_FILE_PATH_MAX);
  

• 	u16_strlcat(info->file_info->current_path, u"\\",
  

• 		    EFICONFIG_FILE_PATH_MAX);
  

• }
  

• } else {

• size_t new_len;
  

• struct eficonfig_filepath_info *filepath;
  

• new_len = u16_strlen(info->file_info->current_path) +
  

• 		     u16_strlen(info->file_name);
  

• if (new_len >= EFICONFIG_FILE_PATH_MAX) {
  

• 	eficonfig_print_msg("File path is too long!");
  

• 	return EFI_INVALID_PARAMETER;
  

• }
  

• u16_strlcat(info->file_info->current_path, info->file_name,
  

• 	    EFICONFIG_FILE_PATH_MAX);
  

• filepath = calloc(1, sizeof(struct eficonfig_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->file_info->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 >= EFICONFIG_FILE_PATH_MAX) {
  

• 		eficonfig_print_msg("Directory path is too long!");
  

• 		return EFI_INVALID_PARAMETER;
  

• 	}
  

• 	u16_strlcat(info->file_info->current_path, u"\\",
  

• 		    EFICONFIG_FILE_PATH_MAX);
  

• } else {
  

• 	info->file_info->file_selected = true;
  

• }
  

• }
• return EFI_SUCCESS;

+}

+static efi_status_t eficonfig_select_volume(struct eficonfig_select_file_info *file_info) +{

• 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 eficonfig_item *menu_item, *iter;
• struct efi_simple_file_system_protocol *v;
• 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) {

• eficonfig_print_msg("No block device found!");
  

• return ret;
  

• }
• menu_item = calloc(count + 1, sizeof(struct eficonfig_item));
• if (!menu_item) {

• efi_free_pool(volume_handles);
  

• return EFI_OUT_OF_RESOURCES;
  

• }
• iter = menu_item;
• for (i = 0; i < count; i++) {

• char *devname;
  

• struct efi_block_io *block_io;
  

• struct eficonfig_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 eficonfig_volume_entry_data));
  

• if (!info) {
  

• 	ret = EFI_OUT_OF_RESOURCES;
  

• 	goto out;
  

• }
  

• devname = calloc(1, BOOTMENU_DEVICE_NAME_MAX);
  

• if (!devname) {
  

• 	free(info);
  

• 	ret = EFI_OUT_OF_RESOURCES;
  

• 	goto out;
  

• }
  

• efi_disk_get_device_name(block_io, devname, BOOTMENU_DEVICE_NAME_MAX);
  

• info->v = v;
  

• info->dp = device_path;
  

• info->file_info = file_info;
  

• iter->title = devname;
  

• iter->func = eficonfig_volume_selected;
  

• iter->data = info;
  

• iter++;
  

• }
• iter->title = strdup("Quit");
• iter->func = eficonfig_process_quit;
• iter->data = NULL;
• count += 1;
• ret = eficonfig_process_common(menu_item, count, " ** Select Volume **");

+out:

• iter = menu_item;
• for (i = 0; i < count; i++, iter++) {

• free(iter->data);
  

• free(iter->title);
  

• }
• free(menu_item);
• efi_free_pool(volume_handles);
• return ret;

+}

+static efi_status_t eficonfig_select_file(struct eficonfig_select_file_info *file_info,

• 			  struct efi_file_handle *root)
  

+{

• u32 i;
• u32 count = 0;
• efi_uintn_t len;
• efi_status_t ret;
• struct efi_file_handle *f;
• struct efi_file_info *buf;
• struct eficonfig_item *menu_item, *iter;
• buf = calloc(1, sizeof(struct efi_file_info) + EFICONFIG_FILE_PATH_BUF_SIZE);
• if (!buf)

• return EFI_OUT_OF_RESOURCES;
  

• while (!file_info->file_selected) {

• count = 0;
  

• ret = efi_file_open_int(root, &f, file_info->current_path, EFI_FILE_MODE_READ, 0);
  

• if (ret != EFI_SUCCESS) {
  

• 	/* TODO: need to fileter out non-FAT partition? */
  

%s/fileter/filter/

• 	eficonfig_print_msg("Reading volume failed! Please make sure the selected\n"
  

• 			    "   volume is FAT12/FAT16/FAT32 partition.");
  

Who cares if the file is on FAT, ext4, or any other file system else if U-Boot can read it?

• 	ret = EFI_ABORTED;
  

• 	goto out;
  

• }
  

• /* calculate directory information total count */
  

/* Count the number of directory entries */

• for (;;) {
  

• 	len = sizeof(struct efi_file_info) + EFICONFIG_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 eficonfig_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++) {
  

• 	char *name, *p;
  

• 	int name_len;
  

• 	struct eficonfig_file_entry_data *info;
  

• 	len = sizeof(struct efi_file_info) + EFICONFIG_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 eficonfig_file_entry_data));
  

• 	if (!info) {
  

• 		ret = EFI_OUT_OF_RESOURCES;
  

• 		goto err;
  

• 	}
  

• 	if (buf->attribute & EFI_FILE_DIRECTORY) {
  

Should we filter out '.' and '..'?

• 		/* append u'/' at the end of directory name */
  

• 		name_len = utf16_utf8_strlen(buf->file_name) + 2;
  

• 		name = calloc(1, name_len);
  

• 		if (!name) {
  

• 			ret = EFI_OUT_OF_RESOURCES;
  

• 			goto err;
  

• 		}
  

• 		p = name;
  

• 		utf16_utf8_strcpy(&p, buf->file_name);
  

• 		name[u16_strlen(buf->file_name)] = u'/';
  

• 		info->is_directory = true;
  

• 	} else {
  

• 		name_len = utf16_utf8_strlen(buf->file_name) + 1;
  

• 		name = calloc(1, name_len);
  

• 		if (!name) {
  

• 			ret = EFI_OUT_OF_RESOURCES;
  

• 			goto err;
  

• 		}
  

• 		p = name;
  

• 		utf16_utf8_strcpy(&p, buf->file_name);
  

• 	}
  

• 	info->file_name = u16_strdup(buf->file_name);
  

• 	info->file_info = file_info;
  

• 	iter->title = name;
  

• 	iter->func = eficonfig_file_selected;
  

• 	iter->data = info;
  

• 	iter++;
  

• }
  

• /* add "Quit" entry */
  

• iter->title = "Quit";
  

• iter->func = eficonfig_process_quit;
  

• iter->data = NULL;
  

• count += 1;
  

We want to reduce code size. Start the function with

unsigned int count = 1;

• ret = eficonfig_process_common(menu_item, count, "  ** Select File **");
  

+err:

• efi_file_close_int(f);
  

• iter = menu_item;
  

• for (i = 0; i < count - 1; i++, iter++) {
  

• 	free(((struct eficonfig_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 eficonfig_boot_add_enter_description(void *data) +{

• u16 *tmp;
• efi_status_t ret;
• struct eficonfig_boot_option *bo = data;
• printf(ANSI_CLEAR_CONSOLE

•       ANSI_CURSOR_POSITION
  

•       "\n  ** Edit Description **\n"
  

•       "\n"
  

•       "  enter description: "
  

•       ANSI_CURSOR_POSITION
  

•       "  Press ENTER to complete, ESC/CTRL+C to quit",
  

•       0, 1, 8, 1);
  

• tmp = calloc(1, EFICONFIG_DESCRIPTION_MAX * sizeof(u16));
• if (!tmp)

• return EFI_OUT_OF_RESOURCES;
  

• ret = efi_console_get_u16_string(cin, cout, tmp,

• 			 EFICONFIG_DESCRIPTION_MAX, NULL, 4, 22);
  

• if (ret == EFI_SUCCESS)

• u16_strcpy(bo->description, tmp);
  

• free(tmp);
• /* to stay the parent menu */
• ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
• return ret;

+}

+static efi_status_t eficonfig_boot_add_optional_data(void *data) +{

• u16 *tmp;
• efi_status_t ret;
• struct eficonfig_boot_option *bo = data;
• printf(ANSI_CLEAR_CONSOLE

•       ANSI_CURSOR_POSITION
  

•       "\n  ** Edit Optional Data **\n"
  

•       "\n"
  

•       "  enter optional data:"
  

•       ANSI_CURSOR_POSITION
  

•       "  Press ENTER to complete, ESC/CTRL+C to quit",
  

•       0, 1, 8, 1);
  

• tmp = calloc(1, EFICONFIG_OPTIONAL_DATA_MAX * sizeof(u16));
• ret = efi_console_get_u16_string(cin, cout, tmp,

• 			 EFICONFIG_OPTIONAL_DATA_MAX, NULL, 4, 24);
  

• if (ret == EFI_SUCCESS)

• u16_strcpy(bo->optional_data, tmp);
  

• free(tmp);
• /* to stay the parent menu */
• ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
• return ret;

+}

+static efi_status_t eficonfig_boot_edit_save(void *data) +{

• struct eficonfig_boot_option *bo = data;
• if (u16_strlen(bo->description) == 0) {

• eficonfig_print_msg("Boot Description is empty!");
  

• bo->edit_completed = false;
  

• return EFI_NOT_READY;
  

• }
• if (u16_strlen(bo->file_info.current_path) == 0) {

• eficonfig_print_msg("File is not selected!");
  

• bo->edit_completed = false;
  

• return EFI_NOT_READY;
  

• }
• bo->edit_completed = true;
• return EFI_SUCCESS;

+}

+static efi_status_t eficonfig_boot_edit_quit(void *data) +{

• return EFI_ABORTED;

+}

+/**

• • eficonfig_select_file_handler() - handle user file selection
• • @data: pointer to the data
• • Return: status code
• */

+efi_status_t eficonfig_select_file_handler(void *data) +{

• size_t len;
• efi_status_t ret;
• struct list_head *pos, *n;
• struct efi_file_handle *root;
• struct eficonfig_filepath_info *item;
• struct eficonfig_select_file_info *file_info = data;
• struct eficonfig_select_file_info *tmp = NULL;
• tmp = calloc(1, sizeof(struct eficonfig_select_file_info));
• if (!tmp)

• return EFI_OUT_OF_RESOURCES;
  

• tmp->current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE);
• if (!tmp->current_path) {

• free(tmp);
  

• return EFI_OUT_OF_RESOURCES;
  

• }
• INIT_LIST_HEAD(&tmp->filepath_list);
• while (!tmp->file_selected) {

• tmp->current_volume = NULL;
  

• memset(tmp->current_path, 0, EFICONFIG_FILE_PATH_BUF_SIZE);
  

• ret = eficonfig_select_volume(tmp);
  

• if (ret != EFI_SUCCESS)
  

• 	goto out;
  

• if (!tmp->current_volume)
  

• 	return EFI_INVALID_PARAMETER;
  

• ret = efi_open_volume_int(tmp->current_volume, &root);
  

• if (ret != EFI_SUCCESS)
  

• 	goto out;
  

• ret = eficonfig_select_file(tmp, root);
  

• if (ret == EFI_ABORTED)
  

• 	continue;
  

• if (ret != EFI_SUCCESS)
  

• 	goto out;
  

• }

+out:

• if (ret == EFI_SUCCESS) {

• len = u16_strlen(tmp->current_path);
  

• len = (len >= EFICONFIG_FILE_PATH_MAX) ? (EFICONFIG_FILE_PATH_MAX - 1) : len;
  

• memcpy(file_info->current_path, tmp->current_path, len * sizeof(u16));
  

• file_info->current_path[len] = u'\0';
  

• file_info->current_volume = tmp->current_volume;
  

• file_info->dp_volume = tmp->dp_volume;
  

• }
• list_for_each_safe(pos, n, &tmp->filepath_list) {

• item = list_entry(pos, struct eficonfig_filepath_info, list);
  

• list_del(&item->list);
  

• free(item->name);
  

• free(item);
  

• }
• free(tmp->current_path);
• free(tmp);
• /* to stay the parent menu */
• ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
• return ret;

+}

+efi_status_t eficonfig_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size,

• 			     unsigned int *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 eficonfig_set_boot_option(u16 *varname, 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(varname, &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 eficonfig_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 eficonfig_convert_dp_to_device_name(struct efi_device_path *dp,

• 					char *buf, int size)
  

This function seems to be generic enough to live in lib/efi_loader/ as a library function.

+{

• u32 i;
• efi_status_t ret;
• struct efi_handler *handler;
• efi_uintn_t count, dp_size, iter_dp_size;
• efi_handle_t *volume_handles = NULL;
• struct efi_device_path *iter_dp;
• if (!dp || !buf || !size)

• return EFI_INVALID_PARAMETER;
  

• ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,

• 			   NULL, &count, (efi_handle_t **)&volume_handles);
  

Please, use efi_dp_find_obj() to retrieve the handle.

• if (ret != EFI_SUCCESS)

• return ret;
  

• dp_size = efi_dp_size(dp);
• for (i = 0; i < count; i++) {

• struct efi_block_io *block_io;
  

• ret = efi_search_protocol(volume_handles[i],
  

• 			  &efi_simple_file_system_protocol_guid, &handler);
  

• 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 **)&iter_dp,
  

• 			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;
  

• iter_dp_size = efi_dp_size(iter_dp);
  

• if (dp_size == iter_dp_size) {
  

• 	if (memcmp(dp, iter_dp, dp_size) == 0) {
  

• 		efi_disk_get_device_name(block_io, buf, size);
  

• 		return EFI_SUCCESS;
  

• 	}
  

• }
  

• }
• return EFI_NOT_FOUND;

+}

+static efi_status_t create_boot_option_entry(void *data, char *title, u16 *val,

• 			     eficonfig_entry_func func, struct eficonfig_item *iter)
  

+{

• u32 len;
• char *p;
• len = strlen(title) + 1;
• if (val)

• len += utf16_utf8_strlen(val);
  

• iter->title = calloc(1, len);
• if (!iter->title)

• return EFI_OUT_OF_RESOURCES;
  

• strcpy(iter->title, title);
• if (val) {

• p = iter->title + strlen(title);
  

• utf16_utf8_strcpy(&p, val);
  

• }
• iter->func = func;
• iter->data = data;
• return EFI_SUCCESS;

+}

+/**

• • eficonfig_show_boot_option() - prepare menu entry for editing boot option
• • Construct the structures to create edit boot option menu
• • @bo: pointer to the boot option
• • @header_str: pointer to the header string
• • Return: status code
• */

+static efi_status_t eficonfig_show_boot_option(struct eficonfig_boot_option *bo,

• 			       char *header_str)
  

+{

• u32 i, len;
• u16 *file_name, *p;
• struct eficonfig_item *menu_item, *iter;
• efi_status_t ret = EFI_OUT_OF_RESOURCES;
• char devname[BOOTMENU_DEVICE_NAME_MAX] = {0};
• menu_item = calloc(EFICONFIG_EDIT_BOOT_OPTION_MENU_ENTRY, sizeof(struct eficonfig_item));
• if (!menu_item)

• goto out;
  

• iter = menu_item;
• ret = create_boot_option_entry(bo, "Description: ", bo->description,

• 		       eficonfig_boot_add_enter_description, iter++);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• /* file name */
• eficonfig_convert_dp_to_device_name(bo->file_info.dp_volume, devname,

• 			    BOOTMENU_DEVICE_NAME_MAX);
  

• /*

• * efi_convert_device_path_to_text() automatically adds u'/' at the beginning of
  

• * file name, add manually u'/' at the last of device name if there is no u'/'
  

There is nothing manual here.

%s/add manually u'/' at the last of device name/append u'/'/

• * at bo->file_info.current_path[0].
  

• */
  

• if (bo->file_info.current_path[0] != u'\0' && bo->file_info.current_path[0] != u'/')

• strlcat(devname, "/", BOOTMENU_DEVICE_NAME_MAX);
  

• len = strlen(devname);
• len += utf16_utf8_strlen(bo->file_info.current_path) + 1;
• file_name = calloc(1, len * sizeof(u16));
• if (!file_name)

• goto out;
  

• p = file_name;
• utf8_utf16_strcpy(&p, devname);
• u16_strlcat(file_name, bo->file_info.current_path, len);
• ret = create_boot_option_entry(&bo->file_info, "File: ", file_name,

• 		       eficonfig_select_file_handler, iter++);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = create_boot_option_entry(bo, "Optional Data: ", bo->optional_data,

• 		       eficonfig_boot_add_optional_data, iter++);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = create_boot_option_entry(bo, "Save", NULL,

• 		       eficonfig_boot_edit_save, iter++);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = create_boot_option_entry(bo, "Quit", NULL,

• 		       eficonfig_boot_edit_quit, iter++);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = eficonfig_process_common(menu_item, EFICONFIG_EDIT_BOOT_OPTION_MENU_ENTRY,

• 		       header_str);
  

+out:

• iter = menu_item;
• for (i = 0; i < EFICONFIG_EDIT_BOOT_OPTION_MENU_ENTRY; i++) {

• free(iter->title);
  

• iter++;
  

• }
• free(file_name);
• free(menu_item);
• return ret;

+}

+/**

• • eficonfig_edit_boot_option() - prepare boot option structure for editing
• • Construct the boot option structure and copy the existing value
• • @varname: pointer to the uefi variable name
• • @bo: pointer to the boot option
• • @description: pointer to the description
• • @optional_data: pointer to the optional_data
• • @optional_data_size: optional_data_size
• • @dp: pointer to the device path
• • @header_str: pointer to the header string
• • Return : status code
• */

+static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_boot_option *bo,

• 			       u16 *description, const u8 *optional_data,
  

• 			       efi_uintn_t optional_data_size,
  

• 			       struct efi_device_path *dp,
  

• 			       char *header_str)
  

+{

• size_t len;
• char *buf = NULL;
• efi_status_t ret;
• char *iter = NULL;
• char *tmp = NULL, *p;
• efi_uintn_t dp_size, fp_size;
• struct efi_device_path *device_dp = NULL;
• struct efi_device_path_file_path *fp;
• bo->file_info.current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE);
• if (!bo->file_info.current_path) {

• ret =  EFI_OUT_OF_RESOURCES;
  

• goto out;
  

• }
• bo->description = calloc(1, EFICONFIG_DESCRIPTION_MAX * sizeof(u16));
• if (!bo->description) {

• ret =  EFI_OUT_OF_RESOURCES;
  

• goto out;
  

• }
• bo->optional_data = calloc(1, EFICONFIG_OPTIONAL_DATA_MAX * sizeof(u16));
• if (!bo->optional_data) {

• ret =  EFI_OUT_OF_RESOURCES;
  

• goto out;
  

• }
• if (description && u16_strlen(description) >= EFICONFIG_DESCRIPTION_MAX) {

• ret = EFI_INVALID_PARAMETER;
  

• goto out;
  

• }
• if (description)

• u16_strcpy(bo->description, description);
  

• if (dp) {

• u16 *file_str;
  

• struct efi_device_path *file_dp = NULL;
  

• efi_dp_split_file_path(dp, &device_dp, &file_dp);
  

• bo->file_info.dp_volume = device_dp;
  

• file_str = efi_dp_str(file_dp);
  

• u16_strcpy(bo->file_info.current_path, file_str);
  

• efi_free_pool(file_dp);
  

• efi_free_pool(file_str);
  

• }
• if (optional_data && optional_data_size >= EFICONFIG_OPTIONAL_DATA_MAX * sizeof(u16)) {

• ret = EFI_INVALID_PARAMETER;
  

• goto out;
  

• }
• if (optional_data && optional_data_size > 0)

• memcpy(bo->optional_data, optional_data, optional_data_size);
  

• while (1) {

• ret = eficonfig_show_boot_option(bo, header_str);
  

• if (ret == EFI_SUCCESS && bo->edit_completed)
  

• 	break;
  

• if (ret == EFI_NOT_READY)
  

• 	continue;
  

• if (ret != EFI_SUCCESS)
  

• 	goto out;
  

• }
• dp_size = efi_dp_size(bo->file_info.dp_volume);
• fp_size = sizeof(struct efi_device_path) +

•   ((u16_strlen(bo->file_info.current_path) + 1) * sizeof(u16));
  

• buf = calloc(1, dp_size + fp_size + sizeof(END));
• if (!buf)

• goto out;
  

• iter = buf;
• memcpy(iter, bo->file_info.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->file_info.current_path);
• iter += fp_size;
• *((struct efi_device_path *)iter) = END;
• len = utf16_utf8_strlen(bo->optional_data) + 1;
• tmp = calloc(1, len);
• if (!tmp)

• goto out;
  

• p = tmp;
• utf16_utf8_strncpy(&p, bo->optional_data, u16_strlen(bo->optional_data));
• ret = eficonfig_set_boot_option(varname, (struct efi_device_path *)buf,

• 			bo->description, tmp);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

+out:

• free(tmp);
• free(buf);
• free(bo->optional_data);
• free(bo->description);
• free(bo->file_info.current_path);
• efi_free_pool(device_dp);
• return ret;

+}

+static efi_status_t eficonfig_process_add_boot_option(void *data) +{

• u16 varname[9];
• efi_status_t ret;
• struct eficonfig_boot_option *bo = NULL;
• bo = calloc(1, sizeof(struct eficonfig_boot_option));
• if (!bo)

• return EFI_OUT_OF_RESOURCES;
  

• ret = eficonfig_get_unused_bootoption(varname, sizeof(varname), &bo->boot_index);
• if (ret != EFI_SUCCESS)

• return ret;
  

• ret = eficonfig_edit_boot_option(varname, bo, NULL, NULL, 0, NULL,

• 			 "  ** Add Boot Option ** ");
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = eficonfig_append_bootorder((u16)bo->boot_index);
• if (ret != EFI_SUCCESS)

• goto out;
  

+out:

• free(bo);
• /* to stay the parent menu */
• ret = (ret == EFI_ABORTED) ? EFI_SUCCESS : ret;
• return ret;

+}

+static efi_status_t eficonfig_init(void) +{

• efi_status_t ret;
• static bool init;
• struct efi_handler *handler;
• if (!init) {

• 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;
  

• }
• init = true;
• return ret;

+}

+static const struct eficonfig_item maintenance_menu_items[] = {

• {"Add Boot Option", eficonfig_process_add_boot_option},
• {"Quit", eficonfig_process_quit},

+};

+int do_eficonfig(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{

• efi_status_t ret;
• if (argc > 1)

• return CMD_RET_USAGE;
  

• ret = efi_init_obj_list();

We should add efi_init_obj_list() to init_sequence_r[] in a future patch to avoid calling it it in many different places.

• if (ret != EFI_SUCCESS) {

• log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
  

• 	ret & ~EFI_ERROR_MASK);
  

• return CMD_RET_FAILURE;
  

• }
• ret = eficonfig_init();
• if (ret != EFI_SUCCESS)

• return CMD_RET_FAILURE;
  

• while (1) {

• ret = eficonfig_process_common(maintenance_menu_items,
  

• 			       ARRAY_SIZE(maintenance_menu_items),
  

• 			     "  ** UEFI Maintenance Menu **");
  

• if (ret == EFI_ABORTED)
  

• 	break;
  

• }
• return CMD_RET_SUCCESS;

+}

+U_BOOT_CMD(

• eficonfig, 1, 0, do_eficonfig,
• "provide menu-driven UEFI variable maintenance interface",
• ""

+); diff --git a/include/efi_config.h b/include/efi_config.h new file mode 100644 index 0000000000..1b48e47c48 --- /dev/null +++ b/include/efi_config.h @@ -0,0 +1,91 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Menu-driven UEFI Variable maintenance
• • Copyright (c) 2022 Masahisa Kojima, Linaro Limited
• */

+#ifndef _EFI_CONFIG_H +#define _EFI_CONFIG_H

+#define EFICONFIG_ENTRY_NUM_MAX 99 +#define EFICONFIG_FILE_PATH_MAX 512 +#define EFICONFIG_FILE_PATH_BUF_SIZE (EFICONFIG_FILE_PATH_MAX * sizeof(u16))

+typedef efi_status_t (*eficonfig_entry_func)(void *data);

+/**

• • struct eficonfig_entry - menu entry structure
• • @num: menu entry index
• • @title: title of entry
• • @key: unique key
• • @efi_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 eficonfig_entry {

• u32 num;
• char *title;
• char key[3];
• struct efimenu *efi_menu;
• struct eficonfig_entry *next;
• eficonfig_entry_func func;
• void *data;

+};

+/**

• • struct efimenu - efi menu structure
• • @delay: delay for autoboot
• • @active: active menu entry index
• • @count: total count of menu entry
• • @menu_header: menu header string
• • @first: pointer to the first menu entry
• */

+struct efimenu {

• int delay;
• int active;
• int count;
• char *menu_header;
• struct eficonfig_entry *first;

+};

+/**

• • struct eficonfig_item - structure to construct eficonfig_entry
• • @title: title of entry
• • @func: callback function to be called when this entry is selected
• • @data: data to be passed to the callback function
• */

+struct eficonfig_item {

• char *title;
• eficonfig_entry_func func;
• void *data;

+};

+/**

• • struct eficonfig_select_file_info - structure to be used for file selection
• • @current_volume: pointer to the efi_simple_file_system_protocol
• • @dp_volume: pointer to device path of the selected device
• • @current_path: pointer to the selected file path string
• • @file_selectred: flag indicates file selecting status
• • @filepath_list: list_head structure for file path list
• */

+struct eficonfig_select_file_info {

• struct efi_simple_file_system_protocol *current_volume;
• struct efi_device_path *dp_volume;
• u16 *current_path;
• struct list_head filepath_list;
• bool file_selected;

+};

+void eficonfig_print_msg(char *msg); +efi_status_t eficonfig_process_quit(void *data); +efi_status_t eficonfig_process_common(const struct eficonfig_item *items, int count,

• 		      char *menu_header);
  

+efi_status_t eficonfig_select_file_handler(void *data);

+#endif diff --git a/include/efi_loader.h b/include/efi_loader.h index c6df29993c..365ce9493e 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;

@@ -249,6 +252,9 @@ extern const struct efi_hii_string_protocol efi_hii_string;

uint16_t *efi_dp_str(struct efi_device_path *dp);

+/* GUID for the auto generated boot menu entry */ +extern const efi_guid_t efi_guid_bootmenu_auto_generated;

• /* GUID of the U-Boot root node */ extern const efi_guid_t efi_u_boot_guid; #ifdef CONFIG_SANDBOX

@@ -314,6 +320,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[]; @@ -883,6 +892,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

@@ -1054,4 +1065,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 eficonfig_get_unused_bootoption(u16 *buf,

• 			     efi_uintn_t buf_size, u32 *index);
  

+efi_status_t eficonfig_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/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..ca8e38b8eb 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,80 @@ 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
• • @count: number of u16 string including NULL terminator that buf has
• • @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 count,
  

• 			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);

One printf() statement is enough and reduces code size:

printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE_TO_END ANSI_CURSOR_SHOW, row col);

• 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));
  

Please, use %ls to print u16 strings:

printf("ANSI_CURSOR_POSITION "%ls" ANSI_CLEAR_LINE_TO_END, row, col, buf);

• 	if (ret != EFI_SUCCESS)
  

• 		return ret;
  

• 	puts(ANSI_CLEAR_LINE_TO_END);
  

• 	continue;
  

• } else if (key.unicode_char == u'\r') {
  

• 	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 >= (count - 1))
  

• 	continue;
  

• buf[len] = key.unicode_char;
  

• len++;
  

• printf(ANSI_CURSOR_POSITION, row, col);
  

• ret = EFI_CALL(cout->output_string(cout, buf));
  

Please use %ls:

printf(ANSI_CURSOR_POSITION "%ls", row, col, buf);

• if (ret != EFI_SUCCESS)
  

• 	return ret;
  

• }

+} diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 1e82f52dc0..efc2f20ff0 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -778,3 +778,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);

This will fail if the EFI_BLOCK_IO_PROTOCOL is provided by a driver loaded via the bootefi command.

Handles can only be created by U-Boot in contrast to protocol interfaces. What you need is the udevice as a field in struct efi_object instead of struct efi_disk_obj. See Takahiro's comment in lib/efi_loader/efi_disk.c:49:

struct udevice *dev; /* TODO: move it to efi_object */

Best regards

Heinrich

• snprintf(buf, size, "%s %d:%d", diskobj->ifname, diskobj->dev_index, diskobj->part);
• return EFI_SUCCESS;

+} 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 *

On 7/12/22 03:55, Takahiro Akashi wrote:

On Sun, Jul 10, 2022 at 11:03:43AM +0200, Heinrich Schuchardt wrote:

...

On 6/19/22 06:56, Masahisa Kojima wrote:

...

This commit add the "eficonfig" command. The "eficonfig" command implements the menu-driven UEFI boot option maintenance feature. This commit implements the addition of new boot option. User can select the block device volume having efi_simple_file_system_protocol and select the file corresponding to the Boot#### variable. User can also enter the description and optional_data of the BOOT#### variable in utf8.

This commit adds "include/efi_config.h", it contains the common definition to be used from other menus such as UEFI Secure Boot key management.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

Changes in v8:

• command name is change from "efimenu" to "eficonfig"
• function and struct prefixes is changed to "eficonfig"
• fix menu header string

Changes in v7:

• add "efimenu" command and uefi variable maintenance code moved into cmd/efimenu.c

• create include/efimenu.h to define the common definition for the other menu such as UEFI Secure Boot key management

• update boot option edit UI, user can select description, file, and optional_data to edit in the same menu like following.

  ** Edit Boot Option **

   Description: debian
   File: virtio 0:1/EFI\debian\grubaa64.efi
   Optional Data: test
   Save
   Quit
  
  

• remove exit parameter from efimenu_process_common()

• menu title type is changed from u16 to char

• efimenu_process_common() add menu title string

• reduce printf/puts function call for displaying the menu

• efi_console_get_u16_string() accept 0 length to allow optional_data is empty

• efi_console_get_u16_string() the "size" parameter name is changes to "count"

• efimenu is now designed to maintain the UEFI variables, remove autoboot related code

• remove one empty line before "Quit" entry

• efimenu_init() processes only the first time

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
• 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

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/Kconfig | 7 + cmd/Makefile | 1 + cmd/eficonfig.c | 1270 +++++++++++++++++++++++++++++++++ include/efi_config.h | 91 +++ include/efi_loader.h | 40 ++ lib/efi_loader/efi_boottime.c | 52 +- lib/efi_loader/efi_console.c | 78 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- 9 files changed, 1578 insertions(+), 47 deletions(-) create mode 100644 cmd/eficonfig.c create mode 100644 include/efi_config.h

diff --git a/cmd/Kconfig b/cmd/Kconfig index 09193b61b9..bb7f1d0463 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1870,6 +1870,13 @@ config CMD_EFIDEBUG particularly for managing boot parameters as well as examining various EFI status for debugging.

+config CMD_EFICONFIG

• bool "eficonfig - provide menu-driven uefi variables maintenance interface"
• depends on CMD_BOOTEFI_BOOTMGR
• help

•  Enable the 'eficonfig' command which provides the menu-driven UEFI
  

•  variable maintenance interface.
  

• config CMD_EXCEPTION bool "exception - raise exception" depends on ARM || RISCV || SANDBOX || X86

diff --git a/cmd/Makefile b/cmd/Makefile index 5e43a1e022..0afa687e94 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -63,6 +63,7 @@ obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI) += efi.o obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o +obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_CMD_EROFS) += erofs.o obj-$(CONFIG_HUSH_PARSER) += exit.o diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c new file mode 100644 index 0000000000..20747db115 --- /dev/null +++ b/cmd/eficonfig.c @@ -0,0 +1,1270 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Menu-driven UEFI Variable maintenance
• • Copyright (c) 2022 Masahisa Kojima, Linaro Limited
• */

+#include <ansi.h> +#include <common.h> +#include <charset.h> +#include <efi_loader.h> +#include <efi_config.h> +#include <efi_variable.h> +#include <log.h> +#include <malloc.h> +#include <menu.h> +#include <watchdog.h> +#include <asm/unaligned.h> +#include <linux/delay.h>

+static struct efi_simple_text_input_protocol *cin; +static struct efi_simple_text_output_protocol *cout;

+#define EFICONFIG_DESCRIPTION_MAX 32 +#define EFICONFIG_OPTIONAL_DATA_MAX 64 +#define EFICONFIG_EDIT_BOOT_OPTION_MENU_ENTRY 5

+#define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \

• EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \

•  0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde)
  

+const efi_guid_t efi_guid_bootmenu_auto_generated =

• EFICONFIG_AUTO_GENERATED_ENTRY_GUID;
  

+struct eficonfig_boot_selection_data {

• u16 bootorder_index;
• int *selected;

+};

+struct eficonfig_filepath_info {

• u16 *name;
• struct list_head list;

+};

+/**

• • struct eficonfig_boot_option - structure to be used for uefi boot option update
• • @file_info: user selected file info
• • @boot_index: index of the uefi BootOrder variable
• • @description: pointer to the description string
• • @optional_data: pointer to the optional_data
• • @edit_completed: flag indicates edit complete
• */

+struct eficonfig_boot_option {

• struct eficonfig_select_file_info file_info;
• unsigned int boot_index;
• u16 *description;
• u16 *optional_data;
• bool edit_completed;

+};

+struct eficonfig_volume_entry_data {

• struct eficonfig_select_file_info *file_info;
• struct efi_simple_file_system_protocol *v;
• struct efi_device_path *dp;

+};

+struct eficonfig_file_entry_data {

• struct eficonfig_select_file_info *file_info;
• bool is_directory;
• u16 *file_name;

+};

+/**

• • eficonfig_print_msg() - print message
• • display the message to the user, user proceeds the screen
• • with ENTER key press.
• • @items: pointer to the structure of each menu entry
• • @count: the number of menu entry
• • @menu_header: pointer to the menu header string
• • Return: status code
• */

+void eficonfig_print_msg(char *msg)

None of you patches adds this function to an include. So it should be static.

...

+{

• char c;
• puts(ANSI_CURSOR_HIDE);
• puts(ANSI_CLEAR_CONSOLE);
• printf(ANSI_CURSOR_POSITION, 3, 4);
• printf(msg);
• /* Flush input */
• while (tstc())

• getchar();
  

• printf("\n\n Press ENTER to continue");

We want to minimize the size of the U-Boot binary.

Isn't the following single function call good enough?

printf(ANSI_CURSOR_HIDE ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION msg "\n\n Press ENTER to continue", 3, 4);

...

• while (1) {

• while (!tstc()) {
  

• 	WATCHDOG_RESET();
  

If there is no user to hit a key, why should be stop the watchdog reset?

...

• 	mdelay(10);
  

There is no benefit of calling mdelay here.

...

• }
  

• c = getchar();
  

• if (c == '\r')
  

• 	break;
  

• }

+}

We should replace the whole loop by:

getchar();

...

Please, provide Sphinx style function descriptions for all functions.

...

+static void eficonfig_print_entry(void *data) +{

• struct eficonfig_entry *entry = data;
• int reverse = (entry->efi_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(menu header) + one empty line
  

• */
  

• printf(ANSI_CURSOR_POSITION " ", entry->num + 4, 1);
• if (reverse)

• puts(ANSI_COLOR_REVERSE);
  

• printf("%s", entry->title);
• if (reverse)

• puts(ANSI_COLOR_RESET);
  

+}

+static void eficonfig_display_statusline(struct menu *m) +{

• struct eficonfig_entry *entry;
• if (menu_default_choice(m, (void *)&entry) < 0)

• return;
  

• printf(ANSI_CURSOR_POSITION

•      "\n%s\n"
  

•       ANSI_CURSOR_POSITION
  

•       "\n"
  

•       "  Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit",
  

•       0, 1, entry->efi_menu->menu_header, entry->efi_menu->count + 5, 1);
  

+}

+static char *eficonfig_choice_entry(void *data) +{

• int i;
• int esc = 0;
• struct eficonfig_entry *iter;
• enum bootmenu_key key = KEY_NONE;
• struct efimenu *efi_menu = data;
• while (1) {

• bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
  

• switch (key) {
  

• case KEY_UP:
  

• 	if (efi_menu->active > 0)
  

• 		--efi_menu->active;
  

• 	/* no menu key selected, regenerate menu */
  

• 	return NULL;
  

• case KEY_DOWN:
  

• 	if (efi_menu->active < efi_menu->count - 1)
  

• 		++efi_menu->active;
  

• 	/* no menu key selected, regenerate menu */
  

• 	return NULL;
  

• case KEY_SELECT:
  

• 	iter = efi_menu->first;
  

• 	for (i = 0; i < efi_menu->active; ++i)
  

• 		iter = iter->next;
  

• 	return iter->key;
  

• case KEY_QUIT:
  

• 	/* Quit by choosing the last entry */
  

• 	iter = efi_menu->first;
  

• 	while (iter->next)
  

• 		iter = iter->next;
  

• 	return iter->key;
  

• default:
  

• 	break;
  

• }
  

• }
• /* never happens */
• debug("eficonfig: this should not happen");

This comment and message is wrong. key = KEY_NONE is an expected value for key.

...

• return NULL;

+}

+static void eficonfig_destroy(struct efimenu *efi_menu) +{

• struct eficonfig_entry *next;
• struct eficonfig_entry *iter = efi_menu->first;
• while (iter) {

• next = iter->next;
  

• free(iter);
  

• iter = next;
  

• }
• free(efi_menu->menu_header);
• free(efi_menu);

+}

+/**

• • eficonfig_process_quit() - callback function for "Quit" entry
• • @data: pointer to the data
• • Return: status code
• */

+efi_status_t eficonfig_process_quit(void *data) +{

• return EFI_ABORTED;

+}

+/**

• • eficonfig_process_common() - main handler for uefi menu

%s/uefi/UEFI/

...

• • 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
• • @menu_header: pointer to the menu header string
• • Return: status code
• */

+efi_status_t eficonfig_process_common(const struct eficonfig_item *items, int count,

• 		      char *menu_header)
  

+{

• u32 i;
• efi_status_t ret;
• struct menu *menu;
• void *choice = NULL;
• struct eficonfig_entry *entry;
• struct efimenu *efi_menu;
• struct eficonfig_entry *iter = NULL;
• if (count > EFICONFIG_ENTRY_NUM_MAX)

• return EFI_OUT_OF_RESOURCES;
  

• efi_menu = calloc(1, sizeof(struct efimenu));
• if (!efi_menu)

• return EFI_OUT_OF_RESOURCES;
  

• efi_menu->delay = -1;
• efi_menu->active = 0;
• efi_menu->first = NULL;
• if (menu_header) {

• efi_menu->menu_header = strdup(menu_header);
  

• if (!efi_menu->menu_header) {
  

• 	free(efi_menu);
  

• 	return EFI_OUT_OF_RESOURCES;
  

• }
  

• }
• for (i = 0; i < count; i++) {

• entry = calloc(1, sizeof(struct eficonfig_entry));
  

• if (!entry) {
  

• 	ret = EFI_LOAD_ERROR;
  

• 	goto out;
  

• }
  

• entry->num = i;
  

• entry->title = items->title;
  

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

• entry->efi_menu = efi_menu;
  

• entry->func = items->func;
  

• entry->data = items->data;
  

• entry->next = NULL;
  

• if (!iter)
  

• 	efi_menu->first = entry;
  

• else
  

• 	iter->next = entry;
  

• iter = entry;
  

• items++;
  

• }
• efi_menu->count = count;
• menu = menu_create(NULL, 0, 1, eficonfig_display_statusline,

• 	   eficonfig_print_entry, eficonfig_choice_entry,
  

• 	   efi_menu);
  

• if (!menu) {

• ret = EFI_INVALID_PARAMETER;
  

• goto out;
  

• }
• for (entry = efi_menu->first; entry; entry = entry->next) {

• if (!menu_item_add(menu, entry->key, entry)) {
  

• 	ret = EFI_INVALID_PARAMETER;
  

• 	goto out;
  

• }
  

• }
• menu_default_set(menu, efi_menu->first->key);
• puts(ANSI_CURSOR_HIDE);
• puts(ANSI_CLEAR_CONSOLE);
• printf(ANSI_CURSOR_POSITION, 1, 1);

Please, use a single function call.

...

• if (menu_get_choice(menu, &choice)) {

• entry = choice;
  

• if (entry->func)
  

• 	ret = entry->func(entry->data);
  

• }

+out:

• menu_destroy(menu);
• eficonfig_destroy(efi_menu);
• puts(ANSI_CLEAR_CONSOLE);
• printf(ANSI_CURSOR_POSITION, 1, 1);
• puts(ANSI_CURSOR_SHOW);

ditto

...

• return ret;

+}

+static efi_status_t eficonfig_volume_selected(void *data) +{

• struct eficonfig_volume_entry_data *info = data;
• if (info) {

• info->file_info->current_volume = info->v;
  

• info->file_info->dp_volume = info->dp;
  

• }
• return EFI_SUCCESS;

+}

+static efi_status_t eficonfig_file_selected(void *data) +{

• struct eficonfig_file_entry_data *info = data;
• 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 eficonfig_filepath_info *iter;
  

• struct list_head *pos, *n;
  

• int is_last;
  

• memset(info->file_info->current_path, 0, EFICONFIG_FILE_PATH_BUF_SIZE);
  

• list_for_each_safe(pos, n, &info->file_info->filepath_list) {
  

• 	iter = list_entry(pos, struct eficonfig_filepath_info, list);
  

• 	is_last = list_is_last(&iter->list, &info->file_info->filepath_list);
  

• 	if (is_last) {
  

• 		list_del(&iter->list);
  

• 		free(iter->name);
  

• 		free(iter);
  

• 		break;
  

• 	}
  

• 	u16_strlcat(info->file_info->current_path, iter->name,
  

• 		    EFICONFIG_FILE_PATH_MAX);
  

• 	u16_strlcat(info->file_info->current_path, u"\\",
  

• 		    EFICONFIG_FILE_PATH_MAX);
  

• }
  

• } else {

• size_t new_len;
  

• struct eficonfig_filepath_info *filepath;
  

• new_len = u16_strlen(info->file_info->current_path) +
  

• 		     u16_strlen(info->file_name);
  

• if (new_len >= EFICONFIG_FILE_PATH_MAX) {
  

• 	eficonfig_print_msg("File path is too long!");
  

• 	return EFI_INVALID_PARAMETER;
  

• }
  

• u16_strlcat(info->file_info->current_path, info->file_name,
  

• 	    EFICONFIG_FILE_PATH_MAX);
  

• filepath = calloc(1, sizeof(struct eficonfig_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->file_info->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 >= EFICONFIG_FILE_PATH_MAX) {
  

• 		eficonfig_print_msg("Directory path is too long!");
  

• 		return EFI_INVALID_PARAMETER;
  

• 	}
  

• 	u16_strlcat(info->file_info->current_path, u"\\",
  

• 		    EFICONFIG_FILE_PATH_MAX);
  

• } else {
  

• 	info->file_info->file_selected = true;
  

• }
  

• }
• return EFI_SUCCESS;

+}

+static efi_status_t eficonfig_select_volume(struct eficonfig_select_file_info *file_info) +{

• 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 eficonfig_item *menu_item, *iter;
• struct efi_simple_file_system_protocol *v;
• 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) {

• eficonfig_print_msg("No block device found!");
  

• return ret;
  

• }
• menu_item = calloc(count + 1, sizeof(struct eficonfig_item));
• if (!menu_item) {

• efi_free_pool(volume_handles);
  

• return EFI_OUT_OF_RESOURCES;
  

• }
• iter = menu_item;
• for (i = 0; i < count; i++) {

• char *devname;
  

• struct efi_block_io *block_io;
  

• struct eficonfig_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 eficonfig_volume_entry_data));
  

• if (!info) {
  

• 	ret = EFI_OUT_OF_RESOURCES;
  

• 	goto out;
  

• }
  

• devname = calloc(1, BOOTMENU_DEVICE_NAME_MAX);
  

• if (!devname) {
  

• 	free(info);
  

• 	ret = EFI_OUT_OF_RESOURCES;
  

• 	goto out;
  

• }
  

• efi_disk_get_device_name(block_io, devname, BOOTMENU_DEVICE_NAME_MAX);
  

• info->v = v;
  

• info->dp = device_path;
  

• info->file_info = file_info;
  

• iter->title = devname;
  

• iter->func = eficonfig_volume_selected;
  

• iter->data = info;
  

• iter++;
  

• }
• iter->title = strdup("Quit");
• iter->func = eficonfig_process_quit;
• iter->data = NULL;
• count += 1;
• ret = eficonfig_process_common(menu_item, count, " ** Select Volume **");

+out:

• iter = menu_item;
• for (i = 0; i < count; i++, iter++) {

• free(iter->data);
  

• free(iter->title);
  

• }
• free(menu_item);
• efi_free_pool(volume_handles);
• return ret;

+}

+static efi_status_t eficonfig_select_file(struct eficonfig_select_file_info *file_info,

• 			  struct efi_file_handle *root)
  

+{

• u32 i;
• u32 count = 0;
• efi_uintn_t len;
• efi_status_t ret;
• struct efi_file_handle *f;
• struct efi_file_info *buf;
• struct eficonfig_item *menu_item, *iter;
• buf = calloc(1, sizeof(struct efi_file_info) + EFICONFIG_FILE_PATH_BUF_SIZE);
• if (!buf)

• return EFI_OUT_OF_RESOURCES;
  

• while (!file_info->file_selected) {

• count = 0;
  

• ret = efi_file_open_int(root, &f, file_info->current_path, EFI_FILE_MODE_READ, 0);
  

• if (ret != EFI_SUCCESS) {
  

• 	/* TODO: need to fileter out non-FAT partition? */
  

%s/fileter/filter/

...

• 	eficonfig_print_msg("Reading volume failed! Please make sure the selected\n"
  

• 			    "   volume is FAT12/FAT16/FAT32 partition.");
  

Who cares if the file is on FAT, ext4, or any other file system else if U-Boot can read it?

UEFI specification and distro's does.

...

From technical viewpoint, there is no reason that we need to impose a restriction on file system type.

But, from the viewpoint of goal of UEFI, we should have a very common file system, that is FAT, on UEFI system partition. UEFI specification says:

---8<--- 1.5 UEFI Design Overview ... system partition. The System partition defines a partition and file system that are designed to allow safe sharing between multiple vendors, and for different purposes. The ability to include ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a separate, sharable system partition presents an opportunity to increase platform value-add without significantly growing the need for nonvolatile platform memory.

13.3.1.3 Directory Structure An EFI system partition that is present on a hard disk must contain an EFI defined directory in the root directory. This directory is named EFI. All OS loaders and applications will be stored in subdirectories below EFI. --->8---