[RESEND RFC PATCH v2 0/4] enable menu-driven boot device selection

[RESEND RFC PATCH v2 0/4] enable menu-driven boot device selection

[Masahisa Kojima]

This patch series adds the menu-driven boot device selection.
This menu also provides the functionality to add and delete
Boot#### variable, and edit the BootOrder variable.

This menu appears with the command "bootefi bootmgr",
the menu structure is as follows.

* Menu structure
[Boot Manager]
    -> select Boot#### to boot
[Boot Manager Maintenance]
    - [Add Boot Option]
        -> add new Boot#### variable
    - [Delete Boot Option]
        -> delete existing Boot#### variable
    - [Change Boot Order]
        -> update BootOrder variable

* Things to consider
 - eliminate EFI_CALLs
 - replacement of ANSI_CLEAR_CONSOLE

* Remaining items
 - Support of adding Boot#### other than block device(e.g. network)
 - error notification

Masahisa Kojima (4):
  efi_loader: add menu-driven boot device selection
  lib/charset: add u16_strlcat() function
  test: unit test for u16_strlcat()
  efi_loader: add menu-driven UEFI Boot Variable maintenance

 cmd/bootmenu.c                |  145 -----
 common/menu.c                 |  137 ++++
 include/charset.h             |   15 +
 include/efi_loader.h          |   27 +
 include/menu.h                |   20 +
 lib/charset.c                 |   20 +
 lib/efi_loader/Kconfig        |   20 +
 lib/efi_loader/efi_bootmgr.c  | 1105 ++++++++++++++++++++++++++++++++-
 lib/efi_loader/efi_boottime.c |   55 +-
 lib/efi_loader/efi_console.c  |   81 +++
 lib/efi_loader/efi_file.c     |   74 ++-
 test/unicode_ut.c             |   45 ++
 12 files changed, 1548 insertions(+), 196 deletions(-)

-- 
2.17.1

[RESEND RFC PATCH v2 1/4] efi_loader: add menu-driven boot device selection

[Masahisa Kojima]

This patch enables the menu-driven boot device selection.
User can select the Boot#### included in BootOrder variable.

If user quits thie menu, or the selected Boot#### fails to boot,
efi bootmgr continues to boot in accordance with BootOrder variable.

This commit also moves the user input handling from cmd/bootmenu.c
to common/menu.c to reuse it from efi boot menu.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changes in v2:
- move user input handling from cmd/bootmenu.c to common/menu.c
- keyboard buffer is drained before asking question in
  common/menu.c::bootmenu_autoboot_loop()
- Add EFI_BOOT_MENU_DELAY Kconfig option to set delay for
  autoboot and disable autoboot
- remove unnecessary "autoboot" member in struc efi_bootmgr_menu

 cmd/bootmenu.c               | 145 ------------
 common/menu.c                | 137 ++++++++++++
 include/menu.h               |  20 ++
 lib/efi_loader/Kconfig       |  20 ++
 lib/efi_loader/efi_bootmgr.c | 418 ++++++++++++++++++++++++++++++++++-
 5 files changed, 592 insertions(+), 148 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 409ef9a848..f9fdebc450 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -33,21 +33,6 @@ struct bootmenu_entry {
 	struct bootmenu_entry *next;	/* next menu entry (num+1) */
 };
 
-struct bootmenu_data {
-	int delay;			/* delay for autoboot */
-	int active;			/* active menu entry */
-	int count;			/* total count of menu entries */
-	struct bootmenu_entry *first;	/* first menu entry */
-};
-
-enum bootmenu_key {
-	KEY_NONE = 0,
-	KEY_UP,
-	KEY_DOWN,
-	KEY_SELECT,
-	KEY_QUIT,
-};
-
 static char *bootmenu_getoption(unsigned short int n)
 {
 	char name[MAX_ENV_SIZE];
@@ -81,136 +66,6 @@ static void bootmenu_print_entry(void *data)
 		puts(ANSI_COLOR_RESET);
 }
 
-static void bootmenu_autoboot_loop(struct bootmenu_data *menu,
-				enum bootmenu_key *key, int *esc)
-{
-	int i, c;
-
-	if (menu->delay > 0) {
-		printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
-		printf("  Hit any key to stop autoboot: %2d ", menu->delay);
-	}
-
-	while (menu->delay > 0) {
-		for (i = 0; i < 100; ++i) {
-			if (!tstc()) {
-				WATCHDOG_RESET();
-				mdelay(10);
-				continue;
-			}
-
-			menu->delay = -1;
-			c = getchar();
-
-			switch (c) {
-			case '\e':
-				*esc = 1;
-				*key = KEY_NONE;
-				break;
-			case '\r':
-				*key = KEY_SELECT;
-				break;
-			case 0x3: /* ^C */
-				*key = KEY_QUIT;
-				break;
-			default:
-				*key = KEY_NONE;
-				break;
-			}
-
-			break;
-		}
-
-		if (menu->delay < 0)
-			break;
-
-		--menu->delay;
-		printf("\b\b\b%2d ", menu->delay);
-	}
-
-	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
-	puts(ANSI_CLEAR_LINE);
-
-	if (menu->delay == 0)
-		*key = KEY_SELECT;
-}
-
-static void bootmenu_loop(struct bootmenu_data *menu,
-		enum bootmenu_key *key, int *esc)
-{
-	int c;
-
-	if (*esc == 1) {
-		if (tstc()) {
-			c = getchar();
-		} else {
-			WATCHDOG_RESET();
-			mdelay(10);
-			if (tstc())
-				c = getchar();
-			else
-				c = '\e';
-		}
-	} else {
-		while (!tstc()) {
-			WATCHDOG_RESET();
-			mdelay(10);
-		}
-		c = getchar();
-	}
-
-	switch (*esc) {
-	case 0:
-		/* First char of ANSI escape sequence '\e' */
-		if (c == '\e') {
-			*esc = 1;
-			*key = KEY_NONE;
-		}
-		break;
-	case 1:
-		/* Second char of ANSI '[' */
-		if (c == '[') {
-			*esc = 2;
-			*key = KEY_NONE;
-		} else {
-		/* Alone ESC key was pressed */
-			*key = KEY_QUIT;
-			*esc = (c == '\e') ? 1 : 0;
-		}
-		break;
-	case 2:
-	case 3:
-		/* Third char of ANSI (number '1') - optional */
-		if (*esc == 2 && c == '1') {
-			*esc = 3;
-			*key = KEY_NONE;
-			break;
-		}
-
-		*esc = 0;
-
-		/* ANSI 'A' - key up was pressed */
-		if (c == 'A')
-			*key = KEY_UP;
-		/* ANSI 'B' - key down was pressed */
-		else if (c == 'B')
-			*key = KEY_DOWN;
-		/* other key was pressed */
-		else
-			*key = KEY_NONE;
-
-		break;
-	}
-
-	/* enter key was pressed */
-	if (c == '\r')
-		*key = KEY_SELECT;
-
-	/* ^C was pressed */
-	if (c == 0x3)
-		*key = KEY_QUIT;
-}
-
 static char *bootmenu_choice_entry(void *data)
 {
 	struct bootmenu_data *menu = data;
diff --git a/common/menu.c b/common/menu.c
index 5fb2ffbd06..786e45f217 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -4,11 +4,14 @@
  * Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
  */
 
+#include <ansi.h>
 #include <common.h>
 #include <cli.h>
 #include <malloc.h>
 #include <errno.h>
+#include <linux/delay.h>
 #include <linux/list.h>
+#include <watchdog.h>
 
 #include "menu.h"
 
@@ -418,3 +421,137 @@ int menu_destroy(struct menu *m)
 
 	return 1;
 }
+
+void bootmenu_autoboot_loop(struct bootmenu_data *menu,
+			    enum bootmenu_key *key, int *esc)
+{
+	int i, c;
+
+	if (menu->delay > 0) {
+		/* flush input */
+		while (tstc())
+			getchar();
+
+		printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
+		printf("  Hit any key to stop autoboot: %2d ", menu->delay);
+	}
+
+	while (menu->delay > 0) {
+		for (i = 0; i < 100; ++i) {
+			if (!tstc()) {
+				WATCHDOG_RESET();
+				mdelay(10);
+				continue;
+			}
+
+			menu->delay = -1;
+			c = getchar();
+
+			switch (c) {
+			case '\e':
+				*esc = 1;
+				*key = KEY_NONE;
+				break;
+			case '\r':
+				*key = KEY_SELECT;
+				break;
+			case 0x3: /* ^C */
+				*key = KEY_QUIT;
+				break;
+			default:
+				*key = KEY_NONE;
+				break;
+			}
+
+			break;
+		}
+
+		if (menu->delay < 0)
+			break;
+
+		--menu->delay;
+		printf("\b\b\b%2d ", menu->delay);
+	}
+
+	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
+	puts(ANSI_CLEAR_LINE);
+
+	if (menu->delay == 0)
+		*key = KEY_SELECT;
+}
+
+void bootmenu_loop(struct bootmenu_data *menu,
+		   enum bootmenu_key *key, int *esc)
+{
+	int c;
+
+	if (*esc == 1) {
+		if (tstc()) {
+			c = getchar();
+		} else {
+			WATCHDOG_RESET();
+			mdelay(10);
+			if (tstc())
+				c = getchar();
+			else
+				c = '\e';
+		}
+	} else {
+		while (!tstc()) {
+			WATCHDOG_RESET();
+			mdelay(10);
+		}
+		c = getchar();
+	}
+
+	switch (*esc) {
+	case 0:
+		/* First char of ANSI escape sequence '\e' */
+		if (c == '\e') {
+			*esc = 1;
+			*key = KEY_NONE;
+		}
+		break;
+	case 1:
+		/* Second char of ANSI '[' */
+		if (c == '[') {
+			*esc = 2;
+			*key = KEY_NONE;
+		} else {
+		/* Alone ESC key was pressed */
+			*key = KEY_QUIT;
+			*esc = (c == '\e') ? 1 : 0;
+		}
+		break;
+	case 2:
+	case 3:
+		/* Third char of ANSI (number '1') - optional */
+		if (*esc == 2 && c == '1') {
+			*esc = 3;
+			*key = KEY_NONE;
+			break;
+		}
+
+		*esc = 0;
+
+		/* ANSI 'A' - key up was pressed */
+		if (c == 'A')
+			*key = KEY_UP;
+		/* ANSI 'B' - key down was pressed */
+		else if (c == 'B')
+			*key = KEY_DOWN;
+		/* other key was pressed */
+		else
+			*key = KEY_NONE;
+
+		break;
+	}
+
+	/* enter key was pressed */
+	if (c == '\r')
+		*key = KEY_SELECT;
+
+	/* ^C was pressed */
+	if (c == 0x3)
+		*key = KEY_QUIT;
+}
diff --git a/include/menu.h b/include/menu.h
index ad5859437e..e74616cae8 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -35,4 +35,24 @@ int menu_default_choice(struct menu *m, void **choice);
  */
 int menu_show(int bootdelay);
 
+struct bootmenu_data {
+	int delay;			/* delay for autoboot */
+	int active;			/* active menu entry */
+	int count;			/* total count of menu entries */
+	struct bootmenu_entry *first;	/* first menu entry */
+};
+
+enum bootmenu_key {
+	KEY_NONE = 0,
+	KEY_UP,
+	KEY_DOWN,
+	KEY_SELECT,
+	KEY_QUIT,
+};
+
+void bootmenu_autoboot_loop(struct bootmenu_data *menu,
+			    enum bootmenu_key *key, int *esc);
+void bootmenu_loop(struct bootmenu_data *menu,
+		   enum bootmenu_key *key, int *esc);
+
 #endif /* __MENU_H__ */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e5e35fe51f..a3fb8b2a75 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -39,6 +39,26 @@ config CMD_BOOTEFI_BOOTMGR
 	  via UEFI variables Boot####, BootOrder, and BootNext. This enables the
 	  'bootefi bootmgr' command.
 
+config EFI_BOOT_MENU
+	bool "UEFI Boot Menu driven boot device selection"
+	default n
+	help
+	  Select this option if you want to enable the menu driven boot device
+	  selection. This menu provides the functionality to select a boot
+	  option to start, and allow users to edit Boot#### and BootOrder.
+	  If this menu is enabled, CLI can be disabled if the system boots
+	  via UEFI variable Boot#### and BootOrder.
+
+config EFI_BOOT_MENU_DELAY
+	int "delay in seconds before automatically booting in UEFI Boot Menu"
+	default 10
+	range -1 2147483647
+	help
+	  Delay before automatically booting in accordance with
+	  "BootOrder" variable.
+	  set to 0 to autoboot with no delay.
+	  set to -1 to disable autoboot.
+
 config EFI_SETUP_EARLY
 	bool
 
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 8c04ecbdc8..bbb3fac5bd 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -7,10 +7,12 @@
 
 #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>
@@ -22,14 +24,400 @@ static const struct efi_runtime_services *rs;
  * bootmgr implements the logic of trying to find a payload to boot
  * based on the BootOrder + BootXXXX variables, and then loading it.
  *
- * TODO detecting a special key held (f9?) and displaying a boot menu
- * like you would get on a PC would be clever.
- *
  * TODO if we had a way to write and persist variables after the OS
  * has started, we'd also want to check OsIndications to see if we
  * should do normal or recovery boot.
  */
 
+#define EFI_BOOTMGR_MENU_ENTRY_NUM_MAX 1024
+
+typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
+
+/**
+ * struct efi_bootmgr_menu_entry - menu entry structure
+ *
+ * @menu_index:		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_bootmgr_menu_entry {
+	u32 menu_index;
+	u16 *title;
+	char key[6];
+	struct efi_bootmgr_menu *bootmgr_menu;
+	struct efi_bootmgr_menu_entry *next;
+	efi_bootmenu_entry_func func;
+	void *data;
+};
+
+/**
+ * struct efi_bootmgr_menuy - 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_bootmgr_menu {
+	int delay;
+	int active;
+	int count;
+	struct efi_bootmgr_menu_entry *first;
+};
+
+struct efi_bootmgr_menu_item {
+	u16 *title;
+	efi_bootmenu_entry_func func;
+	void *data;
+};
+
+struct efi_bootmgr_boot_selection_data {
+	u16 bootorder_index;
+	void *load_option;
+	int *selected;
+};
+
+static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool *exit);
+static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit);
+
+static struct efi_bootmgr_menu_item bootmgr_menu_items[] = {
+	{u"Boot Manager", efi_bootmgr_process_boot_selection},
+	{u"Quit", NULL},
+};
+
+static void efi_bootmgr_menu_print_entry(void *data)
+{
+	struct efi_bootmgr_menu_entry *entry = data;
+	int reverse = (entry->bootmgr_menu->active == entry->menu_index);
+
+	/* TODO: support scroll or page for many entries */
+
+	/*
+	 * Move cursor to line where the entry will be drown (entry->count)
+	 * First 3 lines contain bootmgr menu header + one empty line
+	 * For the last "Quit" entry, add one empty line
+	 */
+	if (entry->menu_index == (entry->bootmgr_menu->count - 1))
+		printf(ANSI_CURSOR_POSITION, entry->menu_index + 5, 1);
+	else
+		printf(ANSI_CURSOR_POSITION, entry->menu_index + 4, 1);
+
+	puts("     ");
+
+	if (reverse)
+		puts(ANSI_COLOR_REVERSE);
+
+	printf("%ls", entry->title);
+
+	if (reverse)
+		puts(ANSI_COLOR_RESET);
+}
+
+static void efi_bootmgr_menu_display_statusline(struct menu *m)
+{
+	struct efi_bootmgr_menu_entry *entry;
+	struct efi_bootmgr_menu *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_bootmgr_menu_choice_entry(void *data)
+{
+	int i;
+	int esc = 0;
+	struct efi_bootmgr_menu_entry *iter;
+	enum bootmenu_key key = KEY_NONE;
+	struct efi_bootmgr_menu *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;
+		}
+	}
+
+	/* never happens */
+	debug("bootmgr menu: this should not happen");
+	return NULL;
+}
+
+static void efi_bootmgr_menu_destroy(struct efi_bootmgr_menu *bootmgr_menu)
+{
+	struct efi_bootmgr_menu_entry *next;
+	struct efi_bootmgr_menu_entry *iter = bootmgr_menu->first;
+
+	while (iter) {
+		next = iter->next;
+		free(iter);
+		iter = next;
+	}
+	free(bootmgr_menu);
+}
+
+/**
+ * efi_bootmgr_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_bootmgr_process_common(struct efi_bootmgr_menu_item *items,
+					       int count, int delay)
+{
+	u32 i;
+	bool exit = false;
+	efi_status_t ret;
+	struct menu *menu;
+	void *choice = NULL;
+	struct efi_bootmgr_menu_entry *entry;
+	struct efi_bootmgr_menu *bootmgr_menu;
+	struct efi_bootmgr_menu_entry *iter = NULL;
+
+	if (count > EFI_BOOTMGR_MENU_ENTRY_NUM_MAX)
+		return EFI_OUT_OF_RESOURCES;
+
+	bootmgr_menu = calloc(1, sizeof(struct efi_bootmgr_menu));
+	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_bootmgr_menu_entry));
+		if (!entry) {
+			ret = EFI_LOAD_ERROR;
+			goto out;
+		}
+
+		entry->menu_index = 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_bootmgr_menu_display_statusline,
+			   efi_bootmgr_menu_print_entry, efi_bootmgr_menu_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->menu_index == (entry->bootmgr_menu->count - 1)) {
+				ret = EFI_ABORTED;
+				break;
+			}
+		}
+	}
+
+out:
+	menu_destroy(menu);
+	efi_bootmgr_menu_destroy(bootmgr_menu);
+
+	puts(ANSI_CLEAR_CONSOLE);
+	printf(ANSI_CURSOR_POSITION, 1, 1);
+	puts(ANSI_CURSOR_SHOW);
+
+	return ret;
+}
+
+static efi_status_t efi_bootmgr_show_boot_selection(u16 *bootorder, efi_uintn_t count,
+						    int *selected)
+{
+	u32 i;
+	efi_status_t ret;
+	efi_uintn_t size;
+	void *load_option;
+	struct efi_load_option lo;
+	u16 varname[] = u"Boot####";
+	struct efi_bootmgr_menu_item *menu_item, *iter;
+
+	menu_item = calloc(count + 1, sizeof(struct efi_bootmgr_menu_item));
+	if (!menu_item) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+
+	iter = menu_item;
+	for (i = 0; i < count; i++) {
+		efi_create_indexed_name(varname, sizeof(varname),
+					"Boot", bootorder[i]);
+		load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
+		if (!load_option)
+			continue;
+
+		ret = efi_deserialize_load_option(&lo, load_option, &size);
+		if (ret != EFI_SUCCESS) {
+			log_warning("Invalid load option for %ls\n", varname);
+			free(load_option);
+			continue;
+		}
+
+		if (lo.attributes & LOAD_OPTION_ACTIVE) {
+			struct efi_bootmgr_boot_selection_data *info;
+
+			info = calloc(1, sizeof(struct efi_bootmgr_boot_selection_data));
+			if (!info) {
+				ret = EFI_OUT_OF_RESOURCES;
+				goto out;
+			}
+
+			info->bootorder_index = i;
+			info->load_option = load_option;
+			info->selected = selected;
+			iter->title = lo.label;
+			iter->func = efi_bootmgr_process_boot_selected;
+			iter->data = info;
+			iter++;
+		}
+	}
+
+	/* add "Quit" entry */
+	iter->title = u"Quit";
+	iter->func = NULL;
+	iter->data = NULL;
+	count += 1;
+
+	ret = efi_bootmgr_process_common(menu_item, count, -1);
+
+out:
+	iter = menu_item;
+	for (i = 0; i < count - 1; i++, iter++) {
+		free(((struct efi_bootmgr_boot_selection_data *)iter->data)->load_option);
+		free(iter->data);
+	}
+
+	free(menu_item);
+
+	return ret;
+}
+
+static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool *exit)
+{
+	struct efi_bootmgr_boot_selection_data *info = data;
+
+	*exit = true;
+
+	if (info)
+		*info->selected = info->bootorder_index;
+
+	return EFI_SUCCESS;
+}
+
+static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit)
+{
+	u16 *bootorder;
+	efi_status_t ret;
+	efi_uintn_t num, size;
+
+	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
+	if (!bootorder)
+		return EFI_NOT_FOUND;
+
+	num = size / sizeof(u16);
+	ret = efi_bootmgr_show_boot_selection(bootorder, num, data);
+	if (ret == EFI_SUCCESS)
+		*exit =  true;
+
+	free(bootorder);
+
+	return ret;
+}
+
 /**
  * try_load_entry() - try to load image for boot option
  *
@@ -177,6 +565,30 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
 		}
 	}
 
+	if (IS_ENABLED(CONFIG_EFI_BOOT_MENU)) {
+		int selected;
+
+		bootmgr_menu_items[0].data = &selected;
+		ret = efi_bootmgr_process_common(bootmgr_menu_items,
+						 ARRAY_SIZE(bootmgr_menu_items),
+						 CONFIG_EFI_BOOT_MENU_DELAY);
+		if (ret == EFI_SUCCESS) {
+			/* bootorder may be updated in the bootmgr menu */
+			bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
+			if (!bootorder) {
+				log_info("BootOrder not defined\n");
+				goto error;
+			}
+			ret = try_load_entry(bootorder[selected], handle, load_options);
+			free(bootorder);
+			if (ret == EFI_SUCCESS)
+				return ret;
+
+			log_err("Failed to start the selected entry(Boot%04X)\n",
+				bootorder[selected]);
+		}
+	}
+
 	/* BootOrder */
 	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
 	if (!bootorder) {
-- 
2.17.1

[RESEND RFC PATCH v2 2/4] lib/charset: add u16_strlcat() function

[Masahisa Kojima]

Provide u16 string version of strlcat().

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changes in v2:
- implement u16_strlcat(with the destination buffer size in argument)
  instead of u16_strcat

 include/charset.h | 15 +++++++++++++++
 lib/charset.c     | 20 ++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/charset.h b/include/charset.h
index b93d023092..dc5fc275ec 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -259,6 +259,21 @@ u16 *u16_strcpy(u16 *dest, const u16 *src);
  */
 u16 *u16_strdup(const void *src);
 
+/**
+ * u16_strlcat() - Append a length-limited, %NUL-terminated string to another
+ *
+ * Append the src string to the dest string, overwriting the terminating
+ * null word at the end of dest, and then adds a terminating null word.
+ * It will append at most size - u16_strlen(dst) - 1 bytes, NUL-terminating the result.
+ *
+ * @dest:		destination buffer (null terminated)
+ * @src:		source buffer (null terminated)
+ * @size:		destination buffer size in bytes
+ * Return:		total size of the created string in bytes.
+ *			If return value >= size, truncation occurred.
+ */
+size_t u16_strlcat(u16 *dest, const u16 *src, size_t size);
+
 /**
  * utf16_to_utf8() - Convert an utf16 string to utf8
  *
diff --git a/lib/charset.c b/lib/charset.c
index f44c58d9d8..f15d5df19a 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -428,6 +428,26 @@ u16 *u16_strdup(const void *src)
 	return new;
 }
 
+size_t u16_strlcat(u16 *dest, const u16 *src, size_t size)
+{
+	size_t dstrlen = u16_strnlen(dest, size >> 1);
+	size_t dlen = dstrlen * sizeof(u16);
+	size_t len = u16_strlen(src) * sizeof(u16);
+	size_t ret = dlen + len;
+
+	if (dlen >= size)
+		return ret;
+
+	dest += dstrlen;
+	size -= dlen;
+	if (len >= size)
+		len = size - sizeof(u16);
+
+	memcpy(dest, src, len);
+	dest[len >> 1] = u'\0';
+	return ret;
+}
+
 /* Convert UTF-16 to UTF-8.  */
 uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size)
 {
-- 
2.17.1

[RESEND RFC PATCH v2 3/4] test: unit test for u16_strlcat()

[Masahisa Kojima]

Provide a unit test for function u16_strlcat().

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Newly created in v2.

 test/unicode_ut.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/test/unicode_ut.c b/test/unicode_ut.c
index f2f63d5367..f79b0439bc 100644
--- a/test/unicode_ut.c
+++ b/test/unicode_ut.c
@@ -758,6 +758,51 @@ static int unicode_test_efi_create_indexed_name(struct unit_test_state *uts)
 UNICODE_TEST(unicode_test_efi_create_indexed_name);
 #endif
 
+static int unicode_test_u16_strlcat(struct unit_test_state *uts)
+{
+	u16 buf[11];
+	u16 dest[] = u"U-Boot";
+	u16 src[] = u"test";
+	u16 expected[] = u"U-Boottest";
+	u16 null_src = u'\0';
+	size_t ret;
+
+	memcpy(buf, dest, sizeof(dest));
+	ret = u16_strlcat(buf, src, sizeof(buf));
+	ut_asserteq(20, ret);
+	ut_assert(!unicode_test_u16_strcmp(buf, expected, 11));
+
+	/* dest is empty string */
+	memset(buf, 0, sizeof(buf));
+	ret = u16_strlcat(buf, src, sizeof(buf));
+	ut_asserteq(8, ret);
+	ut_assert(!unicode_test_u16_strcmp(buf, src, 11));
+
+	/* src is empty string */
+	memset(buf, 0, sizeof(buf));
+	memcpy(buf, dest, sizeof(dest));
+	ret = u16_strlcat(buf, &null_src, sizeof(buf));
+	ut_asserteq(12, ret);
+	ut_assert(!unicode_test_u16_strcmp(buf, dest, 11));
+
+	/* size is smaller than dest size */
+	memset(buf, 0, sizeof(buf));
+	memcpy(buf, dest, sizeof(dest));
+	ret = u16_strlcat(buf, src, 6);
+	ut_asserteq(14, ret);
+	ut_assert(!unicode_test_u16_strcmp(buf, dest, 11));
+
+	/* size is insufficient to append src string */
+	memset(buf, 0, sizeof(buf));
+	memcpy(buf, dest, sizeof(dest));
+	ret = u16_strlcat(buf, src, 20);
+	ut_asserteq(20, ret);
+	ut_assert(!unicode_test_u16_strcmp(buf, u"U-Boottes", 11));
+
+	return 0;
+}
+UNICODE_TEST(unicode_test_u16_strlcat);
+
 int do_ut_unicode(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
 	struct unit_test *tests = UNIT_TEST_SUITE_START(unicode_test);
-- 
2.17.1

[RESEND RFC PATCH v2 4/4] efi_loader: add menu-driven UEFI Boot Variable maintenance

[Masahisa Kojima]

This commit adds the menu-driven UEFI Boot Variable maintenance.
User can add and delete the Boot#### variable, and update the
BootOrder variable through menu operation.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Changes in 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

 include/efi_loader.h          |  27 ++
 lib/efi_loader/efi_bootmgr.c  | 687 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_boottime.c |  55 +--
 lib/efi_loader/efi_console.c  |  81 ++++
 lib/efi_loader/efi_file.c     |  74 ++--
 5 files changed, 876 insertions(+), 48 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index e390d323a9..0623e0a707 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -278,6 +278,7 @@ extern const efi_guid_t efi_guid_loaded_image;
 extern const efi_guid_t efi_guid_loaded_image_device_path;
 extern const efi_guid_t efi_guid_device_path_to_text_protocol;
 extern const efi_guid_t efi_simple_file_system_protocol_guid;
+extern const efi_guid_t efi_system_partition_guid;
 extern const efi_guid_t efi_file_info_guid;
 /* GUID for file system information */
 extern const efi_guid_t efi_file_system_info_guid;
@@ -310,6 +311,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[];
@@ -998,4 +1002,27 @@ 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);
+
 #endif /* _EFI_LOADER_H */
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index bbb3fac5bd..b7d1fa6f4b 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -20,6 +20,9 @@
 static const struct efi_boot_services *bs;
 static const struct efi_runtime_services *rs;
 
+static struct efi_simple_text_input_protocol *cin;
+static struct efi_simple_text_output_protocol *cout;
+
 /*
  * bootmgr implements the logic of trying to find a payload to boot
  * based on the BootOrder + BootXXXX variables, and then loading it.
@@ -30,6 +33,9 @@ static const struct efi_runtime_services *rs;
  */
 
 #define EFI_BOOTMGR_MENU_ENTRY_NUM_MAX 1024
+#define EFI_BOOTMGR_FILE_PATH_MAX 512
+#define EFI_BOOTMGR_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);
 
@@ -83,12 +89,49 @@ struct efi_bootmgr_boot_selection_data {
 
 static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool *exit);
 static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit);
+static efi_status_t efi_bootmgr_process_maintenance(void *data, bool *exit);
+static efi_status_t efi_bootmgr_process_add_boot_option(void *data, bool *exit);
+static efi_status_t efi_bootmgr_process_delete_boot_option(void *data, bool *exit);
+static efi_status_t efi_bootmgr_process_change_boot_order(void *data, bool *exit);
 
 static struct efi_bootmgr_menu_item bootmgr_menu_items[] = {
 	{u"Boot Manager", efi_bootmgr_process_boot_selection},
+	{u"Boot Manager maintenance", efi_bootmgr_process_maintenance},
 	{u"Quit", NULL},
 };
 
+static struct efi_bootmgr_menu_item maintenance_menu_items[] = {
+	{u"Add Boot Option", efi_bootmgr_process_add_boot_option},
+	{u"Delete Boot Option", efi_bootmgr_process_delete_boot_option},
+	{u"Change Boot Order", efi_bootmgr_process_change_boot_order},
+	{u"Quit", NULL},
+};
+
+struct efi_bootmgr_boot_option {
+	struct efi_simple_file_system_protocol *current_volume;
+	struct efi_device_path *dp_volume;
+	u16 *current_path;
+	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),
+};
+
+struct efi_bootmgr_volume_entry_data {
+	struct efi_bootmgr_boot_option *bo;
+	struct efi_simple_file_system_protocol *v;
+	struct efi_device_path *dp;
+};
+
+struct efi_bootmgr_file_entry_data {
+	struct efi_bootmgr_boot_option *bo;
+	struct efi_file_info *f;
+};
+
 static void efi_bootmgr_menu_print_entry(void *data)
 {
 	struct efi_bootmgr_menu_entry *entry = data;
@@ -418,6 +461,646 @@ static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit)
 	return ret;
 }
 
+static efi_status_t efi_bootmgr_volume_selected(void *data, bool *exit)
+{
+	struct efi_bootmgr_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_bootmgr_file_selected(void *data, bool *exit)
+{
+	struct efi_bootmgr_file_entry_data *info = data;
+
+	*exit = true;
+
+	if (!info)
+		return EFI_INVALID_PARAMETER;
+
+	if (u16_strncmp(info->f->file_name, u".", 1) == 0 &&
+	    u16_strlen(info->f->file_name) == 1) {
+		/* stay current path */
+	} else if (u16_strncmp(info->f->file_name, u"..", 2) == 0 &&
+		   u16_strlen(info->f->file_name) == 2) {
+		u32 i;
+		int len = u16_strlen(info->bo->current_path);
+
+		for (i = len - 2; i > 0; i--) {
+			if (info->bo->current_path[i] == u'\\')
+				break;
+		}
+
+		if (i == 0)
+			info->bo->current_path[0] = u'\0';
+		else
+			info->bo->current_path[i + 1] = u'\0';
+	} else {
+		size_t new_len;
+
+		new_len = u16_strlen(info->bo->current_path) +
+				     u16_strlen(info->f->file_name) + 1;
+		if (new_len >= EFI_BOOTMGR_FILE_PATH_MAX) { /* TODO: try with 4K size */
+			/* 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->f->file_name, EFI_BOOTMGR_FILE_PATH_MAX);
+		if (info->f->attribute & EFI_FILE_DIRECTORY) {
+			if (new_len + 1 >= EFI_BOOTMGR_FILE_PATH_MAX) {
+				log_err("file path is too long\n");
+				return EFI_INVALID_PARAMETER;
+			}
+			u16_strlcat(info->bo->current_path, u"\\", EFI_BOOTMGR_FILE_PATH_MAX);
+		} else {
+			info->bo->file_selected = true;
+		}
+	}
+	return EFI_SUCCESS;
+}
+
+static efi_status_t efi_bootmgr_select_volume(struct efi_bootmgr_boot_option *bo)
+{
+	u16 *name;
+	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_device_path_to_text_protocol *text;
+	struct efi_bootmgr_menu_item *menu_item, *iter;
+
+	ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_system_partition_guid,
+					   NULL, &count, (efi_handle_t **)&volume_handles);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	ret = efi_search_protocol(efi_root, &efi_guid_device_path_to_text_protocol, &handler);
+	if (ret != EFI_SUCCESS)
+		goto out1;
+
+	ret = efi_protocol_open(handler, (void **)&text, efi_root, NULL,
+				EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+	if (ret != EFI_SUCCESS)
+		goto out1;
+
+	menu_item = calloc(count + 1, sizeof(struct efi_bootmgr_menu_item));
+	if (!menu_item) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out1;
+	}
+
+	iter = menu_item;
+	for (i = 0; i < count; i++) {
+		struct efi_bootmgr_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;
+
+		name = EFI_CALL(text->convert_device_path_to_text(device_path, true, true));
+		if (!name) {
+			ret = EFI_OUT_OF_RESOURCES;
+			goto out2;
+		}
+
+		info = calloc(1, sizeof(struct efi_bootmgr_volume_entry_data));
+		if (!info) {
+			ret = EFI_OUT_OF_RESOURCES;
+			goto out2;
+		}
+
+		info->v = v;
+		info->dp = device_path;
+		info->bo = bo;
+		iter->title = name;
+		iter->func = efi_bootmgr_volume_selected;
+		iter->data = info;
+		iter++;
+	}
+
+	iter->title = u"Quit";
+	iter->func = NULL;
+	iter->data = NULL;
+	count += 1;
+
+	ret = efi_bootmgr_process_common(menu_item, count, -1);
+
+out2:
+	iter = menu_item;
+	for (i = 0; i < count - 1; i++) {
+		struct efi_bootmgr_volume_entry_data *p;
+
+		p = (struct efi_bootmgr_volume_entry_data *)(iter->data);
+		efi_free_pool(iter->title);
+		free(p);
+		iter++;
+	}
+
+	free(menu_item);
+
+out1:
+	efi_free_pool(volume_handles);
+
+	return ret;
+}
+
+static efi_status_t efi_bootmgr_select_file(struct efi_bootmgr_boot_option *bo,
+					    struct efi_file_handle *root)
+{
+	char *buf;
+	u32 i;
+	char *dir_buf;
+	efi_uintn_t len;
+	efi_status_t ret;
+	efi_uintn_t size;
+	u32 count = 0;
+	struct efi_file_handle *f;
+	struct efi_file_info *ptr;
+	struct efi_bootmgr_menu_item *menu_item, *iter;
+
+	buf = calloc(1, EFI_BOOTMGR_FILE_PATH_MAX);
+	if (!buf)
+		return EFI_OUT_OF_RESOURCES;
+
+	while (!bo->file_selected) {
+		size = 0;
+		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 size */
+		for (;;) {
+			len = EFI_BOOTMGR_FILE_PATH_MAX;
+			ret = efi_file_read_int(f, &len, buf);
+			if (ret != EFI_SUCCESS || len == 0)
+				break;
+
+			size += len;
+			count++;
+		}
+
+		dir_buf = calloc(1, size);
+		if (!dir_buf) {
+			efi_file_close_int(f);
+			ret = EFI_OUT_OF_RESOURCES;
+			goto out;
+		}
+		menu_item = calloc(count + 1, sizeof(struct efi_bootmgr_menu_item));
+		if (!menu_item) {
+			efi_file_close_int(f);
+			free(dir_buf);
+			ret = EFI_OUT_OF_RESOURCES;
+			goto out;
+		}
+
+		/* read directory and construct menu structure */
+		efi_file_setpos_int(f, 0);
+		iter = menu_item;
+		ptr = (struct efi_file_info *)dir_buf;
+		for (i = 0; i < count; i++) {
+			int name_len;
+			u16 *name;
+			struct efi_bootmgr_file_entry_data *info;
+
+			len = size;
+			ret = efi_file_read_int(f, &len, ptr);
+			if (ret != EFI_SUCCESS || len == 0)
+				goto err;
+
+			if (ptr->attribute & EFI_FILE_DIRECTORY) {
+				/* append u'/' at the end of directory name */
+				name_len = u16_strsize(ptr->file_name) + sizeof(u16);
+				name = calloc(1, name_len);
+				if (!name) {
+					ret = EFI_OUT_OF_RESOURCES;
+					goto err;
+				}
+				u16_strcpy(name, ptr->file_name);
+				name[u16_strlen(ptr->file_name)] = u'/';
+			} else {
+				name_len = u16_strsize(ptr->file_name);
+				name = calloc(1, name_len);
+				if (!name) {
+					ret = EFI_OUT_OF_RESOURCES;
+					goto err;
+				}
+				u16_strcpy(name, ptr->file_name);
+			}
+
+			info = calloc(1, sizeof(struct efi_bootmgr_file_entry_data));
+			if (!info) {
+				ret = EFI_OUT_OF_RESOURCES;
+				goto err;
+			}
+			info->f = ptr;
+			info->bo = bo;
+			iter->title = name;
+			iter->func = efi_bootmgr_file_selected;
+			iter->data = info;
+			iter++;
+
+			size -= len;
+			ptr = (struct efi_file_info *)((char *)ptr + len);
+		}
+
+		/* add "Quit" entry */
+		iter->title = u"Quit";
+		iter->func = NULL;
+		iter->data = NULL;
+		count += 1;
+
+		ret = efi_bootmgr_process_common(menu_item, count, -1);
+err:
+		efi_file_close_int(f);
+		iter = menu_item;
+		for (i = 0; i < count - 1; i++, iter++) {
+			free(iter->title);
+			free(iter->data);
+		}
+
+		free(dir_buf);
+		free(menu_item);
+
+		if (ret != EFI_SUCCESS)
+			break;
+	}
+
+out:
+	free(buf);
+	return ret;
+}
+
+static efi_status_t efi_bootmgr_boot_add_enter_name(struct efi_bootmgr_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_BOOTMGR_BOOT_NAME_MAX, NULL, 4, 15);
+
+	return ret;
+}
+
+static efi_status_t allow_decimal(struct efi_input_key *key)
+{
+	if (u'0' <= key->unicode_char && key->unicode_char <= u'9')
+		return EFI_SUCCESS;
+
+	return EFI_INVALID_PARAMETER;
+}
+
+static efi_status_t efi_bootmgr_change_boot_order(int selected, int max, int *new)
+{
+	efi_status_t ret;
+	u16 new_order[EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL] = {0};
+
+	printf(ANSI_CURSOR_POSITION, 2, 1);
+	puts("  *** U-Boot EFI Boot Manager Menu ***");
+	printf(ANSI_CURSOR_POSITION, 4, 1);
+	printf("  current boot order      : %d", selected);
+
+	printf(ANSI_CURSOR_POSITION, 6, 1);
+	printf("  new boot order(0 - %4d): ", max);
+
+	printf(ANSI_CURSOR_POSITION, 8, 1);
+	puts("  ENTER to complete, ESC/CTRL+C to quit");
+
+	printf(ANSI_CURSOR_POSITION, 6, 29);
+	puts(ANSI_CURSOR_SHOW);
+
+	for (;;) {
+		memset(new_order, 0, sizeof(new_order));
+		ret = efi_console_get_u16_string(cin, cout, new_order, 6, allow_decimal, 6, 29);
+		if (ret == EFI_SUCCESS) {
+			int i;
+			int val = 0;
+
+			for (i = 0;
+			     i < u16_strnlen(new_order, EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL - 1);
+			     i++)
+				val = (val * 10) + (new_order[i] - u'0');
+
+			if (val > max)
+				continue;
+
+			*new = val;
+			return EFI_SUCCESS;
+		} else {
+			return ret;
+		}
+	}
+}
+
+static efi_status_t efi_bootmgr_select_file_handler(struct efi_bootmgr_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, sizeof(bo->current_path));
+
+		ret = efi_bootmgr_select_volume(bo);
+		if (ret != EFI_SUCCESS)
+			goto out;
+
+		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_bootmgr_select_file(bo, root);
+
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
+	ret = efi_bootmgr_boot_add_enter_name(bo);
+
+out:
+	return ret;
+}
+
+static efi_status_t efi_bootmgr_process_maintenance(void *data, bool *exit)
+{
+	return efi_bootmgr_process_common(maintenance_menu_items,
+					  ARRAY_SIZE(maintenance_menu_items),
+					  -1);
+}
+
+static efi_status_t efi_bootmgr_process_add_boot_option(void *data, bool *exit)
+{
+	u32 index;
+	void *p = NULL;
+	u16 var_name[9];
+	char *buf = NULL;
+	efi_status_t ret;
+	char *iter = NULL;
+	u16 *bootorder = NULL;
+	u16 *new_bootorder = NULL;
+	struct efi_load_option lo;
+	efi_uintn_t dp_size, fp_size;
+	efi_uintn_t last, size, new_size;
+	struct efi_bootmgr_boot_option bo;
+	struct efi_device_path_file_path *fp;
+
+	/* get unused Boot#### */
+	for (index = 0; index <= 0xFFFF; index++) {
+		size = 0;
+		efi_create_indexed_name(var_name, sizeof(var_name), "Boot", index);
+		ret = efi_get_variable_int(var_name, &efi_global_variable_guid,
+					   NULL, &size, NULL, NULL);
+		if (ret == EFI_BUFFER_TOO_SMALL)
+			continue;
+		else
+			break;
+	}
+
+	if (index >= 0xFFFF)
+		return EFI_OUT_OF_RESOURCES;
+
+	efi_create_indexed_name(var_name, sizeof(var_name), "Boot", index);
+
+	bo.current_path = calloc(1, EFI_BOOTMGR_FILE_PATH_MAX);
+	if (!bo.current_path)
+		goto out;
+
+	bo.boot_name = calloc(1, EFI_BOOTMGR_BOOT_NAME_MAX * sizeof(u16));
+	if (!bo.boot_name)
+		goto out;
+
+	ret = efi_bootmgr_select_file_handler(&bo);
+	if (ret == EFI_ABORTED)
+		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;
+
+	lo.file_path = (struct efi_device_path *)buf;
+	lo.file_path_length = efi_dp_size((struct efi_device_path *)buf) + sizeof(END);
+	lo.attributes = LOAD_OPTION_ACTIVE;
+	lo.optional_data = NULL;
+	lo.label = bo.boot_name;
+
+	size = efi_serialize_load_option(&lo, (u8 **)&p);
+	if (!size) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	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);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	/* 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] = (u16)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(p);
+	free(buf);
+	free(bootorder);
+	free(new_bootorder);
+	free(bo.boot_name);
+	free(bo.current_path);
+
+	return ret;
+}
+
+static efi_status_t efi_bootmgr_process_delete_boot_option(void *data, bool *exit)
+{
+	int selected;
+	u16 *bootorder;
+	u16 var_name[9];
+	efi_status_t ret;
+	efi_uintn_t num, size;
+
+	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
+	if (!bootorder) {
+		ret = EFI_NOT_FOUND;
+		return ret;
+	}
+
+	num = size / sizeof(u16);
+	ret = efi_bootmgr_show_boot_selection(bootorder, num, &selected);
+	if (ret == EFI_SUCCESS) {
+		/* delete selected boot option */
+		efi_create_indexed_name(var_name, sizeof(var_name),
+					"Boot", bootorder[selected]);
+		ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
+					   0, 0, NULL, false);
+		if (ret != EFI_SUCCESS) {
+			log_err("delete boot option(%ls) failed\n", var_name);
+			goto out;
+		}
+
+		/* update BootOrder */
+		memmove(&bootorder[selected], &bootorder[selected + 1],
+			(num - selected - 1) * sizeof(u16));
+		size -= sizeof(u16);
+		ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
+					   EFI_VARIABLE_NON_VOLATILE |
+					   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+					   EFI_VARIABLE_RUNTIME_ACCESS,
+					   size, bootorder, false);
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
+out:
+	free(bootorder);
+
+	return ret;
+}
+
+static efi_status_t efi_bootmgr_process_change_boot_order(void *data, bool *exit)
+{
+	int selected;
+	int new_order;
+	efi_status_t ret;
+	efi_uintn_t num, size;
+	u16 *bootorder = NULL;
+	u16 *new_bootorder = NULL;
+
+	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
+	if (!bootorder)
+		return EFI_NOT_FOUND;
+
+	num = size / sizeof(u16);
+	ret = efi_bootmgr_show_boot_selection(bootorder, num, &selected);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = efi_bootmgr_change_boot_order(selected, num - 1, &new_order);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	new_bootorder = calloc(1, size);
+	if (!new_bootorder)
+		goto out;
+
+	memcpy(new_bootorder, bootorder, size);
+	if (selected > new_order) {
+		new_bootorder[new_order] = bootorder[selected];
+		memcpy(&new_bootorder[new_order + 1], &bootorder[new_order],
+		       (selected - new_order) * sizeof(u16));
+	} else if (selected < new_order) {
+		new_bootorder[new_order] = bootorder[selected];
+		memcpy(&new_bootorder[selected], &bootorder[selected + 1],
+		       (new_order - selected) * sizeof(u16));
+	} else {
+		/* nothing to change */
+		goto out;
+	}
+	ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid,
+				   EFI_VARIABLE_NON_VOLATILE |
+				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				   EFI_VARIABLE_RUNTIME_ACCESS,
+				   size, new_bootorder, false);
+out:
+	free(new_bootorder);
+	free(bootorder);
+
+	return ret;
+}
+
+efi_status_t efi_bootmgr_menu_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;
+}
+
 /**
  * try_load_entry() - try to load image for boot option
  *
@@ -568,6 +1251,10 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
 	if (IS_ENABLED(CONFIG_EFI_BOOT_MENU)) {
 		int selected;
 
+		ret = efi_bootmgr_menu_init();
+		if (ret != EFI_SUCCESS)
+			goto error;
+
 		bootmgr_menu_items[0].data = &selected;
 		ret = efi_bootmgr_process_common(bootmgr_menu_items,
 						 ARRAY_SIZE(bootmgr_menu_items),
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 82128ac1d5..c3c5f299ee 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2454,32 +2454,13 @@ static efi_status_t EFIAPI efi_protocols_per_handle(
 	return EFI_EXIT(EFI_SUCCESS);
 }
 
-/**
- * efi_locate_handle_buffer() - locate handles implementing a protocol
- * @search_type: selection criterion
- * @protocol:    GUID of the protocol
- * @search_key:  registration key
- * @no_handles:  number of returned handles
- * @buffer:      buffer with the returned handles
- *
- * This function implements the LocateHandleBuffer service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * Return: status code
- */
-efi_status_t EFIAPI efi_locate_handle_buffer(
-			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_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;
 
-	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;
@@ -2499,6 +2480,36 @@ efi_status_t EFIAPI efi_locate_handle_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
+ * @protocol:    GUID of the protocol
+ * @search_key:  registration key
+ * @no_handles:  number of returned handles
+ * @buffer:      buffer with the returned handles
+ *
+ * This function implements the LocateHandleBuffer service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * Return: status code
+ */
+efi_status_t EFIAPI efi_locate_handle_buffer(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_ENTRY("%d, %pUs, %p, %p, %p", search_type, protocol, search_key,
+		  no_handles, buffer);
+
+	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 ba68a15017..f5002e1c99 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_file.c b/lib/efi_loader/efi_file.c
index 7a7077e6d0..d2d7e8495d 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,24 @@ 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

Re: [RESEND RFC PATCH v2 1/4] efi_loader: add menu-driven boot device selection

[Heinrich Schuchardt]

On 2/25/22 02:32, Masahisa Kojima wrote:
> This patch enables the menu-driven boot device selection.
> User can select the Boot#### included in BootOrder variable.
>
> If user quits thie menu, or the selected Boot#### fails to boot,
> efi bootmgr continues to boot in accordance with BootOrder variable.
>
> This commit also moves the user input handling from cmd/bootmenu.c
> to common/menu.c to reuse it from efi boot menu.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

With the series applied and CONFIG_EFI_BOOT_MENU=y I first see a line
"Hit any key to stop autoboot:" with a count down and then the UEFI menu
with a countdown. This duplication of countdowns does not make sense.

This is the content of the new menu:

   *** U-Boot EFI Boot Manager ***

      Boot Manager
      Boot Manager maintenance

      Quit

When selecting " Boot Manager maintenance" I see a second menu:

   *** U-Boot EFI Boot Manager ***

      Add Boot Option
      Delete Boot Option
      Change Boot Order

      Quit

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

When I select "Add Boot Option" the system (qemu_arm64_defconfig) simply
hangs.

Why do we need the first menu? Compare it to your laptop experience when
pressing the boot menu key (often <F12> or <F8>).

Can't we have a simple countdown:

"Hit any key for boot menu"

If a key is hit show a menu showing the different boot options and the
distro-boot devices:

Debian (0001)
Ubuntu (0002)
SATA(1)
SATA(2)
NVMe(1)
Boot manager settings
Command line

Simon has proposed a patch series "Initial implementation of standard
boot",
https://patchwork.ozlabs.org/project/uboot/list/?series=281745&state=*.
I think the proposed menu would have to iterate through all active boot
methods.

> ---
> Changes in v2:
> - move user input handling from cmd/bootmenu.c to common/menu.c
> - keyboard buffer is drained before asking question in
>    common/menu.c::bootmenu_autoboot_loop()
> - Add EFI_BOOT_MENU_DELAY Kconfig option to set delay for
>    autoboot and disable autoboot
> - remove unnecessary "autoboot" member in struc efi_bootmgr_menu
>
>   cmd/bootmenu.c               | 145 ------------
>   common/menu.c                | 137 ++++++++++++
>   include/menu.h               |  20 ++
>   lib/efi_loader/Kconfig       |  20 ++
>   lib/efi_loader/efi_bootmgr.c | 418 ++++++++++++++++++++++++++++++++++-
>   5 files changed, 592 insertions(+), 148 deletions(-)
>
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index 409ef9a848..f9fdebc450 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -33,21 +33,6 @@ struct bootmenu_entry {
>   	struct bootmenu_entry *next;	/* next menu entry (num+1) */
>   };
>
> -struct bootmenu_data {
> -	int delay;			/* delay for autoboot */
> -	int active;			/* active menu entry */
> -	int count;			/* total count of menu entries */
> -	struct bootmenu_entry *first;	/* first menu entry */
> -};
> -
> -enum bootmenu_key {
> -	KEY_NONE = 0,
> -	KEY_UP,
> -	KEY_DOWN,
> -	KEY_SELECT,
> -	KEY_QUIT,
> -};
> -
>   static char *bootmenu_getoption(unsigned short int n)
>   {
>   	char name[MAX_ENV_SIZE];
> @@ -81,136 +66,6 @@ static void bootmenu_print_entry(void *data)
>   		puts(ANSI_COLOR_RESET);
>   }
>
> -static void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> -				enum bootmenu_key *key, int *esc)
> -{
> -	int i, c;
> -
> -	if (menu->delay > 0) {
> -		printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> -		printf("  Hit any key to stop autoboot: %2d ", menu->delay);
> -	}
> -
> -	while (menu->delay > 0) {
> -		for (i = 0; i < 100; ++i) {
> -			if (!tstc()) {
> -				WATCHDOG_RESET();
> -				mdelay(10);
> -				continue;
> -			}
> -
> -			menu->delay = -1;
> -			c = getchar();
> -
> -			switch (c) {
> -			case '\e':
> -				*esc = 1;
> -				*key = KEY_NONE;
> -				break;
> -			case '\r':
> -				*key = KEY_SELECT;
> -				break;
> -			case 0x3: /* ^C */
> -				*key = KEY_QUIT;
> -				break;
> -			default:
> -				*key = KEY_NONE;
> -				break;
> -			}
> -
> -			break;
> -		}
> -
> -		if (menu->delay < 0)
> -			break;
> -
> -		--menu->delay;
> -		printf("\b\b\b%2d ", menu->delay);
> -	}
> -
> -	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> -	puts(ANSI_CLEAR_LINE);
> -
> -	if (menu->delay == 0)
> -		*key = KEY_SELECT;
> -}
> -
> -static void bootmenu_loop(struct bootmenu_data *menu,
> -		enum bootmenu_key *key, int *esc)
> -{
> -	int c;
> -
> -	if (*esc == 1) {
> -		if (tstc()) {
> -			c = getchar();
> -		} else {
> -			WATCHDOG_RESET();
> -			mdelay(10);
> -			if (tstc())
> -				c = getchar();
> -			else
> -				c = '\e';
> -		}
> -	} else {
> -		while (!tstc()) {
> -			WATCHDOG_RESET();
> -			mdelay(10);
> -		}
> -		c = getchar();
> -	}
> -
> -	switch (*esc) {
> -	case 0:
> -		/* First char of ANSI escape sequence '\e' */
> -		if (c == '\e') {
> -			*esc = 1;
> -			*key = KEY_NONE;
> -		}
> -		break;
> -	case 1:
> -		/* Second char of ANSI '[' */
> -		if (c == '[') {
> -			*esc = 2;
> -			*key = KEY_NONE;
> -		} else {
> -		/* Alone ESC key was pressed */
> -			*key = KEY_QUIT;
> -			*esc = (c == '\e') ? 1 : 0;
> -		}
> -		break;
> -	case 2:
> -	case 3:
> -		/* Third char of ANSI (number '1') - optional */
> -		if (*esc == 2 && c == '1') {
> -			*esc = 3;
> -			*key = KEY_NONE;
> -			break;
> -		}
> -
> -		*esc = 0;
> -
> -		/* ANSI 'A' - key up was pressed */
> -		if (c == 'A')
> -			*key = KEY_UP;
> -		/* ANSI 'B' - key down was pressed */
> -		else if (c == 'B')
> -			*key = KEY_DOWN;
> -		/* other key was pressed */
> -		else
> -			*key = KEY_NONE;
> -
> -		break;
> -	}
> -
> -	/* enter key was pressed */
> -	if (c == '\r')
> -		*key = KEY_SELECT;
> -
> -	/* ^C was pressed */
> -	if (c == 0x3)
> -		*key = KEY_QUIT;
> -}
> -
>   static char *bootmenu_choice_entry(void *data)
>   {
>   	struct bootmenu_data *menu = data;
> diff --git a/common/menu.c b/common/menu.c
> index 5fb2ffbd06..786e45f217 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -4,11 +4,14 @@
>    * Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
>    */
>

common/menu.c is only compiled if CONFIG_MENU=y but CONFIG_EFI_BOOT_MENU
does not select it. So depending on configuration this simply does not link:

lib/efi_loader/efi_bootmgr.c:315: undefined reference to `menu_create'
aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:330: undefined
reference to `menu_default_set'
aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:351: undefined
reference to `menu_destroy'
aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:324: undefined
reference to `menu_item_add'
aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:337: undefined
reference to `menu_get_choice'
aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.o: in function
`efi_bootmgr_menu_choice_entry':
lib/efi_loader/efi_bootmgr.c:202: undefined reference to
`bootmenu_autoboot_loop'
aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:205: undefined
reference to `bootmenu_loop'
aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.o: in function
`efi_bootmgr_menu_display_statusline':
lib/efi_loader/efi_bootmgr.c:168: undefined reference to
`menu_default_choice

> +#include <ansi.h>
>   #include <common.h>
>   #include <cli.h>
>   #include <malloc.h>
>   #include <errno.h>
> +#include <linux/delay.h>
>   #include <linux/list.h>
> +#include <watchdog.h>
>
>   #include "menu.h"
>
> @@ -418,3 +421,137 @@ int menu_destroy(struct menu *m)
>
>   	return 1;
>   }
> +
> +void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> +			    enum bootmenu_key *key, int *esc)
> +{
> +	int i, c;
> +
> +	if (menu->delay > 0) {
> +		/* flush input */
> +		while (tstc())
> +			getchar();
> +
> +		printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> +		printf("  Hit any key to stop autoboot: %2d ", menu->delay);
> +	}
> +
> +	while (menu->delay > 0) {
> +		for (i = 0; i < 100; ++i) {
> +			if (!tstc()) {
> +				WATCHDOG_RESET();
> +				mdelay(10);
> +				continue;
> +			}
> +
> +			menu->delay = -1;
> +			c = getchar();
> +
> +			switch (c) {
> +			case '\e':
> +				*esc = 1;
> +				*key = KEY_NONE;
> +				break;
> +			case '\r':
> +				*key = KEY_SELECT;
> +				break;
> +			case 0x3: /* ^C */
> +				*key = KEY_QUIT;
> +				break;
> +			default:
> +				*key = KEY_NONE;
> +				break;
> +			}
> +
> +			break;
> +		}
> +
> +		if (menu->delay < 0)
> +			break;
> +
> +		--menu->delay;
> +		printf("\b\b\b%2d ", menu->delay);
> +	}
> +
> +	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> +	puts(ANSI_CLEAR_LINE);
> +
> +	if (menu->delay == 0)
> +		*key = KEY_SELECT;
> +}
> +
> +void bootmenu_loop(struct bootmenu_data *menu,
> +		   enum bootmenu_key *key, int *esc)
> +{
> +	int c;
> +
> +	if (*esc == 1) {
> +		if (tstc()) {
> +			c = getchar();
> +		} else {
> +			WATCHDOG_RESET();
> +			mdelay(10);
> +			if (tstc())
> +				c = getchar();
> +			else
> +				c = '\e';
> +		}
> +	} else {
> +		while (!tstc()) {
> +			WATCHDOG_RESET();
> +			mdelay(10);
> +		}
> +		c = getchar();
> +	}
> +
> +	switch (*esc) {
> +	case 0:
> +		/* First char of ANSI escape sequence '\e' */
> +		if (c == '\e') {
> +			*esc = 1;
> +			*key = KEY_NONE;
> +		}
> +		break;
> +	case 1:
> +		/* Second char of ANSI '[' */
> +		if (c == '[') {
> +			*esc = 2;
> +			*key = KEY_NONE;
> +		} else {
> +		/* Alone ESC key was pressed */
> +			*key = KEY_QUIT;
> +			*esc = (c == '\e') ? 1 : 0;
> +		}
> +		break;
> +	case 2:
> +	case 3:
> +		/* Third char of ANSI (number '1') - optional */
> +		if (*esc == 2 && c == '1') {
> +			*esc = 3;
> +			*key = KEY_NONE;
> +			break;
> +		}
> +
> +		*esc = 0;
> +
> +		/* ANSI 'A' - key up was pressed */
> +		if (c == 'A')
> +			*key = KEY_UP;
> +		/* ANSI 'B' - key down was pressed */
> +		else if (c == 'B')
> +			*key = KEY_DOWN;
> +		/* other key was pressed */
> +		else
> +			*key = KEY_NONE;
> +
> +		break;
> +	}
> +
> +	/* enter key was pressed */
> +	if (c == '\r')
> +		*key = KEY_SELECT;
> +
> +	/* ^C was pressed */
> +	if (c == 0x3)
> +		*key = KEY_QUIT;
> +}
> diff --git a/include/menu.h b/include/menu.h
> index ad5859437e..e74616cae8 100644
> --- a/include/menu.h
> +++ b/include/menu.h
> @@ -35,4 +35,24 @@ int menu_default_choice(struct menu *m, void **choice);
>    */
>   int menu_show(int bootdelay);
>
> +struct bootmenu_data {
> +	int delay;			/* delay for autoboot */
> +	int active;			/* active menu entry */
> +	int count;			/* total count of menu entries */
> +	struct bootmenu_entry *first;	/* first menu entry */
> +};
> +
> +enum bootmenu_key {
> +	KEY_NONE = 0,
> +	KEY_UP,
> +	KEY_DOWN,
> +	KEY_SELECT,
> +	KEY_QUIT,
> +};
> +
> +void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> +			    enum bootmenu_key *key, int *esc);
> +void bootmenu_loop(struct bootmenu_data *menu,
> +		   enum bootmenu_key *key, int *esc);
> +
>   #endif /* __MENU_H__ */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e5e35fe51f..a3fb8b2a75 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -39,6 +39,26 @@ config CMD_BOOTEFI_BOOTMGR
>   	  via UEFI variables Boot####, BootOrder, and BootNext. This enables the
>   	  'bootefi bootmgr' command.
>
> +config EFI_BOOT_MENU
> +	bool "UEFI Boot Menu driven boot device selection"
> +	default n
> +	help
> +	  Select this option if you want to enable the menu driven boot device
> +	  selection. This menu provides the functionality to select a boot
> +	  option to start, and allow users to edit Boot#### and BootOrder.
> +	  If this menu is enabled, CLI can be disabled if the system boots
> +	  via UEFI variable Boot#### and BootOrder.

We end up with these symbols for menus:

CONFIG_MENU
CONFIG_AUTOBOOT_MENUKEY
CONFIG_AUTOBOOT_MENU_SHOW
CONFIG_BOOTDELAY
CONFIG_EFI_BOOT_MENU
CONFIG_EFI_BOOT_MENU_DELAY

Defining both CONFIG_AUTOBOOT_MENU_SHOW and CONFIG_EFI_BOOT_MENU does
not make much sense. Same for CONFIG_BOOTDELAY and
CONFIG_EFI_BOOT_MENU_DELAY. Please, unifiy the settings.

Best regards

Heinrich

> +
> +config EFI_BOOT_MENU_DELAY
> +	int "delay in seconds before automatically booting in UEFI Boot Menu"
> +	default 10
> +	range -1 2147483647
> +	help
> +	  Delay before automatically booting in accordance with
> +	  "BootOrder" variable.
> +	  set to 0 to autoboot with no delay.
> +	  set to -1 to disable autoboot.
> +
>   config EFI_SETUP_EARLY
>   	bool
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 8c04ecbdc8..bbb3fac5bd 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -7,10 +7,12 @@
>
>   #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>
> @@ -22,14 +24,400 @@ static const struct efi_runtime_services *rs;
>    * bootmgr implements the logic of trying to find a payload to boot
>    * based on the BootOrder + BootXXXX variables, and then loading it.
>    *
> - * TODO detecting a special key held (f9?) and displaying a boot menu
> - * like you would get on a PC would be clever.
> - *
>    * TODO if we had a way to write and persist variables after the OS
>    * has started, we'd also want to check OsIndications to see if we
>    * should do normal or recovery boot.
>    */
>
> +#define EFI_BOOTMGR_MENU_ENTRY_NUM_MAX 1024
> +
> +typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
> +
> +/**
> + * struct efi_bootmgr_menu_entry - menu entry structure
> + *
> + * @menu_index:		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_bootmgr_menu_entry {
> +	u32 menu_index;
> +	u16 *title;
> +	char key[6];
> +	struct efi_bootmgr_menu *bootmgr_menu;
> +	struct efi_bootmgr_menu_entry *next;
> +	efi_bootmenu_entry_func func;
> +	void *data;
> +};
> +
> +/**
> + * struct efi_bootmgr_menuy - 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_bootmgr_menu {
> +	int delay;
> +	int active;
> +	int count;
> +	struct efi_bootmgr_menu_entry *first;
> +};
> +
> +struct efi_bootmgr_menu_item {
> +	u16 *title;
> +	efi_bootmenu_entry_func func;
> +	void *data;
> +};
> +
> +struct efi_bootmgr_boot_selection_data {
> +	u16 bootorder_index;
> +	void *load_option;
> +	int *selected;
> +};
> +
> +static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool *exit);
> +static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit);
> +
> +static struct efi_bootmgr_menu_item bootmgr_menu_items[] = {
> +	{u"Boot Manager", efi_bootmgr_process_boot_selection},
> +	{u"Quit", NULL},
> +};
> +
> +static void efi_bootmgr_menu_print_entry(void *data)
> +{
> +	struct efi_bootmgr_menu_entry *entry = data;
> +	int reverse = (entry->bootmgr_menu->active == entry->menu_index);
> +
> +	/* TODO: support scroll or page for many entries */
> +
> +	/*
> +	 * Move cursor to line where the entry will be drown (entry->count)
> +	 * First 3 lines contain bootmgr menu header + one empty line
> +	 * For the last "Quit" entry, add one empty line
> +	 */
> +	if (entry->menu_index == (entry->bootmgr_menu->count - 1))
> +		printf(ANSI_CURSOR_POSITION, entry->menu_index + 5, 1);
> +	else
> +		printf(ANSI_CURSOR_POSITION, entry->menu_index + 4, 1);
> +
> +	puts("     ");
> +
> +	if (reverse)
> +		puts(ANSI_COLOR_REVERSE);
> +
> +	printf("%ls", entry->title);
> +
> +	if (reverse)
> +		puts(ANSI_COLOR_RESET);
> +}
> +
> +static void efi_bootmgr_menu_display_statusline(struct menu *m)
> +{
> +	struct efi_bootmgr_menu_entry *entry;
> +	struct efi_bootmgr_menu *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_bootmgr_menu_choice_entry(void *data)
> +{
> +	int i;
> +	int esc = 0;
> +	struct efi_bootmgr_menu_entry *iter;
> +	enum bootmenu_key key = KEY_NONE;
> +	struct efi_bootmgr_menu *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;
> +		}
> +	}
> +
> +	/* never happens */
> +	debug("bootmgr menu: this should not happen");
> +	return NULL;
> +}
> +
> +static void efi_bootmgr_menu_destroy(struct efi_bootmgr_menu *bootmgr_menu)
> +{
> +	struct efi_bootmgr_menu_entry *next;
> +	struct efi_bootmgr_menu_entry *iter = bootmgr_menu->first;
> +
> +	while (iter) {
> +		next = iter->next;
> +		free(iter);
> +		iter = next;
> +	}
> +	free(bootmgr_menu);
> +}
> +
> +/**
> + * efi_bootmgr_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_bootmgr_process_common(struct efi_bootmgr_menu_item *items,
> +					       int count, int delay)
> +{
> +	u32 i;
> +	bool exit = false;
> +	efi_status_t ret;
> +	struct menu *menu;
> +	void *choice = NULL;
> +	struct efi_bootmgr_menu_entry *entry;
> +	struct efi_bootmgr_menu *bootmgr_menu;
> +	struct efi_bootmgr_menu_entry *iter = NULL;
> +
> +	if (count > EFI_BOOTMGR_MENU_ENTRY_NUM_MAX)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	bootmgr_menu = calloc(1, sizeof(struct efi_bootmgr_menu));
> +	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_bootmgr_menu_entry));
> +		if (!entry) {
> +			ret = EFI_LOAD_ERROR;
> +			goto out;
> +		}
> +
> +		entry->menu_index = 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_bootmgr_menu_display_statusline,
> +			   efi_bootmgr_menu_print_entry, efi_bootmgr_menu_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->menu_index == (entry->bootmgr_menu->count - 1)) {
> +				ret = EFI_ABORTED;
> +				break;
> +			}
> +		}
> +	}
> +
> +out:
> +	menu_destroy(menu);
> +	efi_bootmgr_menu_destroy(bootmgr_menu);
> +
> +	puts(ANSI_CLEAR_CONSOLE);
> +	printf(ANSI_CURSOR_POSITION, 1, 1);
> +	puts(ANSI_CURSOR_SHOW);
> +
> +	return ret;
> +}
> +
> +static efi_status_t efi_bootmgr_show_boot_selection(u16 *bootorder, efi_uintn_t count,
> +						    int *selected)
> +{
> +	u32 i;
> +	efi_status_t ret;
> +	efi_uintn_t size;
> +	void *load_option;
> +	struct efi_load_option lo;
> +	u16 varname[] = u"Boot####";
> +	struct efi_bootmgr_menu_item *menu_item, *iter;
> +
> +	menu_item = calloc(count + 1, sizeof(struct efi_bootmgr_menu_item));
> +	if (!menu_item) {
> +		ret = EFI_OUT_OF_RESOURCES;
> +		goto out;
> +	}
> +
> +	iter = menu_item;
> +	for (i = 0; i < count; i++) {
> +		efi_create_indexed_name(varname, sizeof(varname),
> +					"Boot", bootorder[i]);
> +		load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
> +		if (!load_option)
> +			continue;
> +
> +		ret = efi_deserialize_load_option(&lo, load_option, &size);
> +		if (ret != EFI_SUCCESS) {
> +			log_warning("Invalid load option for %ls\n", varname);
> +			free(load_option);
> +			continue;
> +		}
> +
> +		if (lo.attributes & LOAD_OPTION_ACTIVE) {
> +			struct efi_bootmgr_boot_selection_data *info;
> +
> +			info = calloc(1, sizeof(struct efi_bootmgr_boot_selection_data));
> +			if (!info) {
> +				ret = EFI_OUT_OF_RESOURCES;
> +				goto out;
> +			}
> +
> +			info->bootorder_index = i;
> +			info->load_option = load_option;
> +			info->selected = selected;
> +			iter->title = lo.label;
> +			iter->func = efi_bootmgr_process_boot_selected;
> +			iter->data = info;
> +			iter++;
> +		}
> +	}
> +
> +	/* add "Quit" entry */
> +	iter->title = u"Quit";
> +	iter->func = NULL;
> +	iter->data = NULL;
> +	count += 1;
> +
> +	ret = efi_bootmgr_process_common(menu_item, count, -1);
> +
> +out:
> +	iter = menu_item;
> +	for (i = 0; i < count - 1; i++, iter++) {
> +		free(((struct efi_bootmgr_boot_selection_data *)iter->data)->load_option);
> +		free(iter->data);
> +	}
> +
> +	free(menu_item);
> +
> +	return ret;
> +}
> +
> +static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool *exit)
> +{
> +	struct efi_bootmgr_boot_selection_data *info = data;
> +
> +	*exit = true;
> +
> +	if (info)
> +		*info->selected = info->bootorder_index;
> +
> +	return EFI_SUCCESS;
> +}
> +
> +static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit)
> +{
> +	u16 *bootorder;
> +	efi_status_t ret;
> +	efi_uintn_t num, size;
> +
> +	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> +	if (!bootorder)
> +		return EFI_NOT_FOUND;
> +
> +	num = size / sizeof(u16);
> +	ret = efi_bootmgr_show_boot_selection(bootorder, num, data);
> +	if (ret == EFI_SUCCESS)
> +		*exit =  true;
> +
> +	free(bootorder);
> +
> +	return ret;
> +}
> +
>   /**
>    * try_load_entry() - try to load image for boot option
>    *
> @@ -177,6 +565,30 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
>   		}
>   	}
>
> +	if (IS_ENABLED(CONFIG_EFI_BOOT_MENU)) {
> +		int selected;
> +
> +		bootmgr_menu_items[0].data = &selected;
> +		ret = efi_bootmgr_process_common(bootmgr_menu_items,
> +						 ARRAY_SIZE(bootmgr_menu_items),
> +						 CONFIG_EFI_BOOT_MENU_DELAY);
> +		if (ret == EFI_SUCCESS) {
> +			/* bootorder may be updated in the bootmgr menu */
> +			bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> +			if (!bootorder) {
> +				log_info("BootOrder not defined\n");
> +				goto error;
> +			}
> +			ret = try_load_entry(bootorder[selected], handle, load_options);
> +			free(bootorder);
> +			if (ret == EFI_SUCCESS)
> +				return ret;
> +
> +			log_err("Failed to start the selected entry(Boot%04X)\n",
> +				bootorder[selected]);
> +		}
> +	}
> +
>   	/* BootOrder */
>   	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
>   	if (!bootorder) {

Re: [RESEND RFC PATCH v2 1/4] efi_loader: add menu-driven boot device selection

[Masahisa Kojima]

Hi Heinrich,

On Fri, 25 Feb 2022 at 16:26, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 2/25/22 02:32, Masahisa Kojima wrote:
> > This patch enables the menu-driven boot device selection.
> > User can select the Boot#### included in BootOrder variable.
> >
> > If user quits thie menu, or the selected Boot#### fails to boot,
> > efi bootmgr continues to boot in accordance with BootOrder variable.
> >
> > This commit also moves the user input handling from cmd/bootmenu.c
> > to common/menu.c to reuse it from efi boot menu.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
>
> With the series applied and CONFIG_EFI_BOOT_MENU=y I first see a line
> "Hit any key to stop autoboot:" with a count down and then the UEFI menu
> with a countdown. This duplication of countdowns does not make sense.

Thank you for your review.
I agree, this duplication does not make sense.

Anyway, my primary goal is to disable CLI, user can select the
UEFI boot option("Boot####" variable) to boot the system, and user can edit
the UEFI boot option("Boot####" and "BootOrder") through the menu.

I now realize that my patch series probably do not work without CLI,
and hooking the "bootefi bootmgr" command to show the new efi bootmenu
is probably wrong. I will investigate further.

>
> This is the content of the new menu:
>
>    *** U-Boot EFI Boot Manager ***
>
>       Boot Manager
>       Boot Manager maintenance
>
>       Quit
>
> When selecting " Boot Manager maintenance" I see a second menu:
>
>    *** U-Boot EFI Boot Manager ***
>
>       Add Boot Option
>       Delete Boot Option
>       Change Boot Order
>
>       Quit
>
>    Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
>
> When I select "Add Boot Option" the system (qemu_arm64_defconfig) simply
> hangs.

Sorry, there is something wrong on my code. I will try to reproduce.

>
> Why do we need the first menu? Compare it to your laptop experience when
> pressing the boot menu key (often <F12> or <F8>).
>
> Can't we have a simple countdown:

Yes, I could skip the first menu.

>
> "Hit any key for boot menu"
>
> If a key is hit show a menu showing the different boot options and the
> distro-boot devices:
>
> Debian (0001)
> Ubuntu (0002)
> SATA(1)
> SATA(2)
> NVMe(1)
> Boot manager settings
> Command line

The current scope of this patch series are disabling CLI and booting
the system with UEFI boot option.
I will check what I can do for distro-boot.

>
> Simon has proposed a patch series "Initial implementation of standard
> boot",
> https://patchwork.ozlabs.org/project/uboot/list/?series=281745&state=*.
> I think the proposed menu would have to iterate through all active boot
> methods.
>
> > ---
> > Changes in v2:
> > - move user input handling from cmd/bootmenu.c to common/menu.c
> > - keyboard buffer is drained before asking question in
> >    common/menu.c::bootmenu_autoboot_loop()
> > - Add EFI_BOOT_MENU_DELAY Kconfig option to set delay for
> >    autoboot and disable autoboot
> > - remove unnecessary "autoboot" member in struc efi_bootmgr_menu
> >
> >   cmd/bootmenu.c               | 145 ------------
> >   common/menu.c                | 137 ++++++++++++
> >   include/menu.h               |  20 ++
> >   lib/efi_loader/Kconfig       |  20 ++
> >   lib/efi_loader/efi_bootmgr.c | 418 ++++++++++++++++++++++++++++++++++-
> >   5 files changed, 592 insertions(+), 148 deletions(-)
> >
> > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> > index 409ef9a848..f9fdebc450 100644
> > --- a/cmd/bootmenu.c
> > +++ b/cmd/bootmenu.c
> > @@ -33,21 +33,6 @@ struct bootmenu_entry {
> >       struct bootmenu_entry *next;    /* next menu entry (num+1) */
> >   };
> >
> > -struct bootmenu_data {
> > -     int delay;                      /* delay for autoboot */
> > -     int active;                     /* active menu entry */
> > -     int count;                      /* total count of menu entries */
> > -     struct bootmenu_entry *first;   /* first menu entry */
> > -};
> > -
> > -enum bootmenu_key {
> > -     KEY_NONE = 0,
> > -     KEY_UP,
> > -     KEY_DOWN,
> > -     KEY_SELECT,
> > -     KEY_QUIT,
> > -};
> > -
> >   static char *bootmenu_getoption(unsigned short int n)
> >   {
> >       char name[MAX_ENV_SIZE];
> > @@ -81,136 +66,6 @@ static void bootmenu_print_entry(void *data)
> >               puts(ANSI_COLOR_RESET);
> >   }
> >
> > -static void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> > -                             enum bootmenu_key *key, int *esc)
> > -{
> > -     int i, c;
> > -
> > -     if (menu->delay > 0) {
> > -             printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> > -             printf("  Hit any key to stop autoboot: %2d ", menu->delay);
> > -     }
> > -
> > -     while (menu->delay > 0) {
> > -             for (i = 0; i < 100; ++i) {
> > -                     if (!tstc()) {
> > -                             WATCHDOG_RESET();
> > -                             mdelay(10);
> > -                             continue;
> > -                     }
> > -
> > -                     menu->delay = -1;
> > -                     c = getchar();
> > -
> > -                     switch (c) {
> > -                     case '\e':
> > -                             *esc = 1;
> > -                             *key = KEY_NONE;
> > -                             break;
> > -                     case '\r':
> > -                             *key = KEY_SELECT;
> > -                             break;
> > -                     case 0x3: /* ^C */
> > -                             *key = KEY_QUIT;
> > -                             break;
> > -                     default:
> > -                             *key = KEY_NONE;
> > -                             break;
> > -                     }
> > -
> > -                     break;
> > -             }
> > -
> > -             if (menu->delay < 0)
> > -                     break;
> > -
> > -             --menu->delay;
> > -             printf("\b\b\b%2d ", menu->delay);
> > -     }
> > -
> > -     printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> > -     puts(ANSI_CLEAR_LINE);
> > -
> > -     if (menu->delay == 0)
> > -             *key = KEY_SELECT;
> > -}
> > -
> > -static void bootmenu_loop(struct bootmenu_data *menu,
> > -             enum bootmenu_key *key, int *esc)
> > -{
> > -     int c;
> > -
> > -     if (*esc == 1) {
> > -             if (tstc()) {
> > -                     c = getchar();
> > -             } else {
> > -                     WATCHDOG_RESET();
> > -                     mdelay(10);
> > -                     if (tstc())
> > -                             c = getchar();
> > -                     else
> > -                             c = '\e';
> > -             }
> > -     } else {
> > -             while (!tstc()) {
> > -                     WATCHDOG_RESET();
> > -                     mdelay(10);
> > -             }
> > -             c = getchar();
> > -     }
> > -
> > -     switch (*esc) {
> > -     case 0:
> > -             /* First char of ANSI escape sequence '\e' */
> > -             if (c == '\e') {
> > -                     *esc = 1;
> > -                     *key = KEY_NONE;
> > -             }
> > -             break;
> > -     case 1:
> > -             /* Second char of ANSI '[' */
> > -             if (c == '[') {
> > -                     *esc = 2;
> > -                     *key = KEY_NONE;
> > -             } else {
> > -             /* Alone ESC key was pressed */
> > -                     *key = KEY_QUIT;
> > -                     *esc = (c == '\e') ? 1 : 0;
> > -             }
> > -             break;
> > -     case 2:
> > -     case 3:
> > -             /* Third char of ANSI (number '1') - optional */
> > -             if (*esc == 2 && c == '1') {
> > -                     *esc = 3;
> > -                     *key = KEY_NONE;
> > -                     break;
> > -             }
> > -
> > -             *esc = 0;
> > -
> > -             /* ANSI 'A' - key up was pressed */
> > -             if (c == 'A')
> > -                     *key = KEY_UP;
> > -             /* ANSI 'B' - key down was pressed */
> > -             else if (c == 'B')
> > -                     *key = KEY_DOWN;
> > -             /* other key was pressed */
> > -             else
> > -                     *key = KEY_NONE;
> > -
> > -             break;
> > -     }
> > -
> > -     /* enter key was pressed */
> > -     if (c == '\r')
> > -             *key = KEY_SELECT;
> > -
> > -     /* ^C was pressed */
> > -     if (c == 0x3)
> > -             *key = KEY_QUIT;
> > -}
> > -
> >   static char *bootmenu_choice_entry(void *data)
> >   {
> >       struct bootmenu_data *menu = data;
> > diff --git a/common/menu.c b/common/menu.c
> > index 5fb2ffbd06..786e45f217 100644
> > --- a/common/menu.c
> > +++ b/common/menu.c
> > @@ -4,11 +4,14 @@
> >    * Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> >    */
> >
>
> common/menu.c is only compiled if CONFIG_MENU=y but CONFIG_EFI_BOOT_MENU
> does not select it. So depending on configuration this simply does not link:

I will fix this.

>
> lib/efi_loader/efi_bootmgr.c:315: undefined reference to `menu_create'
> aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:330: undefined
> reference to `menu_default_set'
> aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:351: undefined
> reference to `menu_destroy'
> aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:324: undefined
> reference to `menu_item_add'
> aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:337: undefined
> reference to `menu_get_choice'
> aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.o: in function
> `efi_bootmgr_menu_choice_entry':
> lib/efi_loader/efi_bootmgr.c:202: undefined reference to
> `bootmenu_autoboot_loop'
> aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.c:205: undefined
> reference to `bootmenu_loop'
> aarch64-linux-gnu-ld.bfd: lib/efi_loader/efi_bootmgr.o: in function
> `efi_bootmgr_menu_display_statusline':
> lib/efi_loader/efi_bootmgr.c:168: undefined reference to
> `menu_default_choice
>
> > +#include <ansi.h>
> >   #include <common.h>
> >   #include <cli.h>
> >   #include <malloc.h>
> >   #include <errno.h>
> > +#include <linux/delay.h>
> >   #include <linux/list.h>
> > +#include <watchdog.h>
> >
> >   #include "menu.h"
> >
> > @@ -418,3 +421,137 @@ int menu_destroy(struct menu *m)
> >
> >       return 1;
> >   }
> > +
> > +void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> > +                         enum bootmenu_key *key, int *esc)
> > +{
> > +     int i, c;
> > +
> > +     if (menu->delay > 0) {
> > +             /* flush input */
> > +             while (tstc())
> > +                     getchar();
> > +
> > +             printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> > +             printf("  Hit any key to stop autoboot: %2d ", menu->delay);
> > +     }
> > +
> > +     while (menu->delay > 0) {
> > +             for (i = 0; i < 100; ++i) {
> > +                     if (!tstc()) {
> > +                             WATCHDOG_RESET();
> > +                             mdelay(10);
> > +                             continue;
> > +                     }
> > +
> > +                     menu->delay = -1;
> > +                     c = getchar();
> > +
> > +                     switch (c) {
> > +                     case '\e':
> > +                             *esc = 1;
> > +                             *key = KEY_NONE;
> > +                             break;
> > +                     case '\r':
> > +                             *key = KEY_SELECT;
> > +                             break;
> > +                     case 0x3: /* ^C */
> > +                             *key = KEY_QUIT;
> > +                             break;
> > +                     default:
> > +                             *key = KEY_NONE;
> > +                             break;
> > +                     }
> > +
> > +                     break;
> > +             }
> > +
> > +             if (menu->delay < 0)
> > +                     break;
> > +
> > +             --menu->delay;
> > +             printf("\b\b\b%2d ", menu->delay);
> > +     }
> > +
> > +     printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> > +     puts(ANSI_CLEAR_LINE);
> > +
> > +     if (menu->delay == 0)
> > +             *key = KEY_SELECT;
> > +}
> > +
> > +void bootmenu_loop(struct bootmenu_data *menu,
> > +                enum bootmenu_key *key, int *esc)
> > +{
> > +     int c;
> > +
> > +     if (*esc == 1) {
> > +             if (tstc()) {
> > +                     c = getchar();
> > +             } else {
> > +                     WATCHDOG_RESET();
> > +                     mdelay(10);
> > +                     if (tstc())
> > +                             c = getchar();
> > +                     else
> > +                             c = '\e';
> > +             }
> > +     } else {
> > +             while (!tstc()) {
> > +                     WATCHDOG_RESET();
> > +                     mdelay(10);
> > +             }
> > +             c = getchar();
> > +     }
> > +
> > +     switch (*esc) {
> > +     case 0:
> > +             /* First char of ANSI escape sequence '\e' */
> > +             if (c == '\e') {
> > +                     *esc = 1;
> > +                     *key = KEY_NONE;
> > +             }
> > +             break;
> > +     case 1:
> > +             /* Second char of ANSI '[' */
> > +             if (c == '[') {
> > +                     *esc = 2;
> > +                     *key = KEY_NONE;
> > +             } else {
> > +             /* Alone ESC key was pressed */
> > +                     *key = KEY_QUIT;
> > +                     *esc = (c == '\e') ? 1 : 0;
> > +             }
> > +             break;
> > +     case 2:
> > +     case 3:
> > +             /* Third char of ANSI (number '1') - optional */
> > +             if (*esc == 2 && c == '1') {
> > +                     *esc = 3;
> > +                     *key = KEY_NONE;
> > +                     break;
> > +             }
> > +
> > +             *esc = 0;
> > +
> > +             /* ANSI 'A' - key up was pressed */
> > +             if (c == 'A')
> > +                     *key = KEY_UP;
> > +             /* ANSI 'B' - key down was pressed */
> > +             else if (c == 'B')
> > +                     *key = KEY_DOWN;
> > +             /* other key was pressed */
> > +             else
> > +                     *key = KEY_NONE;
> > +
> > +             break;
> > +     }
> > +
> > +     /* enter key was pressed */
> > +     if (c == '\r')
> > +             *key = KEY_SELECT;
> > +
> > +     /* ^C was pressed */
> > +     if (c == 0x3)
> > +             *key = KEY_QUIT;
> > +}
> > diff --git a/include/menu.h b/include/menu.h
> > index ad5859437e..e74616cae8 100644
> > --- a/include/menu.h
> > +++ b/include/menu.h
> > @@ -35,4 +35,24 @@ int menu_default_choice(struct menu *m, void **choice);
> >    */
> >   int menu_show(int bootdelay);
> >
> > +struct bootmenu_data {
> > +     int delay;                      /* delay for autoboot */
> > +     int active;                     /* active menu entry */
> > +     int count;                      /* total count of menu entries */
> > +     struct bootmenu_entry *first;   /* first menu entry */
> > +};
> > +
> > +enum bootmenu_key {
> > +     KEY_NONE = 0,
> > +     KEY_UP,
> > +     KEY_DOWN,
> > +     KEY_SELECT,
> > +     KEY_QUIT,
> > +};
> > +
> > +void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> > +                         enum bootmenu_key *key, int *esc);
> > +void bootmenu_loop(struct bootmenu_data *menu,
> > +                enum bootmenu_key *key, int *esc);
> > +
> >   #endif /* __MENU_H__ */
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index e5e35fe51f..a3fb8b2a75 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -39,6 +39,26 @@ config CMD_BOOTEFI_BOOTMGR
> >         via UEFI variables Boot####, BootOrder, and BootNext. This enables the
> >         'bootefi bootmgr' command.
> >
> > +config EFI_BOOT_MENU
> > +     bool "UEFI Boot Menu driven boot device selection"
> > +     default n
> > +     help
> > +       Select this option if you want to enable the menu driven boot device
> > +       selection. This menu provides the functionality to select a boot
> > +       option to start, and allow users to edit Boot#### and BootOrder.
> > +       If this menu is enabled, CLI can be disabled if the system boots
> > +       via UEFI variable Boot#### and BootOrder.
>
> We end up with these symbols for menus:
>
> CONFIG_MENU
> CONFIG_AUTOBOOT_MENUKEY
> CONFIG_AUTOBOOT_MENU_SHOW
> CONFIG_BOOTDELAY
> CONFIG_EFI_BOOT_MENU
> CONFIG_EFI_BOOT_MENU_DELAY
>
> Defining both CONFIG_AUTOBOOT_MENU_SHOW and CONFIG_EFI_BOOT_MENU does
> not make much sense. Same for CONFIG_BOOTDELAY and
> CONFIG_EFI_BOOT_MENU_DELAY. Please, unifiy the settings.

I will consider how I can unify these options.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> > +
> > +config EFI_BOOT_MENU_DELAY
> > +     int "delay in seconds before automatically booting in UEFI Boot Menu"
> > +     default 10
> > +     range -1 2147483647
> > +     help
> > +       Delay before automatically booting in accordance with
> > +       "BootOrder" variable.
> > +       set to 0 to autoboot with no delay.
> > +       set to -1 to disable autoboot.
> > +
> >   config EFI_SETUP_EARLY
> >       bool
> >
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index 8c04ecbdc8..bbb3fac5bd 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -7,10 +7,12 @@
> >
> >   #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>
> > @@ -22,14 +24,400 @@ static const struct efi_runtime_services *rs;
> >    * bootmgr implements the logic of trying to find a payload to boot
> >    * based on the BootOrder + BootXXXX variables, and then loading it.
> >    *
> > - * TODO detecting a special key held (f9?) and displaying a boot menu
> > - * like you would get on a PC would be clever.
> > - *
> >    * TODO if we had a way to write and persist variables after the OS
> >    * has started, we'd also want to check OsIndications to see if we
> >    * should do normal or recovery boot.
> >    */
> >
> > +#define EFI_BOOTMGR_MENU_ENTRY_NUM_MAX 1024
> > +
> > +typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
> > +
> > +/**
> > + * struct efi_bootmgr_menu_entry - menu entry structure
> > + *
> > + * @menu_index:              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_bootmgr_menu_entry {
> > +     u32 menu_index;
> > +     u16 *title;
> > +     char key[6];
> > +     struct efi_bootmgr_menu *bootmgr_menu;
> > +     struct efi_bootmgr_menu_entry *next;
> > +     efi_bootmenu_entry_func func;
> > +     void *data;
> > +};
> > +
> > +/**
> > + * struct efi_bootmgr_menuy - 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_bootmgr_menu {
> > +     int delay;
> > +     int active;
> > +     int count;
> > +     struct efi_bootmgr_menu_entry *first;
> > +};
> > +
> > +struct efi_bootmgr_menu_item {
> > +     u16 *title;
> > +     efi_bootmenu_entry_func func;
> > +     void *data;
> > +};
> > +
> > +struct efi_bootmgr_boot_selection_data {
> > +     u16 bootorder_index;
> > +     void *load_option;
> > +     int *selected;
> > +};
> > +
> > +static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool *exit);
> > +static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit);
> > +
> > +static struct efi_bootmgr_menu_item bootmgr_menu_items[] = {
> > +     {u"Boot Manager", efi_bootmgr_process_boot_selection},
> > +     {u"Quit", NULL},
> > +};
> > +
> > +static void efi_bootmgr_menu_print_entry(void *data)
> > +{
> > +     struct efi_bootmgr_menu_entry *entry = data;
> > +     int reverse = (entry->bootmgr_menu->active == entry->menu_index);
> > +
> > +     /* TODO: support scroll or page for many entries */
> > +
> > +     /*
> > +      * Move cursor to line where the entry will be drown (entry->count)
> > +      * First 3 lines contain bootmgr menu header + one empty line
> > +      * For the last "Quit" entry, add one empty line
> > +      */
> > +     if (entry->menu_index == (entry->bootmgr_menu->count - 1))
> > +             printf(ANSI_CURSOR_POSITION, entry->menu_index + 5, 1);
> > +     else
> > +             printf(ANSI_CURSOR_POSITION, entry->menu_index + 4, 1);
> > +
> > +     puts("     ");
> > +
> > +     if (reverse)
> > +             puts(ANSI_COLOR_REVERSE);
> > +
> > +     printf("%ls", entry->title);
> > +
> > +     if (reverse)
> > +             puts(ANSI_COLOR_RESET);
> > +}
> > +
> > +static void efi_bootmgr_menu_display_statusline(struct menu *m)
> > +{
> > +     struct efi_bootmgr_menu_entry *entry;
> > +     struct efi_bootmgr_menu *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_bootmgr_menu_choice_entry(void *data)
> > +{
> > +     int i;
> > +     int esc = 0;
> > +     struct efi_bootmgr_menu_entry *iter;
> > +     enum bootmenu_key key = KEY_NONE;
> > +     struct efi_bootmgr_menu *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;
> > +             }
> > +     }
> > +
> > +     /* never happens */
> > +     debug("bootmgr menu: this should not happen");
> > +     return NULL;
> > +}
> > +
> > +static void efi_bootmgr_menu_destroy(struct efi_bootmgr_menu *bootmgr_menu)
> > +{
> > +     struct efi_bootmgr_menu_entry *next;
> > +     struct efi_bootmgr_menu_entry *iter = bootmgr_menu->first;
> > +
> > +     while (iter) {
> > +             next = iter->next;
> > +             free(iter);
> > +             iter = next;
> > +     }
> > +     free(bootmgr_menu);
> > +}
> > +
> > +/**
> > + * efi_bootmgr_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_bootmgr_process_common(struct efi_bootmgr_menu_item *items,
> > +                                            int count, int delay)
> > +{
> > +     u32 i;
> > +     bool exit = false;
> > +     efi_status_t ret;
> > +     struct menu *menu;
> > +     void *choice = NULL;
> > +     struct efi_bootmgr_menu_entry *entry;
> > +     struct efi_bootmgr_menu *bootmgr_menu;
> > +     struct efi_bootmgr_menu_entry *iter = NULL;
> > +
> > +     if (count > EFI_BOOTMGR_MENU_ENTRY_NUM_MAX)
> > +             return EFI_OUT_OF_RESOURCES;
> > +
> > +     bootmgr_menu = calloc(1, sizeof(struct efi_bootmgr_menu));
> > +     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_bootmgr_menu_entry));
> > +             if (!entry) {
> > +                     ret = EFI_LOAD_ERROR;
> > +                     goto out;
> > +             }
> > +
> > +             entry->menu_index = 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_bootmgr_menu_display_statusline,
> > +                        efi_bootmgr_menu_print_entry, efi_bootmgr_menu_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->menu_index == (entry->bootmgr_menu->count - 1)) {
> > +                             ret = EFI_ABORTED;
> > +                             break;
> > +                     }
> > +             }
> > +     }
> > +
> > +out:
> > +     menu_destroy(menu);
> > +     efi_bootmgr_menu_destroy(bootmgr_menu);
> > +
> > +     puts(ANSI_CLEAR_CONSOLE);
> > +     printf(ANSI_CURSOR_POSITION, 1, 1);
> > +     puts(ANSI_CURSOR_SHOW);
> > +
> > +     return ret;
> > +}
> > +
> > +static efi_status_t efi_bootmgr_show_boot_selection(u16 *bootorder, efi_uintn_t count,
> > +                                                 int *selected)
> > +{
> > +     u32 i;
> > +     efi_status_t ret;
> > +     efi_uintn_t size;
> > +     void *load_option;
> > +     struct efi_load_option lo;
> > +     u16 varname[] = u"Boot####";
> > +     struct efi_bootmgr_menu_item *menu_item, *iter;
> > +
> > +     menu_item = calloc(count + 1, sizeof(struct efi_bootmgr_menu_item));
> > +     if (!menu_item) {
> > +             ret = EFI_OUT_OF_RESOURCES;
> > +             goto out;
> > +     }
> > +
> > +     iter = menu_item;
> > +     for (i = 0; i < count; i++) {
> > +             efi_create_indexed_name(varname, sizeof(varname),
> > +                                     "Boot", bootorder[i]);
> > +             load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
> > +             if (!load_option)
> > +                     continue;
> > +
> > +             ret = efi_deserialize_load_option(&lo, load_option, &size);
> > +             if (ret != EFI_SUCCESS) {
> > +                     log_warning("Invalid load option for %ls\n", varname);
> > +                     free(load_option);
> > +                     continue;
> > +             }
> > +
> > +             if (lo.attributes & LOAD_OPTION_ACTIVE) {
> > +                     struct efi_bootmgr_boot_selection_data *info;
> > +
> > +                     info = calloc(1, sizeof(struct efi_bootmgr_boot_selection_data));
> > +                     if (!info) {
> > +                             ret = EFI_OUT_OF_RESOURCES;
> > +                             goto out;
> > +                     }
> > +
> > +                     info->bootorder_index = i;
> > +                     info->load_option = load_option;
> > +                     info->selected = selected;
> > +                     iter->title = lo.label;
> > +                     iter->func = efi_bootmgr_process_boot_selected;
> > +                     iter->data = info;
> > +                     iter++;
> > +             }
> > +     }
> > +
> > +     /* add "Quit" entry */
> > +     iter->title = u"Quit";
> > +     iter->func = NULL;
> > +     iter->data = NULL;
> > +     count += 1;
> > +
> > +     ret = efi_bootmgr_process_common(menu_item, count, -1);
> > +
> > +out:
> > +     iter = menu_item;
> > +     for (i = 0; i < count - 1; i++, iter++) {
> > +             free(((struct efi_bootmgr_boot_selection_data *)iter->data)->load_option);
> > +             free(iter->data);
> > +     }
> > +
> > +     free(menu_item);
> > +
> > +     return ret;
> > +}
> > +
> > +static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool *exit)
> > +{
> > +     struct efi_bootmgr_boot_selection_data *info = data;
> > +
> > +     *exit = true;
> > +
> > +     if (info)
> > +             *info->selected = info->bootorder_index;
> > +
> > +     return EFI_SUCCESS;
> > +}
> > +
> > +static efi_status_t efi_bootmgr_process_boot_selection(void *data, bool *exit)
> > +{
> > +     u16 *bootorder;
> > +     efi_status_t ret;
> > +     efi_uintn_t num, size;
> > +
> > +     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> > +     if (!bootorder)
> > +             return EFI_NOT_FOUND;
> > +
> > +     num = size / sizeof(u16);
> > +     ret = efi_bootmgr_show_boot_selection(bootorder, num, data);
> > +     if (ret == EFI_SUCCESS)
> > +             *exit =  true;
> > +
> > +     free(bootorder);
> > +
> > +     return ret;
> > +}
> > +
> >   /**
> >    * try_load_entry() - try to load image for boot option
> >    *
> > @@ -177,6 +565,30 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options)
> >               }
> >       }
> >
> > +     if (IS_ENABLED(CONFIG_EFI_BOOT_MENU)) {
> > +             int selected;
> > +
> > +             bootmgr_menu_items[0].data = &selected;
> > +             ret = efi_bootmgr_process_common(bootmgr_menu_items,
> > +                                              ARRAY_SIZE(bootmgr_menu_items),
> > +                                              CONFIG_EFI_BOOT_MENU_DELAY);
> > +             if (ret == EFI_SUCCESS) {
> > +                     /* bootorder may be updated in the bootmgr menu */
> > +                     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> > +                     if (!bootorder) {
> > +                             log_info("BootOrder not defined\n");
> > +                             goto error;
> > +                     }
> > +                     ret = try_load_entry(bootorder[selected], handle, load_options);
> > +                     free(bootorder);
> > +                     if (ret == EFI_SUCCESS)
> > +                             return ret;
> > +
> > +                     log_err("Failed to start the selected entry(Boot%04X)\n",
> > +                             bootorder[selected]);
> > +             }
> > +     }
> > +
> >       /* BootOrder */
> >       bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
> >       if (!bootorder) {
>

Re: [RESEND RFC PATCH v2 2/4] lib/charset: add u16_strlcat() function

[Simon Glass]

Hi Masahisa,

On Thu, 24 Feb 2022 at 18:31, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> Provide u16 string version of strlcat().
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changes in v2:
> - implement u16_strlcat(with the destination buffer size in argument)
>   instead of u16_strcat
>
>  include/charset.h | 15 +++++++++++++++
>  lib/charset.c     | 20 ++++++++++++++++++++
>  2 files changed, 35 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

nits below

>
> diff --git a/include/charset.h b/include/charset.h
> index b93d023092..dc5fc275ec 100644
> --- a/include/charset.h
> +++ b/include/charset.h
> @@ -259,6 +259,21 @@ u16 *u16_strcpy(u16 *dest, const u16 *src);
>   */
>  u16 *u16_strdup(const void *src);
>
> +/**
> + * u16_strlcat() - Append a length-limited, %NUL-terminated string to another
> + *
> + * Append the src string to the dest string, overwriting the terminating
> + * null word at the end of dest, and then adds a terminating null word.

add

> + * It will append at most size - u16_strlen(dst) - 1 bytes, NUL-terminating the result.
> + *
> + * @dest:              destination buffer (null terminated)
> + * @src:               source buffer (null terminated)
> + * @size:              destination buffer size in bytes
> + * Return:             total size of the created string in bytes.
> + *                     If return value >= size, truncation occurred.
> + */
> +size_t u16_strlcat(u16 *dest, const u16 *src, size_t size);
> +
>  /**
>   * utf16_to_utf8() - Convert an utf16 string to utf8
>   *
> diff --git a/lib/charset.c b/lib/charset.c
> index f44c58d9d8..f15d5df19a 100644
> --- a/lib/charset.c
> +++ b/lib/charset.c
> @@ -428,6 +428,26 @@ u16 *u16_strdup(const void *src)
>         return new;
>  }
>
> +size_t u16_strlcat(u16 *dest, const u16 *src, size_t size)
> +{
> +       size_t dstrlen = u16_strnlen(dest, size >> 1);
> +       size_t dlen = dstrlen * sizeof(u16);
> +       size_t len = u16_strlen(src) * sizeof(u16);
> +       size_t ret = dlen + len;
> +
> +       if (dlen >= size)
> +               return ret;
> +
> +       dest += dstrlen;
> +       size -= dlen;
> +       if (len >= size)
> +               len = size - sizeof(u16);
> +
> +       memcpy(dest, src, len);
> +       dest[len >> 1] = u'\0';

blank line before final return

> +       return ret;
> +}
> +
>  /* Convert UTF-16 to UTF-8.  */
>  uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size)
>  {
> --
> 2.17.1
>

Regards,
Simon

Re: [RESEND RFC PATCH v2 3/4] test: unit test for u16_strlcat()

[Simon Glass]

On Thu, 24 Feb 2022 at 18:31, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> Provide a unit test for function u16_strlcat().
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Newly created in v2.
>
>  test/unicode_ut.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RESEND RFC PATCH v2 2/4] lib/charset: add u16_strlcat() function

[Masahisa Kojima]

Hi Simon,

On Sat, 12 Mar 2022 at 11:24, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Masahisa,
>
> On Thu, 24 Feb 2022 at 18:31, Masahisa Kojima
> <masahisa.kojima@linaro.org> wrote:
> >
> > Provide u16 string version of strlcat().
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Changes in v2:
> > - implement u16_strlcat(with the destination buffer size in argument)
> >   instead of u16_strcat
> >
> >  include/charset.h | 15 +++++++++++++++
> >  lib/charset.c     | 20 ++++++++++++++++++++
> >  2 files changed, 35 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> nits below
>
> >
> > diff --git a/include/charset.h b/include/charset.h
> > index b93d023092..dc5fc275ec 100644
> > --- a/include/charset.h
> > +++ b/include/charset.h
> > @@ -259,6 +259,21 @@ u16 *u16_strcpy(u16 *dest, const u16 *src);
> >   */
> >  u16 *u16_strdup(const void *src);
> >
> > +/**
> > + * u16_strlcat() - Append a length-limited, %NUL-terminated string to another
> > + *
> > + * Append the src string to the dest string, overwriting the terminating
> > + * null word at the end of dest, and then adds a terminating null word.
>
> add
>
> > + * It will append at most size - u16_strlen(dst) - 1 bytes, NUL-terminating the result.
> > + *
> > + * @dest:              destination buffer (null terminated)
> > + * @src:               source buffer (null terminated)
> > + * @size:              destination buffer size in bytes
> > + * Return:             total size of the created string in bytes.
> > + *                     If return value >= size, truncation occurred.
> > + */
> > +size_t u16_strlcat(u16 *dest, const u16 *src, size_t size);
> > +
> >  /**
> >   * utf16_to_utf8() - Convert an utf16 string to utf8
> >   *
> > diff --git a/lib/charset.c b/lib/charset.c
> > index f44c58d9d8..f15d5df19a 100644
> > --- a/lib/charset.c
> > +++ b/lib/charset.c
> > @@ -428,6 +428,26 @@ u16 *u16_strdup(const void *src)
> >         return new;
> >  }
> >
> > +size_t u16_strlcat(u16 *dest, const u16 *src, size_t size)
> > +{
> > +       size_t dstrlen = u16_strnlen(dest, size >> 1);
> > +       size_t dlen = dstrlen * sizeof(u16);
> > +       size_t len = u16_strlen(src) * sizeof(u16);
> > +       size_t ret = dlen + len;
> > +
> > +       if (dlen >= size)
> > +               return ret;
> > +
> > +       dest += dstrlen;
> > +       size -= dlen;
> > +       if (len >= size)
> > +               len = size - sizeof(u16);
> > +
> > +       memcpy(dest, src, len);
> > +       dest[len >> 1] = u'\0';
>
> blank line before final return

OK.
Thank you for your review.

Regards,
Masahisa Kojima

>
> > +       return ret;
> > +}
> > +
> >  /* Convert UTF-16 to UTF-8.  */
> >  uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size)
> >  {
> > --
> > 2.17.1
> >
>
> Regards,
> Simon