[PATCH v4 00/11] enable menu-driven boot device selection
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
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 UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 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
[Major changes from RFC v3] - add Kconfig option to disable U-Boot console - add UEFI boot variable maintenance feature - support removable media support and user selection - app bootmenu enhancement documentation
[How to run on QEMU(arm64)] 1) clone source code $ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
2) prepare U-Boot .config $ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
3) run on QEMU(arm64) example $ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
U-Boot menu framework(common/menu.c) returns 1 if it is successful, returns negative value if it fails.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- Newly added in v4
cmd/bootmenu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 409ef9a848..d573487272 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -463,7 +463,7 @@ static void bootmenu_show(int delay) }
for (iter = bootmenu->first; iter; iter = iter->next) { - if (!menu_item_add(menu, iter->key, iter)) + if (menu_item_add(menu, iter->key, iter) != 1) goto cleanup; }
@@ -476,7 +476,7 @@ static void bootmenu_show(int delay)
init = 1;
- if (menu_get_choice(menu, &choice)) { + if (menu_get_choice(menu, &choice) == 1) { iter = choice; title = strdup(iter->title); command = strdup(iter->command);
On Thu, Mar 24, 2022 at 10:54:33PM +0900, Masahisa Kojima wrote:
U-Boot menu framework(common/menu.c) returns 1 if it is successful, returns negative value if it fails.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Newly added in v4
cmd/bootmenu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 409ef9a848..d573487272 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -463,7 +463,7 @@ static void bootmenu_show(int delay) }
for (iter = bootmenu->first; iter; iter = iter->next) {
if (!menu_item_add(menu, iter->key, iter))
if (menu_item_add(menu, iter->key, iter) != 1) goto cleanup;@@ -476,7 +476,7 @@ static void bootmenu_show(int delay)
init = 1;
- if (menu_get_choice(menu, &choice)) {
- if (menu_get_choice(menu, &choice) == 1) { iter = choice; title = strdup(iter->title); command = strdup(iter->command);
-- 2.17.1
Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Provide u16 string version of strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- Changes in v4: - add blank line above the return statement
Changes in v2: - implement u16_strlcat(with the destination buffer size in argument) instead of u16_strcat
include/charset.h | 15 +++++++++++++++ lib/charset.c | 21 +++++++++++++++++++++ 2 files changed, 36 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..47997eca7d 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -428,6 +428,27 @@ 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) {
On 3/24/22 14:54, Masahisa Kojima wrote:
Provide u16 string version of strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v4:
- add blank line above the return statement
Changes in v2:
implement u16_strlcat(with the destination buffer size in argument) instead of u16_strcat
include/charset.h | 15 +++++++++++++++ lib/charset.c | 21 +++++++++++++++++++++ 2 files changed, 36 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
The function should be called u16_strncat() in reference to the strncat() function.
- 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..47997eca7d 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -428,6 +428,27 @@ 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);
For size = dlen + 1 this results in
len = SIZE_MAX = 0xffffffffffffffff
Something must be missing in your unit test.
Best regards
Heinrich
- 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) {
Hi Heinrich,
On Sat, 2 Apr 2022 at 16:19, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
Provide u16 string version of strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v4:
- add blank line above the return statement
Changes in v2:
implement u16_strlcat(with the destination buffer size in argument) instead of u16_strcat
include/charset.h | 15 +++++++++++++++ lib/charset.c | 21 +++++++++++++++++++++ 2 files changed, 36 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
The function should be called u16_strncat() in reference to the strncat() function.
I intended to implement the string concatenation function with destination buffer size check, it is u16_strlcat(). strncat() is not safe. strncat() has size parameter, but it indicates the size to be copied to the destination, not the size of the destination buffer.
- 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..47997eca7d 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -428,6 +428,27 @@ 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);For size = dlen + 1 this results in
len = SIZE_MAX = 0xffffffffffffffff
Something must be missing in your unit test.
Yes, you are correct. I need to care about the case that the size is an odd number.
Thanks, Masahisa Kojima
Best regards
Heinrich
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) {
On 3/24/22 14:54, Masahisa Kojima wrote:
Provide u16 string version of strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v4:
- add blank line above the return statement
Changes in v2:
implement u16_strlcat(with the destination buffer size in argument) instead of u16_strcat
include/charset.h | 15 +++++++++++++++ lib/charset.c | 21 +++++++++++++++++++++ 2 files changed, 36 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.
Why "- 1"?
If size is even, we append up to size - u16_strlen(dst) - 2 bytes. The two extra bytes used for 0x0000. If size is odd, we append up to size - u16_strlen(dst) - 3 bytes leaving one byte of the buffer unused.
- @dest: destination buffer (null terminated)
- @src: source buffer (null terminated)
- @size: destination buffer size in bytes
s/$/ including the trailing 0x0000/
- 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..47997eca7d 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -428,6 +428,27 @@ u16 *u16_strdup(const void *src) return new; }
+size_t u16_strlcat(u16 *dest, const u16 *src, size_t size) +{
If you start the function with
size >>= 1;
or
size /= sizeof(u16);
this might simplify the code.
- 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;
This misses the trailing 0x0000.
Best regards
Heinrich
- 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) {
On Sat, 16 Apr 2022 at 16:32, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
Provide u16 string version of strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v4:
- add blank line above the return statement
Changes in v2:
implement u16_strlcat(with the destination buffer size in argument) instead of u16_strcat
include/charset.h | 15 +++++++++++++++ lib/charset.c | 21 +++++++++++++++++++++ 2 files changed, 36 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.
Why "- 1"?
It is my mistake, it should be 2.
If size is even, we append up to size - u16_strlen(dst) - 2 bytes. The two extra bytes used for 0x0000. If size is odd, we append up to size - u16_strlen(dst) - 3 bytes leaving one byte of the buffer unused.
Thanks, It clearly explains the behavior.
- @dest: destination buffer (null terminated)
- @src: source buffer (null terminated)
- @size: destination buffer size in bytes
s/$/ including the trailing 0x0000/
OK, I will update "(null terminated)" to the suggested one.
- 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..47997eca7d 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -428,6 +428,27 @@ u16 *u16_strdup(const void *src) return new; }
+size_t u16_strlcat(u16 *dest, const u16 *src, size_t size) +{
If you start the function with
size >>= 1;or
size /= sizeof(u16);this might simplify the code.
In u16_strlcat(), there are two size definitions, u16 string size and buffer size. I will rename some of the variables to clearly identify the meaning.
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;This misses the trailing 0x0000.
Strlcat() is not the C standard function, but the linux implementation of strlcat() does not include trailing 0x00[1], also the same for openbsd. [1] https://github.com/torvalds/linux/blob/master/lib/string.c#L319.
The current U-Boot strlcat() contains trailing 0x00, I think it needs to be updated.
Thanks, Masahisa Kojima
Best regards
Heinrich
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) {
Hi Heinrich,
On Mon, 18 Apr 2022 at 16:47, Masahisa Kojima masahisa.kojima@linaro.org wrote:
On Sat, 16 Apr 2022 at 16:32, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
Provide u16 string version of strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
Changes in v4:
- add blank line above the return statement
Changes in v2:
implement u16_strlcat(with the destination buffer size in argument) instead of u16_strcat
include/charset.h | 15 +++++++++++++++ lib/charset.c | 21 +++++++++++++++++++++ 2 files changed, 36 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.
Why "- 1"?
It is my mistake, it should be 2.
If size is even, we append up to size - u16_strlen(dst) - 2 bytes. The two extra bytes used for 0x0000. If size is odd, we append up to size - u16_strlen(dst) - 3 bytes leaving one byte of the buffer unused.
To make behavior simple, I update the meaning of the 3rd parameter from buffer size to u16 string count. It is the same behavior as other u16_strxxx functions in U-boot.
Thanks, Masahisa Kojima
Thanks, It clearly explains the behavior.
- @dest: destination buffer (null terminated)
- @src: source buffer (null terminated)
- @size: destination buffer size in bytes
s/$/ including the trailing 0x0000/
OK, I will update "(null terminated)" to the suggested one.
- 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..47997eca7d 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -428,6 +428,27 @@ u16 *u16_strdup(const void *src) return new; }
+size_t u16_strlcat(u16 *dest, const u16 *src, size_t size) +{
If you start the function with
size >>= 1;or
size /= sizeof(u16);this might simplify the code.
In u16_strlcat(), there are two size definitions, u16 string size and buffer size. I will rename some of the variables to clearly identify the meaning.
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;This misses the trailing 0x0000.
Strlcat() is not the C standard function, but the linux implementation of strlcat() does not include trailing 0x00[1], also the same for openbsd. [1] https://github.com/torvalds/linux/blob/master/lib/string.c#L319.
The current U-Boot strlcat() contains trailing 0x00, I think it needs to be updated.
Thanks, Masahisa Kojima
Best regards
Heinrich
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) {
Provide a unit test for function u16_strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- No change since v2
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);
On 3/24/22 14:54, Masahisa Kojima wrote:
Provide a unit test for function u16_strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
No change since v2
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];
buf should be longer then the expected test result, e.g. buf[40].
- u16 dest[] = u"U-Boot";
- u16 src[] = u"test";
The letters in the test should be different to each other and should not have zero bytes. You could use:
dest = {0x043b, 0x043e, 0x0434, 0x043a, 0x0430, 0} /* u"лодка" */ src = {0x6f5c, 0x8247, 0} /* u"潜艇" */
- u16 expected[] = u"U-Boottest";
- u16 null_src = u'\0';
- size_t ret;
If you want to detect failures, you have to set the unused buffer bytes to non-zero values except for the last two bytes which should be set to zero.
- 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));
This test does not catch all corner cases. You never use an odd value for size. E.g. size = 13 lead to a buffer overrun which you did not catch.
Please, enclose your tests in a loop for size from 0 to sizeof(buf) - 2 where sizeof(buf) is larger then any expected string size.
Test with dest and/or src being { 0 } too.
Best regards
Heinrich
- 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);
On Sat, 2 Apr 2022 at 16:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
Provide a unit test for function u16_strlcat().
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
No change since v2
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];buf should be longer then the expected test result, e.g. buf[40].
u16 dest[] = u"U-Boot";u16 src[] = u"test";The letters in the test should be different to each other and should not have zero bytes. You could use:
dest = {0x043b, 0x043e, 0x0434, 0x043a, 0x0430, 0} /* u"лодка" */ src = {0x6f5c, 0x8247, 0} /* u"潜艇" */
u16 expected[] = u"U-Boottest";u16 null_src = u'\0';size_t ret;If you want to detect failures, you have to set the unused buffer bytes to non-zero values except for the last two bytes which should be set to zero.
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));This test does not catch all corner cases. You never use an odd value for size. E.g. size = 13 lead to a buffer overrun which you did not catch.
Please, enclose your tests in a loop for size from 0 to sizeof(buf) - 2 where sizeof(buf) is larger then any expected string size.
Test with dest and/or src being { 0 } too.
I will add these test cases in the next version.
Thanks, Masahisa Kojima
Best regards
Heinrich
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);
To make user aware of the menu entry selection, menu always appears regardless of the number of entry.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- Newly created in v4
common/menu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/menu.c b/common/menu.c index 5fb2ffbd06..b577d80b4f 100644 --- a/common/menu.c +++ b/common/menu.c @@ -271,7 +271,7 @@ int menu_get_choice(struct menu *m, void **choice) if (!m || !choice) return -EINVAL;
- if (!m->prompt || m->item_cnt == 1) + if (!m->prompt) return menu_default_choice(m, choice);
return menu_interactive_choice(m, choice);
On 3/24/22 14:54, Masahisa Kojima wrote:
To make user aware of the menu entry selection, menu always appears regardless of the number of entry.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Newly created in v4
common/menu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/menu.c b/common/menu.c index 5fb2ffbd06..b577d80b4f 100644 --- a/common/menu.c +++ b/common/menu.c @@ -271,7 +271,7 @@ int menu_get_choice(struct menu *m, void **choice) if (!m || !choice) return -EINVAL;
- if (!m->prompt || m->item_cnt == 1)
- if (!m->prompt) return menu_default_choice(m, choice);
As we can disable entering the console item_cnt may be zero. This case needs to be handled.
Best regards
Heinrich
return menu_interactive_choice(m, choice);
From: AKASHI Takahiro takahiro.akashi@linaro.org
This function will be used in the next commit where some behavior of EFI boot manager will be expanded.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- No changes from original version
include/efi_loader.h | 4 ++++ lib/efi_loader/efi_boottime.c | 7 +++---- 2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index af36639ec6..ce22660d5b 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -593,6 +593,10 @@ efi_status_t efi_create_handle(efi_handle_t *handle); void efi_delete_handle(efi_handle_t obj); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(const efi_handle_t handle); +/* Locate device_path handle */ +efi_status_t EFIAPI efi_locate_device_path(const efi_guid_t *protocol, + struct efi_device_path **device_path, + efi_handle_t *device); /* Load image */ efi_status_t EFIAPI efi_load_image(bool boot_policy, efi_handle_t parent_image, diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 5bcb8253ed..4da64b5d29 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1799,10 +1799,9 @@ failure: * * Return: status code */ -static efi_status_t EFIAPI efi_locate_device_path( - const efi_guid_t *protocol, - struct efi_device_path **device_path, - efi_handle_t *device) +efi_status_t EFIAPI efi_locate_device_path(const efi_guid_t *protocol, + struct efi_device_path **device_path, + efi_handle_t *device) { struct efi_device_path *dp; size_t i;
On Thu, Mar 24, 2022 at 10:54:37PM +0900, Masahisa Kojima wrote:
From: AKASHI Takahiro takahiro.akashi@linaro.org
This function will be used in the next commit where some behavior of EFI boot manager will be expanded.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
No changes from original version
include/efi_loader.h | 4 ++++ lib/efi_loader/efi_boottime.c | 7 +++---- 2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index af36639ec6..ce22660d5b 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -593,6 +593,10 @@ efi_status_t efi_create_handle(efi_handle_t *handle); void efi_delete_handle(efi_handle_t obj); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(const efi_handle_t handle); +/* Locate device_path handle */ +efi_status_t EFIAPI efi_locate_device_path(const efi_guid_t *protocol,
struct efi_device_path **device_path,efi_handle_t *device);/* Load image */ efi_status_t EFIAPI efi_load_image(bool boot_policy, efi_handle_t parent_image, diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 5bcb8253ed..4da64b5d29 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1799,10 +1799,9 @@ failure:
- Return: status code
*/ -static efi_status_t EFIAPI efi_locate_device_path(
const efi_guid_t *protocol,struct efi_device_path **device_path,efi_handle_t *device)+efi_status_t EFIAPI efi_locate_device_path(const efi_guid_t *protocol,
struct efi_device_path **device_path,efi_handle_t *device){ struct efi_device_path *dp; size_t i; -- 2.17.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
From: AKASHI Takahiro takahiro.akashi@linaro.org
Under the current implementation, booting from removable media using a architecture-specific default image name, say BOOTAA64.EFI, is supported only in distro_bootcmd script. See the commit 74522c898b35 ("efi_loader: Add distro boot script for removable media").
This is, however, half-baked implementation because 1) UEFI specification requires this feature to be implemented as part of Boot Manager's responsibility:
3 - Boot Manager 3.5.1 Boot via the Simple File Protocol When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will start with a device path that points to the device that implements the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next part of the FilePath may point to the file name, including subdirectories, which contain the bootable image. If the file name is a null device path, the file name must be generated from the rules defined below. ... 3.5.1.1 Removable Media Boot Behavior To generate a file name when none is present in the FilePath, the firmware must append a default file name in the form \EFI\BOOT\BOOT{machine type short-name}.EFI ...
2) So (1) entails the hehavior that the user's preference of boot media order should be determined by Boot#### and BootOrder variables.
With this patch, the semantics mentioned above is fully implemented. For example, if you want to boot the system from USB and SCSI in this order, * define Boot0001 which contains only a device path to the USB device (without any file path/name) * define Boot0002 which contains only a device path to the SCSI device, and * set BootOrder to Boot0001:Boot0002
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- Changes from original version: - create new include file "efi_default_filename.h" to avoid conflict with config_distro_bootcmd.h - modify the target pointer of efi_free_pool(), expand_media_path() should only free the pointer allocated by efi_dp_from_file() function.
include/config_distro_bootcmd.h | 14 +-------- include/efi_default_filename.h | 26 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 ++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 include/efi_default_filename.h
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 2f90929178..ef2c9f330e 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -91,19 +91,7 @@ #endif
#ifdef CONFIG_EFI_LOADER -#if defined(CONFIG_ARM64) -#define BOOTEFI_NAME "bootaa64.efi" -#elif defined(CONFIG_ARM) -#define BOOTEFI_NAME "bootarm.efi" -#elif defined(CONFIG_X86_RUN_32BIT) -#define BOOTEFI_NAME "bootia32.efi" -#elif defined(CONFIG_X86_RUN_64BIT) -#define BOOTEFI_NAME "bootx64.efi" -#elif defined(CONFIG_ARCH_RV32I) -#define BOOTEFI_NAME "bootriscv32.efi" -#elif defined(CONFIG_ARCH_RV64I) -#define BOOTEFI_NAME "bootriscv64.efi" -#endif +#include <efi_default_filename.h> #endif
#ifdef BOOTEFI_NAME diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h new file mode 100644 index 0000000000..de030d2692 --- /dev/null +++ b/include/efi_default_filename.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Default boot file name when none is present in the FilePath. + * + * Copyright (c) 2022, Linaro Limited + */ +#ifndef _EFI_DEFAULT_FILENAME_H +#define _EFI_DEFAULT_FILENAME_H + +#if defined(CONFIG_ARM64) +#define BOOTEFI_NAME "BOOTAA64.EFI" +#elif defined(CONFIG_ARM) +#define BOOTEFI_NAME "BOOTARM.EFI" +#elif defined(CONFIG_X86_64) +#define BOOTEFI_NAME "BOOTX64.EFI" +#elif defined(CONFIG_X86) +#define BOOTEFI_NAME "BOOTIA32.EFI" +#elif defined(CONFIG_ARCH_RV32I) +#define BOOTEFI_NAME "BOOTRISCV32.EFI" +#elif defined(CONFIG_ARCH_RV64I) +#define BOOTEFI_NAME "BOOTRISCV64.EFI" +#else +#error Unsupported UEFI architecture +#endif + +#endif diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 8c04ecbdc8..22a4302aac 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -11,6 +11,7 @@ #include <charset.h> #include <log.h> #include <malloc.h> +#include <efi_default_filename.h> #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> @@ -30,6 +31,50 @@ static const struct efi_runtime_services *rs; * should do normal or recovery boot. */
+/** + * expand_media_path() - expand a device path for default file name + * @device_path: device path to check against + * + * If @device_path is a media or disk partition which houses a file + * system, this function returns a full device path which contains + * an architecture-specific default file name for removable media. + * + * Return: a newly allocated device path + */ +static +struct efi_device_path *expand_media_path(struct efi_device_path *device_path) +{ + struct efi_device_path *dp, *full_path; + efi_handle_t handle; + efi_status_t ret; + + if (!device_path) + return NULL; + + /* + * If device_path is a (removable) media or partition which provides + * simple file system protocol, append a default file name to support + * booting from removable media. + */ + dp = device_path; + ret = efi_locate_device_path(&efi_simple_file_system_protocol_guid, + &dp, &handle); + if (ret == EFI_SUCCESS) { + if (dp->type == DEVICE_PATH_TYPE_END) { + dp = efi_dp_from_file(NULL, 0, + "/EFI/BOOT/" BOOTEFI_NAME); + full_path = efi_dp_append(device_path, dp); + efi_free_pool(dp); + } else { + full_path = efi_dp_dup(device_path); + } + } else { + full_path = efi_dp_dup(device_path); + } + + return full_path; +} + /** * try_load_entry() - try to load image for boot option * @@ -68,13 +113,16 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, }
if (lo.attributes & LOAD_OPTION_ACTIVE) { + struct efi_device_path *file_path; u32 attributes;
log_debug("%s: trying to load "%ls" from %pD\n", __func__, lo.label, lo.file_path);
- ret = EFI_CALL(efi_load_image(true, efi_root, lo.file_path, + file_path = expand_media_path(lo.file_path); + ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, handle)); + efi_free_pool(file_path); if (ret != EFI_SUCCESS) { log_warning("Loading %ls '%ls' failed\n", varname, lo.label);
Hello Akashi-san,
On Thu, Mar 24, 2022 at 10:54:38PM +0900, Masahisa Kojima wrote:
From: AKASHI Takahiro takahiro.akashi@linaro.org
Under the current implementation, booting from removable media using a architecture-specific default image name, say BOOTAA64.EFI, is supported only in distro_bootcmd script. See the commit 74522c898b35 ("efi_loader: Add distro boot script for removable media").
This is, however, half-baked implementation because
- UEFI specification requires this feature to be implemented as part of Boot Manager's responsibility:
3 - Boot Manager 3.5.1 Boot via the Simple File Protocol When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will start with a device path that points to the device that implements the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next part of the FilePath may point to the file name, including subdirectories, which contain the bootable image. If the file name is a null device path, the file name must be generated from the rules defined below. ... 3.5.1.1 Removable Media Boot Behavior To generate a file name when none is present in the FilePath, the firmware must append a default file name in the form \EFI\BOOT\BOOT{machine type short-name}.EFI ...
- So (1) entails the hehavior that the user's preference of boot media order should be determined by Boot#### and BootOrder variables.
With this patch, the semantics mentioned above is fully implemented. For example, if you want to boot the system from USB and SCSI in this order,
- define Boot0001 which contains only a device path to the USB device (without any file path/name)
- define Boot0002 which contains only a device path to the SCSI device,
and
- set BootOrder to Boot0001:Boot0002
Mark had some concerns wrt to this approach and from what I can tell this hasn't changed in this revision [1]. Can we use boot_targets and generate Boot#### with an empty FilePath as Mark suggested? Or is the user expected to select that somehow from the menu?
[1] https://lore.kernel.org/u-boot/d3cac2e5b37f96b5@bloch.sibelius.xs4all.nl/
Thanks /Ilias
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Changes from original version:
- create new include file "efi_default_filename.h" to avoid conflict with config_distro_bootcmd.h
- modify the target pointer of efi_free_pool(), expand_media_path() should only free the pointer allocated by efi_dp_from_file() function.
include/config_distro_bootcmd.h | 14 +-------- include/efi_default_filename.h | 26 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 ++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 include/efi_default_filename.h
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 2f90929178..ef2c9f330e 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -91,19 +91,7 @@ #endif
#ifdef CONFIG_EFI_LOADER -#if defined(CONFIG_ARM64) -#define BOOTEFI_NAME "bootaa64.efi" -#elif defined(CONFIG_ARM) -#define BOOTEFI_NAME "bootarm.efi" -#elif defined(CONFIG_X86_RUN_32BIT) -#define BOOTEFI_NAME "bootia32.efi" -#elif defined(CONFIG_X86_RUN_64BIT) -#define BOOTEFI_NAME "bootx64.efi" -#elif defined(CONFIG_ARCH_RV32I) -#define BOOTEFI_NAME "bootriscv32.efi" -#elif defined(CONFIG_ARCH_RV64I) -#define BOOTEFI_NAME "bootriscv64.efi" -#endif +#include <efi_default_filename.h> #endif
#ifdef BOOTEFI_NAME diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h new file mode 100644 index 0000000000..de030d2692 --- /dev/null +++ b/include/efi_default_filename.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Default boot file name when none is present in the FilePath.
- Copyright (c) 2022, Linaro Limited
- */
+#ifndef _EFI_DEFAULT_FILENAME_H +#define _EFI_DEFAULT_FILENAME_H
+#if defined(CONFIG_ARM64) +#define BOOTEFI_NAME "BOOTAA64.EFI" +#elif defined(CONFIG_ARM) +#define BOOTEFI_NAME "BOOTARM.EFI" +#elif defined(CONFIG_X86_64) +#define BOOTEFI_NAME "BOOTX64.EFI" +#elif defined(CONFIG_X86) +#define BOOTEFI_NAME "BOOTIA32.EFI" +#elif defined(CONFIG_ARCH_RV32I) +#define BOOTEFI_NAME "BOOTRISCV32.EFI" +#elif defined(CONFIG_ARCH_RV64I) +#define BOOTEFI_NAME "BOOTRISCV64.EFI" +#else +#error Unsupported UEFI architecture +#endif
+#endif diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 8c04ecbdc8..22a4302aac 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -11,6 +11,7 @@ #include <charset.h> #include <log.h> #include <malloc.h> +#include <efi_default_filename.h> #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> @@ -30,6 +31,50 @@ static const struct efi_runtime_services *rs;
- should do normal or recovery boot.
*/
+/**
- expand_media_path() - expand a device path for default file name
- @device_path: device path to check against
- If @device_path is a media or disk partition which houses a file
- system, this function returns a full device path which contains
- an architecture-specific default file name for removable media.
- Return: a newly allocated device path
- */
+static +struct efi_device_path *expand_media_path(struct efi_device_path *device_path) +{
- struct efi_device_path *dp, *full_path;
- efi_handle_t handle;
- efi_status_t ret;
- if (!device_path)
return NULL;- /*
* If device_path is a (removable) media or partition which provides* simple file system protocol, append a default file name to support* booting from removable media.*/- dp = device_path;
- ret = efi_locate_device_path(&efi_simple_file_system_protocol_guid,
&dp, &handle);- if (ret == EFI_SUCCESS) {
if (dp->type == DEVICE_PATH_TYPE_END) {dp = efi_dp_from_file(NULL, 0,"/EFI/BOOT/" BOOTEFI_NAME);full_path = efi_dp_append(device_path, dp);efi_free_pool(dp);} else {full_path = efi_dp_dup(device_path);}- } else {
full_path = efi_dp_dup(device_path);- }
- return full_path;
+}
/**
- try_load_entry() - try to load image for boot option
@@ -68,13 +113,16 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, }
if (lo.attributes & LOAD_OPTION_ACTIVE) {
struct efi_device_path *file_path;u32 attributes;
log_debug("%s: trying to load "%ls" from %pD\n", __func__, lo.label, lo.file_path);
ret = EFI_CALL(efi_load_image(true, efi_root, lo.file_path,
file_path = expand_media_path(lo.file_path);ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, handle));efi_free_pool(file_path);-- 2.17.1
Hi Ilias,
On Thu, 31 Mar 2022 at 04:13, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hello Akashi-san,
On Thu, Mar 24, 2022 at 10:54:38PM +0900, Masahisa Kojima wrote:
From: AKASHI Takahiro takahiro.akashi@linaro.org
Under the current implementation, booting from removable media using a architecture-specific default image name, say BOOTAA64.EFI, is supported only in distro_bootcmd script. See the commit 74522c898b35 ("efi_loader: Add distro boot script for removable media").
This is, however, half-baked implementation because
- UEFI specification requires this feature to be implemented as part of Boot Manager's responsibility:
3 - Boot Manager 3.5.1 Boot via the Simple File Protocol When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will start with a device path that points to the device that implements the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next part of the FilePath may point to the file name, including subdirectories, which contain the bootable image. If the file name is a null device path, the file name must be generated from the rules defined below. ... 3.5.1.1 Removable Media Boot Behavior To generate a file name when none is present in the FilePath, the firmware must append a default file name in the form \EFI\BOOT\BOOT{machine type short-name}.EFI ...
- So (1) entails the hehavior that the user's preference of boot media order should be determined by Boot#### and BootOrder variables.
With this patch, the semantics mentioned above is fully implemented. For example, if you want to boot the system from USB and SCSI in this order,
- define Boot0001 which contains only a device path to the USB device (without any file path/name)
- define Boot0002 which contains only a device path to the SCSI device,
and
- set BootOrder to Boot0001:Boot0002
Mark had some concerns wrt to this approach and from what I can tell this hasn't changed in this revision [1]. Can we use boot_targets and generate Boot#### with an empty FilePath as Mark suggested? Or is the user expected to select that somehow from the menu?
[1] https://lore.kernel.org/u-boot/d3cac2e5b37f96b5@bloch.sibelius.xs4all.nl/
My patch series tries to resolve the above issue to some extent by a different approach.
The patch "[PATCH v4 10/11] bootmenu: add removable media entries" [*1] enumerates the all (removable) medias supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
Example bootmenu is as below.
*** U-Boot Boot Menu ***
UEFI BOOT0000 : mmc0:1 UEFI BOOT0001 : mmc0:2 UEFI BOOT0002 : debian UEFI BOOT0003 : nvme0:1 UEFI BOOT0004 : ubuntu UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2
mmcX:X, usbX:X and nvmeX:X are the entries of the removable media having device path without FilePath. They are automatically created by the bootmenu and BOOT#### and BootOrder variables are updated. They are also managed by the bootmenu, if the usb device is removed from the system at the next boot, the BOOT#### variable is removed by bootmenu and BootOrder is also updated.
[*1] https://lore.kernel.org/u-boot/20220324135443.1571-11-masahisa.kojima@linaro...
Thanks, Masahisa Kojima
Thanks /Ilias
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Changes from original version:
- create new include file "efi_default_filename.h" to avoid conflict with config_distro_bootcmd.h
- modify the target pointer of efi_free_pool(), expand_media_path() should only free the pointer allocated by efi_dp_from_file() function.
include/config_distro_bootcmd.h | 14 +-------- include/efi_default_filename.h | 26 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 ++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 include/efi_default_filename.h
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 2f90929178..ef2c9f330e 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -91,19 +91,7 @@ #endif
#ifdef CONFIG_EFI_LOADER -#if defined(CONFIG_ARM64) -#define BOOTEFI_NAME "bootaa64.efi" -#elif defined(CONFIG_ARM) -#define BOOTEFI_NAME "bootarm.efi" -#elif defined(CONFIG_X86_RUN_32BIT) -#define BOOTEFI_NAME "bootia32.efi" -#elif defined(CONFIG_X86_RUN_64BIT) -#define BOOTEFI_NAME "bootx64.efi" -#elif defined(CONFIG_ARCH_RV32I) -#define BOOTEFI_NAME "bootriscv32.efi" -#elif defined(CONFIG_ARCH_RV64I) -#define BOOTEFI_NAME "bootriscv64.efi" -#endif +#include <efi_default_filename.h> #endif
#ifdef BOOTEFI_NAME diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h new file mode 100644 index 0000000000..de030d2692 --- /dev/null +++ b/include/efi_default_filename.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Default boot file name when none is present in the FilePath.
- Copyright (c) 2022, Linaro Limited
- */
+#ifndef _EFI_DEFAULT_FILENAME_H +#define _EFI_DEFAULT_FILENAME_H
+#if defined(CONFIG_ARM64) +#define BOOTEFI_NAME "BOOTAA64.EFI" +#elif defined(CONFIG_ARM) +#define BOOTEFI_NAME "BOOTARM.EFI" +#elif defined(CONFIG_X86_64) +#define BOOTEFI_NAME "BOOTX64.EFI" +#elif defined(CONFIG_X86) +#define BOOTEFI_NAME "BOOTIA32.EFI" +#elif defined(CONFIG_ARCH_RV32I) +#define BOOTEFI_NAME "BOOTRISCV32.EFI" +#elif defined(CONFIG_ARCH_RV64I) +#define BOOTEFI_NAME "BOOTRISCV64.EFI" +#else +#error Unsupported UEFI architecture +#endif
+#endif diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 8c04ecbdc8..22a4302aac 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -11,6 +11,7 @@ #include <charset.h> #include <log.h> #include <malloc.h> +#include <efi_default_filename.h> #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> @@ -30,6 +31,50 @@ static const struct efi_runtime_services *rs;
- should do normal or recovery boot.
*/
+/**
- expand_media_path() - expand a device path for default file name
- @device_path: device path to check against
- If @device_path is a media or disk partition which houses a file
- system, this function returns a full device path which contains
- an architecture-specific default file name for removable media.
- Return: a newly allocated device path
- */
+static +struct efi_device_path *expand_media_path(struct efi_device_path *device_path) +{
struct efi_device_path *dp, *full_path;efi_handle_t handle;efi_status_t ret;if (!device_path)return NULL;/** If device_path is a (removable) media or partition which provides* simple file system protocol, append a default file name to support* booting from removable media.*/dp = device_path;ret = efi_locate_device_path(&efi_simple_file_system_protocol_guid,&dp, &handle);if (ret == EFI_SUCCESS) {if (dp->type == DEVICE_PATH_TYPE_END) {dp = efi_dp_from_file(NULL, 0,"/EFI/BOOT/" BOOTEFI_NAME);full_path = efi_dp_append(device_path, dp);efi_free_pool(dp);} else {full_path = efi_dp_dup(device_path);}} else {full_path = efi_dp_dup(device_path);}return full_path;+}
/**
- try_load_entry() - try to load image for boot option
@@ -68,13 +113,16 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, }
if (lo.attributes & LOAD_OPTION_ACTIVE) {
struct efi_device_path *file_path; u32 attributes; log_debug("%s: trying to load \"%ls\" from %pD\n", __func__, lo.label, lo.file_path);
ret = EFI_CALL(efi_load_image(true, efi_root, lo.file_path,
file_path = expand_media_path(lo.file_path);ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, handle));efi_free_pool(file_path); if (ret != EFI_SUCCESS) { log_warning("Loading %ls '%ls' failed\n", varname, lo.label);-- 2.17.1
Kojima-san,
On Thu, 31 Mar 2022 at 03:51, Masahisa Kojima masahisa.kojima@linaro.org wrote:
Hi Ilias,
On Thu, 31 Mar 2022 at 04:13, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hello Akashi-san,
On Thu, Mar 24, 2022 at 10:54:38PM +0900, Masahisa Kojima wrote:
From: AKASHI Takahiro takahiro.akashi@linaro.org
Under the current implementation, booting from removable media using a architecture-specific default image name, say BOOTAA64.EFI, is supported only in distro_bootcmd script. See the commit 74522c898b35 ("efi_loader: Add distro boot script for removable media").
This is, however, half-baked implementation because
- UEFI specification requires this feature to be implemented as part of Boot Manager's responsibility:
3 - Boot Manager 3.5.1 Boot via the Simple File Protocol When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will start with a device path that points to the device that implements the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next part of the FilePath may point to the file name, including subdirectories, which contain the bootable image. If the file name is a null device path, the file name must be generated from the rules defined below. ... 3.5.1.1 Removable Media Boot Behavior To generate a file name when none is present in the FilePath, the firmware must append a default file name in the form \EFI\BOOT\BOOT{machine type short-name}.EFI ...
- So (1) entails the hehavior that the user's preference of boot media order should be determined by Boot#### and BootOrder variables.
With this patch, the semantics mentioned above is fully implemented. For example, if you want to boot the system from USB and SCSI in this order,
- define Boot0001 which contains only a device path to the USB device (without any file path/name)
- define Boot0002 which contains only a device path to the SCSI device,
and
- set BootOrder to Boot0001:Boot0002
Mark had some concerns wrt to this approach and from what I can tell this hasn't changed in this revision [1]. Can we use boot_targets and generate Boot#### with an empty FilePath as Mark suggested? Or is the user expected to select that somehow from the menu?
[1] https://lore.kernel.org/u-boot/d3cac2e5b37f96b5@bloch.sibelius.xs4all.nl/
My patch series tries to resolve the above issue to some extent by a different approach.
The patch "[PATCH v4 10/11] bootmenu: add removable media entries" [*1] enumerates the all (removable) medias supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
Example bootmenu is as below.
*** U-Boot Boot Menu *** UEFI BOOT0000 : mmc0:1 UEFI BOOT0001 : mmc0:2 UEFI BOOT0002 : debian UEFI BOOT0003 : nvme0:1 UEFI BOOT0004 : ubuntu UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2mmcX:X, usbX:X and nvmeX:X are the entries of the removable media having device path without FilePath. They are automatically created by the bootmenu and BOOT#### and BootOrder variables are updated. They are also managed by the bootmenu, if the usb device is removed from the system at the next boot, the BOOT#### variable is removed by bootmenu and BootOrder is also updated.
[*1] https://lore.kernel.org/u-boot/20220324135443.1571-11-masahisa.kojima@linaro...
Great thanks, that sounds reasonable. I'll go through the rest of the series and let you know /Ilias
Thanks, Masahisa Kojima
On 3/24/22 14:54, Masahisa Kojima wrote:
From: AKASHI Takahiro takahiro.akashi@linaro.org
Under the current implementation, booting from removable media using a architecture-specific default image name, say BOOTAA64.EFI, is supported only in distro_bootcmd script. See the commit 74522c898b35 ("efi_loader: Add distro boot script for removable media").
This is, however, half-baked implementation because
UEFI specification requires this feature to be implemented as part of Boot Manager's responsibility:
3 - Boot Manager 3.5.1 Boot via the Simple File Protocol When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will start with a device path that points to the device that implements the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next part of the FilePath may point to the file name, including subdirectories, which contain the bootable image. If the file name is a null device path, the file name must be generated from the rules defined below. ... 3.5.1.1 Removable Media Boot Behavior To generate a file name when none is present in the FilePath, the firmware must append a default file name in the form \EFI\BOOT\BOOT{machine type short-name}.EFI ...
So (1) entails the hehavior that the user's preference of boot media order should be determined by Boot#### and BootOrder variables.
At every boot you will have to delete autogenerated boot options and create new ones according to the media which are present.
Your implementation does not offer any possibility to identify autogenerated boot options.
On my laptop all autogenerated boot options use a VenMsg() device path
Boot0016* USB CD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,86701296aa5a7848b66cd49dd3ba6a55) Boot0017* USB FDD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,6ff015a28830b543a8b8641009461e49) Boot0018* NVMe0 VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,001c199932d94c4eae9aa0b6e98eb8a400) Boot0019* ATA HDD0 VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,91af625956449f41a7b91f4f892ab0f600) Boot001A* USB HDD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,33e821aaaf33bc4789bd419f88c50803) Boot001B* PCI LAN VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,78a84aaf2b2afc4ea79cf5cc8f3d3803)
while manual boot options use normal device paths
Boot0001* debian HD(2,GPT,54e58b03-c1db-4c6b-afda-24340c3acda5,0x109000,0x32000)/File(\EFI\debian\shimx64.efi) Boot0002* ubuntu HD(2,GPT,54e58b03-c1db-4c6b-afda-24340c3acda5,0x109000,0x32000)/File(\EFI\ubuntu\shimx64.efi)
Please, provide a concept that can differentiate between autogenerated and manually set boot options.
Best regards
Heinrich
With this patch, the semantics mentioned above is fully implemented. For example, if you want to boot the system from USB and SCSI in this order,
- define Boot0001 which contains only a device path to the USB device (without any file path/name)
- define Boot0002 which contains only a device path to the SCSI device,
and
- set BootOrder to Boot0001:Boot0002
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Changes from original version:
create new include file "efi_default_filename.h" to avoid conflict with config_distro_bootcmd.h
modify the target pointer of efi_free_pool(), expand_media_path() should only free the pointer allocated by efi_dp_from_file() function.
include/config_distro_bootcmd.h | 14 +-------- include/efi_default_filename.h | 26 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 ++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 include/efi_default_filename.h
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 2f90929178..ef2c9f330e 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -91,19 +91,7 @@ #endif
#ifdef CONFIG_EFI_LOADER -#if defined(CONFIG_ARM64) -#define BOOTEFI_NAME "bootaa64.efi" -#elif defined(CONFIG_ARM) -#define BOOTEFI_NAME "bootarm.efi" -#elif defined(CONFIG_X86_RUN_32BIT) -#define BOOTEFI_NAME "bootia32.efi" -#elif defined(CONFIG_X86_RUN_64BIT) -#define BOOTEFI_NAME "bootx64.efi" -#elif defined(CONFIG_ARCH_RV32I) -#define BOOTEFI_NAME "bootriscv32.efi" -#elif defined(CONFIG_ARCH_RV64I) -#define BOOTEFI_NAME "bootriscv64.efi" -#endif +#include <efi_default_filename.h> #endif
#ifdef BOOTEFI_NAME diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h new file mode 100644 index 0000000000..de030d2692 --- /dev/null +++ b/include/efi_default_filename.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Default boot file name when none is present in the FilePath.
- Copyright (c) 2022, Linaro Limited
- */
+#ifndef _EFI_DEFAULT_FILENAME_H +#define _EFI_DEFAULT_FILENAME_H
+#if defined(CONFIG_ARM64) +#define BOOTEFI_NAME "BOOTAA64.EFI" +#elif defined(CONFIG_ARM) +#define BOOTEFI_NAME "BOOTARM.EFI" +#elif defined(CONFIG_X86_64) +#define BOOTEFI_NAME "BOOTX64.EFI" +#elif defined(CONFIG_X86) +#define BOOTEFI_NAME "BOOTIA32.EFI" +#elif defined(CONFIG_ARCH_RV32I) +#define BOOTEFI_NAME "BOOTRISCV32.EFI" +#elif defined(CONFIG_ARCH_RV64I) +#define BOOTEFI_NAME "BOOTRISCV64.EFI" +#else +#error Unsupported UEFI architecture +#endif
+#endif diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 8c04ecbdc8..22a4302aac 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -11,6 +11,7 @@ #include <charset.h> #include <log.h> #include <malloc.h> +#include <efi_default_filename.h> #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> @@ -30,6 +31,50 @@ static const struct efi_runtime_services *rs;
- should do normal or recovery boot.
*/
+/**
- expand_media_path() - expand a device path for default file name
- @device_path: device path to check against
- If @device_path is a media or disk partition which houses a file
- system, this function returns a full device path which contains
- an architecture-specific default file name for removable media.
- Return: a newly allocated device path
- */
+static +struct efi_device_path *expand_media_path(struct efi_device_path *device_path) +{
- struct efi_device_path *dp, *full_path;
- efi_handle_t handle;
- efi_status_t ret;
- if (!device_path)
return NULL;- /*
* If device_path is a (removable) media or partition which provides* simple file system protocol, append a default file name to support* booting from removable media.*/- dp = device_path;
- ret = efi_locate_device_path(&efi_simple_file_system_protocol_guid,
&dp, &handle);- if (ret == EFI_SUCCESS) {
if (dp->type == DEVICE_PATH_TYPE_END) {dp = efi_dp_from_file(NULL, 0,"/EFI/BOOT/" BOOTEFI_NAME);full_path = efi_dp_append(device_path, dp);efi_free_pool(dp);} else {full_path = efi_dp_dup(device_path);}- } else {
full_path = efi_dp_dup(device_path);- }
- return full_path;
+}
- /**
- try_load_entry() - try to load image for boot option
@@ -68,13 +113,16 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, }
if (lo.attributes & LOAD_OPTION_ACTIVE) {
struct efi_device_path *file_path;u32 attributes;
log_debug("%s: trying to load "%ls" from %pD\n", __func__, lo.label, lo.file_path);
ret = EFI_CALL(efi_load_image(true, efi_root, lo.file_path,
file_path = expand_media_path(lo.file_path);ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, handle));efi_free_pool(file_path);
On Sat, 2 Apr 2022 at 15:12, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
From: AKASHI Takahiro takahiro.akashi@linaro.org
Under the current implementation, booting from removable media using a architecture-specific default image name, say BOOTAA64.EFI, is supported only in distro_bootcmd script. See the commit 74522c898b35 ("efi_loader: Add distro boot script for removable media").
This is, however, half-baked implementation because
UEFI specification requires this feature to be implemented as part of Boot Manager's responsibility:
3 - Boot Manager 3.5.1 Boot via the Simple File Protocol When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will start with a device path that points to the device that implements the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next part of the FilePath may point to the file name, including subdirectories, which contain the bootable image. If the file name is a null device path, the file name must be generated from the rules defined below. ... 3.5.1.1 Removable Media Boot Behavior To generate a file name when none is present in the FilePath, the firmware must append a default file name in the form \EFI\BOOT\BOOT{machine type short-name}.EFI ...
So (1) entails the hehavior that the user's preference of boot media order should be determined by Boot#### and BootOrder variables.
At every boot you will have to delete autogenerated boot options and create new ones according to the media which are present.
Your implementation does not offer any possibility to identify autogenerated boot options.
On my laptop all autogenerated boot options use a VenMsg() device path
Boot0016* USB CD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,86701296aa5a7848b66cd49dd3ba6a55) Boot0017* USB FDD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,6ff015a28830b543a8b8641009461e49) Boot0018* NVMe0 VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,001c199932d94c4eae9aa0b6e98eb8a400) Boot0019* ATA HDD0 VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,91af625956449f41a7b91f4f892ab0f600) Boot001A* USB HDD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,33e821aaaf33bc4789bd419f88c50803) Boot001B* PCI LAN VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,78a84aaf2b2afc4ea79cf5cc8f3d3803)
while manual boot options use normal device paths
Boot0001* debian HD(2,GPT,54e58b03-c1db-4c6b-afda-24340c3acda5,0x109000,0x32000)/File(\EFI\debian\shimx64.efi) Boot0002* ubuntu HD(2,GPT,54e58b03-c1db-4c6b-afda-24340c3acda5,0x109000,0x32000)/File(\EFI\ubuntu\shimx64.efi)
Please, provide a concept that can differentiate between autogenerated and manually set boot options.
The patch "[PATCH v4 10/11] bootmenu: add removable media entries" [*1] handles this auto generation and auto deletion. # Sorry, I should clearly describe this in the commit message.
u"bootmenu" string is stored in EFI_LOAD_OPTION.OptionalData of the autogenerated Boot#### variable, to differentiate the boot options between autogenerated and manually set. # In EDK2 implementation, a special GUID is used for this purpose and GUID is stored in EFI_LOAD_OPTION.OptionalData. # In U-Boot, lib/efi_loader/efi_load_option.c::efi_serialize_load_option() handles the EFI_LOAD_OPTION.OptionalData as u16 string, so I use "bootmenu" string.
The example is as below, Boot0003 is an auto generated boot option, and Boot0004 is manually set. ========== Boot0003: attributes: A-- (0x00000001) label: virtio1:2 file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(63293792-adf5-9325-b99f-4e0e455c1b1e,01)/HD(2,GPT,fb12642d-1b5a-4e31-b074-ba2d813bed71,0x100800,0x18fff) data: 00000000: 62 00 6f 00 6f 00 74 00 6d 00 65 00 6e 00 75 00 b.o.o.t.m.e.n.u. 00000010: 00 00 .. Boot0004: attributes: A-- (0x00000001) label: debian file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(63293792-adf5-9325-b99f-4e0e455c1b1e,00)/HD(1,GPT,c2475a57-2735-4744-9f01-67fefa1d06ae,0x800,0x100000)/EFI\debian\grubaa64.efi data: ==========
In "[PATCH v4 10/11]" [*1], auto generation is handled as follows. 1) bootmenu enumerates the all devices having EFI_SIMPLE_FILE_SYSTEM_PROTOCOL 2) bootmenu compares the device path enumerated in step#1 and existing boot option read from EFI variable 3-a) if the enumerated device path in step#1 already exist in EFI variable, nothing to do. 3-b) if the enumerated device path in step#1 does not exist in EFI variable, boot option is auto generated. 3-c) If the boot option stored in EFI variable with "bootmenu" OptionalData does not appear in the device path enumerated in step#1, this boot option is treated as "invalid" and auto deleted.
[*1] https://lore.kernel.org/u-boot/20220324135443.1571-11-masahisa.kojima@linaro...
Thanks, Masahisa Kojima
Best regards
Heinrich
With this patch, the semantics mentioned above is fully implemented. For example, if you want to boot the system from USB and SCSI in this order,
- define Boot0001 which contains only a device path to the USB device (without any file path/name)
- define Boot0002 which contains only a device path to the SCSI device,
and
- set BootOrder to Boot0001:Boot0002
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Changes from original version:
create new include file "efi_default_filename.h" to avoid conflict with config_distro_bootcmd.h
modify the target pointer of efi_free_pool(), expand_media_path() should only free the pointer allocated by efi_dp_from_file() function.
include/config_distro_bootcmd.h | 14 +-------- include/efi_default_filename.h | 26 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 ++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 include/efi_default_filename.h
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 2f90929178..ef2c9f330e 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -91,19 +91,7 @@ #endif
#ifdef CONFIG_EFI_LOADER -#if defined(CONFIG_ARM64) -#define BOOTEFI_NAME "bootaa64.efi" -#elif defined(CONFIG_ARM) -#define BOOTEFI_NAME "bootarm.efi" -#elif defined(CONFIG_X86_RUN_32BIT) -#define BOOTEFI_NAME "bootia32.efi" -#elif defined(CONFIG_X86_RUN_64BIT) -#define BOOTEFI_NAME "bootx64.efi" -#elif defined(CONFIG_ARCH_RV32I) -#define BOOTEFI_NAME "bootriscv32.efi" -#elif defined(CONFIG_ARCH_RV64I) -#define BOOTEFI_NAME "bootriscv64.efi" -#endif +#include <efi_default_filename.h> #endif
#ifdef BOOTEFI_NAME diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h new file mode 100644 index 0000000000..de030d2692 --- /dev/null +++ b/include/efi_default_filename.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Default boot file name when none is present in the FilePath.
- Copyright (c) 2022, Linaro Limited
- */
+#ifndef _EFI_DEFAULT_FILENAME_H +#define _EFI_DEFAULT_FILENAME_H
+#if defined(CONFIG_ARM64) +#define BOOTEFI_NAME "BOOTAA64.EFI" +#elif defined(CONFIG_ARM) +#define BOOTEFI_NAME "BOOTARM.EFI" +#elif defined(CONFIG_X86_64) +#define BOOTEFI_NAME "BOOTX64.EFI" +#elif defined(CONFIG_X86) +#define BOOTEFI_NAME "BOOTIA32.EFI" +#elif defined(CONFIG_ARCH_RV32I) +#define BOOTEFI_NAME "BOOTRISCV32.EFI" +#elif defined(CONFIG_ARCH_RV64I) +#define BOOTEFI_NAME "BOOTRISCV64.EFI" +#else +#error Unsupported UEFI architecture +#endif
+#endif diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 8c04ecbdc8..22a4302aac 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -11,6 +11,7 @@ #include <charset.h> #include <log.h> #include <malloc.h> +#include <efi_default_filename.h> #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> @@ -30,6 +31,50 @@ static const struct efi_runtime_services *rs;
- should do normal or recovery boot.
*/
+/**
- expand_media_path() - expand a device path for default file name
- @device_path: device path to check against
- If @device_path is a media or disk partition which houses a file
- system, this function returns a full device path which contains
- an architecture-specific default file name for removable media.
- Return: a newly allocated device path
- */
+static +struct efi_device_path *expand_media_path(struct efi_device_path *device_path) +{
struct efi_device_path *dp, *full_path;efi_handle_t handle;efi_status_t ret;if (!device_path)return NULL;/** If device_path is a (removable) media or partition which provides* simple file system protocol, append a default file name to support* booting from removable media.*/dp = device_path;ret = efi_locate_device_path(&efi_simple_file_system_protocol_guid,&dp, &handle);if (ret == EFI_SUCCESS) {if (dp->type == DEVICE_PATH_TYPE_END) {dp = efi_dp_from_file(NULL, 0,"/EFI/BOOT/" BOOTEFI_NAME);full_path = efi_dp_append(device_path, dp);efi_free_pool(dp);} else {full_path = efi_dp_dup(device_path);}} else {full_path = efi_dp_dup(device_path);}return full_path;+}
- /**
- try_load_entry() - try to load image for boot option
@@ -68,13 +113,16 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, }
if (lo.attributes & LOAD_OPTION_ACTIVE) {
struct efi_device_path *file_path; u32 attributes; log_debug("%s: trying to load \"%ls\" from %pD\n", __func__, lo.label, lo.file_path);
ret = EFI_CALL(efi_load_image(true, efi_root, lo.file_path,
file_path = expand_media_path(lo.file_path);ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, handle));efi_free_pool(file_path); if (ret != EFI_SUCCESS) { log_warning("Loading %ls '%ls' failed\n", varname, lo.label);
On 4/4/22 08:48, Masahisa Kojima wrote:
On Sat, 2 Apr 2022 at 15:12, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
From: AKASHI Takahiro takahiro.akashi@linaro.org
Under the current implementation, booting from removable media using a architecture-specific default image name, say BOOTAA64.EFI, is supported only in distro_bootcmd script. See the commit 74522c898b35 ("efi_loader: Add distro boot script for removable media").
This is, however, half-baked implementation because
UEFI specification requires this feature to be implemented as part of Boot Manager's responsibility:
3 - Boot Manager 3.5.1 Boot via the Simple File Protocol When booting via the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, the FilePath will start with a device path that points to the device that implements the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL or the EFI_BLOCK_IO_PROTOCOL. The next part of the FilePath may point to the file name, including subdirectories, which contain the bootable image. If the file name is a null device path, the file name must be generated from the rules defined below. ... 3.5.1.1 Removable Media Boot Behavior To generate a file name when none is present in the FilePath, the firmware must append a default file name in the form \EFI\BOOT\BOOT{machine type short-name}.EFI ...
So (1) entails the hehavior that the user's preference of boot media order should be determined by Boot#### and BootOrder variables.
At every boot you will have to delete autogenerated boot options and create new ones according to the media which are present.
Your implementation does not offer any possibility to identify autogenerated boot options.
On my laptop all autogenerated boot options use a VenMsg() device path
Boot0016* USB CD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,86701296aa5a7848b66cd49dd3ba6a55) Boot0017* USB FDD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,6ff015a28830b543a8b8641009461e49) Boot0018* NVMe0 VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,001c199932d94c4eae9aa0b6e98eb8a400) Boot0019* ATA HDD0 VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,91af625956449f41a7b91f4f892ab0f600) Boot001A* USB HDD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,33e821aaaf33bc4789bd419f88c50803) Boot001B* PCI LAN VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,78a84aaf2b2afc4ea79cf5cc8f3d3803)
while manual boot options use normal device paths
Boot0001* debian HD(2,GPT,54e58b03-c1db-4c6b-afda-24340c3acda5,0x109000,0x32000)/File(\EFI\debian\shimx64.efi) Boot0002* ubuntu HD(2,GPT,54e58b03-c1db-4c6b-afda-24340c3acda5,0x109000,0x32000)/File(\EFI\ubuntu\shimx64.efi)
Please, provide a concept that can differentiate between autogenerated and manually set boot options.
The patch "[PATCH v4 10/11] bootmenu: add removable media entries" [*1] handles this auto generation and auto deletion. # Sorry, I should clearly describe this in the commit message.
u"bootmenu" string is stored in EFI_LOAD_OPTION.OptionalData of the autogenerated Boot#### variable, to differentiate the boot options between autogenerated and manually set. # In EDK2 implementation, a special GUID is used for this purpose and GUID is stored in EFI_LOAD_OPTION.OptionalData.
EDK II uses GUID mBmAutoCreateBootOptionGuid in MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c.
The chance of a collision for a GUID is much less then when using a string. Hence I would prefer a GUID based solution.
An alternative to storing the GUID in the optional data is in the device path. This is what I see on my laptop:
Boot001A* USB HDD VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b, 33e821aaaf33bc4783bd419f88c70803)
But using a GUID in the optional data is good enough.
Best regards
Heinrich
# In U-Boot, lib/efi_loader/efi_load_option.c::efi_serialize_load_option() handles the EFI_LOAD_OPTION.OptionalData as u16 string, so I use "bootmenu" string.
The example is as below, Boot0003 is an auto generated boot option, and Boot0004 is manually set. ========== Boot0003: attributes: A-- (0x00000001) label: virtio1:2 file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(63293792-adf5-9325-b99f-4e0e455c1b1e,01)/HD(2,GPT,fb12642d-1b5a-4e31-b074-ba2d813bed71,0x100800,0x18fff) data: 00000000: 62 00 6f 00 6f 00 74 00 6d 00 65 00 6e 00 75 00 b.o.o.t.m.e.n.u. 00000010: 00 00 .. Boot0004: attributes: A-- (0x00000001) label: debian file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(63293792-adf5-9325-b99f-4e0e455c1b1e,00)/HD(1,GPT,c2475a57-2735-4744-9f01-67fefa1d06ae,0x800,0x100000)/EFI\debian\grubaa64.efi data: ==========
In "[PATCH v4 10/11]" [*1], auto generation is handled as follows.
- bootmenu enumerates the all devices having EFI_SIMPLE_FILE_SYSTEM_PROTOCOL
- bootmenu compares the device path enumerated in step#1 and existing boot option read from EFI variable
3-a) if the enumerated device path in step#1 already exist in EFI variable, nothing to do. 3-b) if the enumerated device path in step#1 does not exist in EFI variable, boot option is auto generated. 3-c) If the boot option stored in EFI variable with "bootmenu" OptionalData does not appear in the device path enumerated in step#1, this boot option is treated as "invalid" and auto deleted.
[*1] https://lore.kernel.org/u-boot/20220324135443.1571-11-masahisa.kojima@linaro...
Thanks, Masahisa Kojima
Best regards
Heinrich
With this patch, the semantics mentioned above is fully implemented. For example, if you want to boot the system from USB and SCSI in this order,
- define Boot0001 which contains only a device path to the USB device (without any file path/name)
- define Boot0002 which contains only a device path to the SCSI device,
and
- set BootOrder to Boot0001:Boot0002
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Changes from original version:
create new include file "efi_default_filename.h" to avoid conflict with config_distro_bootcmd.h
modify the target pointer of efi_free_pool(), expand_media_path() should only free the pointer allocated by efi_dp_from_file() function.
include/config_distro_bootcmd.h | 14 +-------- include/efi_default_filename.h | 26 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 ++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 14 deletions(-) create mode 100644 include/efi_default_filename.h
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 2f90929178..ef2c9f330e 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -91,19 +91,7 @@ #endif
#ifdef CONFIG_EFI_LOADER -#if defined(CONFIG_ARM64) -#define BOOTEFI_NAME "bootaa64.efi" -#elif defined(CONFIG_ARM) -#define BOOTEFI_NAME "bootarm.efi" -#elif defined(CONFIG_X86_RUN_32BIT) -#define BOOTEFI_NAME "bootia32.efi" -#elif defined(CONFIG_X86_RUN_64BIT) -#define BOOTEFI_NAME "bootx64.efi" -#elif defined(CONFIG_ARCH_RV32I) -#define BOOTEFI_NAME "bootriscv32.efi" -#elif defined(CONFIG_ARCH_RV64I) -#define BOOTEFI_NAME "bootriscv64.efi" -#endif +#include <efi_default_filename.h> #endif
#ifdef BOOTEFI_NAME diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h new file mode 100644 index 0000000000..de030d2692 --- /dev/null +++ b/include/efi_default_filename.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Default boot file name when none is present in the FilePath.
- Copyright (c) 2022, Linaro Limited
- */
+#ifndef _EFI_DEFAULT_FILENAME_H +#define _EFI_DEFAULT_FILENAME_H
+#if defined(CONFIG_ARM64) +#define BOOTEFI_NAME "BOOTAA64.EFI" +#elif defined(CONFIG_ARM) +#define BOOTEFI_NAME "BOOTARM.EFI" +#elif defined(CONFIG_X86_64) +#define BOOTEFI_NAME "BOOTX64.EFI" +#elif defined(CONFIG_X86) +#define BOOTEFI_NAME "BOOTIA32.EFI" +#elif defined(CONFIG_ARCH_RV32I) +#define BOOTEFI_NAME "BOOTRISCV32.EFI" +#elif defined(CONFIG_ARCH_RV64I) +#define BOOTEFI_NAME "BOOTRISCV64.EFI" +#else +#error Unsupported UEFI architecture +#endif
+#endif diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 8c04ecbdc8..22a4302aac 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -11,6 +11,7 @@ #include <charset.h> #include <log.h> #include <malloc.h> +#include <efi_default_filename.h> #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> @@ -30,6 +31,50 @@ static const struct efi_runtime_services *rs; * should do normal or recovery boot. */
+/**
- expand_media_path() - expand a device path for default file name
- @device_path: device path to check against
- If @device_path is a media or disk partition which houses a file
- system, this function returns a full device path which contains
- an architecture-specific default file name for removable media.
- Return: a newly allocated device path
- */
+static +struct efi_device_path *expand_media_path(struct efi_device_path *device_path) +{
struct efi_device_path *dp, *full_path;efi_handle_t handle;efi_status_t ret;if (!device_path)return NULL;/** If device_path is a (removable) media or partition which provides* simple file system protocol, append a default file name to support* booting from removable media.*/dp = device_path;ret = efi_locate_device_path(&efi_simple_file_system_protocol_guid,&dp, &handle);if (ret == EFI_SUCCESS) {if (dp->type == DEVICE_PATH_TYPE_END) {dp = efi_dp_from_file(NULL, 0,"/EFI/BOOT/" BOOTEFI_NAME);full_path = efi_dp_append(device_path, dp);efi_free_pool(dp);} else {full_path = efi_dp_dup(device_path);}} else {full_path = efi_dp_dup(device_path);}return full_path;+}
- /**
- try_load_entry() - try to load image for boot option
@@ -68,13 +113,16 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle, }
if (lo.attributes & LOAD_OPTION_ACTIVE) {
struct efi_device_path *file_path; u32 attributes; log_debug("%s: trying to load \"%ls\" from %pD\n", __func__, lo.label, lo.file_path);
ret = EFI_CALL(efi_load_image(true, efi_root, lo.file_path,
file_path = expand_media_path(lo.file_path);ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0, handle));efi_free_pool(file_path); if (ret != EFI_SUCCESS) { log_warning("Loading %ls '%ls' failed\n", varname, lo.label);
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.kojima@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. + 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 pali@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(); + 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); }
-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; +} + +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; +} + +static int prepare_distro_boot_entry(struct bootmenu_data *menu, + struct bootmenu_entry **current, + unsigned short int *index) +{ + 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, + 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
Hi Kojima-san,
[...]
entry->title = u16_strdup(lo.label);if (!entry->title) {free(load_option);free(entry);
We need to free bootorder as well
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;elseiter->next = entry;iter = entry;i++;}free(load_option);if (i == MAX_COUNT - 1)break;- }
- free(bootorder);
- *index = i;
- *current = iter;
- return 1;
+}
+static int prepare_distro_boot_entry(struct bootmenu_data *menu,
struct bootmenu_entry **current,unsigned short int *index)+{
- 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;
We need to free p as well
}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");elseentry->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;
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);- } 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,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);
run_command(command, 0);
if (efi_ret == EFI_SUCCESS)cmd_ret = run_command(command, 0);#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
2.17.1
Thanks /Ilias
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 : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Signed-off-by: Masahisa Kojima masahisa.kojima@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 enterthe U-Boot console from bootmenu. It increasesthe 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 pali@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;
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;@@ -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;elseiter->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;elseiter->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");elseentry->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;
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);- } 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);
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
On Sat, 2 Apr 2022 at 15:33, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
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 : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
Signed-off-by: Masahisa Kojima masahisa.kojima@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_BOOTMENUdefault nhelpAdd an entry to enter U-Boot console in bootmenu.If this option is disabled, user can not enterthe U-Boot console from bootmenu. It increasesthe 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 pali@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);elseprintf("%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;elseiter->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;elseiter->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.
Thank you for your review. All above comments(patch separation and adding comment) will be done in the next version.
How does this fit to Simon's work to overhaul distroboot?
The current implementation targets to support existing distroboot. If user selects one of the distroboot entry in bootmenu, the bootmenu runs "run bootcmd_[selected device]" command. We can integrate Simon's work by replacing this command invocation to bootmeth style.
+{
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");elseentry->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?
Yes, it is possible. The saved non-volatile BootNext is handled before showing bootmenu, so the above U-Boot internal use case of BootNext can be volatile. One concern is that UEFI spec says BootNext is an NV variable.
Thanks, Masahisa Kojima
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 */
This commit moves the user input handling from cmd/bootmenu.c to common/menu.c to reuse it from other modules.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- Newly created in v4.
cmd/bootmenu.c | 149 ------------------------------------------------- common/menu.c | 137 +++++++++++++++++++++++++++++++++++++++++++++ include/menu.h | 20 +++++++ 3 files changed, 157 insertions(+), 149 deletions(-)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 947b3a49ff..d3591aa4a9 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -52,21 +52,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]; @@ -107,140 +92,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) { - /* 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; -} - -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 b577d80b4f..6dcac035b7 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__ */
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 --- Chanes in v4: - UEFI boot option maintenance menu is integrated into bootmenu - display the simplified volume name(e.g. usb0:1, nvme1:2) for the volume selection - instead of extending lib/efi_loader/efi_bootmgr.c, newly create lib/efi_loader/efi_bootmenu_maintenance.c and implement boot variable maintenance into it.
Changes in v3: not included in v3 series
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 cmd/bootmenu.c | 70 +- include/efi_loader.h | 37 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1116 +++++++++++++++++++++ lib/efi_loader/efi_boottime.c | 52 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- 8 files changed, 1388 insertions(+), 55 deletions(-) create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index d3591aa4a9..458eb90b63 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -19,6 +19,8 @@
/* maximum bootmenu entries */ #define MAX_COUNT 99 +#define STATIC_ENTRY 2 +#define MAX_DYNAMIC_ENTRY (MAX_COUNT - STATIC_ENTRY)
/* maximal size of bootmenu env * 9 = strlen("bootmenu_") @@ -39,10 +41,11 @@ enum boot_type { BOOTMENU_TYPE_BOOTMENU, BOOTMENU_TYPE_UEFI_BOOT_OPTION, BOOTMENU_TYPE_DISTRO_BOOT, + BOOTMENU_TYPE_UEFI_MAINTENANCE, };
struct bootmenu_entry { - unsigned short int num; /* unique number 0 .. MAX_COUNT */ + unsigned short int num; /* unique number 0 .. MAX_DYNAMIC_ENTRY */ char key[3]; /* key identifier of number */ u16 *title; /* title of entry */ char *command; /* hush command of entry */ @@ -56,7 +59,7 @@ static char *bootmenu_getoption(unsigned short int n) { char name[MAX_ENV_SIZE];
- if (n > MAX_COUNT) + if (n > MAX_DYNAMIC_ENTRY) return NULL;
sprintf(name, "bootmenu_%d", n); @@ -215,7 +218,7 @@ static int prepare_bootmenu_entry(struct bootmenu_data *menu, iter = entry; i++;
- if (i == MAX_COUNT - 1) + if (i == MAX_DYNAMIC_ENTRY) break; }
@@ -290,7 +293,7 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu,
free(load_option);
- if (i == MAX_COUNT - 1) + if (i == MAX_DYNAMIC_ENTRY) break; }
@@ -365,7 +368,7 @@ static int prepare_distro_boot_entry(struct bootmenu_data *menu, iter = entry; i++;
- if (i == MAX_COUNT - 1) + if (i == MAX_DYNAMIC_ENTRY) break;
token = strtok(NULL, " "); @@ -405,19 +408,56 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup;
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { - if (i < MAX_COUNT - 1) { + if (i < MAX_DYNAMIC_ENTRY) { ret = prepare_uefi_bootorder_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT) goto cleanup; } }
- if (i < MAX_COUNT - 1) { + if (i < MAX_DYNAMIC_ENTRY) { ret = prepare_distro_boot_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT) goto cleanup; }
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { + /* Add UEFI Boot Manager Maintenance entry */ + if (i <= MAX_DYNAMIC_ENTRY) { + entry = malloc(sizeof(struct bootmenu_entry)); + if (!entry) + goto cleanup; + + entry->title = u16_strdup(u"UEFI Boot Manager Maintenance"); + if (!entry->title) { + free(entry); + goto cleanup; + } + + entry->command = strdup(""); + if (!entry->command) { + free(entry->title); + free(entry); + goto cleanup; + } + + sprintf(entry->key, "%d", i); + + entry->num = i; + entry->menu = menu; + entry->type = BOOTMENU_TYPE_UEFI_MAINTENANCE; + entry->next = NULL; + + if (!iter) + menu->first = entry; + else + iter->next = entry; + + iter = entry; + i++; + } + } + /* Add U-Boot console entry at the end */ if (i <= MAX_COUNT - 1) { entry = malloc(sizeof(struct bootmenu_entry)); @@ -585,6 +625,13 @@ static enum bootmenu_ret bootmenu_show(int delay)
if (menu_get_choice(menu, &choice) == 1) { iter = choice; + if (iter->type == BOOTMENU_TYPE_UEFI_MAINTENANCE) { + efi_bootmenu_show_maintenance_menu(); + menu_destroy(menu); + bootmenu_destroy(bootmenu); + return BOOTMENU_RET_UPDATED; + } + /* last entry is U-Boot console or Quit */ if (iter->num == iter->menu->count - 1) { menu_destroy(menu); @@ -674,6 +721,7 @@ int do_bootmenu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { char *delay_str = NULL; int delay = 10; + int ret;
#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) delay = CONFIG_BOOTDELAY; @@ -688,7 +736,13 @@ int do_bootmenu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (delay_str) delay = (int)simple_strtol(delay_str, NULL, 10);
- bootmenu_show(delay); + while (1) { + ret = bootmenu_show(delay); + delay = -1; + if (ret != BOOTMENU_RET_UPDATED) + break; + } + return 0; }
diff --git a/include/efi_loader.h b/include/efi_loader.h index ce22660d5b..7b100ca030 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -226,6 +226,9 @@ const char *__efi_nesting_dec(void); #define EFI_CACHELINE_SIZE 128 #endif
+/* max bootmenu title size for volume selection */ +#define BOOTMENU_DEVICE_NAME_MAX 16 + /* Key identifying current memory map */ extern efi_uintn_t efi_memory_map_key;
@@ -312,6 +315,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[]; @@ -869,6 +875,8 @@ efi_status_t efi_set_load_options(efi_handle_t handle, void *load_options); efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);
+efi_status_t efi_bootmenu_show_maintenance_menu(void); + /** * struct efi_image_regions - A list of memory regions * @@ -1004,4 +1012,33 @@ efi_status_t efi_esrt_populate(void); efi_status_t efi_load_capsule_drivers(void);
efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr, u32 *sz); + +efi_status_t efi_locate_handle_buffer_int(enum efi_locate_search_type search_type, + const efi_guid_t *protocol, void *search_key, + efi_uintn_t *no_handles, efi_handle_t **buffer); + +efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol *this, + struct efi_file_handle **root); +efi_status_t efi_file_open_int(struct efi_file_handle *this, + struct efi_file_handle **new_handle, + u16 *file_name, u64 open_mode, + u64 attributes); +efi_status_t efi_file_close_int(struct efi_file_handle *file); +efi_status_t efi_file_read_int(struct efi_file_handle *this, + efi_uintn_t *buffer_size, void *buffer); +efi_status_t efi_file_setpos_int(struct efi_file_handle *file, u64 pos); + +typedef efi_status_t (*efi_console_filter_func)(struct efi_input_key *key); +efi_status_t efi_console_get_u16_string + (struct efi_simple_text_input_protocol *cin, + struct efi_simple_text_output_protocol *cout, + u16 *buf, efi_uintn_t count, efi_console_filter_func filer_func, + int row, int col); + +efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, + efi_uintn_t buf_size, u32 *index); +efi_status_t efi_bootmenu_append_bootorder(u16 index); + +efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size); + #endif /* _EFI_LOADER_H */ diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index befed7144e..fd900a23c7 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -77,6 +77,7 @@ obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_riscv.o obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o +obj-$(CONFIG_CMD_BOOTMENU) += efi_bootmenu_maintenance.o
EFI_VAR_SEED_FILE := $(subst $",,$(CONFIG_EFI_VAR_SEED_FILE)) $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE) diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c new file mode 100644 index 0000000000..2ce5a47b10 --- /dev/null +++ b/lib/efi_loader/efi_bootmenu_maintenance.c @@ -0,0 +1,1116 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Menu-driven UEFI Boot Variable maintenance + * + * Copyright (c) 2022 Masahisa Kojima, Linaro Limited + */ + +#define LOG_CATEGORY LOGC_EFI + +#include <ansi.h> +#include <common.h> +#include <charset.h> +#include <log.h> +#include <malloc.h> +#include <menu.h> +#include <efi_loader.h> +#include <efi_variable.h> +#include <asm/unaligned.h> + +static struct efi_simple_text_input_protocol *cin; +static struct efi_simple_text_output_protocol *cout; + +#define EFI_BOOTMENU_ENTRY_NUM_MAX 99 +#define EFI_BOOTMENU_FILE_PATH_MAX 512 +#define EFI_BOOTMENU_BOOT_NAME_MAX 32 +#define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 + +typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit); + +/** + * struct efi_bootmenu_entry - menu entry structure + * + * @num: menu entry index + * @title: title of entry + * @key: unique key + * @bootmgr_menu: pointer to the menu structure + * @next: pointer to the next entry + * @func: callback function to be called when this entry is selected + * @data: data to be passed to the callback function + */ +struct efi_bootmenu_entry { + u32 num; + u16 *title; + char key[6]; + struct efi_bootmenu *bootmgr_menu; + struct efi_bootmenu_entry *next; + efi_bootmenu_entry_func func; + void *data; +}; + +/** + * struct efi_bootmenu - bootmgr menu structure + * + * @delay: delay for autoboot + * @active: active menu entry index + * @count: total count of menu entry + * @first: pointer to the first menu entry + */ +struct efi_bootmenu { + int delay; + int active; + int count; + struct efi_bootmenu_entry *first; +}; + +struct efi_bootmenu_item { + u16 *title; + efi_bootmenu_entry_func func; + void *data; +}; + +struct efi_bootmenu_boot_selection_data { + u16 bootorder_index; + void *load_option; + int *selected; +}; + +struct efi_bootmenu_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_bootmenu_volume_entry_data { + struct efi_bootmenu_boot_option *bo; + struct efi_simple_file_system_protocol *v; + struct efi_device_path *dp; +}; + +struct efi_bootmenu_file_entry_data { + struct efi_bootmenu_boot_option *bo; + struct efi_file_info *f; +}; + +static efi_status_t efi_bootmenu_process_boot_selected(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_add_boot_option(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_delete_boot_option(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_change_boot_order(void *data, bool *exit); + +static struct efi_bootmenu_item maintenance_menu_items[] = { + {u"Add Boot Option", efi_bootmenu_process_add_boot_option}, + {u"Delete Boot Option", efi_bootmenu_process_delete_boot_option}, + {u"Change Boot Order", efi_bootmenu_process_change_boot_order}, + {u"Quit", NULL}, +}; + +static void efi_bootmenu_print_entry(void *data) +{ + struct efi_bootmenu_entry *entry = data; + int reverse = (entry->bootmgr_menu->active == entry->num); + + /* TODO: support scroll or page for many entries */ + + /* + * Move cursor to line where the entry will be drown (entry->count) + * First 3 lines contain bootmgr menu header + one empty line + * For the last "Quit" entry, add one empty line + */ + if (entry->num == (entry->bootmgr_menu->count - 1)) + printf(ANSI_CURSOR_POSITION, entry->num + 5, 1); + else + printf(ANSI_CURSOR_POSITION, entry->num + 4, 1); + + puts(" "); + + if (reverse) + puts(ANSI_COLOR_REVERSE); + + printf("%ls", entry->title); + + if (reverse) + puts(ANSI_COLOR_RESET); +} + +static void efi_bootmenu_display_statusline(struct menu *m) +{ + struct efi_bootmenu_entry *entry; + struct efi_bootmenu *bootmgr_menu; + + if (menu_default_choice(m, (void *)&entry) < 0) + return; + + bootmgr_menu = entry->bootmgr_menu; + + printf(ANSI_CURSOR_POSITION, 1, 1); + puts(ANSI_CLEAR_LINE); + printf(ANSI_CURSOR_POSITION, 2, 1); + puts(" *** U-Boot EFI Boot Manager ***"); + puts(ANSI_CLEAR_LINE_TO_END); + printf(ANSI_CURSOR_POSITION, 3, 1); + puts(ANSI_CLEAR_LINE); + + /* First 3 lines are bootmgr_menu header + 2 empty lines between entries */ + printf(ANSI_CURSOR_POSITION, bootmgr_menu->count + 5, 1); + puts(ANSI_CLEAR_LINE); + printf(ANSI_CURSOR_POSITION, bootmgr_menu->count + 6, 1); + puts(" Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit"); + puts(ANSI_CLEAR_LINE_TO_END); + printf(ANSI_CURSOR_POSITION, bootmgr_menu->count + 7, 1); + puts(ANSI_CLEAR_LINE); +} + +static char *efi_bootmenu_choice_entry(void *data) +{ + int i; + int esc = 0; + struct efi_bootmenu_entry *iter; + enum bootmenu_key key = KEY_NONE; + struct efi_bootmenu *bootmgr_menu = data; + + while (1) { + if (bootmgr_menu->delay >= 0) { + /* Autoboot was not stopped */ + bootmenu_autoboot_loop((struct bootmenu_data *)bootmgr_menu, &key, &esc); + } else { + /* Some key was pressed, so autoboot was stopped */ + bootmenu_loop((struct bootmenu_data *)bootmgr_menu, &key, &esc); + } + + if (bootmgr_menu->delay == 0) + key = KEY_QUIT; + + switch (key) { + case KEY_UP: + if (bootmgr_menu->active > 0) + --bootmgr_menu->active; + /* no menu key selected, regenerate menu */ + return NULL; + case KEY_DOWN: + if (bootmgr_menu->active < bootmgr_menu->count - 1) + ++bootmgr_menu->active; + /* no menu key selected, regenerate menu */ + return NULL; + case KEY_SELECT: + iter = bootmgr_menu->first; + for (i = 0; i < bootmgr_menu->active; ++i) + iter = iter->next; + return iter->key; + case KEY_QUIT: + /* Quit by choosing the last entry */ + iter = bootmgr_menu->first; + while (iter->next) + iter = iter->next; + return iter->key; + default: + break; + } + } + + /* never happens */ + debug("bootmgr menu: this should not happen"); + return NULL; +} + +static void efi_bootmenu_destroy(struct efi_bootmenu *bootmgr_menu) +{ + struct efi_bootmenu_entry *next; + struct efi_bootmenu_entry *iter = bootmgr_menu->first; + + while (iter) { + next = iter->next; + free(iter); + iter = next; + } + free(bootmgr_menu); +} + +/** + * efi_bootmenu_process_common() - main handler for uefi bootmgr menu + * + * Construct the structures required to show the menu, then handle + * the user input intracting with u-boot menu functions. + * + * @items: pointer to the structure of each menu entry + * @count: the number of menu entry + * @delay: delay for autoboot/autoselect + * Return: status code + */ +static efi_status_t efi_bootmenu_process_common(struct efi_bootmenu_item *items, + int count, int delay) +{ + u32 i; + bool exit = false; + efi_status_t ret; + struct menu *menu; + void *choice = NULL; + struct efi_bootmenu_entry *entry; + struct efi_bootmenu *bootmgr_menu; + struct efi_bootmenu_entry *iter = NULL; + + if (count > EFI_BOOTMENU_ENTRY_NUM_MAX) + return EFI_OUT_OF_RESOURCES; + + bootmgr_menu = calloc(1, sizeof(struct efi_bootmenu)); + if (!bootmgr_menu) + return EFI_OUT_OF_RESOURCES; + + bootmgr_menu->delay = delay; + bootmgr_menu->active = 0; + bootmgr_menu->first = NULL; + + for (i = 0; i < count; i++) { + entry = calloc(1, sizeof(struct efi_bootmenu_entry)); + if (!entry) { + ret = EFI_LOAD_ERROR; + goto out; + } + + entry->num = i; + entry->title = items->title; + snprintf(entry->key, sizeof(entry->key), "%04X", i); + entry->bootmgr_menu = bootmgr_menu; + entry->func = items->func; + entry->data = items->data; + entry->next = NULL; + + if (!iter) + bootmgr_menu->first = entry; + else + iter->next = entry; + + iter = entry; + items++; + } + bootmgr_menu->count = count; + + menu = menu_create(NULL, 0, 1, efi_bootmenu_display_statusline, + efi_bootmenu_print_entry, efi_bootmenu_choice_entry, + bootmgr_menu); + if (!menu) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + for (entry = bootmgr_menu->first; entry; entry = entry->next) { + if (!menu_item_add(menu, entry->key, entry)) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + } + + menu_default_set(menu, bootmgr_menu->first->key); + + while (!exit) { + puts(ANSI_CURSOR_HIDE); + puts(ANSI_CLEAR_CONSOLE); + printf(ANSI_CURSOR_POSITION, 1, 1); + + if (menu_get_choice(menu, &choice)) { + entry = choice; + if (entry->func) + ret = entry->func(entry->data, &exit); + + /* last entry "Quit" is selected, exit this menu */ + if (entry->num == (entry->bootmgr_menu->count - 1)) { + ret = EFI_ABORTED; + break; + } + } + } + +out: + menu_destroy(menu); + efi_bootmenu_destroy(bootmgr_menu); + + puts(ANSI_CLEAR_CONSOLE); + printf(ANSI_CURSOR_POSITION, 1, 1); + puts(ANSI_CURSOR_SHOW); + + return ret; +} + +static efi_status_t efi_bootmenu_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_bootmenu_item *menu_item, *iter; + + menu_item = calloc(count + 1, sizeof(struct efi_bootmenu_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_bootmenu_boot_selection_data *info; + + info = calloc(1, sizeof(struct efi_bootmenu_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_bootmenu_process_boot_selected; + iter->data = info; + iter++; + } + } + + /* add "Quit" entry */ + iter->title = u"Quit"; + iter->func = NULL; + iter->data = NULL; + count += 1; + + ret = efi_bootmenu_process_common(menu_item, count, -1); + +out: + iter = menu_item; + for (i = 0; i < count - 1; i++, iter++) { + free(((struct efi_bootmenu_boot_selection_data *)iter->data)->load_option); + free(iter->data); + } + + free(menu_item); + + return ret; +} + +static efi_status_t efi_bootmenu_process_boot_selected(void *data, bool *exit) +{ + struct efi_bootmenu_boot_selection_data *info = data; + + *exit = true; + + if (info) + *info->selected = info->bootorder_index; + + return EFI_SUCCESS; +} + +static efi_status_t efi_bootmenu_volume_selected(void *data, bool *exit) +{ + struct efi_bootmenu_volume_entry_data *info = data; + + *exit = true; + + if (info) { + info->bo->current_volume = info->v; + info->bo->dp_volume = info->dp; + } + + return EFI_SUCCESS; +} + +static efi_status_t efi_bootmenu_file_selected(void *data, bool *exit) +{ + struct efi_bootmenu_file_entry_data *info = data; + + *exit = true; + + if (!info) + return EFI_INVALID_PARAMETER; + + if (u16_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_BOOTMENU_FILE_PATH_MAX) { + /* TODO: show error notification to user */ + log_err("file path is too long\n"); + return EFI_INVALID_PARAMETER; + } + u16_strlcat(info->bo->current_path, info->f->file_name, EFI_BOOTMENU_FILE_PATH_MAX); + if (info->f->attribute & EFI_FILE_DIRECTORY) { + if (new_len + 1 >= EFI_BOOTMENU_FILE_PATH_MAX) { + log_err("file path is too long\n"); + return EFI_INVALID_PARAMETER; + } + u16_strlcat(info->bo->current_path, u"\", EFI_BOOTMENU_FILE_PATH_MAX); + } else { + info->bo->file_selected = true; + } + } + return EFI_SUCCESS; +} + +static efi_status_t efi_bootmenu_select_volume(struct efi_bootmenu_boot_option *bo) +{ + u32 i; + efi_status_t ret; + efi_uintn_t count; + struct efi_handler *handler; + struct efi_device_path *device_path; + efi_handle_t *volume_handles = NULL; + struct efi_simple_file_system_protocol *v; + struct efi_bootmenu_item *menu_item, *iter; + + ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid, + NULL, &count, (efi_handle_t **)&volume_handles); + if (ret != EFI_SUCCESS) + return ret; + + menu_item = calloc(count + 1, sizeof(struct efi_bootmenu_item)); + if (!menu_item) { + ret = EFI_OUT_OF_RESOURCES; + goto out1; + } + + iter = menu_item; + for (i = 0; i < count; i++) { + u16 *dev_name, *p; + struct efi_block_io *block_io; + char buf[BOOTMENU_DEVICE_NAME_MAX]; + struct efi_bootmenu_volume_entry_data *info; + + ret = efi_search_protocol(volume_handles[i], + &efi_simple_file_system_protocol_guid, &handler); + if (ret != EFI_SUCCESS) + continue; + ret = efi_protocol_open(handler, (void **)&v, efi_root, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + continue; + + ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler); + if (ret != EFI_SUCCESS) + continue; + ret = efi_protocol_open(handler, (void **)&device_path, + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + continue; + + ret = efi_search_protocol(volume_handles[i], &efi_block_io_guid, &handler); + if (ret != EFI_SUCCESS) + continue; + ret = efi_protocol_open(handler, (void **)&block_io, + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + continue; + + info = calloc(1, sizeof(struct efi_bootmenu_volume_entry_data)); + if (!info) { + ret = EFI_OUT_OF_RESOURCES; + goto out2; + } + + efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX); + dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16)); + if (!dev_name) { + free(info); + ret = EFI_OUT_OF_RESOURCES; + goto out2; + } + p = dev_name; + utf8_utf16_strncpy(&p, buf, strlen(buf)); + + info->v = v; + info->dp = device_path; + info->bo = bo; + iter->title = dev_name; + iter->func = efi_bootmenu_volume_selected; + iter->data = info; + iter++; + } + + iter->title = u16_strdup(u"Quit"); + iter->func = NULL; + iter->data = NULL; + count += 1; + + ret = efi_bootmenu_process_common(menu_item, count, -1); + +out2: + iter = menu_item; + for (i = 0; i < count; i++) { + struct efi_bootmenu_volume_entry_data *p; + + p = (struct efi_bootmenu_volume_entry_data *)(iter->data); + free(iter->title); + free(p); + iter++; + } + + free(menu_item); + +out1: + efi_free_pool(volume_handles); + + return ret; +} + +static efi_status_t efi_bootmenu_select_file(struct efi_bootmenu_boot_option *bo, + struct efi_file_handle *root) +{ + 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_bootmenu_item *menu_item, *iter; + + buf = calloc(1, EFI_BOOTMENU_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_BOOTMENU_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_bootmenu_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_bootmenu_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_bootmenu_file_entry_data)); + if (!info) { + ret = EFI_OUT_OF_RESOURCES; + goto err; + } + info->f = ptr; + info->bo = bo; + iter->title = name; + iter->func = efi_bootmenu_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_bootmenu_process_common(menu_item, count, -1); +err: + efi_file_close_int(f); + iter = menu_item; + for (i = 0; i < count - 1; i++, iter++) { + free(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_bootmenu_boot_add_enter_name(struct efi_bootmenu_boot_option *bo) +{ + efi_status_t ret; + + printf(ANSI_CURSOR_POSITION, 2, 1); + puts(" *** U-Boot EFI Boot Manager Menu ***"); + printf(ANSI_CURSOR_POSITION, 4, 1); + puts(" enter name:"); + + printf(ANSI_CURSOR_POSITION, 8, 1); + puts(" ENTER to complete, ESC/CTRL+C to quit"); + + ret = efi_console_get_u16_string(cin, cout, bo->boot_name, + EFI_BOOTMENU_BOOT_NAME_MAX, NULL, 4, 15); + + return ret; +} + +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_bootmenu_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_bootmenu_select_file_handler(struct efi_bootmenu_boot_option *bo) +{ + efi_status_t ret; + struct efi_file_handle *root; + + bo->file_selected = false; + + while (!bo->file_selected) { + bo->current_volume = NULL; + memset(bo->current_path, 0, sizeof(bo->current_path)); + + ret = efi_bootmenu_select_volume(bo); + if (ret != EFI_SUCCESS) + return ret; + + if (!bo->current_volume) + return EFI_INVALID_PARAMETER; + + ret = efi_open_volume_int(bo->current_volume, &root); + if (ret != EFI_SUCCESS) + return ret; + + ret = efi_bootmenu_select_file(bo, root); + if (ret != EFI_SUCCESS) + return ret; + } + + ret = efi_bootmenu_boot_add_enter_name(bo); + + return ret; +} + +efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, + efi_uintn_t buf_size, u32 *index) +{ + u32 i; + efi_status_t ret; + efi_uintn_t size; + + if (buf_size < u16_strsize(u"Boot####")) + return EFI_BUFFER_TOO_SMALL; + + for (i = 0; i <= 0xFFFF; i++) { + size = 0; + efi_create_indexed_name(buf, buf_size, "Boot", i); + ret = efi_get_variable_int(buf, &efi_global_variable_guid, + NULL, &size, NULL, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) + continue; + else + break; + } + + if (i > 0xFFFF) + return EFI_OUT_OF_RESOURCES; + + *index = i; + + return EFI_SUCCESS; +} + +static efi_status_t efi_bootmenu_set_boot_option(u16 *var_name, struct efi_device_path *dp, + u16 *label, char *optional_data) +{ + void *p = NULL; + efi_status_t ret; + efi_uintn_t size; + struct efi_load_option lo; + + lo.file_path = dp; + lo.file_path_length = efi_dp_size(dp) + sizeof(END); + lo.attributes = LOAD_OPTION_ACTIVE; + lo.optional_data = optional_data; + lo.label = label; + + size = efi_serialize_load_option(&lo, (u8 **)&p); + if (!size) + return EFI_INVALID_PARAMETER; + + ret = efi_set_variable_int(var_name, &efi_global_variable_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + size, p, false); + free(p); + + return ret; +} + +efi_status_t efi_bootmenu_append_bootorder(u16 index) +{ + u16 *bootorder; + efi_status_t ret; + u16 *new_bootorder = NULL; + efi_uintn_t last, size, new_size; + + /* append new boot option */ + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &size); + last = size / sizeof(u16); + new_size = size + sizeof(u16); + new_bootorder = calloc(1, new_size); + if (!new_bootorder) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + memcpy(new_bootorder, bootorder, size); + new_bootorder[last] = index; + + ret = efi_set_variable_int(u"BootOrder", &efi_global_variable_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + new_size, new_bootorder, false); + if (ret != EFI_SUCCESS) + goto out; + +out: + free(bootorder); + free(new_bootorder); + + return ret; +} + +static efi_status_t efi_bootmenu_process_add_boot_option(void *data, bool *exit) +{ + u32 index; + u16 var_name[9]; + char *buf = NULL; + efi_status_t ret; + char *iter = NULL; + efi_uintn_t dp_size, fp_size; + struct efi_bootmenu_boot_option bo; + struct efi_device_path_file_path *fp; + + ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name), + &index); + if (ret != EFI_SUCCESS) + return ret; + + bo.current_path = calloc(1, EFI_BOOTMENU_FILE_PATH_MAX); + if (!bo.current_path) + goto out; + + bo.boot_name = calloc(1, EFI_BOOTMENU_BOOT_NAME_MAX * sizeof(u16)); + if (!bo.boot_name) + goto out; + + ret = efi_bootmenu_select_file_handler(&bo); + if (ret != EFI_SUCCESS) + goto out; + + dp_size = efi_dp_size(bo.dp_volume); + fp_size = sizeof(struct efi_device_path) + + ((u16_strlen(bo.current_path) + 1) * sizeof(u16)); + buf = calloc(1, dp_size + fp_size + sizeof(END)); + if (!buf) + goto out; + + iter = buf; + memcpy(iter, bo.dp_volume, dp_size); + iter += dp_size; + + fp = (struct efi_device_path_file_path *)iter; + fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE; + fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH; + fp->dp.length = (u16)fp_size; + u16_strcpy(fp->str, bo.current_path); + iter += fp_size; + *((struct efi_device_path *)iter) = END; + + ret = efi_bootmenu_set_boot_option(var_name, (struct efi_device_path *)buf, + bo.boot_name, NULL); + if (ret != EFI_SUCCESS) + goto out; + + efi_bootmenu_append_bootorder((u16)index); + if (ret != EFI_SUCCESS) + goto out; + +out: + free(buf); + free(bo.boot_name); + free(bo.current_path); + + return ret; +} + +static efi_status_t delete_boot_option(u16 *bootorder, u16 index, efi_uintn_t size) +{ + u16 var_name[9]; + efi_status_t ret; + efi_uintn_t num; + + efi_create_indexed_name(var_name, sizeof(var_name), + "Boot", bootorder[index]); + 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); + return ret; + } + + /* update BootOrder */ + num = size / sizeof(u16); + memmove(&bootorder[index], &bootorder[index + 1], + (num - index - 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); + + return ret; +} + +static efi_status_t efi_bootmenu_process_delete_boot_option(void *data, bool *exit) +{ + int selected; + u16 *bootorder; + 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_bootmenu_show_boot_selection(bootorder, num, &selected); + if (ret == EFI_SUCCESS) + ret = delete_boot_option(bootorder, selected, size); + + free(bootorder); + + return ret; +} + +static efi_status_t efi_bootmenu_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_bootmenu_show_boot_selection(bootorder, num, &selected); + if (ret != EFI_SUCCESS) + goto out; + + ret = efi_bootmenu_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; +} + +static efi_status_t efi_bootmenu_init(void) +{ + efi_status_t ret; + struct efi_handler *handler; + + ret = efi_search_protocol(efi_root, &efi_guid_text_input_protocol, &handler); + if (ret != EFI_SUCCESS) + return ret; + + ret = efi_protocol_open(handler, (void **)&cin, efi_root, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + return ret; + + ret = efi_search_protocol(efi_root, &efi_guid_text_output_protocol, &handler); + if (ret != EFI_SUCCESS) + return ret; + + ret = efi_protocol_open(handler, (void **)&cout, efi_root, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + return ret; + + return ret; +} + +efi_status_t efi_bootmenu_show_maintenance_menu(void) +{ + efi_status_t ret; + + ret = efi_bootmenu_init(); + if (ret != EFI_SUCCESS) + return ret; + + return efi_bootmenu_process_common(maintenance_menu_items, + ARRAY_SIZE(maintenance_menu_items), + -1); +} diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4da64b5d29..1233418e77 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2453,6 +2453,35 @@ static efi_status_t EFIAPI efi_protocols_per_handle( return EFI_EXIT(EFI_SUCCESS); }
+efi_status_t efi_locate_handle_buffer_int(enum efi_locate_search_type search_type, + const efi_guid_t *protocol, void *search_key, + efi_uintn_t *no_handles, efi_handle_t **buffer) +{ + efi_status_t r; + efi_uintn_t buffer_size = 0; + + if (!no_handles || !buffer) { + r = EFI_INVALID_PARAMETER; + goto out; + } + *no_handles = 0; + *buffer = NULL; + r = efi_locate_handle(search_type, protocol, search_key, &buffer_size, + *buffer); + if (r != EFI_BUFFER_TOO_SMALL) + goto out; + r = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size, + (void **)buffer); + if (r != EFI_SUCCESS) + goto out; + r = efi_locate_handle(search_type, protocol, search_key, &buffer_size, + *buffer); + if (r == EFI_SUCCESS) + *no_handles = buffer_size / sizeof(efi_handle_t); +out: + return r; +} + /** * efi_locate_handle_buffer() - locate handles implementing a protocol * @search_type: selection criterion @@ -2474,30 +2503,13 @@ efi_status_t EFIAPI efi_locate_handle_buffer( efi_uintn_t *no_handles, efi_handle_t **buffer) { efi_status_t r; - efi_uintn_t buffer_size = 0;
EFI_ENTRY("%d, %pUs, %p, %p, %p", search_type, protocol, search_key, no_handles, buffer);
- if (!no_handles || !buffer) { - r = EFI_INVALID_PARAMETER; - goto out; - } - *no_handles = 0; - *buffer = NULL; - r = efi_locate_handle(search_type, protocol, search_key, &buffer_size, - *buffer); - if (r != EFI_BUFFER_TOO_SMALL) - goto out; - r = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size, - (void **)buffer); - if (r != EFI_SUCCESS) - goto out; - r = efi_locate_handle(search_type, protocol, search_key, &buffer_size, - *buffer); - if (r == EFI_SUCCESS) - *no_handles = buffer_size / sizeof(efi_handle_t); -out: + r = efi_locate_handle_buffer_int(search_type, protocol, search_key, + no_handles, buffer); + return EFI_EXIT(r); }
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 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_disk.c b/lib/efi_loader/efi_disk.c index c905c12abc..4963fefd86 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -587,3 +587,14 @@ efi_status_t efi_disk_register(void)
return EFI_SUCCESS; } + +efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size) +{ + struct efi_disk_obj *diskobj; + + diskobj = container_of(this, struct efi_disk_obj, ops); + + snprintf(buf, size, "%s%d:%d", diskobj->ifname, diskobj->dev_index, diskobj->part); + + return EFI_SUCCESS; +} diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 7a7077e6d0..c96a7f7ca3 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -246,10 +246,10 @@ error: return NULL; }
-static efi_status_t efi_file_open_int(struct efi_file_handle *this, - struct efi_file_handle **new_handle, - u16 *file_name, u64 open_mode, - u64 attributes) +efi_status_t efi_file_open_int(struct efi_file_handle *this, + struct efi_file_handle **new_handle, + u16 *file_name, u64 open_mode, + u64 attributes) { struct file_handle *fh = to_fh(this); efi_status_t ret; @@ -369,11 +369,17 @@ static efi_status_t file_close(struct file_handle *fh) return EFI_SUCCESS; }
-static efi_status_t EFIAPI efi_file_close(struct efi_file_handle *file) +efi_status_t efi_file_close_int(struct efi_file_handle *file) { struct file_handle *fh = to_fh(file); + + return file_close(fh); +} + +static efi_status_t EFIAPI efi_file_close(struct efi_file_handle *file) +{ EFI_ENTRY("%p", file); - return EFI_EXIT(file_close(fh)); + return EFI_EXIT(efi_file_close_int(file)); }
static efi_status_t EFIAPI efi_file_delete(struct efi_file_handle *file) @@ -562,8 +568,8 @@ static efi_status_t dir_read(struct file_handle *fh, u64 *buffer_size, return EFI_SUCCESS; }
-static efi_status_t efi_file_read_int(struct efi_file_handle *this, - efi_uintn_t *buffer_size, void *buffer) +efi_status_t efi_file_read_int(struct efi_file_handle *this, + efi_uintn_t *buffer_size, void *buffer) { struct file_handle *fh = to_fh(this); efi_status_t ret = EFI_SUCCESS; @@ -773,24 +779,11 @@ out: return EFI_EXIT(ret); }
-/** - * efi_file_setpos() - set current position in file - * - * This function implements the SetPosition service of the EFI file protocol. - * See the UEFI spec for details. - * - * @file: file handle - * @pos: new file position - * Return: status code - */ -static efi_status_t EFIAPI efi_file_setpos(struct efi_file_handle *file, - u64 pos) +efi_status_t efi_file_setpos_int(struct efi_file_handle *file, u64 pos) { struct file_handle *fh = to_fh(file); efi_status_t ret = EFI_SUCCESS;
- EFI_ENTRY("%p, %llu", file, pos); - if (fh->isdir) { if (pos != 0) { ret = EFI_UNSUPPORTED; @@ -812,6 +805,28 @@ static efi_status_t EFIAPI efi_file_setpos(struct efi_file_handle *file, fh->offset = pos;
error: + return ret; +} + +/** + * efi_file_setpos() - set current position in file + * + * This function implements the SetPosition service of the EFI file protocol. + * See the UEFI spec for details. + * + * @file: file handle + * @pos: new file position + * Return: status code + */ +static efi_status_t EFIAPI efi_file_setpos(struct efi_file_handle *file, + u64 pos) +{ + efi_status_t ret = EFI_SUCCESS; + + EFI_ENTRY("%p, %llu", file, pos); + + ret = efi_file_setpos_int(file, pos); + return EFI_EXIT(ret); }
@@ -1138,17 +1153,23 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) return f; }
+efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol *this, + struct efi_file_handle **root) +{ + struct file_system *fs = to_fs(this); + + *root = file_open(fs, NULL, NULL, 0, 0); + + return EFI_SUCCESS; +} + static efi_status_t EFIAPI efi_open_volume(struct efi_simple_file_system_protocol *this, struct efi_file_handle **root) { - struct file_system *fs = to_fs(this); - EFI_ENTRY("%p, %p", this, root);
- *root = file_open(fs, NULL, NULL, 0, 0); - - return EFI_EXIT(EFI_SUCCESS); + return EFI_EXIT(efi_open_volume_int(this, root)); }
struct efi_simple_file_system_protocol *
Hi Kojima-san, On Thu, Mar 24, 2022 at 10:54:41PM +0900, Masahisa Kojima wrote:
I haven't been able to get the patch working yet. I'll send more feedback once I do. Here's a few comments I have
[...]
+struct efi_bootmenu_file_entry_data {
- struct efi_bootmenu_boot_option *bo;
- struct efi_file_info *f;
+};
+static efi_status_t efi_bootmenu_process_boot_selected(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_add_boot_option(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_delete_boot_option(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_change_boot_order(void *data, bool *exit);
I think you can re-arrange some of the functions and get rid of the forward declarations
+static struct efi_bootmenu_item maintenance_menu_items[] = {
const ?
- {u"Add Boot Option", efi_bootmenu_process_add_boot_option},
- {u"Delete Boot Option", efi_bootmenu_process_delete_boot_option},
- {u"Change Boot Order", efi_bootmenu_process_change_boot_order},
- {u"Quit", NULL},
+};
+static void efi_bootmenu_print_entry(void *data) +{
- struct efi_bootmenu_entry *entry = data;
[...]
new_len = u16_strlen(info->bo->current_path) +/* TODO: show error notification to user */log_err("file path is too long\n");return EFI_INVALID_PARAMETER;
Can we just check for new_len + 1 here and get rid of the follow up check ?
}u16_strlcat(info->bo->current_path, info->f->file_name, EFI_BOOTMENU_FILE_PATH_MAX);if (info->f->attribute & EFI_FILE_DIRECTORY) {if (new_len + 1 >= EFI_BOOTMENU_FILE_PATH_MAX) {log_err("file path is too long\n");return EFI_INVALID_PARAMETER;}u16_strlcat(info->bo->current_path, u"\\", EFI_BOOTMENU_FILE_PATH_MAX);} else {info->bo->file_selected = true;}
[...]
menu_item = calloc(count + 1, sizeof(struct efi_bootmenu_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;
This will cause an unaligned access later on when you access ptr->attribute. Any reason we can't allocate ptr directly?
for (i = 0; i < count; i++) {int name_len;u16 *name;struct efi_bootmenu_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_bootmenu_file_entry_data));if (!info) {ret = EFI_OUT_OF_RESOURCES;goto err;}info->f = ptr;info->bo = bo;iter->title = name;iter->func = efi_bootmenu_file_selected;iter->data = info;iter++;size -= len;ptr = (struct efi_file_info *)((char *)ptr + len);
ditto
}/* add "Quit" entry */iter->title = u"Quit";iter->func = NULL;iter->data = NULL;count += 1;ret = efi_bootmenu_process_common(menu_item, count, -1);+err:
efi_file_close_int(f);iter = menu_item;for (i = 0; i < count - 1; i++, iter++) {free(iter->title);free(iter->data);}free(dir_buf);free(menu_item);if (ret != EFI_SUCCESS)break;- }
+out:
- free(buf);
- return ret;
+}
[...]
Regards /Ilias
Hi Ilias,
On Thu, 31 Mar 2022 at 17:31, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Kojima-san, On Thu, Mar 24, 2022 at 10:54:41PM +0900, Masahisa Kojima wrote:
I haven't been able to get the patch working yet. I'll send more feedback once I do. Here's a few comments I have
[...]
+struct efi_bootmenu_file_entry_data {
struct efi_bootmenu_boot_option *bo;struct efi_file_info *f;+};
+static efi_status_t efi_bootmenu_process_boot_selected(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_add_boot_option(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_delete_boot_option(void *data, bool *exit); +static efi_status_t efi_bootmenu_process_change_boot_order(void *data, bool *exit);
I think you can re-arrange some of the functions and get rid of the forward declarations
+static struct efi_bootmenu_item maintenance_menu_items[] = {
const ?
{u"Add Boot Option", efi_bootmenu_process_add_boot_option},{u"Delete Boot Option", efi_bootmenu_process_delete_boot_option},{u"Change Boot Order", efi_bootmenu_process_change_boot_order},{u"Quit", NULL},+};
+static void efi_bootmenu_print_entry(void *data) +{
struct efi_bootmenu_entry *entry = data;[...]
new_len = u16_strlen(info->bo->current_path) +/* TODO: show error notification to user */log_err("file path is too long\n");return EFI_INVALID_PARAMETER;Can we just check for new_len + 1 here and get rid of the follow up check ?
Sorry for the late reply. If the selected entry is a file, not the directory, no u'\' will be added. Strictly speaking, checking for new_len + 1 here ends up that file path name max size is (EFI_BOOTMENU_FILE_PATH_MAX -1).
}u16_strlcat(info->bo->current_path, info->f->file_name, EFI_BOOTMENU_FILE_PATH_MAX);if (info->f->attribute & EFI_FILE_DIRECTORY) {if (new_len + 1 >= EFI_BOOTMENU_FILE_PATH_MAX) {log_err("file path is too long\n");return EFI_INVALID_PARAMETER;}u16_strlcat(info->bo->current_path, u"\\", EFI_BOOTMENU_FILE_PATH_MAX);} else {info->bo->file_selected = true;}[...]
menu_item = calloc(count + 1, sizeof(struct efi_bootmenu_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;This will cause an unaligned access later on when you access ptr->attribute. Any reason we can't allocate ptr directly?
I will fix other comments.
Thanks, Masahisa Kojima
for (i = 0; i < count; i++) {int name_len;u16 *name;struct efi_bootmenu_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_bootmenu_file_entry_data));if (!info) {ret = EFI_OUT_OF_RESOURCES;goto err;}info->f = ptr;info->bo = bo;iter->title = name;iter->func = efi_bootmenu_file_selected;iter->data = info;iter++;size -= len;ptr = (struct efi_file_info *)((char *)ptr + len);ditto
}/* add "Quit" entry */iter->title = u"Quit";iter->func = NULL;iter->data = NULL;count += 1;ret = efi_bootmenu_process_common(menu_item, count, -1);+err:
efi_file_close_int(f);iter = menu_item;for (i = 0; i < count - 1; i++, iter++) {free(iter->title);free(iter->data);}free(dir_buf);free(menu_item);if (ret != EFI_SUCCESS)break;}+out:
free(buf);return ret;+}
[...]
Regards /Ilias
UEFI specification requires booting from removal media using a architecture-specific default image name such as BOOTAA64.EFI. This commit adds the removable media entries into bootmenu, so that user can select the removable media and boot with default image.
The bootmenu automatically enumerates the possible bootable media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, add it as new UEFI boot option(BOOT####) and update BootOrder variable. Depending on the system hardware setup, some devices may not exist at a later system boot, so invalid BOOT#### variable must be deleted.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- Newly created in v4
cmd/bootmenu.c | 78 +++++++++++++ include/efi_loader.h | 22 ++++ lib/efi_loader/efi_bootmenu_maintenance.c | 128 ++++++++++++++++++++++ 3 files changed, 228 insertions(+)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 458eb90b63..7357cfeae5 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -304,6 +304,74 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, return 1; }
+static efi_status_t prepare_media_device_entry(void) +{ + u32 i; + efi_status_t ret; + efi_uintn_t count; + efi_handle_t *volume_handles = NULL; + struct efi_bootmenu_media_boot_option *opt = NULL; + + ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid, + NULL, &count, (efi_handle_t **)&volume_handles); + if (ret != EFI_SUCCESS) + return ret; + + opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option)); + if (!opt) + goto out; + + /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ + ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count); + if (ret != EFI_SUCCESS) + goto out; + + /* + * System hardware configuration may vary depending on the user setup. + * The boot option is automatically added by the bootmenu. + * If the device is not attached to the system, the boot option needs + * to be deleted. + */ + ret = efi_bootmenu_delete_invalid_boot_option(opt, count); + if (ret != EFI_SUCCESS) + goto out; + + /* add non-existent boot option */ + for (i = 0; i < count; i++) { + u32 boot_index; + u16 var_name[9]; + + if (!opt[i].exist) { + ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name), + &boot_index); + if (ret != EFI_SUCCESS) + 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, + opt[i].size, opt[i].lo, false); + if (ret != EFI_SUCCESS) + goto out; + + ret = efi_bootmenu_append_bootorder(boot_index); + if (ret != EFI_SUCCESS) + goto out; + } + } + +out: + if (opt) { + for (i = 0; i < count; i++) + free(opt[i].lo); + } + free(opt); + efi_free_pool(volume_handles); + + return ret; +} + static int prepare_distro_boot_entry(struct bootmenu_data *menu, struct bootmenu_entry **current, unsigned short int *index) @@ -408,6 +476,16 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup;
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) { + if (i < MAX_DYNAMIC_ENTRY) { + /* + * UEFI specification requires booting from removal media using + * a architecture-specific default image name such as BOOTAA64.EFI. + */ + ret = prepare_media_device_entry(); + if (ret != EFI_SUCCESS) + goto cleanup; + } + if (i < MAX_DYNAMIC_ENTRY) { ret = prepare_uefi_bootorder_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT) diff --git a/include/efi_loader.h b/include/efi_loader.h index 7b100ca030..c326dfdaf2 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -926,6 +926,22 @@ struct efi_signature_store { struct x509_certificate; struct pkcs7_message;
+/** + * struct efi_bootmenu_media_boot_option - boot option for (removable) media device + * + * This structure is used to enumerate possible boot option + * + * @lo: Serialized load option data + * @size: Size of serialized load option data + * @exist: Flag to indicate the load option already exists + * in Non-volatile load option + */ +struct efi_bootmenu_media_boot_option { + void *lo; + efi_uintn_t size; + bool exist; +}; + bool efi_signature_lookup_digest(struct efi_image_regions *regs, struct efi_signature_store *db, bool dbx); @@ -1038,6 +1054,12 @@ efi_status_t efi_console_get_u16_string efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size, u32 *index); efi_status_t efi_bootmenu_append_bootorder(u16 index); +efi_status_t efi_bootmenu_enumerate_boot_option( + struct efi_bootmenu_media_boot_option *opt, + efi_handle_t *volume_handles, efi_status_t count); +efi_status_t efi_bootmenu_delete_invalid_boot_option( + struct efi_bootmenu_media_boot_option *opt, + efi_status_t count);
efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size);
diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c index 2ce5a47b10..3a64cf767a 100644 --- a/lib/efi_loader/efi_bootmenu_maintenance.c +++ b/lib/efi_loader/efi_bootmenu_maintenance.c @@ -24,6 +24,8 @@ static struct efi_simple_text_output_protocol *cout; #define EFI_BOOTMENU_FILE_PATH_MAX 512 #define EFI_BOOTMENU_BOOT_NAME_MAX 32 #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 +#define BOOTMENU_AUTO_ADD_OPTION_DATA "bootmenu" +#define BOOTMENU_AUTO_ADD_OPTION_DATA_U16 u"bootmenu"
typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
@@ -1114,3 +1116,129 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) ARRAY_SIZE(maintenance_menu_items), -1); } + +efi_status_t efi_bootmenu_enumerate_boot_option( + struct efi_bootmenu_media_boot_option *opt, + efi_handle_t *volume_handles, efi_status_t count) +{ + u32 i; + struct efi_handler *handler; + efi_status_t ret = EFI_SUCCESS; + + for (i = 0; i < count; i++) { + u16 *dev_name, *p; + struct efi_load_option lo; + struct efi_block_io *block_io; + char buf[BOOTMENU_DEVICE_NAME_MAX]; + struct efi_device_path *device_path; + + ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler); + if (ret != EFI_SUCCESS) + continue; + ret = efi_protocol_open(handler, (void **)&device_path, + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + continue; + + ret = efi_search_protocol(volume_handles[i], &efi_block_io_guid, &handler); + if (ret != EFI_SUCCESS) + continue; + ret = efi_protocol_open(handler, (void **)&block_io, + efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + continue; + + efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX); + dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16)); + if (!dev_name) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + p = dev_name; + utf8_utf16_strncpy(&p, buf, strlen(buf)); + + lo.label = dev_name; + lo.attributes = LOAD_OPTION_ACTIVE; + lo.file_path = device_path; + lo.file_path_length = efi_dp_size(device_path) + sizeof(END); + /* + * To identify the boot option that automatically added by + * the bootmenu, optional_data has special string. + * optional_data will be converted into u16 string + * in efi_serialize_load_option(). + */ + lo.optional_data = BOOTMENU_AUTO_ADD_OPTION_DATA; + opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo); + if (!opt[i].size) { + ret = EFI_OUT_OF_RESOURCES; + free(dev_name); + goto out; + } + free(dev_name); + } + +out: + return ret; +} + +efi_status_t efi_bootmenu_delete_invalid_boot_option( + struct efi_bootmenu_media_boot_option *opt, + efi_status_t count) +{ + u16 *bootorder; + u32 i, j; + efi_status_t ret; + efi_uintn_t num, size, bootorder_size; + void *load_option; + struct efi_load_option lo; + u16 varname[] = u"Boot####"; + + bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size); + if (!bootorder) + return EFI_NOT_FOUND; + + num = bootorder_size / sizeof(u16); + for (i = 0; i < num;) { + efi_uintn_t tmp; + + efi_create_indexed_name(varname, sizeof(varname), + "Boot", bootorder[i]); + load_option = efi_get_var(varname, &efi_global_variable_guid, &size); + if (!load_option) + goto next; + + tmp = size; + ret = efi_deserialize_load_option(&lo, load_option, &tmp); + if (ret != EFI_SUCCESS) + goto next; + + if (u16_strncmp((u16 *)lo.optional_data, BOOTMENU_AUTO_ADD_OPTION_DATA_U16, + u16_strlen(BOOTMENU_AUTO_ADD_OPTION_DATA_U16)) == 0) { + for (j = 0; j < count; j++) { + if (memcmp(opt[j].lo, load_option, size) == 0) { + opt[j].exist = true; + break; + } + } + + if (j == count) { + ret = delete_boot_option(bootorder, i, bootorder_size); + if (ret != EFI_SUCCESS) { + free(load_option); + goto out; + } + + num--; + bootorder_size -= sizeof(u16); + free(load_option); + continue; + } + } +next: + free(load_option); + i++; + } + +out: + return ret; +}
On Thu, Mar 24, 2022 at 10:54:42PM +0900, Masahisa Kojima wrote:
UEFI specification requires booting from removal media using a architecture-specific default image name such as BOOTAA64.EFI. This commit adds the removable media entries into bootmenu, so that user can select the removable media and boot with default image.
The bootmenu automatically enumerates the possible bootable media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, add it as new UEFI boot option(BOOT####) and update BootOrder variable. Depending on the system hardware setup, some devices may not exist at a later system boot, so invalid BOOT#### variable must be deleted.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Newly created in v4
cmd/bootmenu.c | 78 +++++++++++++ include/efi_loader.h | 22 ++++ lib/efi_loader/efi_bootmenu_maintenance.c | 128 ++++++++++++++++++++++ 3 files changed, 228 insertions(+)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 458eb90b63..7357cfeae5 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -304,6 +304,74 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, return 1; }
+static efi_status_t prepare_media_device_entry(void) +{
- u32 i;
- efi_status_t ret;
- efi_uintn_t count;
- efi_handle_t *volume_handles = NULL;
- struct efi_bootmenu_media_boot_option *opt = NULL;
- ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,
NULL, &count, (efi_handle_t **)&volume_handles);- if (ret != EFI_SUCCESS)
return ret;- opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option));
- if (!opt)
goto out;- /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
- ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count);
- if (ret != EFI_SUCCESS)
goto out;- /*
* System hardware configuration may vary depending on the user setup.* The boot option is automatically added by the bootmenu.* If the device is not attached to the system, the boot option needs* to be deleted.*/- ret = efi_bootmenu_delete_invalid_boot_option(opt, count);
Won't that fail on first boot if the BootOrder is undefined? efi_bootmenu_delete_invalid_boot_option() should return EFI_SUCCESS if it doesn't have a boot order maybe?
- if (ret != EFI_SUCCESS)
goto out;- /* add non-existent boot option */
- for (i = 0; i < count; i++) {
u32 boot_index;u16 var_name[9];if (!opt[i].exist) {ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name),&boot_index);if (ret != EFI_SUCCESS)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,opt[i].size, opt[i].lo, false);if (ret != EFI_SUCCESS)goto out;ret = efi_bootmenu_append_bootorder(boot_index);if (ret != EFI_SUCCESS)goto out;}- }
+out:
- if (opt) {
for (i = 0; i < count; i++)free(opt[i].lo);- }
- free(opt);
- efi_free_pool(volume_handles);
- return ret;
+}
static int prepare_distro_boot_entry(struct bootmenu_data *menu, struct bootmenu_entry **current, unsigned short int *index) @@ -408,6 +476,16 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup;
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
if (i < MAX_DYNAMIC_ENTRY) {/** UEFI specification requires booting from removal media using* a architecture-specific default image name such as BOOTAA64.EFI.*/ret = prepare_media_device_entry();if (ret != EFI_SUCCESS)goto cleanup;}- if (i < MAX_DYNAMIC_ENTRY) { ret = prepare_uefi_bootorder_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 7b100ca030..c326dfdaf2 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -926,6 +926,22 @@ struct efi_signature_store { struct x509_certificate; struct pkcs7_message;
+/**
- struct efi_bootmenu_media_boot_option - boot option for (removable) media device
- This structure is used to enumerate possible boot option
- @lo: Serialized load option data
- @size: Size of serialized load option data
- @exist: Flag to indicate the load option already exists
in Non-volatile load option- */
+struct efi_bootmenu_media_boot_option {
- void *lo;
- efi_uintn_t size;
- bool exist;
+};
bool efi_signature_lookup_digest(struct efi_image_regions *regs, struct efi_signature_store *db, bool dbx); @@ -1038,6 +1054,12 @@ efi_status_t efi_console_get_u16_string efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size, u32 *index); efi_status_t efi_bootmenu_append_bootorder(u16 index); +efi_status_t efi_bootmenu_enumerate_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_handle_t *volume_handles, efi_status_t count);+efi_status_t efi_bootmenu_delete_invalid_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_status_t count);efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size);
diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c index 2ce5a47b10..3a64cf767a 100644 --- a/lib/efi_loader/efi_bootmenu_maintenance.c +++ b/lib/efi_loader/efi_bootmenu_maintenance.c @@ -24,6 +24,8 @@ static struct efi_simple_text_output_protocol *cout; #define EFI_BOOTMENU_FILE_PATH_MAX 512 #define EFI_BOOTMENU_BOOT_NAME_MAX 32 #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 +#define BOOTMENU_AUTO_ADD_OPTION_DATA "bootmenu" +#define BOOTMENU_AUTO_ADD_OPTION_DATA_U16 u"bootmenu"
typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
@@ -1114,3 +1116,129 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) ARRAY_SIZE(maintenance_menu_items), -1); }
+efi_status_t efi_bootmenu_enumerate_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_handle_t *volume_handles, efi_status_t count)+{
- u32 i;
- struct efi_handler *handler;
- efi_status_t ret = EFI_SUCCESS;
- for (i = 0; i < count; i++) {
u16 *dev_name, *p;struct efi_load_option lo;struct efi_block_io *block_io;char buf[BOOTMENU_DEVICE_NAME_MAX];struct efi_device_path *device_path;ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);if (ret != EFI_SUCCESS)continue;ret = efi_protocol_open(handler, (void **)&device_path,efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);if (ret != EFI_SUCCESS)continue;ret = efi_search_protocol(volume_handles[i], &efi_block_io_guid, &handler);if (ret != EFI_SUCCESS)continue;ret = efi_protocol_open(handler, (void **)&block_io,efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);if (ret != EFI_SUCCESS)continue;efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX);dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16));if (!dev_name) {ret = EFI_OUT_OF_RESOURCES;goto out;}p = dev_name;utf8_utf16_strncpy(&p, buf, strlen(buf));lo.label = dev_name;lo.attributes = LOAD_OPTION_ACTIVE;lo.file_path = device_path;lo.file_path_length = efi_dp_size(device_path) + sizeof(END);/** To identify the boot option that automatically added by* the bootmenu, optional_data has special string.* optional_data will be converted into u16 string* in efi_serialize_load_option().*/lo.optional_data = BOOTMENU_AUTO_ADD_OPTION_DATA;opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo);if (!opt[i].size) {ret = EFI_OUT_OF_RESOURCES;free(dev_name);goto out;}free(dev_name);- }
+out:
- return ret;
+}
+efi_status_t efi_bootmenu_delete_invalid_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_status_t count)+{
- u16 *bootorder;
- u32 i, j;
- efi_status_t ret;
- efi_uintn_t num, size, bootorder_size;
- void *load_option;
- struct efi_load_option lo;
- u16 varname[] = u"Boot####";
- bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size);
- if (!bootorder)
return EFI_NOT_FOUND;- num = bootorder_size / sizeof(u16);
- for (i = 0; i < num;) {
efi_uintn_t tmp;efi_create_indexed_name(varname, sizeof(varname),"Boot", bootorder[i]);load_option = efi_get_var(varname, &efi_global_variable_guid, &size);if (!load_option)goto next;tmp = size;ret = efi_deserialize_load_option(&lo, load_option, &tmp);if (ret != EFI_SUCCESS)goto next;if (u16_strncmp((u16 *)lo.optional_data, BOOTMENU_AUTO_ADD_OPTION_DATA_U16,
You should convert the lo.optional_data to a u16 before doing the check here, other wise this might cause an unaligned access
u16_strlen(BOOTMENU_AUTO_ADD_OPTION_DATA_U16)) == 0) {for (j = 0; j < count; j++) {if (memcmp(opt[j].lo, load_option, size) == 0) {opt[j].exist = true;break;}}if (j == count) {ret = delete_boot_option(bootorder, i, bootorder_size);if (ret != EFI_SUCCESS) {free(load_option);goto out;}num--;bootorder_size -= sizeof(u16);free(load_option);continue;}}+next:
free(load_option);i++;- }
+out:
- return ret;
+}
2.17.1
Thanks /Ilias
On Thu, 31 Mar 2022 at 17:48, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Thu, Mar 24, 2022 at 10:54:42PM +0900, Masahisa Kojima wrote:
UEFI specification requires booting from removal media using a architecture-specific default image name such as BOOTAA64.EFI. This commit adds the removable media entries into bootmenu, so that user can select the removable media and boot with default image.
The bootmenu automatically enumerates the possible bootable media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, add it as new UEFI boot option(BOOT####) and update BootOrder variable. Depending on the system hardware setup, some devices may not exist at a later system boot, so invalid BOOT#### variable must be deleted.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Newly created in v4
cmd/bootmenu.c | 78 +++++++++++++ include/efi_loader.h | 22 ++++ lib/efi_loader/efi_bootmenu_maintenance.c | 128 ++++++++++++++++++++++ 3 files changed, 228 insertions(+)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 458eb90b63..7357cfeae5 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -304,6 +304,74 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, return 1; }
+static efi_status_t prepare_media_device_entry(void) +{
u32 i;efi_status_t ret;efi_uintn_t count;efi_handle_t *volume_handles = NULL;struct efi_bootmenu_media_boot_option *opt = NULL;ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,NULL, &count, (efi_handle_t **)&volume_handles);if (ret != EFI_SUCCESS)return ret;opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option));if (!opt)goto out;/* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count);if (ret != EFI_SUCCESS)goto out;/** System hardware configuration may vary depending on the user setup.* The boot option is automatically added by the bootmenu.* If the device is not attached to the system, the boot option needs* to be deleted.*/ret = efi_bootmenu_delete_invalid_boot_option(opt, count);Won't that fail on first boot if the BootOrder is undefined? efi_bootmenu_delete_invalid_boot_option() should return EFI_SUCCESS if it doesn't have a boot order maybe?
You are correct. Returning EFI_SUCCESS if there is no BootOrder defined will resolve your issue.
if (ret != EFI_SUCCESS)goto out;/* add non-existent boot option */for (i = 0; i < count; i++) {u32 boot_index;u16 var_name[9];if (!opt[i].exist) {ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name),&boot_index);if (ret != EFI_SUCCESS)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,opt[i].size, opt[i].lo, false);if (ret != EFI_SUCCESS)goto out;ret = efi_bootmenu_append_bootorder(boot_index);if (ret != EFI_SUCCESS)goto out;}}+out:
if (opt) {for (i = 0; i < count; i++)free(opt[i].lo);}free(opt);efi_free_pool(volume_handles);return ret;+}
static int prepare_distro_boot_entry(struct bootmenu_data *menu, struct bootmenu_entry **current, unsigned short int *index) @@ -408,6 +476,16 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup;
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
if (i < MAX_DYNAMIC_ENTRY) {/** UEFI specification requires booting from removal media using* a architecture-specific default image name such as BOOTAA64.EFI.*/ret = prepare_media_device_entry();if (ret != EFI_SUCCESS)goto cleanup;}if (i < MAX_DYNAMIC_ENTRY) { ret = prepare_uefi_bootorder_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT)diff --git a/include/efi_loader.h b/include/efi_loader.h index 7b100ca030..c326dfdaf2 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -926,6 +926,22 @@ struct efi_signature_store { struct x509_certificate; struct pkcs7_message;
+/**
- struct efi_bootmenu_media_boot_option - boot option for (removable) media device
- This structure is used to enumerate possible boot option
- @lo: Serialized load option data
- @size: Size of serialized load option data
- @exist: Flag to indicate the load option already exists
in Non-volatile load option- */
+struct efi_bootmenu_media_boot_option {
void *lo;efi_uintn_t size;bool exist;+};
bool efi_signature_lookup_digest(struct efi_image_regions *regs, struct efi_signature_store *db, bool dbx); @@ -1038,6 +1054,12 @@ efi_status_t efi_console_get_u16_string efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size, u32 *index); efi_status_t efi_bootmenu_append_bootorder(u16 index); +efi_status_t efi_bootmenu_enumerate_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_handle_t *volume_handles, efi_status_t count);+efi_status_t efi_bootmenu_delete_invalid_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_status_t count);efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size);
diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c index 2ce5a47b10..3a64cf767a 100644 --- a/lib/efi_loader/efi_bootmenu_maintenance.c +++ b/lib/efi_loader/efi_bootmenu_maintenance.c @@ -24,6 +24,8 @@ static struct efi_simple_text_output_protocol *cout; #define EFI_BOOTMENU_FILE_PATH_MAX 512 #define EFI_BOOTMENU_BOOT_NAME_MAX 32 #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 +#define BOOTMENU_AUTO_ADD_OPTION_DATA "bootmenu" +#define BOOTMENU_AUTO_ADD_OPTION_DATA_U16 u"bootmenu"
typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
@@ -1114,3 +1116,129 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) ARRAY_SIZE(maintenance_menu_items), -1); }
+efi_status_t efi_bootmenu_enumerate_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_handle_t *volume_handles, efi_status_t count)+{
u32 i;struct efi_handler *handler;efi_status_t ret = EFI_SUCCESS;for (i = 0; i < count; i++) {u16 *dev_name, *p;struct efi_load_option lo;struct efi_block_io *block_io;char buf[BOOTMENU_DEVICE_NAME_MAX];struct efi_device_path *device_path;ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);if (ret != EFI_SUCCESS)continue;ret = efi_protocol_open(handler, (void **)&device_path,efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);if (ret != EFI_SUCCESS)continue;ret = efi_search_protocol(volume_handles[i], &efi_block_io_guid, &handler);if (ret != EFI_SUCCESS)continue;ret = efi_protocol_open(handler, (void **)&block_io,efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);if (ret != EFI_SUCCESS)continue;efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX);dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16));if (!dev_name) {ret = EFI_OUT_OF_RESOURCES;goto out;}p = dev_name;utf8_utf16_strncpy(&p, buf, strlen(buf));lo.label = dev_name;lo.attributes = LOAD_OPTION_ACTIVE;lo.file_path = device_path;lo.file_path_length = efi_dp_size(device_path) + sizeof(END);/** To identify the boot option that automatically added by* the bootmenu, optional_data has special string.* optional_data will be converted into u16 string* in efi_serialize_load_option().*/lo.optional_data = BOOTMENU_AUTO_ADD_OPTION_DATA;opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo);if (!opt[i].size) {ret = EFI_OUT_OF_RESOURCES;free(dev_name);goto out;}free(dev_name);}+out:
return ret;+}
+efi_status_t efi_bootmenu_delete_invalid_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_status_t count)+{
u16 *bootorder;u32 i, j;efi_status_t ret;efi_uintn_t num, size, bootorder_size;void *load_option;struct efi_load_option lo;u16 varname[] = u"Boot####";bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size);if (!bootorder)return EFI_NOT_FOUND;num = bootorder_size / sizeof(u16);for (i = 0; i < num;) {efi_uintn_t tmp;efi_create_indexed_name(varname, sizeof(varname),"Boot", bootorder[i]);load_option = efi_get_var(varname, &efi_global_variable_guid, &size);if (!load_option)goto next;tmp = size;ret = efi_deserialize_load_option(&lo, load_option, &tmp);if (ret != EFI_SUCCESS)goto next;if (u16_strncmp((u16 *)lo.optional_data, BOOTMENU_AUTO_ADD_OPTION_DATA_U16,You should convert the lo.optional_data to a u16 before doing the check here, other wise this might cause an unaligned access
Yes, unaligned access might occur. I need to copy lo.optional_data to the allocated buffer first, then compare it.
Thanks, Masahisa Kojima
u16_strlen(BOOTMENU_AUTO_ADD_OPTION_DATA_U16)) == 0) {for (j = 0; j < count; j++) {if (memcmp(opt[j].lo, load_option, size) == 0) {opt[j].exist = true;break;}}if (j == count) {ret = delete_boot_option(bootorder, i, bootorder_size);if (ret != EFI_SUCCESS) {free(load_option);goto out;}num--;bootorder_size -= sizeof(u16);free(load_option);continue;}}+next:
free(load_option);i++;}+out:
return ret;+}
2.17.1
Thanks /Ilias
On 3/31/22 10:48, Ilias Apalodimas wrote:
On Thu, Mar 24, 2022 at 10:54:42PM +0900, Masahisa Kojima wrote:
UEFI specification requires booting from removal media using a architecture-specific default image name such as BOOTAA64.EFI. This commit adds the removable media entries into bootmenu, so that user can select the removable media and boot with default image.
The bootmenu automatically enumerates the possible bootable media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, add it as new UEFI boot option(BOOT####) and update BootOrder variable. Depending on the system hardware setup, some devices may not exist at a later system boot, so invalid BOOT#### variable must be deleted.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Newly created in v4
cmd/bootmenu.c | 78 +++++++++++++ include/efi_loader.h | 22 ++++ lib/efi_loader/efi_bootmenu_maintenance.c | 128 ++++++++++++++++++++++ 3 files changed, 228 insertions(+)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index 458eb90b63..7357cfeae5 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -304,6 +304,74 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, return 1; }
+static efi_status_t prepare_media_device_entry(void) +{
- u32 i;
- efi_status_t ret;
- efi_uintn_t count;
- efi_handle_t *volume_handles = NULL;
- struct efi_bootmenu_media_boot_option *opt = NULL;
- ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,
NULL, &count, (efi_handle_t **)&volume_handles);- if (ret != EFI_SUCCESS)
return ret;- opt = calloc(count, sizeof(struct efi_bootmenu_media_boot_option));
- if (!opt)
goto out;- /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */
- ret = efi_bootmenu_enumerate_boot_option(opt, volume_handles, count);
- if (ret != EFI_SUCCESS)
goto out;- /*
* System hardware configuration may vary depending on the user setup.* The boot option is automatically added by the bootmenu.* If the device is not attached to the system, the boot option needs* to be deleted.*/- ret = efi_bootmenu_delete_invalid_boot_option(opt, count);
Won't that fail on first boot if the BootOrder is undefined? efi_bootmenu_delete_invalid_boot_option() should return EFI_SUCCESS if it doesn't have a boot order maybe?
- if (ret != EFI_SUCCESS)
goto out;- /* add non-existent boot option */
- for (i = 0; i < count; i++) {
u32 boot_index;u16 var_name[9];if (!opt[i].exist) {ret = efi_bootmenu_get_unused_bootoption(var_name, sizeof(var_name),&boot_index);if (ret != EFI_SUCCESS)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,opt[i].size, opt[i].lo, false);if (ret != EFI_SUCCESS)goto out;ret = efi_bootmenu_append_bootorder(boot_index);if (ret != EFI_SUCCESS)goto out;}- }
+out:
- if (opt) {
for (i = 0; i < count; i++)free(opt[i].lo);- }
- free(opt);
- efi_free_pool(volume_handles);
- return ret;
+}
- static int prepare_distro_boot_entry(struct bootmenu_data *menu, struct bootmenu_entry **current, unsigned short int *index)
@@ -408,6 +476,16 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup;
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
if (i < MAX_DYNAMIC_ENTRY) {/** UEFI specification requires booting from removal media using* a architecture-specific default image name such as BOOTAA64.EFI.*/ret = prepare_media_device_entry();if (ret != EFI_SUCCESS)goto cleanup;}- if (i < MAX_DYNAMIC_ENTRY) { ret = prepare_uefi_bootorder_entry(menu, &iter, &i); if (ret < 0 && ret != -ENOENT)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 7b100ca030..c326dfdaf2 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -926,6 +926,22 @@ struct efi_signature_store { struct x509_certificate; struct pkcs7_message;
+/**
- struct efi_bootmenu_media_boot_option - boot option for (removable) media device
- This structure is used to enumerate possible boot option
- @lo: Serialized load option data
- @size: Size of serialized load option data
- @exist: Flag to indicate the load option already exists
in Non-volatile load option- */
+struct efi_bootmenu_media_boot_option {
- void *lo;
- efi_uintn_t size;
- bool exist;
+};
- bool efi_signature_lookup_digest(struct efi_image_regions *regs, struct efi_signature_store *db, bool dbx);
@@ -1038,6 +1054,12 @@ efi_status_t efi_console_get_u16_string efi_status_t efi_bootmenu_get_unused_bootoption(u16 *buf, efi_uintn_t buf_size, u32 *index); efi_status_t efi_bootmenu_append_bootorder(u16 index); +efi_status_t efi_bootmenu_enumerate_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_handle_t *volume_handles, efi_status_t count);+efi_status_t efi_bootmenu_delete_invalid_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_status_t count);efi_status_t efi_disk_get_device_name(struct efi_block_io *this, char *buf, int size);
diff --git a/lib/efi_loader/efi_bootmenu_maintenance.c b/lib/efi_loader/efi_bootmenu_maintenance.c index 2ce5a47b10..3a64cf767a 100644 --- a/lib/efi_loader/efi_bootmenu_maintenance.c +++ b/lib/efi_loader/efi_bootmenu_maintenance.c @@ -24,6 +24,8 @@ static struct efi_simple_text_output_protocol *cout; #define EFI_BOOTMENU_FILE_PATH_MAX 512 #define EFI_BOOTMENU_BOOT_NAME_MAX 32 #define EFI_BOOT_ORDER_MAX_SIZE_IN_DECIMAL 6 +#define BOOTMENU_AUTO_ADD_OPTION_DATA "bootmenu" +#define BOOTMENU_AUTO_ADD_OPTION_DATA_U16 u"bootmenu"
typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool *exit);
@@ -1114,3 +1116,129 @@ efi_status_t efi_bootmenu_show_maintenance_menu(void) ARRAY_SIZE(maintenance_menu_items), -1); }
+efi_status_t efi_bootmenu_enumerate_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_handle_t *volume_handles, efi_status_t count)+{
- u32 i;
- struct efi_handler *handler;
- efi_status_t ret = EFI_SUCCESS;
- for (i = 0; i < count; i++) {
u16 *dev_name, *p;struct efi_load_option lo;struct efi_block_io *block_io;char buf[BOOTMENU_DEVICE_NAME_MAX];struct efi_device_path *device_path;ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);if (ret != EFI_SUCCESS)continue;ret = efi_protocol_open(handler, (void **)&device_path,efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);if (ret != EFI_SUCCESS)continue;ret = efi_search_protocol(volume_handles[i], &efi_block_io_guid, &handler);if (ret != EFI_SUCCESS)continue;ret = efi_protocol_open(handler, (void **)&block_io,efi_root, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);if (ret != EFI_SUCCESS)continue;efi_disk_get_device_name(block_io, buf, BOOTMENU_DEVICE_NAME_MAX);dev_name = calloc(1, (strlen(buf) + 1) * sizeof(u16));if (!dev_name) {ret = EFI_OUT_OF_RESOURCES;goto out;}p = dev_name;utf8_utf16_strncpy(&p, buf, strlen(buf));lo.label = dev_name;lo.attributes = LOAD_OPTION_ACTIVE;lo.file_path = device_path;lo.file_path_length = efi_dp_size(device_path) + sizeof(END);/** To identify the boot option that automatically added by* the bootmenu, optional_data has special string.* optional_data will be converted into u16 string* in efi_serialize_load_option().*/lo.optional_data = BOOTMENU_AUTO_ADD_OPTION_DATA;opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo);if (!opt[i].size) {ret = EFI_OUT_OF_RESOURCES;free(dev_name);goto out;}free(dev_name);- }
+out:
- return ret;
+}
+efi_status_t efi_bootmenu_delete_invalid_boot_option(
struct efi_bootmenu_media_boot_option *opt,efi_status_t count)+{
- u16 *bootorder;
- u32 i, j;
- efi_status_t ret;
- efi_uintn_t num, size, bootorder_size;
- void *load_option;
- struct efi_load_option lo;
- u16 varname[] = u"Boot####";
- bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid, &bootorder_size);
- if (!bootorder)
return EFI_NOT_FOUND;- num = bootorder_size / sizeof(u16);
- for (i = 0; i < num;) {
efi_uintn_t tmp;efi_create_indexed_name(varname, sizeof(varname),"Boot", bootorder[i]);load_option = efi_get_var(varname, &efi_global_variable_guid, &size);if (!load_option)goto next;tmp = size;ret = efi_deserialize_load_option(&lo, load_option, &tmp);if (ret != EFI_SUCCESS)goto next;if (u16_strncmp((u16 *)lo.optional_data, BOOTMENU_AUTO_ADD_OPTION_DATA_U16,You should convert the lo.optional_data to a u16 before doing the check here, other wise this might cause an unaligned access
Once we have initialized the EFI sub-system unaligned access is allowed. But it may be slow. E.g. on RISC-V it may be emulated by the SBI.
Best regards
Heinrich
u16_strlen(BOOTMENU_AUTO_ADD_OPTION_DATA_U16)) == 0) {for (j = 0; j < count; j++) {if (memcmp(opt[j].lo, load_option, size) == 0) {opt[j].exist = true;break;}}if (j == count) {ret = delete_boot_option(bootorder, i, bootorder_size);if (ret != EFI_SUCCESS) {free(load_option);goto out;}num--;bootorder_size -= sizeof(u16);free(load_option);continue;}}+next:
free(load_option);i++;- }
+out:
- return ret;
+}
2.17.1
Thanks /Ilias
The bootmenu enumerates the UEFI boot variable and distro boot (boot_targets) for boot device selection.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org --- Newly created in v4
doc/usage/bootmenu.rst | 65 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)
diff --git a/doc/usage/bootmenu.rst b/doc/usage/bootmenu.rst index 1f094ad6ed..dc1d22062b 100644 --- a/doc/usage/bootmenu.rst +++ b/doc/usage/bootmenu.rst @@ -93,3 +93,68 @@ When you intend to use the bootmenu on a color frame buffer console, make sure to additionally define::
CONFIG_CFB_CONSOLE_ANSI=y + +UEFI boot variable enumeration +============================== + +The bootmenu automatically generates the UEFI boot variable("BOOT####") +in order of "BootOrder". When the user selects the UEFI boot +variable entry, bootmenu sets the selected boot variable index +to "BootNext", then call the uefi boot manager with the command +"bootefi bootmgr". + +The bootmenu automatically enumerates the possible bootable +media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. + +The bootmenu prints the EFI_LOAD_OPTION.Description[] as title +of the entry together with "UEFI" prefix and BOOT#### variable name. + +Example bootmenu is as below:: + + *** U-Boot Boot Menu *** + + UEFI BOOT0000 : mmc0:1 + UEFI BOOT0001 : mmc0:2 + UEFI BOOT0002 : debian + UEFI BOOT0003 : nvme0:1 + UEFI BOOT0004 : ubuntu + UEFI BOOT0005 : nvme0:2 + UEFI BOOT0006 : usb0:2 + +To scan the discoverable devices connected to the buses such as +USB and PCIe prior to bootmenu showing up, CONFIG_PREBOOT can be +used to run the command before showing the bootmenu, i.e.:: + + CONFIG_USE_PREBOOT=y + CONFIG_PREBOOT="pci enum; usb start; scsi scan; nvme scan; virtio scan" + +distro boot command enumeration +=============================== + +The bootmenu also automatically generates the entries for +distro boot command. The bootmenu shows the devices in boot_targets +environment variable. +When the user selects the distro boot command entry, the bootmenu +runs the command defined in "bootcmd_xxx" environment variable. +As an example, if user selects "usb0" entry, bootmenu runs the +command defined in "bootcmd_usb0". + +Example boot_targets:: + + usb0, scsi0, nvme0, dhcp + +Example bootmenu is as below:: + + *** U-Boot Boot Menu *** + + distro_boot : usb0 + distro_boot : scsi0 + distro_boot : nvme0 + distro_boot : dhcp + +Disable to enter the U-Boot console +=================================== + +To improve the product security, U-Boot console can be disabled by:: + + CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE=n
On 3/24/22 14:54, Masahisa Kojima wrote:
The bootmenu enumerates the UEFI boot variable and distro boot (boot_targets) for boot device selection.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Newly created in v4
doc/usage/bootmenu.rst | 65 ++++++++++++++++++++++++++++++++++++++++++
In origin/master the file is called doc/usage/cmd/bootmenu.rst.
1 file changed, 65 insertions(+)
diff --git a/doc/usage/bootmenu.rst b/doc/usage/bootmenu.rst index 1f094ad6ed..dc1d22062b 100644 --- a/doc/usage/bootmenu.rst +++ b/doc/usage/bootmenu.rst @@ -93,3 +93,68 @@ When you intend to use the bootmenu on a color frame buffer console, make sure to additionally define::
CONFIG_CFB_CONSOLE_ANSI=y
+UEFI boot variable enumeration +==============================
+The bootmenu automatically generates the UEFI boot variable("BOOT####")
How do you ensure that those variables that are autogenerated are not saved?
+in order of "BootOrder". When the user selects the UEFI boot +variable entry, bootmenu sets the selected boot variable index +to "BootNext", then call the uefi boot manager with the command +"bootefi bootmgr".
+The bootmenu automatically enumerates the possible bootable +media devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.
+The bootmenu prints the EFI_LOAD_OPTION.Description[] as title +of the entry together with "UEFI" prefix and BOOT#### variable name.
+Example bootmenu is as below::
- *** U-Boot Boot Menu ***
UEFI BOOT0000 : mmc0:1UEFI BOOT0001 : mmc0:2UEFI BOOT0002 : debianUEFI BOOT0003 : nvme0:1UEFI BOOT0004 : ubuntuUEFI BOOT0005 : nvme0:2UEFI BOOT0006 : usb0:2+To scan the discoverable devices connected to the buses such as +USB and PCIe prior to bootmenu showing up, CONFIG_PREBOOT can be +used to run the command before showing the bootmenu, i.e.::
- CONFIG_USE_PREBOOT=y
- CONFIG_PREBOOT="pci enum; usb start; scsi scan; nvme scan; virtio scan"
+distro boot command enumeration +===============================
+The bootmenu also automatically generates the entries for +distro boot command. The bootmenu shows the devices in boot_targets +environment variable. +When the user selects the distro boot command entry, the bootmenu +runs the command defined in "bootcmd_xxx" environment variable. +As an example, if user selects "usb0" entry, bootmenu runs the +command defined in "bootcmd_usb0".
+Example boot_targets::
- usb0, scsi0, nvme0, dhcp
+Example bootmenu is as below::
- *** U-Boot Boot Menu ***
distro_boot : usb0distro_boot : scsi0distro_boot : nvme0distro_boot : dhcp+Disable to enter the U-Boot console +===================================
+To improve the product security, U-Boot console can be disabled by::
- CONFIG_CMD_BOOTMENU_ENTER_UBOOT_CONSOLE=n
The man page should have a section 'Configuration' which lists all relevant configuration options for the bootmenu command.
Kojima-san,
On Thu, Mar 24, 2022 at 10:54:32PM +0900, Masahisa Kojima wrote:
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
The menu example is as follows.
Good job done, Kojima-san. I like it. Before reviewing each commit, I would suggest a couple of improvements on the menu itself. They are more or less my opinion and other people may have their own preference, though.
1) Top menu (U-Boot Boot Menu) - In general, it's a bit difficult to understand from where each menu item comes and what it means. For instance,
UEFI BOOT0000 : debian
is a user-defined boot option, while
UEFI BOOT0002 : mmc0:1
is an option for removable media. Correct?
- I'd prefer to categorize items into sub-menus, particularly, UEFI items.
bootmenu_... distro_boot ... UEFI Boot UEFI Boot Manager Maintenance
and "UEFI Boot" sub-menu shows
UEFI BOOT0000 : debian UEFI BOOT0001 : ubuntu
in an order of "BootOrder",
and <removable media> /* not selectable */
UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2
- For UEFI items, I want to do "e" (edit/modify) directly here to change the option.
- When "U-Boot console" is selected, the prompt ("=>") is displayed like UEFI Boot Manager Maintenance U-Boot console=> It should be output at the beginning of the next line or the screen be cleaned up before showing the prompt.
- What not have "Quit" here?
2) UEFI Boot Manager Maintenance - The title should be "UEFI Boot Manager Maintenance". - I want to have "Edit(Modify) Boot Option" - "Add Boot Option" - The menu titles should be "Select a device" and "Select a file". - Some devices are shown, some are not. Why? Do we have to run, say, "scsi rescan" beforehand? - How can we specify "removable media" without a file path? - At "file selection" menu, "Esc" should let us go back to the "device selection" menu rather than the top, "Add Boot Option". - We should be able to specify initrd path, i.e. the second device path in a boot option. - We should be able to specify "optional data" in a boot option. - "Change Boot Order" - I like a more intuitive operation here. Say, select an item with "Enter" and then use "Up" and "Down" to move it around. - Probably, it would be better to have the final confirmation, like "Do you want to save the change?"
Thanks, -Takahiro Akashi
*** U-Boot Boot Menu ***
bootmenu_00 : Boot 1. kernel bootmenu_01 : Boot 2. kernel bootmenu_02 : Reset board UEFI BOOT0000 : debian UEFI BOOT0001 : ubuntu UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
[Major changes from RFC v3]
- add Kconfig option to disable U-Boot console
- add UEFI boot variable maintenance feature
- support removable media support and user selection
- app bootmenu enhancement documentation
[How to run on QEMU(arm64)]
- clone source code
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
- prepare U-Boot .config
$ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
- run on QEMU(arm64) example
$ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
-- 2.17.1
Hi Akashi-san,
On Fri, 25 Mar 2022 at 10:20, Takahiro Akashi takahiro.akashi@linaro.org wrote:
Kojima-san,
On Thu, Mar 24, 2022 at 10:54:32PM +0900, Masahisa Kojima wrote:
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
The menu example is as follows.
Good job done, Kojima-san. I like it. Before reviewing each commit, I would suggest a couple of improvements on the menu itself. They are more or less my opinion and other people may have their own preference, though.
Thank you for your comment and verifying the actual behavior.
- Top menu (U-Boot Boot Menu)
- In general, it's a bit difficult to understand from where each menu item comes and what it means. For instance,
UEFI BOOT0000 : debianis a user-defined boot option, while
UEFI BOOT0002 : mmc0:1is an option for removable media. Correct?
Yes, and I agree that the current menu title is a bit difficult to understand, especially for automatically listed removable media(e.g. mmc0:1).
I'd prefer to categorize items into sub-menus, particularly, UEFI items.
bootmenu_... distro_boot ... UEFI Boot UEFI Boot Manager Maintenance
Yes, it is a good idea. On the other hand, if user prefers simpler menu and only uses UEFI Boot for example, bootmenu_xx and distro boot entries can be removed by updating the U-Boot environment variable(remove bootmenu_xx and boot_targets).
and "UEFI Boot" sub-menu shows
UEFI BOOT0000 : debian UEFI BOOT0001 : ubuntuin an order of "BootOrder",
and <removable media> /* not selectable */
UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2
It is a little difficult. UEFI BootOrder variable is also automatically updated by bootmenu to include the removable media, so the entries that user defined Boot#### and automatically added removable media are mixed in BootOrder. The UEFI Boot entry is displayed in the order of BootOrder.
- For UEFI items, I want to do "e" (edit/modify) directly here to change the option.
It is the same as the Grub menu. I will consider it but let me set as a low priority now.
- When "U-Boot console" is selected, the prompt ("=>") is displayed like UEFI Boot Manager Maintenance U-Boot console=> It should be output at the beginning of the next line or the screen be cleaned up before showing the prompt.
Thank you. It should be fixed.
- What not have "Quit" here?
OK, I will consider to add.
- UEFI Boot Manager Maintenance
- The title should be "UEFI Boot Manager Maintenance".
"UEFI Boot Manager Maintenance" is the same as the current one.
- I want to have "Edit(Modify) Boot Option"
Yes, I agree.
- "Add Boot Option"
- The menu titles should be "Select a device" and "Select a file".
Current "Add Boot Option" will support the "Select a file" feature. I think the user will normally use the boot option having FilePath.
- Some devices are shown, some are not. Why? Do we have to run, say, "scsi rescan" beforehand?
Current bootmenu enumerates the all devices implementing EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. If some devices are not shown, please run the discover command like "scsi rescan" then re-enter the bootmenu with "bootmenu" command. If the all devices listed by "efidebug dh" having EFI_SIMPLE_FILE_SYSTEM_PROTOCOL are not shown in bootmenu, something wrong in my code.
- How can we specify "removable media" without a file path?
This patch series only supports the auto enumeration. I think the user will not manually add the "removable media".
- At "file selection" menu, "Esc" should let us go back to the "device selection" menu rather than the top, "Add Boot Option".
Yes, I agree.
- We should be able to specify initrd path, i.e. the second device path in a boot option.
- We should be able to specify "optional data" in a boot option.
- "Change Boot Order"
I consider supporting these two items.
- I like a more intuitive operation here. Say, select an item with "Enter" and then use "Up" and "Down" to move it around.
Yes, I agree and try to improve.
- Probably, it would be better to have the final confirmation, like "Do you want to save the change?"
Yes, same for "Delete Boot Option".
Thanks, Masahisa Kojima
Thanks, -Takahiro Akashi
*** U-Boot Boot Menu ***
bootmenu_00 : Boot 1. kernel bootmenu_01 : Boot 2. kernel bootmenu_02 : Reset board UEFI BOOT0000 : debian UEFI BOOT0001 : ubuntu UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
[Major changes from RFC v3]
- add Kconfig option to disable U-Boot console
- add UEFI boot variable maintenance feature
- support removable media support and user selection
- app bootmenu enhancement documentation
[How to run on QEMU(arm64)]
- clone source code
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
- prepare U-Boot .config
$ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
- run on QEMU(arm64) example
$ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
-- 2.17.1
On 3/24/22 14:54, Masahisa Kojima wrote:
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
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 UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
[Major changes from RFC v3]
- add Kconfig option to disable U-Boot console
- add UEFI boot variable maintenance feature
- support removable media support and user selection
- app bootmenu enhancement documentation
[How to run on QEMU(arm64)]
- clone source code
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
- prepare U-Boot .config
$ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
- run on QEMU(arm64) example
$ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
I applied the series to origin/master.
make sandbox_defconfig CONFIG_CMD_BOOTMENU=y CONFIG_AUTOBOOT_MENU_SHOW=y CONFIG_USE_PREBOOT=y CONFIG_PREBOOT="host bind 0 ../sandbox.img"
sandbox.img contains file ubootefi.var defining two boot options 1: Boot0003: test3 2: Boot00A0: hello
./u-boot -T just shows the U-Boot prompt and no menu.
What do I miss?
Best regards
Heinrich
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
Hi Heinrich,
On Sat, 2 Apr 2022 at 14:48, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
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 UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
[Major changes from RFC v3]
- add Kconfig option to disable U-Boot console
- add UEFI boot variable maintenance feature
- support removable media support and user selection
- app bootmenu enhancement documentation
[How to run on QEMU(arm64)]
- clone source code
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
- prepare U-Boot .config
$ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
- run on QEMU(arm64) example
$ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
I applied the series to origin/master.
make sandbox_defconfig CONFIG_CMD_BOOTMENU=y CONFIG_AUTOBOOT_MENU_SHOW=y CONFIG_USE_PREBOOT=y CONFIG_PREBOOT="host bind 0 ../sandbox.img"
sandbox.img contains file ubootefi.var defining two boot options 1: Boot0003: test3 2: Boot00A0: hello
./u-boot -T just shows the U-Boot prompt and no menu.
What do I miss?
Sorry, the current patch series has an issue that Ilias pointed out[*1], could you also define the BootOrder variable? If there is no BootOrder defined, bootmenu ends up with EFI_NOT_FOUND and go to U-Boot console without showing bootmenu.
[*1] https://lore.kernel.org/u-boot/YkVqzw3nIG3uR33g@hades/#t
Thanks, Masahisa Kojima
Best regards
Heinrich
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
On 3/24/22 14:54, Masahisa Kojima wrote:
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
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 UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
[Major changes from RFC v3]
- add Kconfig option to disable U-Boot console
- add UEFI boot variable maintenance feature
- support removable media support and user selection
- app bootmenu enhancement documentation
[How to run on QEMU(arm64)]
- clone source code
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
- prepare U-Boot .config
$ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
- run on QEMU(arm64) example
$ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
I applied the series to origin/master and added the two config options.
This is the output if the disk image does not have an EFI partition:
Found 0 disks No EFI system partition Unable to find TPMv2 device
Even if there is no EFI system partition on virtio 0 I would expect the menu to be shown.
When having an image with an EFI system partition I get:
*** U-Boot Boot Menu ***
UEFI BOOT0003 : test3 UEFI BOOT00A0 : hello UEFI BOOT0000 : virtio0:1 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : nvme0 distro_boot : dhcp UEFI Boot Manager Maintenance U-Boot console
The entries usb0, scsi0, nvme0 should not exist as there is no such device.
=> lsblk Block Driver Devices ----------------------------- efi_blk : <none> nvme-blk : <none> scsi_blk : <none> usb_storage_blk : <none> virtio-blk : virtio 0
When I select "U-Boot console" the prompt is written in the middle of the screen. I would expect the screen to be cleared.
If I disable networking, I get
Missing RNG device for EFI_RNG_PROTOCOL No EFI system partition Unable to find TPMv2 device =>
though the disk has an EFI partition.
So this series needs some rework.
Best regards
Heinrich
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
Hi Heinrich,
On Sat, 16 Apr 2022 at 15:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 3/24/22 14:54, Masahisa Kojima wrote:
This patch series adds the menu-driven boot device selection, by extending the existing "bootmenu" to include UEFI and distro_boot related entries, and supports menu-driven UEFI boot variable maintenance.
This patch series also includes the removable media support that UEFI specification requires to support.
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 UEFI BOOT0002 : mmc0:1 UEFI BOOT0003 : mmc0:2 UEFI BOOT0004 : nvme0:1 UEFI BOOT0005 : nvme0:2 UEFI BOOT0006 : usb0:2 UEFI BOOT0007 : usb1:1 UEFI BOOT0008 : usb1:2 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : dhcpPress UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
[Major changes from RFC v3]
- add Kconfig option to disable U-Boot console
- add UEFI boot variable maintenance feature
- support removable media support and user selection
- app bootmenu enhancement documentation
[How to run on QEMU(arm64)]
- clone source code
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git \ -b kojima/bootmenu_v4_upstream_0324 --depth 1
- prepare U-Boot .config
$ make qemu_arm64_menuconfig then, enable CONFIG_CMD_BOOTMENU and CONFIG_AUTOBOOT_MENU_SHOW
- run on QEMU(arm64) example
$ qemu-system-aarch64 -machine virt,gic-version=3 -cpu cortex-a57 -m 4G -nographic \ -no-acpi -bios ./u-boot.bin -hda xxx.img
I applied the series to origin/master and added the two config options.
This is the output if the disk image does not have an EFI partition:
Found 0 disks No EFI system partition Unable to find TPMv2 device
Even if there is no EFI system partition on virtio 0 I would expect the menu to be shown.
This issue is fixed in the next version.
When having an image with an EFI system partition I get:
*** U-Boot Boot Menu ***
UEFI BOOT0003 : test3 UEFI BOOT00A0 : hello UEFI BOOT0000 : virtio0:1 distro_boot : usb0 distro_boot : scsi0 distro_boot : virtio0 distro_boot : nvme0 distro_boot : dhcp UEFI Boot Manager Maintenance U-Boot consoleThe entries usb0, scsi0, nvme0 should not exist as there is no such device.
Non-existent devices are filtered out in the next version.
=> lsblk Block Driver Devices
efi_blk : <none> nvme-blk : <none> scsi_blk : <none> usb_storage_blk : <none> virtio-blk : virtio 0
When I select "U-Boot console" the prompt is written in the middle of the screen. I would expect the screen to be cleared.
If I disable networking, I get
Missing RNG device for EFI_RNG_PROTOCOL No EFI system partition Unable to find TPMv2 device =>
though the disk has an EFI partition.
I fixed the issue that no menu appears if networking is disabled.
But the issue of no EFI system partition found if networking is disabled seems a fundamental issue of U-Boot, not specific to this series. If networking is disabled, "lsblk" also can not find any block device.
o networking disabled => lsblk Block Driver Devices ----------------------------- efi_blk : <none> nvme-blk : <none> scsi_blk : <none> usb_storage_blk : <none> virtio-blk : <none>
o networking enabled => lsblk Block Driver Devices ----------------------------- efi_blk : <none> nvme-blk : <none> scsi_blk : <none> usb_storage_blk : <none> virtio-blk : virtio 0
Thanks, Masahisa Kojima
So this series needs some rework.
Best regards
Heinrich
AKASHI Takahiro (2): efi_loader: export efi_locate_device_handle() efi_loader: bootmgr: add booting from removable media
Masahisa Kojima (9): bootmenu: fix menu API error handling lib/charset: add u16_strlcat() function test: unit test for u16_strlcat() menu: always show the menu regardless of the number or entry bootmenu: add UEFI and disto_boot entries bootmenu: factor out the user input handling efi_loader: add menu-driven UEFI Boot Variable maintenance bootmenu: add removable media entries doc:bootmenu: add UEFI boot variable and distro boot support
cmd/Kconfig | 10 + cmd/bootmenu.c | 678 +++++++---- common/menu.c | 139 ++- doc/usage/bootmenu.rst | 65 ++ include/charset.h | 15 + include/config_distro_bootcmd.h | 14 +- include/efi_default_filename.h | 26 + include/efi_loader.h | 63 ++ include/menu.h | 20 + lib/charset.c | 21 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_bootmenu_maintenance.c | 1244 +++++++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 50 +- lib/efi_loader/efi_boottime.c | 59 +- lib/efi_loader/efi_console.c | 81 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- test/unicode_ut.c | 45 + 18 files changed, 2357 insertions(+), 260 deletions(-) create mode 100644 include/efi_default_filename.h create mode 100644 lib/efi_loader/efi_bootmenu_maintenance.c
participants (4)
-
Heinrich Schuchardt -
Ilias Apalodimas -
Masahisa Kojima -
Takahiro Akashi