On 3/24/22 14:54, Masahisa Kojima wrote:
This commit adds the UEFI related menu entries and
distro_boot entries into the bootmenu.
For UEFI, user can select which UEFI "Boot####" option
to execute, call UEFI bootmgr and UEFI boot variable
maintenance menu. UEFI bootmgr entry is required to
correctly handle "BootNext" variable.
For distro_boot, user can select the boot device
included in "boot_targets" u-boot environment variable.
The menu example is as follows.
*** U-Boot Boot Menu ***
bootmenu_00 : Boot 1. kernel
bootmenu_01 : Boot 2. kernel
bootmenu_02 : Reset board
UEFI BOOT0000 : debian
UEFI BOOT0001 : ubuntu
distro_boot : usb0
distro_boot : scsi0
distro_boot : virtio0
distro_boot : dhcp
Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
---
Changes in v4:
- add Kconfig option "CMD_BOOTMENU_ENTER_UBOOT_CONSOLE" to
disable to enter U-Boot console from bootmenu
- update the menu entry display format
- create local function to create menu entry for bootmenu, UEFI boot option
and distro boot command
- handle "BootNext" before showing bootmenu
- call "bootefi bootmgr" instead of "bootefi bootindex"
- move bootmenu_show() into loop
- add default boot behavior(run "bootefi bootmgr" then "run bootcmd") when
user quit the bootmenu if U-Boot console is disabled
Changes in v3:
- newly created
cmd/Kconfig | 10 ++
cmd/bootmenu.c | 393 +++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 360 insertions(+), 43 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 5e25e45fd2..5fbeab266f 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -300,6 +300,16 @@ config CMD_BOOTMENU
help
Add an ANSI terminal boot menu command.
+config CMD_BOOTMENU_ENTER_UBOOT_CONSOLE
+ bool "Allow Bootmenu to enter the U-Boot console"
+ depends on CMD_BOOTMENU
+ default n
+ help
+ Add an entry to enter U-Boot console in bootmenu.
+ If this option is disabled, user can not enter
+ the U-Boot console from bootmenu. It increases
+ the system security.
+
This is an unrelated change which is not described in the commit
message. As it not specific to UEFI it deserves to live in a separate patch.
config CMD_ADTIMG
bool "adtimg"
help
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index d573487272..947b3a49ff 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -3,9 +3,12 @@
* (C) Copyright 2011-2013 Pali Rohár <p...@kernel.org>
*/
+#include <charset.h>
#include <common.h>
#include <command.h>
#include <ansi.h>
+#include <efi_loader.h>
+#include <efi_variable.h>
#include <env.h>
#include <log.h>
#include <menu.h>
@@ -24,11 +27,27 @@
*/
#define MAX_ENV_SIZE (9 + 2 + 1)
+enum bootmenu_ret {
+ BOOTMENU_RET_SUCCESS = 0,
+ BOOTMENU_RET_FAIL,
+ BOOTMENU_RET_QUIT,
+ BOOTMENU_RET_UPDATED,
+};
+
+enum boot_type {
+ BOOTMENU_TYPE_NONE = 0,
+ BOOTMENU_TYPE_BOOTMENU,
+ BOOTMENU_TYPE_UEFI_BOOT_OPTION,
+ BOOTMENU_TYPE_DISTRO_BOOT,
+};
+
struct bootmenu_entry {
unsigned short int num; /* unique number 0 .. MAX_COUNT */
char key[3]; /* key identifier of number */
- char *title; /* title of entry */
+ u16 *title; /* title of entry */
char *command; /* hush command of entry */
+ enum boot_type type; /* boot type of entry */
+ u16 bootorder; /* order for each boot type */
struct bootmenu_data *menu; /* this bootmenu */
struct bootmenu_entry *next; /* next menu entry (num+1) */
};
@@ -75,7 +94,14 @@ static void bootmenu_print_entry(void *data)
if (reverse)
puts(ANSI_COLOR_REVERSE);
- puts(entry->title);
+ if (entry->type == BOOTMENU_TYPE_BOOTMENU)
+ printf("bootmenu_%02d : %ls", entry->bootorder, entry->title);
+ else if (entry->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION)
+ printf("UEFI BOOT%04X : %ls", entry->bootorder, entry->title);
+ else if (entry->type == BOOTMENU_TYPE_DISTRO_BOOT)
+ printf("distro_boot : %ls", entry->title);
+ else
+ printf("%ls", entry->title);
if (reverse)
puts(ANSI_COLOR_RESET);
@@ -87,6 +113,10 @@ static void bootmenu_autoboot_loop(struct bootmenu_data
*menu,
int i, c;
if (menu->delay > 0) {
+ /* flush input */
+ while (tstc())
+ getchar();
+
This change is not related to UEFI and not described in the commit message.
Put it into a separate patch.
printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
printf(" Hit any key to stop autoboot: %2d ", menu->delay);
}
@@ -275,31 +305,20 @@ static void bootmenu_destroy(struct bootmenu_data *menu)
free(menu);
}
Please, provide a function description.
-static struct bootmenu_data *bootmenu_create(int delay)
+static int prepare_bootmenu_entry(struct bootmenu_data *menu,
+ struct bootmenu_entry **current,
+ unsigned short int *index)
{
- unsigned short int i = 0;
- const char *option;
- struct bootmenu_data *menu;
- struct bootmenu_entry *iter = NULL;
-
int len;
char *sep;
- char *default_str;
- struct bootmenu_entry *entry;
-
- menu = malloc(sizeof(struct bootmenu_data));
- if (!menu)
- return NULL;
-
- menu->delay = delay;
- menu->active = 0;
- menu->first = NULL;
-
- default_str = env_get("bootmenu_default");
- if (default_str)
- menu->active = (int)simple_strtol(default_str, NULL, 10);
+ const char *option;
+ unsigned short int i = *index;
+ struct bootmenu_entry *entry = NULL;
+ struct bootmenu_entry *iter = *current;
while ((option = bootmenu_getoption(i))) {
+ u16 *buf;
+
sep = strchr(option, '=');
if (!sep) {
printf("Invalid bootmenu entry: %s\n", option);
@@ -308,23 +327,23 @@ static struct bootmenu_data *bootmenu_create(int delay)
entry = malloc(sizeof(struct bootmenu_entry));
if (!entry)
- goto cleanup;
+ return -ENOMEM;
len = sep-option;
- entry->title = malloc(len + 1);
+ buf = calloc(1, (len + 1) * sizeof(u16));
+ entry->title = buf;
if (!entry->title) {
free(entry);
- goto cleanup;
+ return -ENOMEM;
}
- memcpy(entry->title, option, len);
- entry->title[len] = 0;
+ utf8_utf16_strncpy(&buf, option, len);
len = strlen(sep + 1);
entry->command = malloc(len + 1);
if (!entry->command) {
free(entry->title);
free(entry);
- goto cleanup;
+ return -ENOMEM;
}
memcpy(entry->command, sep + 1, len);
entry->command[len] = 0;
@@ -333,6 +352,158 @@ static struct bootmenu_data *bootmenu_create(int delay)
entry->num = i;
entry->menu = menu;
+ entry->type = BOOTMENU_TYPE_BOOTMENU;
+ entry->bootorder = i;
+ entry->next = NULL;
+
+ if (!iter)
+ menu->first = entry;
+ else
+ iter->next = entry;
+
+ iter = entry;
+ i++;
+
+ if (i == MAX_COUNT - 1)
+ break;
+ }
+
+ *index = i;
+ *current = iter;
+
+ return 1;
+}
This is an unrelated change not described in the commit message.
Put it into a separate patch.
Please, add a function description.
+
+static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
+ struct bootmenu_entry **current,
+ unsigned short int *index)
+{
+ u16 *bootorder;
+ efi_status_t ret;
+ unsigned short j;
+ efi_uintn_t num, size;
+ void *load_option;
+ struct efi_load_option lo;
+ u16 varname[] = u"Boot####";
+ unsigned short int i = *index;
+ struct bootmenu_entry *entry = NULL;
+ struct bootmenu_entry *iter = *current;
+
+ bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size);
+ if (!bootorder)
+ return -ENOENT;
+
+ num = size / sizeof(u16);
+ for (j = 0; j < num; j++) {
+ entry = malloc(sizeof(struct bootmenu_entry));
+ if (!entry)
+ return -ENOMEM;
+
+ efi_create_indexed_name(varname, sizeof(varname),
+ "Boot", bootorder[j]);
+ load_option = efi_get_var(varname, &efi_global_variable_guid,
&size);
+ if (!load_option)
+ continue;
+
+ ret = efi_deserialize_load_option(&lo, load_option, &size);
+ if (ret != EFI_SUCCESS) {
+ log_warning("Invalid load option for %ls\n", varname);
+ free(load_option);
+ free(entry);
+ continue;
+ }
+
+ if (lo.attributes & LOAD_OPTION_ACTIVE) {
+ entry->title = u16_strdup(lo.label);
+ if (!entry->title) {
+ free(load_option);
+ free(entry);
+ return -ENOMEM;
+ }
+ entry->command = strdup("bootefi bootmgr");
+ sprintf(entry->key, "%d", i);
+ entry->num = i;
+ entry->menu = menu;
+ entry->type = BOOTMENU_TYPE_UEFI_BOOT_OPTION;
+ entry->bootorder = bootorder[j];
+ entry->next = NULL;
+
+ if (!iter)
+ menu->first = entry;
+ else
+ iter->next = entry;
+
+ iter = entry;
+ i++;
+ }
+
+ free(load_option);
+
+ if (i == MAX_COUNT - 1)
+ break;
+ }
+
+ free(bootorder);
+ *index = i;
+ *current = iter;
+
+ return 1;
+}
+
Please, provide a Sphinx style function description explaining what this
function is meant to do.
+static int prepare_distro_boot_entry(struct bootmenu_data *menu,
+ struct bootmenu_entry **current,
+ unsigned short int *index)
Please, put distro boot entry generation and UEFI boot entry generation
into separate patches. This will make reviewing much easier.
How does this fit to Simon's work to overhaul distroboot?
+{
+ char *p;
+ int len;
+ char *token;
+ char *boot_targets;
+ unsigned short int i = *index;
+ struct bootmenu_entry *entry = NULL;
+ struct bootmenu_entry *iter = *current;
+
+ /* list the distro boot "boot_targets" */
+ boot_targets = env_get("boot_targets");
+ if (!boot_targets)
+ return -ENOENT;
+
+ len = strlen(boot_targets);
+ p = calloc(1, len + 1);
+ strlcpy(p, boot_targets, len);
+
+ token = strtok(p, " ");
+
+ do {
+ u16 *buf;
+ char *command;
+ int command_size;
+
+ entry = malloc(sizeof(struct bootmenu_entry));
+ if (!entry)
+ return -ENOMEM;
+
+ len = strlen(token);
+ buf = calloc(1, (len + 1) * sizeof(u16));
+ entry->title = buf;
+ if (!entry->title) {
+ free(entry);
+ return -ENOMEM;
+ }
+ utf8_utf16_strncpy(&buf, token, len);
+ sprintf(entry->key, "%d", i);
+ entry->num = i;
+ entry->menu = menu;
+
+ command_size = sizeof("run bootcmd_") + len;
+ command = calloc(1, command_size);
+ if (!command) {
+ free(entry->title);
+ free(entry);
+ return -ENOMEM;
+ }
+ snprintf(command, command_size, "run bootcmd_%s", token);
+ entry->command = command;
+ entry->type = BOOTMENU_TYPE_DISTRO_BOOT;
entry->next = NULL;
if (!iter)
@@ -341,10 +512,59 @@ static struct bootmenu_data *bootmenu_create(int delay)
iter->next = entry;
iter = entry;
- ++i;
+ i++;
if (i == MAX_COUNT - 1)
break;
+
+ token = strtok(NULL, " ");
+ } while (token);
+
+ free(p);
+ *index = i;
+ *current = iter;
+
+ return 1;
+}
+
+static struct bootmenu_data *bootmenu_create(int delay)
+{
+ int ret;
+ unsigned short int i = 0;
+ struct bootmenu_data *menu;
+ struct bootmenu_entry *iter = NULL;
+ struct bootmenu_entry *entry;
+
+ char *default_str;
+
+ menu = malloc(sizeof(struct bootmenu_data));
+ if (!menu)
+ return NULL;
+
+ menu->delay = delay;
+ menu->active = 0;
+ menu->first = NULL;
+
+ default_str = env_get("bootmenu_default");
+ if (default_str)
+ menu->active = (int)simple_strtol(default_str, NULL, 10);
+
+ ret = prepare_bootmenu_entry(menu, &iter, &i);
+ if (ret < 0)
+ goto cleanup;
+
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
+ if (i < MAX_COUNT - 1) {
+ ret = prepare_uefi_bootorder_entry(menu, &iter, &i);
+ if (ret < 0 && ret != -ENOENT)
+ goto cleanup;
+ }
+ }
+
+ if (i < MAX_COUNT - 1) {
+ ret = prepare_distro_boot_entry(menu, &iter, &i);
+ if (ret < 0 && ret != -ENOENT)
+ goto cleanup;
}
/* Add U-Boot console entry at the end */
@@ -353,7 +573,12 @@ static struct bootmenu_data *bootmenu_create(int delay)
if (!entry)
goto cleanup;
- entry->title = strdup("U-Boot console");
+ /* Add dummy entry if entering U-Boot console is disabled */
+ if (IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE))
+ entry->title = u16_strdup(u"U-Boot console");
+ else
+ entry->title = u16_strdup(u"");
+
if (!entry->title) {
free(entry);
goto cleanup;
@@ -370,6 +595,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
entry->num = i;
entry->menu = menu;
+ entry->type = BOOTMENU_TYPE_NONE;
entry->next = NULL;
if (!iter)
@@ -378,7 +604,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
iter->next = entry;
iter = entry;
- ++i;
+ i++;
}
menu->count = i;
@@ -423,43 +649,73 @@ static void menu_display_statusline(struct menu *m)
puts(ANSI_CLEAR_LINE);
}
-static void bootmenu_show(int delay)
+static void handle_uefi_bootnext(void)
{
+ u16 bootnext;
+ efi_status_t ret;
+ efi_uintn_t size;
+
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+
+ return;
+ }
+
+ /* If UEFI BootNext variable is set, boot the BootNext load option */
+ size = sizeof(u16);
+ ret = efi_get_variable_int(u"BootNext",
+ &efi_global_variable_guid,
+ NULL, &size, &bootnext, NULL);
+ if (ret == EFI_SUCCESS)
+ /* BootNext does exist here, try to boot */
+ run_command("bootefi bootmgr", 0);
+}
+
+static enum bootmenu_ret bootmenu_show(int delay)
+{
+ int cmd_ret;
int init = 0;
void *choice = NULL;
- char *title = NULL;
+ u16 *title = NULL;
char *command = NULL;
struct menu *menu;
struct bootmenu_data *bootmenu;
struct bootmenu_entry *iter;
+ efi_status_t efi_ret = EFI_SUCCESS;
char *option, *sep;
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
+ handle_uefi_bootnext();
+
/* If delay is 0 do not create menu, just run first entry */
if (delay == 0) {
option = bootmenu_getoption(0);
if (!option) {
puts("bootmenu option 0 was not found\n");
- return;
+ return BOOTMENU_RET_FAIL;
}
sep = strchr(option, '=');
if (!sep) {
puts("bootmenu option 0 is invalid\n");
- return;
+ return BOOTMENU_RET_FAIL;
}
- run_command(sep+1, 0);
- return;
+ cmd_ret = run_command(sep + 1, 0);
+ return (cmd_ret == CMD_RET_SUCCESS ? BOOTMENU_RET_SUCCESS :
BOOTMENU_RET_FAIL);
}
bootmenu = bootmenu_create(delay);
if (!bootmenu)
- return;
+ return BOOTMENU_RET_FAIL;
menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
bootmenu_print_entry, bootmenu_choice_entry,
bootmenu);
if (!menu) {
bootmenu_destroy(bootmenu);
- return;
+ return BOOTMENU_RET_FAIL;
}
for (iter = bootmenu->first; iter; iter = iter->next) {
@@ -478,8 +734,33 @@ static void bootmenu_show(int delay)
if (menu_get_choice(menu, &choice) == 1) {
iter = choice;
- title = strdup(iter->title);
+ /* last entry is U-Boot console or Quit */
+ if (iter->num == iter->menu->count - 1) {
+ menu_destroy(menu);
+ bootmenu_destroy(bootmenu);
+ return BOOTMENU_RET_QUIT;
+ }
+
+ title = u16_strdup(iter->title);
command = strdup(iter->command);
+ } else {
+ goto cleanup;
+ }
+
+ /*
+ * If the selected entry is UEFI BOOT####, set the BootNext variable.
+ * Then uefi bootmgr is invoked by the preset command in iter->command.
+ */
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
+ if (iter->type == BOOTMENU_TYPE_UEFI_BOOT_OPTION) {
+ efi_ret = efi_set_variable_int(u"BootNext",
&efi_global_variable_guid,
We should avoid needlessly writing to flash. Can't we avoid the
non-volatile flag here?
Best regards
Heinrich
+
EFI_VARIABLE_NON_VOLATILE |
+
EFI_VARIABLE_BOOTSERVICE_ACCESS |
+
EFI_VARIABLE_RUNTIME_ACCESS,
+ sizeof(u16),
&iter->bootorder, false);
+ if (efi_ret != EFI_SUCCESS)
+ goto cleanup;
+ }
}
cleanup:
@@ -493,21 +774,47 @@ cleanup:
}
if (title && command) {
- debug("Starting entry '%s'\n", title);
+ debug("Starting entry '%ls'\n", title);
free(title);
- run_command(command, 0);
+ if (efi_ret == EFI_SUCCESS)
+ cmd_ret = run_command(command, 0);
free(command);
}
#ifdef CONFIG_POSTBOOTMENU
run_command(CONFIG_POSTBOOTMENU, 0);
#endif
+
+ if (efi_ret == EFI_SUCCESS && cmd_ret == CMD_RET_SUCCESS)
+ return BOOTMENU_RET_SUCCESS;
+
+ return BOOTMENU_RET_FAIL;
}
#ifdef CONFIG_AUTOBOOT_MENU_SHOW
int menu_show(int bootdelay)
{
- bootmenu_show(bootdelay);
+ int ret;
+
+ while (1) {
+ ret = bootmenu_show(bootdelay);
+ bootdelay = -1;
+ if (ret == BOOTMENU_RET_UPDATED)
+ continue;
+
+ if (!IS_ENABLED(CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE)) {
+ if (ret == BOOTMENU_RET_QUIT) {
+ /* default boot process */
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
+ run_command("bootefi bootmgr", 0);
+
+ run_command("run bootcmd", 0);
+ }
+ } else {
+ break;
+ }
+ }
+
return -1; /* -1 - abort boot and run monitor code */
}
#endif
