[U-Boot] [PATCH v3 0/8] cmd: add efishell for efi environment
# This patch set contains only efishell-specific part of my original patch[1]. See also the other patch[2].
This patch set is a collection of patches to enhance efi user interfaces /commands. It will help improve user experience on efi boot and make it more usable *without* edk2's shell utility.
Let's see how it works: => efishell boot add 1 SHELL mmc 0:1 /Shell.efi "" => efishell boot add 2 HELLO mmc 0:1 /hello.efi "" => efishell boot dump Boot0001: attributes: A-- (0x00000001) label: SHELL file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\Shell.efi data: Boot0002: attributes: A-- (0x00000001) label: HELLO file_path: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(1,MBR,0x086246ba,0x800,0x40000)/\hello.efi data:
=> efishell boot order 1 2 => efishell boot order 1: Boot0001: SHELL 2: Boot0002: HELLO
=> run -e Boot0002 (or bootefi bootmgr - 2) ; '-' means no dtb specified WARNING: booting without device tree Booting: HELLO ## Starting EFI application at 000000007db8b040 ... Hello, world! ## Application terminated, r = 0
=> env set -e PlatformLang en ; important! (or you can do "efishell setvar PlatformLang en") => env print -e Boot0001: {boot,run}(blob) 00000000: 01 00 00 00 68 00 53 00 ....h.S. 00000010: 48 00 45 00 4c 00 4c 00 H.E.L.L. 00000020: 00 00 01 04 14 00 b9 73 .......s 00000030: 1d e6 84 a3 cc 4a ae ab .....J.. 00000040: 82 e8 28 f3 62 8b 03 1a ..(.b... 00000050: 05 00 00 03 1a 05 00 00 ........ 00000060: 04 01 2a 00 01 00 00 00 ..*..... 00000070: 00 08 00 00 00 00 00 00 ........ 00000080: 00 00 04 00 00 00 00 00 ........ 00000090: ba 46 62 08 00 00 00 00 .Fb..... 000000a0: 00 00 00 00 00 00 00 00 ........ 000000b0: 01 01 04 04 1c 00 5c 00 ....... 000000c0: 5c 00 53 00 68 00 65 00 .S.h.e. 000000d0: 6c 00 6c 00 2e 00 65 00 l.l...e. 000000e0: 66 00 69 00 00 00 7f ff f.i.... 000000f0: 04 00 00 ... Boot0002: {boot,run}(blob) 00000000: 01 00 00 00 68 00 48 00 ....h.H. 00000010: 45 00 4c 00 4c 00 4f 00 E.L.L.O. 00000020: 00 00 01 04 14 00 b9 73 .......s 00000030: 1d e6 84 a3 cc 4a ae ab .....J.. 00000040: 82 e8 28 f3 62 8b 03 1a ..(.b... 00000050: 05 00 00 03 1a 05 00 00 ........ 00000060: 04 01 2a 00 01 00 00 00 ..*..... 00000070: 00 08 00 00 00 00 00 00 ........ 00000080: 00 00 04 00 00 00 00 00 ........ 00000090: ba 46 62 08 00 00 00 00 .Fb..... 000000a0: 00 00 00 00 00 00 00 00 ........ 000000b0: 01 01 04 04 1c 00 5c 00 ....... 000000c0: 5c 00 68 00 65 00 6c 00 .h.e.l. 000000d0: 6c 00 6f 00 2e 00 65 00 l.o...e. 000000e0: 66 00 69 00 00 00 7f ff f.i.... 000000f0: 04 00 00 ... BootOrder: {boot,run}(blob) 00000000: 01 00 02 00 .... OsIndicationsSupported: {ro,boot}(blob) 00000000: 00 00 00 00 00 00 00 00 ........ PlatformLang: {boot,run}(blob) 00000000: 65 6e en
=> run -e Boot0001 or bootefi bootmgr
(shell ...)
"setvar" command now supports efi shell-like syntax:
=> env set -e foo =S"akashi" =0x012345 =Habcdef => env print -e foo foo: {boot,run}(blob) 00000000: 61 6b 61 73 68 69 45 23 akashiE# 00000010: 01 00 ab cd ef .....
Other useful sub commands are: => efishell devices ; print uefi devices => efishell drivers ; print uefi drivers => efishell images ; print loaded images => efishell dh ; print uefi handles => efishell memmap ; dump uefi memory map
# I admit there is some room to improve the output from those commands.
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html [2] https://lists.denx.de/pipermail/u-boot/2018-December/352403.html
Enjoy! -Takahiro Akashi
Changes in v3 (Dec 18, 2018) * split v2 into two separate patch series * add CONFIG_CMD_EFISHELL to enable/disable efishell command * add missing free() at several places in efishell command
Changes in v2 (Nov 5, 2018) * modify efi_dp_from_name() for use in efishell * rename efi_marshal_load_option() to efi_serialize_load_option(), taking a "struct efi_load_option" as an argument * improve a format in dumping uefi variables * enhance a setvar syntax as efi's shell does * change a syntax from "bootefi bootmgr -2" to "bootefi bootmgr - 2" * add -e option to run command * add -e option to env command * add more sub-commands
AKASHI Takahiro (8): cmd: add efishell command cmd: efishell: add devices command cmd: efishell: add drivers command cmd: efishell: add images command cmd: efishell: add memmap command cmd: efishell: add dh command cmd: efishell: export uefi variable helper functions cmd: env: add "-e" option for handling UEFI variables
cmd/Kconfig | 10 + cmd/Makefile | 1 + cmd/efishell.c | 1000 +++++++++++++++++++++++++++++++++++++++++++++ cmd/nvedit.c | 61 ++- include/command.h | 4 + 5 files changed, 1074 insertions(+), 2 deletions(-) create mode 100644 cmd/efishell.c
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces: * efishell boot add: add BootXXXX variable * efishell boot rm: remove BootXXXX variable * efishell boot dump: display all BootXXXX variables * efishell boot order: set/display a boot order (BootOrder) * efishell setvar: set an UEFI variable (with limited functionality) * efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/Kconfig | 10 + cmd/Makefile | 1 + cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 684 insertions(+) create mode 100644 cmd/efishell.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index f2f3b5e2b76b..a8a4bf7db45e 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1390,6 +1390,16 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EFISHELL + bool "Enable the 'efishell' command for EFI environment" + depends on EFI_LOADER + default n + help + Enable the 'efishell' command which provides a subset of UEFI + shell utility with simplified functionality. It will be useful + particularly for managing boot parameters as well as examining + various EFI status for debugging. + config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 5ec2f9e8ebfd..0258d8a373b1 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -49,6 +49,7 @@ obj-$(CONFIG_CMD_ECHO) += echo.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o +obj-$(CONFIG_CMD_EFISHELL) += efishell.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_HUSH_PARSER) += exit.o obj-$(CONFIG_CMD_EXT4) += ext4.o diff --git a/cmd/efishell.c b/cmd/efishell.c new file mode 100644 index 000000000000..5819e52cf575 --- /dev/null +++ b/cmd/efishell.c @@ -0,0 +1,673 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * EFI Shell-like command + * + * Copyright (c) 2018 AKASHI Takahiro, Linaro Limited + */ + +#include <charset.h> +#include <common.h> +#include <command.h> +#include <efi_loader.h> +#include <environment.h> +#include <errno.h> +#include <exports.h> +#include <hexdump.h> +#include <malloc.h> +#include <search.h> +#include <linux/ctype.h> +#include <asm/global_data.h> + +static void dump_var_data(char *data, unsigned long len) +{ + char *start, *end, *p; + unsigned long pos, count; + char hex[3], line[9]; + int i; + + end = data + len; + for (start = data, pos = 0; start < end; start += count, pos += count) { + count = end - start; + if (count > 16) + count = 16; + + /* count should be multiple of two */ + printf("%08lx: ", pos); + + /* in hex format */ + p = start; + for (i = 0; i < count / 2; p += 2, i++) + printf(" %c%c", *p, *(p + 1)); + for (; i < 8; i++) + printf(" "); + + /* in character format */ + p = start; + hex[2] = '\0'; + for (i = 0; i < count / 2; i++) { + hex[0] = *p++; + hex[1] = *p++; + line[i] = simple_strtoul(hex, 0, 16); + if (line[i] < 0x20 || line[i] > 0x7f) + line[i] = '.'; + } + line[i] = '\0'; + printf(" %s\n", line); + } +} + +/* + * From efi_variable.c, + * + * Mapping between EFI variables and u-boot variables: + * + * efi_$guid_$varname = {attributes}(type)value + */ +static int do_efi_dump_var(int argc, char * const argv[]) +{ + char regex[256]; + char * const regexlist[] = {regex}; + char *res = NULL, *start, *end; + int len; + + if (argc > 2) + return CMD_RET_USAGE; + + if (argc == 2) + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]); + else + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*"); + debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex); + + len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY, + &res, 0, 1, regexlist); + + if (len < 0) + return CMD_RET_FAILURE; + + if (len > 0) { + end = res; + while (true) { + /* variable name */ + start = strchr(end, '_'); + if (!start) + break; + start = strchr(++start, '_'); + if (!start) + break; + end = strchr(++start, '='); + if (!end) + break; + printf("%.*s:", (int)(end - start), start); + end++; + + /* value */ + start = end; + end = strstr(start, "(blob)"); + if (!end) { + putc('\n'); + break; + } + end += 6; + printf(" %.*s\n", (int)(end - start), start); + + start = end; + end = strchr(start, '\n'); + if (!end) + break; + dump_var_data(start, end - start); + } + free(res); + + if (len < 2 && argc == 2) + printf("%s: not found\n", argv[1]); + } + + return CMD_RET_SUCCESS; +} + +static int append_value(char **bufp, unsigned long *sizep, char *data) +{ + char *tmp_buf = NULL, *new_buf = NULL, *value; + unsigned long len = 0; + + if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */ + union { + u8 u8; + u16 u16; + u32 u32; + u64 u64; + } tmp_data; + unsigned long hex_value; + void *hex_ptr; + + data += 3; + len = strlen(data); + if ((len & 0x1)) /* not multiple of two */ + return -1; + + len /= 2; + if (len > 8) + return -1; + else if (len > 4) + len = 8; + else if (len > 2) + len = 4; + + /* convert hex hexadecimal number */ + if (strict_strtoul(data, 16, &hex_value) < 0) + return -1; + + tmp_buf = malloc(len); + if (!tmp_buf) + return -1; + + if (len == 1) { + tmp_data.u8 = hex_value; + hex_ptr = &tmp_data.u8; + } else if (len == 2) { + tmp_data.u16 = hex_value; + hex_ptr = &tmp_data.u16; + } else if (len == 4) { + tmp_data.u32 = hex_value; + hex_ptr = &tmp_data.u32; + } else { + tmp_data.u64 = hex_value; + hex_ptr = &tmp_data.u64; + } + memcpy(tmp_buf, hex_ptr, len); + value = tmp_buf; + + } else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */ + data += 2; + len = strlen(data); + if (len & 0x1) /* not multiple of two */ + return -1; + + len /= 2; + tmp_buf = malloc(len); + if (!tmp_buf) + return -1; + + if (hex2bin((u8 *)tmp_buf, data, len) < 0) + return -1; + + value = tmp_buf; + } else { /* string */ + if (!strncmp(data, "="", 2) || !strncmp(data, "=S"", 3)) { + if (data[1] == '"') + data += 2; + else + data += 3; + value = data; + len = strlen(data) - 1; + if (data[len] != '"') + return -1; + } else { + value = data; + len = strlen(data); + } + } + + new_buf = realloc(*bufp, *sizep + len); + if (!new_buf) + goto out; + + memcpy(new_buf + *sizep, value, len); + *bufp = new_buf; + *sizep += len; + +out: + free(tmp_buf); + + return 0; +} + +static int do_efi_set_var(int argc, char * const argv[]) +{ + char *var_name, *value = NULL; + unsigned long size = 0; + u16 *var_name16, *p; + efi_guid_t guid; + efi_status_t ret; + + if (argc == 1) + return CMD_RET_SUCCESS; + + var_name = argv[1]; + if (argc == 2) { + /* remove */ + value = NULL; + size = 0; + } else { /* set */ + argc -= 2; + argv += 2; + + for ( ; argc > 0; argc--, argv++) + if (append_value(&value, &size, argv[0]) < 0) { + ret = CMD_RET_FAILURE; + goto out; + } + } + + var_name16 = malloc((strlen(var_name) + 1) * 2); + p = var_name16; + utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1); + + guid = efi_global_variable_guid; + ret = efi_set_variable(var_name16, &guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, size, value); + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE); +out: + return ret; +} + +static int do_efi_boot_add(int argc, char * const argv[]) +{ + int id; + char *endp; + char var_name[9]; + u16 var_name16[9], *p; + efi_guid_t guid; + size_t label_len, label_len16; + u16 *label; + struct efi_device_path *device_path = NULL, *file_path = NULL; + struct efi_load_option lo; + void *data = NULL; + unsigned long size; + int ret; + + if (argc < 6 || argc > 7) + return CMD_RET_USAGE; + + id = (int)simple_strtoul(argv[1], &endp, 0); + if (*endp != '\0' || id > 0xffff) + return CMD_RET_FAILURE; + + sprintf(var_name, "Boot%04X", id); + p = var_name16; + utf8_utf16_strncpy(&p, var_name, 9); + + guid = efi_global_variable_guid; + + /* attributes */ + lo.attributes = 0x1; /* always ACTIVE */ + + /* label */ + label_len = strlen(argv[2]); + label_len16 = utf8_utf16_strnlen(argv[2], label_len); + label = malloc((label_len16 + 1) * sizeof(u16)); + if (!label) + return CMD_RET_FAILURE; + lo.label = label; /* label will be changed below */ + utf8_utf16_strncpy(&label, argv[2], label_len); + + /* file path */ + ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path, + &file_path); + if (ret != EFI_SUCCESS) { + ret = CMD_RET_FAILURE; + goto out; + } + lo.file_path = file_path; + lo.file_path_length = efi_dp_size(file_path) + + sizeof(struct efi_device_path); /* for END */ + + /* optional data */ + lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]); + + size = efi_serialize_load_option(&lo, (u8 **)&data); + if (!size) { + ret = CMD_RET_FAILURE; + goto out; + } + + ret = efi_set_variable(var_name16, &guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, size, data); + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE); +out: + free(data); + efi_free_pool(device_path); + efi_free_pool(file_path); + free(lo.label); + + return ret; +} + +static int do_efi_boot_rm(int argc, char * const argv[]) +{ + efi_guid_t guid; + int id, i; + char *endp; + char var_name[9]; + u16 var_name16[9]; + efi_status_t ret; + + if (argc == 1) + return CMD_RET_USAGE; + + guid = efi_global_variable_guid; + for (i = 1; i < argc; i++, argv++) { + id = (int)simple_strtoul(argv[1], &endp, 0); + if (*endp != '\0' || id > 0xffff) + return CMD_RET_FAILURE; + + sprintf(var_name, "Boot%04X", id); + utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9); + + ret = efi_set_variable(var_name16, &guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL); + if (ret) { + printf("cannot remove Boot%04X", id); + return CMD_RET_FAILURE; + } + } + + return CMD_RET_SUCCESS; +} + +static void show_efi_boot_opt_data(int id, void *data) +{ + struct efi_load_option lo; + char *label, *p; + size_t label_len16, label_len; + u16 *dp_str; + + efi_deserialize_load_option(&lo, data); + + label_len16 = u16_strlen(lo.label); + label_len = utf16_utf8_strnlen(lo.label, label_len16); + label = malloc(label_len + 1); + if (!label) + return; + p = label; + utf16_utf8_strncpy(&p, lo.label, label_len16); + + printf("Boot%04X:\n", id); + printf("\tattributes: %c%c%c (0x%08x)\n", + /* ACTIVE */ + lo.attributes & 0x1 ? 'A' : '-', + /* FORCE RECONNECT */ + lo.attributes & 0x2 ? 'R' : '-', + /* HIDDEN */ + lo.attributes & 0x8 ? 'H' : '-', + lo.attributes); + printf("\tlabel: %s\n", label); + + dp_str = efi_dp_str(lo.file_path); + printf("\tfile_path: %ls\n", dp_str); + efi_free_pool(dp_str); + + printf("\tdata: %s\n", lo.optional_data); + + free(label); +} + +static void show_efi_boot_opt(int id) +{ + char var_name[9]; + u16 var_name16[9], *p; + efi_guid_t guid; + void *data = NULL; + unsigned long size; + int ret; + + sprintf(var_name, "Boot%04X", id); + p = var_name16; + utf8_utf16_strncpy(&p, var_name, 9); + guid = efi_global_variable_guid; + + size = 0; + ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL); + if (ret == (int)EFI_BUFFER_TOO_SMALL) { + data = malloc(size); + ret = efi_get_variable(var_name16, &guid, NULL, &size, data); + } + if (ret == EFI_SUCCESS) + show_efi_boot_opt_data(id, data); + else if (ret == EFI_NOT_FOUND) + printf("Boot%04X: not found\n", id); + + free(data); +} + +static int do_efi_boot_dump(int argc, char * const argv[]) +{ + char regex[256]; + char * const regexlist[] = {regex}; + char *variables = NULL, *boot, *value; + int len; + int id; + + if (argc > 1) + return CMD_RET_USAGE; + + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+"); + + len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY, + &variables, 0, 1, regexlist); + + if (!len) + return CMD_RET_SUCCESS; + + if (len < 0) + return CMD_RET_FAILURE; + + boot = variables; + while (*boot) { + value = strstr(boot, "Boot") + 4; + id = (int)simple_strtoul(value, NULL, 16); + show_efi_boot_opt(id); + boot = strchr(boot, '\n'); + if (!*boot) + break; + boot++; + } + free(variables); + + return CMD_RET_SUCCESS; +} + +static int show_efi_boot_order(void) +{ + efi_guid_t guid; + u16 *bootorder = NULL; + unsigned long size; + int num, i; + char var_name[9]; + u16 var_name16[9], *p16; + void *data; + struct efi_load_option lo; + char *label, *p; + size_t label_len16, label_len; + efi_status_t ret; + + guid = efi_global_variable_guid; + size = 0; + ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) { + bootorder = malloc(size); + ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, + bootorder); + } + if (ret == EFI_NOT_FOUND) { + printf("BootOrder not defined\n"); + ret = CMD_RET_SUCCESS; + goto out; + } else if (ret != EFI_SUCCESS) { + ret = CMD_RET_FAILURE; + goto out; + } + + num = size / sizeof(u16); + for (i = 0; i < num; i++) { + sprintf(var_name, "Boot%04X", bootorder[i]); + p16 = var_name16; + utf8_utf16_strncpy(&p16, var_name, 9); + + size = 0; + ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL); + if (ret != EFI_BUFFER_TOO_SMALL) { + printf("%2d: Boot%04X: (not defined)\n", + i + 1, bootorder[i]); + continue; + } + + data = malloc(size); + if (!data) { + ret = CMD_RET_FAILURE; + goto out; + } + ret = efi_get_variable(var_name16, &guid, NULL, &size, data); + if (ret != EFI_SUCCESS) { + free(data); + ret = CMD_RET_FAILURE; + goto out; + } + + efi_deserialize_load_option(&lo, data); + + label_len16 = u16_strlen(lo.label); + label_len = utf16_utf8_strnlen(lo.label, label_len16); + label = malloc(label_len + 1); + if (!label) { + free(data); + ret = CMD_RET_FAILURE; + goto out; + } + p = label; + utf16_utf8_strncpy(&p, lo.label, label_len16); + printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label); + free(label); + + free(data); + } +out: + free(bootorder); + + return ret; +} + +static int do_efi_boot_order(int argc, char * const argv[]) +{ + u16 *bootorder = NULL; + unsigned long size; + int id, i; + char *endp; + efi_guid_t guid; + efi_status_t ret; + + if (argc == 1) + return show_efi_boot_order(); + + argc--; + argv++; + + size = argc * sizeof(u16); + bootorder = malloc(size); + if (!bootorder) + return CMD_RET_FAILURE; + + for (i = 0; i < argc; i++) { + id = (int)simple_strtoul(argv[i], &endp, 0); + if (*endp != '\0' || id > 0xffff) { + printf("invalid value: %s\n", argv[i]); + ret = CMD_RET_FAILURE; + goto out; + } + + bootorder[i] = (u16)id; + } + + guid = efi_global_variable_guid; + ret = efi_set_variable(L"BootOrder", &guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder); + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE); +out: + free(bootorder); + + return ret; +} + +static int do_efi_boot_opt(int argc, char * const argv[]) +{ + char *sub_command; + + if (argc < 2) + return CMD_RET_USAGE; + + argc--; argv++; + sub_command = argv[0]; + + if (!strcmp(sub_command, "add")) + return do_efi_boot_add(argc, argv); + else if (!strcmp(sub_command, "rm")) + return do_efi_boot_rm(argc, argv); + else if (!strcmp(sub_command, "dump")) + return do_efi_boot_dump(argc, argv); + else if (!strcmp(sub_command, "order")) + return do_efi_boot_order(argc, argv); + else + return CMD_RET_USAGE; +} + +/* Interpreter command to configure EFI environment */ +static int do_efishell(cmd_tbl_t *cmdtp, int flag, + int argc, char * const argv[]) +{ + char *command; + efi_status_t r; + + if (argc < 2) + return CMD_RET_USAGE; + + argc--; argv++; + command = argv[0]; + + /* Initialize EFI drivers */ + r = efi_init_obj_list(); + if (r != EFI_SUCCESS) { + printf("Error: Cannot set up EFI drivers, r = %lu\n", + r & ~EFI_ERROR_MASK); + return CMD_RET_FAILURE; + } + + if (!strcmp(command, "boot")) + return do_efi_boot_opt(argc, argv); + else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore")) + return do_efi_dump_var(argc, argv); + else if (!strcmp(command, "setvar")) + return do_efi_set_var(argc, argv); + else + return CMD_RET_USAGE; +} + +#ifdef CONFIG_SYS_LONGHELP +static char efishell_help_text[] = + " - EFI Shell-like interface to configure EFI environment\n" + "\n" + "efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n" + " - set uefi's BOOT variable\n" + "efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" + " - set/display uefi boot order\n" + "efishell boot dump\n" + " - display all uefi's BOOT variables\n" + "efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n" + " - set/display uefi boot order\n" + "\n" + "efishell dumpvar [<name>]\n" + " - get uefi variable's value\n" + "efishell setvar <name> [<value>]\n" + " - set/delete uefi variable's value\n" + " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n"; +#endif + +U_BOOT_CMD( + efishell, 10, 0, do_efishell, + "Configure EFI environment", + efishell_help_text +);
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces:
- efishell boot add: add BootXXXX variable
- efishell boot rm: remove BootXXXX variable
- efishell boot dump: display all BootXXXX variables
- efishell boot order: set/display a boot order (BootOrder)
- efishell setvar: set an UEFI variable (with limited functionality)
- efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/Kconfig | 10 + cmd/Makefile | 1 + cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 684 insertions(+) create mode 100644 cmd/efishell.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index f2f3b5e2b76b..a8a4bf7db45e 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1390,6 +1390,16 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EFISHELL
- bool "Enable the 'efishell' command for EFI environment"
- depends on EFI_LOADER
- default n
- help
Enable the 'efishell' command which provides a subset of UEFIshell utility with simplified functionality. It will be usefulparticularly for managing boot parameters as well as examiningvarious EFI status for debugging.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 5ec2f9e8ebfd..0258d8a373b1 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -49,6 +49,7 @@ obj-$(CONFIG_CMD_ECHO) += echo.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o +obj-$(CONFIG_CMD_EFISHELL) += efishell.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_HUSH_PARSER) += exit.o obj-$(CONFIG_CMD_EXT4) += ext4.o diff --git a/cmd/efishell.c b/cmd/efishell.c new file mode 100644 index 000000000000..5819e52cf575 --- /dev/null +++ b/cmd/efishell.c @@ -0,0 +1,673 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- EFI Shell-like command
- Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
- */
+#include <charset.h> +#include <common.h> +#include <command.h> +#include <efi_loader.h> +#include <environment.h> +#include <errno.h> +#include <exports.h> +#include <hexdump.h> +#include <malloc.h> +#include <search.h> +#include <linux/ctype.h> +#include <asm/global_data.h>
+static void dump_var_data(char *data, unsigned long len) +{
- char *start, *end, *p;
- unsigned long pos, count;
- char hex[3], line[9];
- int i;
- end = data + len;
- for (start = data, pos = 0; start < end; start += count, pos += count) {
count = end - start;if (count > 16)count = 16;/* count should be multiple of two */printf("%08lx: ", pos);/* in hex format */p = start;for (i = 0; i < count / 2; p += 2, i++)printf(" %c%c", *p, *(p + 1));for (; i < 8; i++)printf(" ");/* in character format */p = start;hex[2] = '\0';for (i = 0; i < count / 2; i++) {hex[0] = *p++;hex[1] = *p++;line[i] = simple_strtoul(hex, 0, 16);if (line[i] < 0x20 || line[i] > 0x7f)line[i] = '.';}line[i] = '\0';printf(" %s\n", line);- }
+}
+/*
- From efi_variable.c,
- Mapping between EFI variables and u-boot variables:
- efi_$guid_$varname = {attributes}(type)value
- */
+static int do_efi_dump_var(int argc, char * const argv[]) +{
- char regex[256];
- char * const regexlist[] = {regex};
- char *res = NULL, *start, *end;
- int len;
- if (argc > 2)
return CMD_RET_USAGE;- if (argc == 2)
snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);- else
snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");- debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
- len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
&res, 0, 1, regexlist);- if (len < 0)
return CMD_RET_FAILURE;- if (len > 0) {
end = res;while (true) {/* variable name */start = strchr(end, '_');if (!start)break;start = strchr(++start, '_');if (!start)break;end = strchr(++start, '=');if (!end)break;printf("%.*s:", (int)(end - start), start);end++;/* value */start = end;end = strstr(start, "(blob)");if (!end) {putc('\n');break;}end += 6;printf(" %.*s\n", (int)(end - start), start);start = end;end = strchr(start, '\n');if (!end)break;dump_var_data(start, end - start);}free(res);if (len < 2 && argc == 2)printf("%s: not found\n", argv[1]);- }
- return CMD_RET_SUCCESS;
+}
+static int append_value(char **bufp, unsigned long *sizep, char *data) +{
- char *tmp_buf = NULL, *new_buf = NULL, *value;
- unsigned long len = 0;
- if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
union {u8 u8;u16 u16;u32 u32;u64 u64;} tmp_data;unsigned long hex_value;void *hex_ptr;data += 3;len = strlen(data);if ((len & 0x1)) /* not multiple of two */return -1;len /= 2;if (len > 8)return -1;else if (len > 4)len = 8;else if (len > 2)len = 4;/* convert hex hexadecimal number */if (strict_strtoul(data, 16, &hex_value) < 0)return -1;tmp_buf = malloc(len);if (!tmp_buf)return -1;if (len == 1) {tmp_data.u8 = hex_value;hex_ptr = &tmp_data.u8;} else if (len == 2) {tmp_data.u16 = hex_value;hex_ptr = &tmp_data.u16;} else if (len == 4) {tmp_data.u32 = hex_value;hex_ptr = &tmp_data.u32;} else {tmp_data.u64 = hex_value;hex_ptr = &tmp_data.u64;}memcpy(tmp_buf, hex_ptr, len);value = tmp_buf;- } else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
data += 2;len = strlen(data);if (len & 0x1) /* not multiple of two */return -1;len /= 2;tmp_buf = malloc(len);if (!tmp_buf)return -1;if (hex2bin((u8 *)tmp_buf, data, len) < 0)return -1;value = tmp_buf;- } else { /* string */
if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {if (data[1] == '"')data += 2;elsedata += 3;value = data;len = strlen(data) - 1;if (data[len] != '"')return -1;} else {value = data;len = strlen(data);}- }
- new_buf = realloc(*bufp, *sizep + len);
- if (!new_buf)
goto out;- memcpy(new_buf + *sizep, value, len);
- *bufp = new_buf;
- *sizep += len;
+out:
- free(tmp_buf);
- return 0;
+}
+static int do_efi_set_var(int argc, char * const argv[]) +{
- char *var_name, *value = NULL;
- unsigned long size = 0;
- u16 *var_name16, *p;
- efi_guid_t guid;
- efi_status_t ret;
- if (argc == 1)
return CMD_RET_SUCCESS;- var_name = argv[1];
- if (argc == 2) {
/* remove */value = NULL;size = 0;- } else { /* set */
argc -= 2;argv += 2;for ( ; argc > 0; argc--, argv++)if (append_value(&value, &size, argv[0]) < 0) {ret = CMD_RET_FAILURE;goto out;}- }
- var_name16 = malloc((strlen(var_name) + 1) * 2);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
- guid = efi_global_variable_guid;
- ret = efi_set_variable(var_name16, &guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, size, value);- ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
- return ret;
+}
+static int do_efi_boot_add(int argc, char * const argv[]) +{
- int id;
- char *endp;
- char var_name[9];
- u16 var_name16[9], *p;
- efi_guid_t guid;
- size_t label_len, label_len16;
- u16 *label;
- struct efi_device_path *device_path = NULL, *file_path = NULL;
- struct efi_load_option lo;
- void *data = NULL;
- unsigned long size;
- int ret;
- if (argc < 6 || argc > 7)
return CMD_RET_USAGE;- id = (int)simple_strtoul(argv[1], &endp, 0);
- if (*endp != '\0' || id > 0xffff)
return CMD_RET_FAILURE;- sprintf(var_name, "Boot%04X", id);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, 9);
- guid = efi_global_variable_guid;
- /* attributes */
- lo.attributes = 0x1; /* always ACTIVE */
- /* label */
- label_len = strlen(argv[2]);
- label_len16 = utf8_utf16_strnlen(argv[2], label_len);
- label = malloc((label_len16 + 1) * sizeof(u16));
- if (!label)
return CMD_RET_FAILURE;- lo.label = label; /* label will be changed below */
- utf8_utf16_strncpy(&label, argv[2], label_len);
- /* file path */
- ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
&file_path);- if (ret != EFI_SUCCESS) {
ret = CMD_RET_FAILURE;goto out;- }
- lo.file_path = file_path;
- lo.file_path_length = efi_dp_size(file_path)
+ sizeof(struct efi_device_path); /* for END */- /* optional data */
- lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
- size = efi_serialize_load_option(&lo, (u8 **)&data);
The load options should be converted to a UTF-16 string before calling efi_serialize_load_option so that we can copy them to the loaded image protocol.
- if (!size) {
ret = CMD_RET_FAILURE;goto out;- }
- ret = efi_set_variable(var_name16, &guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, size, data);- ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
- free(data);
- efi_free_pool(device_path);
- efi_free_pool(file_path);
- free(lo.label);
- return ret;
+}
+static int do_efi_boot_rm(int argc, char * const argv[]) +{
- efi_guid_t guid;
- int id, i;
- char *endp;
- char var_name[9];
- u16 var_name16[9];
- efi_status_t ret;
- if (argc == 1)
return CMD_RET_USAGE;- guid = efi_global_variable_guid;
- for (i = 1; i < argc; i++, argv++) {
id = (int)simple_strtoul(argv[1], &endp, 0);if (*endp != '\0' || id > 0xffff)return CMD_RET_FAILURE;sprintf(var_name, "Boot%04X", id);utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);ret = efi_set_variable(var_name16, &guid,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);if (ret) {printf("cannot remove Boot%04X", id);return CMD_RET_FAILURE;}- }
- return CMD_RET_SUCCESS;
+}
+static void show_efi_boot_opt_data(int id, void *data) +{
- struct efi_load_option lo;
- char *label, *p;
- size_t label_len16, label_len;
- u16 *dp_str;
- efi_deserialize_load_option(&lo, data);
- label_len16 = u16_strlen(lo.label);
- label_len = utf16_utf8_strnlen(lo.label, label_len16);
- label = malloc(label_len + 1);
- if (!label)
return;- p = label;
- utf16_utf8_strncpy(&p, lo.label, label_len16);
- printf("Boot%04X:\n", id);
- printf("\tattributes: %c%c%c (0x%08x)\n",
/* ACTIVE */lo.attributes & 0x1 ? 'A' : '-',/* FORCE RECONNECT */lo.attributes & 0x2 ? 'R' : '-',/* HIDDEN */lo.attributes & 0x8 ? 'H' : '-',lo.attributes);- printf("\tlabel: %s\n", label);
- dp_str = efi_dp_str(lo.file_path);
- printf("\tfile_path: %ls\n", dp_str);
- efi_free_pool(dp_str);
- printf("\tdata: %s\n", lo.optional_data);
- free(label);
+}
+static void show_efi_boot_opt(int id) +{
- char var_name[9];
- u16 var_name16[9], *p;
- efi_guid_t guid;
- void *data = NULL;
- unsigned long size;
'size' must be of type efi_uint_t. Otherwise you get warnings like this one:
cmd/efishell.c:729:50: warning: passing argument 4 of ‘efi_get_variable’ from incompatible pointer type [-Wincompatible-pointer-types] ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
- int ret;
- sprintf(var_name, "Boot%04X", id);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, 9);
- guid = efi_global_variable_guid;
- size = 0;
- ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
- if (ret == (int)EFI_BUFFER_TOO_SMALL) {
data = malloc(size);ret = efi_get_variable(var_name16, &guid, NULL, &size, data);- }
- if (ret == EFI_SUCCESS)
show_efi_boot_opt_data(id, data);- else if (ret == EFI_NOT_FOUND)
printf("Boot%04X: not found\n", id);- free(data);
+}
+static int do_efi_boot_dump(int argc, char * const argv[]) +{
- char regex[256];
- char * const regexlist[] = {regex};
- char *variables = NULL, *boot, *value;
- int len;
- int id;
- if (argc > 1)
return CMD_RET_USAGE;- snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
- len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
&variables, 0, 1, regexlist);- if (!len)
return CMD_RET_SUCCESS;- if (len < 0)
return CMD_RET_FAILURE;- boot = variables;
- while (*boot) {
value = strstr(boot, "Boot") + 4;id = (int)simple_strtoul(value, NULL, 16);show_efi_boot_opt(id);boot = strchr(boot, '\n');if (!*boot)break;boot++;- }
- free(variables);
- return CMD_RET_SUCCESS;
+}
+static int show_efi_boot_order(void) +{
- efi_guid_t guid;
- u16 *bootorder = NULL;
- unsigned long size;
- int num, i;
- char var_name[9];
- u16 var_name16[9], *p16;
- void *data;
- struct efi_load_option lo;
- char *label, *p;
- size_t label_len16, label_len;
- efi_status_t ret;
- guid = efi_global_variable_guid;
- size = 0;
- ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
- if (ret == EFI_BUFFER_TOO_SMALL) {
bootorder = malloc(size);ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,bootorder);- }
- if (ret == EFI_NOT_FOUND) {
printf("BootOrder not defined\n");ret = CMD_RET_SUCCESS;goto out;- } else if (ret != EFI_SUCCESS) {
ret = CMD_RET_FAILURE;goto out;- }
- num = size / sizeof(u16);
- for (i = 0; i < num; i++) {
sprintf(var_name, "Boot%04X", bootorder[i]);p16 = var_name16;utf8_utf16_strncpy(&p16, var_name, 9);size = 0;ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);if (ret != EFI_BUFFER_TOO_SMALL) {printf("%2d: Boot%04X: (not defined)\n",i + 1, bootorder[i]);continue;}data = malloc(size);if (!data) {ret = CMD_RET_FAILURE;goto out;}ret = efi_get_variable(var_name16, &guid, NULL, &size, data);if (ret != EFI_SUCCESS) {free(data);ret = CMD_RET_FAILURE;goto out;}efi_deserialize_load_option(&lo, data);label_len16 = u16_strlen(lo.label);label_len = utf16_utf8_strnlen(lo.label, label_len16);label = malloc(label_len + 1);if (!label) {free(data);ret = CMD_RET_FAILURE;goto out;}p = label;utf16_utf8_strncpy(&p, lo.label, label_len16);printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);free(label);free(data);- }
+out:
- free(bootorder);
- return ret;
+}
+static int do_efi_boot_order(int argc, char * const argv[]) +{
- u16 *bootorder = NULL;
- unsigned long size;
- int id, i;
- char *endp;
- efi_guid_t guid;
- efi_status_t ret;
- if (argc == 1)
return show_efi_boot_order();- argc--;
- argv++;
- size = argc * sizeof(u16);
- bootorder = malloc(size);
- if (!bootorder)
return CMD_RET_FAILURE;- for (i = 0; i < argc; i++) {
id = (int)simple_strtoul(argv[i], &endp, 0);if (*endp != '\0' || id > 0xffff) {printf("invalid value: %s\n", argv[i]);ret = CMD_RET_FAILURE;goto out;}bootorder[i] = (u16)id;- }
- guid = efi_global_variable_guid;
- ret = efi_set_variable(L"BootOrder", &guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);- ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
- free(bootorder);
- return ret;
+}
+static int do_efi_boot_opt(int argc, char * const argv[]) +{
- char *sub_command;
- if (argc < 2)
return CMD_RET_USAGE;- argc--; argv++;
- sub_command = argv[0];
- if (!strcmp(sub_command, "add"))
return do_efi_boot_add(argc, argv);- else if (!strcmp(sub_command, "rm"))
return do_efi_boot_rm(argc, argv);- else if (!strcmp(sub_command, "dump"))
return do_efi_boot_dump(argc, argv);- else if (!strcmp(sub_command, "order"))
return do_efi_boot_order(argc, argv);- else
return CMD_RET_USAGE;+}
+/* Interpreter command to configure EFI environment */ +static int do_efishell(cmd_tbl_t *cmdtp, int flag,
int argc, char * const argv[])+{
- char *command;
- efi_status_t r;
- if (argc < 2)
return CMD_RET_USAGE;- argc--; argv++;
- command = argv[0];
- /* Initialize EFI drivers */
- r = efi_init_obj_list();
- if (r != EFI_SUCCESS) {
printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;- }
- if (!strcmp(command, "boot"))
return do_efi_boot_opt(argc, argv);- else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
return do_efi_dump_var(argc, argv);- else if (!strcmp(command, "setvar"))
return do_efi_set_var(argc, argv);- else
return CMD_RET_USAGE;+}
+#ifdef CONFIG_SYS_LONGHELP +static char efishell_help_text[] =
- " - EFI Shell-like interface to configure EFI environment\n"
- "\n"
- "efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
The 'option' parameter is optional. Please, add brackets ([]).
The parameter 'option' is misleading. Please call it 'load_options'.
Please, add a text explaining that 'load_options' is meant to be the command line string passed to the EFI binary.
Best regards
Heinrich
- " - set uefi's BOOT variable\n"
- "efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
- " - set/display uefi boot order\n"
- "efishell boot dump\n"
- " - display all uefi's BOOT variables\n"
- "efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
- " - set/display uefi boot order\n"
- "\n"
- "efishell dumpvar [<name>]\n"
- " - get uefi variable's value\n"
- "efishell setvar <name> [<value>]\n"
- " - set/delete uefi variable's value\n"
- " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n";
+#endif
+U_BOOT_CMD(
- efishell, 10, 0, do_efishell,
- "Configure EFI environment",
- efishell_help_text
+);
On 12/30/18 4:44 PM, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces:
- efishell boot add: add BootXXXX variable
- efishell boot rm: remove BootXXXX variable
- efishell boot dump: display all BootXXXX variables
- efishell boot order: set/display a boot order (BootOrder)
- efishell setvar: set an UEFI variable (with limited functionality)
- efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
The behavior is a bit unexpected:
=> efishell boot order 200 => efishell boot order 1: Boot00C8: (not defined) exit not allowed from main input shell.
I would expect 'efishell boot order' to take a 4 digit hexadecimal number and to do no conversion from decimal to hexadecimal.
I was also surprised to see 'exit not allowed from main input shell.'
Best regards
Heinrich
On Sun, Dec 30, 2018 at 06:10:51PM +0100, Heinrich Schuchardt wrote:
On 12/30/18 4:44 PM, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces:
- efishell boot add: add BootXXXX variable
- efishell boot rm: remove BootXXXX variable
- efishell boot dump: display all BootXXXX variables
- efishell boot order: set/display a boot order (BootOrder)
- efishell setvar: set an UEFI variable (with limited functionality)
- efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
The behavior is a bit unexpected:
=> efishell boot order 200 => efishell boot order 1: Boot00C8: (not defined) exit not allowed from main input shell.
I would expect 'efishell boot order' to take a 4 digit hexadecimal number and to do no conversion from decimal to hexadecimal.
OK, but we should allow a less-than-4-digit number.
I was also surprised to see 'exit not allowed from main input shell.'
I cannot reproduce this problem.
-Takahiro Akashi
Best regards
Heinrich
On 07.01.19 06:08, AKASHI Takahiro wrote:
On Sun, Dec 30, 2018 at 06:10:51PM +0100, Heinrich Schuchardt wrote:
On 12/30/18 4:44 PM, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces:
- efishell boot add: add BootXXXX variable
- efishell boot rm: remove BootXXXX variable
- efishell boot dump: display all BootXXXX variables
- efishell boot order: set/display a boot order (BootOrder)
- efishell setvar: set an UEFI variable (with limited functionality)
- efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
The behavior is a bit unexpected:
=> efishell boot order 200 => efishell boot order 1: Boot00C8: (not defined) exit not allowed from main input shell.
I would expect 'efishell boot order' to take a 4 digit hexadecimal number and to do no conversion from decimal to hexadecimal.
OK, but we should allow a less-than-4-digit number.
Yes, but definitely stick to Hex regardless. Hex is the default number space in U-Boot - and it just happens to fit quite well with the variable definition too :).
Alex
On Sun, Dec 30, 2018 at 04:44:53PM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces:
- efishell boot add: add BootXXXX variable
- efishell boot rm: remove BootXXXX variable
- efishell boot dump: display all BootXXXX variables
- efishell boot order: set/display a boot order (BootOrder)
- efishell setvar: set an UEFI variable (with limited functionality)
- efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/Kconfig | 10 + cmd/Makefile | 1 + cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 684 insertions(+) create mode 100644 cmd/efishell.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index f2f3b5e2b76b..a8a4bf7db45e 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1390,6 +1390,16 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
+config CMD_EFISHELL
- bool "Enable the 'efishell' command for EFI environment"
- depends on EFI_LOADER
- default n
- help
Enable the 'efishell' command which provides a subset of UEFIshell utility with simplified functionality. It will be usefulparticularly for managing boot parameters as well as examiningvarious EFI status for debugging.config CMD_LED bool "led" default y if LED diff --git a/cmd/Makefile b/cmd/Makefile index 5ec2f9e8ebfd..0258d8a373b1 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -49,6 +49,7 @@ obj-$(CONFIG_CMD_ECHO) += echo.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o obj-$(CONFIG_EFI_STUB) += efi.o +obj-$(CONFIG_CMD_EFISHELL) += efishell.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_HUSH_PARSER) += exit.o obj-$(CONFIG_CMD_EXT4) += ext4.o diff --git a/cmd/efishell.c b/cmd/efishell.c new file mode 100644 index 000000000000..5819e52cf575 --- /dev/null +++ b/cmd/efishell.c @@ -0,0 +1,673 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- EFI Shell-like command
- Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
- */
+#include <charset.h> +#include <common.h> +#include <command.h> +#include <efi_loader.h> +#include <environment.h> +#include <errno.h> +#include <exports.h> +#include <hexdump.h> +#include <malloc.h> +#include <search.h> +#include <linux/ctype.h> +#include <asm/global_data.h>
+static void dump_var_data(char *data, unsigned long len) +{
- char *start, *end, *p;
- unsigned long pos, count;
- char hex[3], line[9];
- int i;
- end = data + len;
- for (start = data, pos = 0; start < end; start += count, pos += count) {
count = end - start;if (count > 16)count = 16;/* count should be multiple of two */printf("%08lx: ", pos);/* in hex format */p = start;for (i = 0; i < count / 2; p += 2, i++)printf(" %c%c", *p, *(p + 1));for (; i < 8; i++)printf(" ");/* in character format */p = start;hex[2] = '\0';for (i = 0; i < count / 2; i++) {hex[0] = *p++;hex[1] = *p++;line[i] = simple_strtoul(hex, 0, 16);if (line[i] < 0x20 || line[i] > 0x7f)line[i] = '.';}line[i] = '\0';printf(" %s\n", line);- }
+}
+/*
- From efi_variable.c,
- Mapping between EFI variables and u-boot variables:
- efi_$guid_$varname = {attributes}(type)value
- */
+static int do_efi_dump_var(int argc, char * const argv[]) +{
- char regex[256];
- char * const regexlist[] = {regex};
- char *res = NULL, *start, *end;
- int len;
- if (argc > 2)
return CMD_RET_USAGE;- if (argc == 2)
snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);- else
snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");- debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
- len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
&res, 0, 1, regexlist);- if (len < 0)
return CMD_RET_FAILURE;- if (len > 0) {
end = res;while (true) {/* variable name */start = strchr(end, '_');if (!start)break;start = strchr(++start, '_');if (!start)break;end = strchr(++start, '=');if (!end)break;printf("%.*s:", (int)(end - start), start);end++;/* value */start = end;end = strstr(start, "(blob)");if (!end) {putc('\n');break;}end += 6;printf(" %.*s\n", (int)(end - start), start);start = end;end = strchr(start, '\n');if (!end)break;dump_var_data(start, end - start);}free(res);if (len < 2 && argc == 2)printf("%s: not found\n", argv[1]);- }
- return CMD_RET_SUCCESS;
+}
+static int append_value(char **bufp, unsigned long *sizep, char *data) +{
- char *tmp_buf = NULL, *new_buf = NULL, *value;
- unsigned long len = 0;
- if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
union {u8 u8;u16 u16;u32 u32;u64 u64;} tmp_data;unsigned long hex_value;void *hex_ptr;data += 3;len = strlen(data);if ((len & 0x1)) /* not multiple of two */return -1;len /= 2;if (len > 8)return -1;else if (len > 4)len = 8;else if (len > 2)len = 4;/* convert hex hexadecimal number */if (strict_strtoul(data, 16, &hex_value) < 0)return -1;tmp_buf = malloc(len);if (!tmp_buf)return -1;if (len == 1) {tmp_data.u8 = hex_value;hex_ptr = &tmp_data.u8;} else if (len == 2) {tmp_data.u16 = hex_value;hex_ptr = &tmp_data.u16;} else if (len == 4) {tmp_data.u32 = hex_value;hex_ptr = &tmp_data.u32;} else {tmp_data.u64 = hex_value;hex_ptr = &tmp_data.u64;}memcpy(tmp_buf, hex_ptr, len);value = tmp_buf;- } else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
data += 2;len = strlen(data);if (len & 0x1) /* not multiple of two */return -1;len /= 2;tmp_buf = malloc(len);if (!tmp_buf)return -1;if (hex2bin((u8 *)tmp_buf, data, len) < 0)return -1;value = tmp_buf;- } else { /* string */
if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {if (data[1] == '"')data += 2;elsedata += 3;value = data;len = strlen(data) - 1;if (data[len] != '"')return -1;} else {value = data;len = strlen(data);}- }
- new_buf = realloc(*bufp, *sizep + len);
- if (!new_buf)
goto out;- memcpy(new_buf + *sizep, value, len);
- *bufp = new_buf;
- *sizep += len;
+out:
- free(tmp_buf);
- return 0;
+}
+static int do_efi_set_var(int argc, char * const argv[]) +{
- char *var_name, *value = NULL;
- unsigned long size = 0;
- u16 *var_name16, *p;
- efi_guid_t guid;
- efi_status_t ret;
- if (argc == 1)
return CMD_RET_SUCCESS;- var_name = argv[1];
- if (argc == 2) {
/* remove */value = NULL;size = 0;- } else { /* set */
argc -= 2;argv += 2;for ( ; argc > 0; argc--, argv++)if (append_value(&value, &size, argv[0]) < 0) {ret = CMD_RET_FAILURE;goto out;}- }
- var_name16 = malloc((strlen(var_name) + 1) * 2);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
- guid = efi_global_variable_guid;
- ret = efi_set_variable(var_name16, &guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, size, value);- ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
- return ret;
+}
+static int do_efi_boot_add(int argc, char * const argv[]) +{
- int id;
- char *endp;
- char var_name[9];
- u16 var_name16[9], *p;
- efi_guid_t guid;
- size_t label_len, label_len16;
- u16 *label;
- struct efi_device_path *device_path = NULL, *file_path = NULL;
- struct efi_load_option lo;
- void *data = NULL;
- unsigned long size;
- int ret;
- if (argc < 6 || argc > 7)
return CMD_RET_USAGE;- id = (int)simple_strtoul(argv[1], &endp, 0);
- if (*endp != '\0' || id > 0xffff)
return CMD_RET_FAILURE;- sprintf(var_name, "Boot%04X", id);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, 9);
- guid = efi_global_variable_guid;
- /* attributes */
- lo.attributes = 0x1; /* always ACTIVE */
- /* label */
- label_len = strlen(argv[2]);
- label_len16 = utf8_utf16_strnlen(argv[2], label_len);
- label = malloc((label_len16 + 1) * sizeof(u16));
- if (!label)
return CMD_RET_FAILURE;- lo.label = label; /* label will be changed below */
- utf8_utf16_strncpy(&label, argv[2], label_len);
- /* file path */
- ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
&file_path);- if (ret != EFI_SUCCESS) {
ret = CMD_RET_FAILURE;goto out;- }
- lo.file_path = file_path;
- lo.file_path_length = efi_dp_size(file_path)
+ sizeof(struct efi_device_path); /* for END */- /* optional data */
- lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
- size = efi_serialize_load_option(&lo, (u8 **)&data);
The load options should be converted to a UTF-16 string before calling efi_serialize_load_option so that we can copy them to the loaded image protocol.
I don't get your point. What do you mean by "load options," lo or optional_data? The latter is defined as a u8 string in efi_loader.h, which was originally located in efi_bootmgr.c.
- if (!size) {
ret = CMD_RET_FAILURE;goto out;- }
- ret = efi_set_variable(var_name16, &guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, size, data);- ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
- free(data);
- efi_free_pool(device_path);
- efi_free_pool(file_path);
- free(lo.label);
- return ret;
+}
+static int do_efi_boot_rm(int argc, char * const argv[]) +{
- efi_guid_t guid;
- int id, i;
- char *endp;
- char var_name[9];
- u16 var_name16[9];
- efi_status_t ret;
- if (argc == 1)
return CMD_RET_USAGE;- guid = efi_global_variable_guid;
- for (i = 1; i < argc; i++, argv++) {
id = (int)simple_strtoul(argv[1], &endp, 0);if (*endp != '\0' || id > 0xffff)return CMD_RET_FAILURE;sprintf(var_name, "Boot%04X", id);utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);ret = efi_set_variable(var_name16, &guid,EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);if (ret) {printf("cannot remove Boot%04X", id);return CMD_RET_FAILURE;}- }
- return CMD_RET_SUCCESS;
+}
+static void show_efi_boot_opt_data(int id, void *data) +{
- struct efi_load_option lo;
- char *label, *p;
- size_t label_len16, label_len;
- u16 *dp_str;
- efi_deserialize_load_option(&lo, data);
- label_len16 = u16_strlen(lo.label);
- label_len = utf16_utf8_strnlen(lo.label, label_len16);
- label = malloc(label_len + 1);
- if (!label)
return;- p = label;
- utf16_utf8_strncpy(&p, lo.label, label_len16);
- printf("Boot%04X:\n", id);
- printf("\tattributes: %c%c%c (0x%08x)\n",
/* ACTIVE */lo.attributes & 0x1 ? 'A' : '-',/* FORCE RECONNECT */lo.attributes & 0x2 ? 'R' : '-',/* HIDDEN */lo.attributes & 0x8 ? 'H' : '-',lo.attributes);- printf("\tlabel: %s\n", label);
- dp_str = efi_dp_str(lo.file_path);
- printf("\tfile_path: %ls\n", dp_str);
- efi_free_pool(dp_str);
- printf("\tdata: %s\n", lo.optional_data);
- free(label);
+}
+static void show_efi_boot_opt(int id) +{
- char var_name[9];
- u16 var_name16[9], *p;
- efi_guid_t guid;
- void *data = NULL;
- unsigned long size;
'size' must be of type efi_uint_t. Otherwise you get warnings like this one:
cmd/efishell.c:729:50: warning: passing argument 4 of ‘efi_get_variable’ from incompatible pointer type [-Wincompatible-pointer-types] ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
OK.
- int ret;
- sprintf(var_name, "Boot%04X", id);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, 9);
- guid = efi_global_variable_guid;
- size = 0;
- ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
- if (ret == (int)EFI_BUFFER_TOO_SMALL) {
data = malloc(size);ret = efi_get_variable(var_name16, &guid, NULL, &size, data);- }
- if (ret == EFI_SUCCESS)
show_efi_boot_opt_data(id, data);- else if (ret == EFI_NOT_FOUND)
printf("Boot%04X: not found\n", id);- free(data);
+}
+static int do_efi_boot_dump(int argc, char * const argv[]) +{
- char regex[256];
- char * const regexlist[] = {regex};
- char *variables = NULL, *boot, *value;
- int len;
- int id;
- if (argc > 1)
return CMD_RET_USAGE;- snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
- len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
&variables, 0, 1, regexlist);- if (!len)
return CMD_RET_SUCCESS;- if (len < 0)
return CMD_RET_FAILURE;- boot = variables;
- while (*boot) {
value = strstr(boot, "Boot") + 4;id = (int)simple_strtoul(value, NULL, 16);show_efi_boot_opt(id);boot = strchr(boot, '\n');if (!*boot)break;boot++;- }
- free(variables);
- return CMD_RET_SUCCESS;
+}
+static int show_efi_boot_order(void) +{
- efi_guid_t guid;
- u16 *bootorder = NULL;
- unsigned long size;
- int num, i;
- char var_name[9];
- u16 var_name16[9], *p16;
- void *data;
- struct efi_load_option lo;
- char *label, *p;
- size_t label_len16, label_len;
- efi_status_t ret;
- guid = efi_global_variable_guid;
- size = 0;
- ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
- if (ret == EFI_BUFFER_TOO_SMALL) {
bootorder = malloc(size);ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,bootorder);- }
- if (ret == EFI_NOT_FOUND) {
printf("BootOrder not defined\n");ret = CMD_RET_SUCCESS;goto out;- } else if (ret != EFI_SUCCESS) {
ret = CMD_RET_FAILURE;goto out;- }
- num = size / sizeof(u16);
- for (i = 0; i < num; i++) {
sprintf(var_name, "Boot%04X", bootorder[i]);p16 = var_name16;utf8_utf16_strncpy(&p16, var_name, 9);size = 0;ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);if (ret != EFI_BUFFER_TOO_SMALL) {printf("%2d: Boot%04X: (not defined)\n",i + 1, bootorder[i]);continue;}data = malloc(size);if (!data) {ret = CMD_RET_FAILURE;goto out;}ret = efi_get_variable(var_name16, &guid, NULL, &size, data);if (ret != EFI_SUCCESS) {free(data);ret = CMD_RET_FAILURE;goto out;}efi_deserialize_load_option(&lo, data);label_len16 = u16_strlen(lo.label);label_len = utf16_utf8_strnlen(lo.label, label_len16);label = malloc(label_len + 1);if (!label) {free(data);ret = CMD_RET_FAILURE;goto out;}p = label;utf16_utf8_strncpy(&p, lo.label, label_len16);printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);free(label);free(data);- }
+out:
- free(bootorder);
- return ret;
+}
+static int do_efi_boot_order(int argc, char * const argv[]) +{
- u16 *bootorder = NULL;
- unsigned long size;
- int id, i;
- char *endp;
- efi_guid_t guid;
- efi_status_t ret;
- if (argc == 1)
return show_efi_boot_order();- argc--;
- argv++;
- size = argc * sizeof(u16);
- bootorder = malloc(size);
- if (!bootorder)
return CMD_RET_FAILURE;- for (i = 0; i < argc; i++) {
id = (int)simple_strtoul(argv[i], &endp, 0);if (*endp != '\0' || id > 0xffff) {printf("invalid value: %s\n", argv[i]);ret = CMD_RET_FAILURE;goto out;}bootorder[i] = (u16)id;- }
- guid = efi_global_variable_guid;
- ret = efi_set_variable(L"BootOrder", &guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);- ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
- free(bootorder);
- return ret;
+}
+static int do_efi_boot_opt(int argc, char * const argv[]) +{
- char *sub_command;
- if (argc < 2)
return CMD_RET_USAGE;- argc--; argv++;
- sub_command = argv[0];
- if (!strcmp(sub_command, "add"))
return do_efi_boot_add(argc, argv);- else if (!strcmp(sub_command, "rm"))
return do_efi_boot_rm(argc, argv);- else if (!strcmp(sub_command, "dump"))
return do_efi_boot_dump(argc, argv);- else if (!strcmp(sub_command, "order"))
return do_efi_boot_order(argc, argv);- else
return CMD_RET_USAGE;+}
+/* Interpreter command to configure EFI environment */ +static int do_efishell(cmd_tbl_t *cmdtp, int flag,
int argc, char * const argv[])+{
- char *command;
- efi_status_t r;
- if (argc < 2)
return CMD_RET_USAGE;- argc--; argv++;
- command = argv[0];
- /* Initialize EFI drivers */
- r = efi_init_obj_list();
- if (r != EFI_SUCCESS) {
printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;- }
- if (!strcmp(command, "boot"))
return do_efi_boot_opt(argc, argv);- else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
return do_efi_dump_var(argc, argv);- else if (!strcmp(command, "setvar"))
return do_efi_set_var(argc, argv);- else
return CMD_RET_USAGE;+}
+#ifdef CONFIG_SYS_LONGHELP +static char efishell_help_text[] =
- " - EFI Shell-like interface to configure EFI environment\n"
- "\n"
- "efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
The 'option' parameter is optional. Please, add brackets ([]).
OK.
The parameter 'option' is misleading. Please call it 'load_options'.
OK.
Please, add a text explaining that 'load_options' is meant to be the command line string passed to the EFI binary.
OK.
-Takahiro Akashi
Best regards
Heinrich
- " - set uefi's BOOT variable\n"
- "efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
- " - set/display uefi boot order\n"
- "efishell boot dump\n"
- " - display all uefi's BOOT variables\n"
- "efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
- " - set/display uefi boot order\n"
- "\n"
- "efishell dumpvar [<name>]\n"
- " - get uefi variable's value\n"
- "efishell setvar <name> [<value>]\n"
- " - set/delete uefi variable's value\n"
- " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n";
+#endif
+U_BOOT_CMD(
- efishell, 10, 0, do_efishell,
- "Configure EFI environment",
- efishell_help_text
+);
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces:
- efishell boot add: add BootXXXX variable
- efishell boot rm: remove BootXXXX variable
- efishell boot dump: display all BootXXXX variables
- efishell boot order: set/display a boot order (BootOrder)
- efishell setvar: set an UEFI variable (with limited functionality)
- efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/Kconfig | 10 + cmd/Makefile | 1 + cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 684 insertions(+) create mode 100644 cmd/efishell.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index f2f3b5e2b76b..a8a4bf7db45e 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1390,6 +1390,16 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
<snip>
+static int do_efi_boot_add(int argc, char * const argv[]) +{
- int id;
- char *endp;
- char var_name[9];
- u16 var_name16[9], *p;
- efi_guid_t guid;
- size_t label_len, label_len16;
- u16 *label;
- struct efi_device_path *device_path = NULL, *file_path = NULL;
- struct efi_load_option lo;
- void *data = NULL;
- unsigned long size;
- int ret;
- if (argc < 6 || argc > 7)
return CMD_RET_USAGE;- id = (int)simple_strtoul(argv[1], &endp, 0);
- if (*endp != '\0' || id > 0xffff)
return CMD_RET_FAILURE;- sprintf(var_name, "Boot%04X", id);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, 9);
- guid = efi_global_variable_guid;
- /* attributes */
- lo.attributes = 0x1; /* always ACTIVE */
- /* label */
- label_len = strlen(argv[2]);
- label_len16 = utf8_utf16_strnlen(argv[2], label_len);
- label = malloc((label_len16 + 1) * sizeof(u16));
- if (!label)
return CMD_RET_FAILURE;- lo.label = label; /* label will be changed below */
- utf8_utf16_strncpy(&label, argv[2], label_len);
- /* file path */
- ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
&file_path);
This will create a full device path like
/VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f)/<file_path>
This is unlike what the Linux program efibootmgr would do. Efibootmgr will create a shortened device path where the first node is the partition, e.g.
HD(1,MBR,0xd1535d21,0x1,0x7f)/<file_path>
The advantage of this shortened device path is that it only depends on the disk content and not on the firmware. With a full device path approach adding a node (e.g. the disk controller) in front of the partition would invalidate the boot entry. Furthermore the operating system will not be aware of the full device path.
EDK2 uses the following logic in the boot manager to expand the device path (BmGetNextLoadOptionDevicePath()):
Check if the file path is a full device path. Check if the device path matches a partition on any drive. Check if the file path matches a file on any partition.
I think in U-Boot we will have to support shortened device paths to collaborate with operating systems.
Best regards
Heinrich
On Mon, Dec 31, 2018 at 12:47:07AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.
The new command, efishell, helps address these issues and give us more friendly interfaces:
- efishell boot add: add BootXXXX variable
- efishell boot rm: remove BootXXXX variable
- efishell boot dump: display all BootXXXX variables
- efishell boot order: set/display a boot order (BootOrder)
- efishell setvar: set an UEFI variable (with limited functionality)
- efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/Kconfig | 10 + cmd/Makefile | 1 + cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 684 insertions(+) create mode 100644 cmd/efishell.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index f2f3b5e2b76b..a8a4bf7db45e 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1390,6 +1390,16 @@ config CMD_DISPLAY displayed on a simple board-specific display. Implement display_putc() to use it.
<snip>
+static int do_efi_boot_add(int argc, char * const argv[]) +{
- int id;
- char *endp;
- char var_name[9];
- u16 var_name16[9], *p;
- efi_guid_t guid;
- size_t label_len, label_len16;
- u16 *label;
- struct efi_device_path *device_path = NULL, *file_path = NULL;
- struct efi_load_option lo;
- void *data = NULL;
- unsigned long size;
- int ret;
- if (argc < 6 || argc > 7)
return CMD_RET_USAGE;- id = (int)simple_strtoul(argv[1], &endp, 0);
- if (*endp != '\0' || id > 0xffff)
return CMD_RET_FAILURE;- sprintf(var_name, "Boot%04X", id);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, 9);
- guid = efi_global_variable_guid;
- /* attributes */
- lo.attributes = 0x1; /* always ACTIVE */
- /* label */
- label_len = strlen(argv[2]);
- label_len16 = utf8_utf16_strnlen(argv[2], label_len);
- label = malloc((label_len16 + 1) * sizeof(u16));
- if (!label)
return CMD_RET_FAILURE;- lo.label = label; /* label will be changed below */
- utf8_utf16_strncpy(&label, argv[2], label_len);
- /* file path */
- ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
&file_path);This will create a full device path like
/VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f)/<file_path>
This is unlike what the Linux program efibootmgr would do. Efibootmgr will create a shortened device path where the first node is the partition, e.g.
HD(1,MBR,0xd1535d21,0x1,0x7f)/<file_path>
The advantage of this shortened device path is that it only depends on the disk content and not on the firmware. With a full device path approach adding a node (e.g. the disk controller) in front of the partition would invalidate the boot entry. Furthermore the operating system will not be aware of the full device path.
EDK2 uses the following logic in the boot manager to expand the device path (BmGetNextLoadOptionDevicePath()):
Check if the file path is a full device path. Check if the device path matches a partition on any drive. Check if the file path matches a file on any partition.
I think in U-Boot we will have to support shortened device paths to collaborate with operating systems.
So I assume that your comment here is not on my patch, but u-boot's bootmgr. Once bootmgr is modified, I will be able to make my code aligned.
-Takahiro Akashi
Best regards
Heinrich
"devices" command prints all the uefi variables on the system. => efishell devices Device Name ============================================ /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\ HD(2,MBR,0x086246ba,0x40800,0x3f800) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\ HD(1,MBR,0x086246ba,0x800,0x40000)
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 5819e52cf575..929b6343b1b2 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -18,6 +18,8 @@ #include <linux/ctype.h> #include <asm/global_data.h>
+static const struct efi_boot_services *bs; + static void dump_var_data(char *data, unsigned long len) { char *start, *end, *p; @@ -263,6 +265,84 @@ out: return ret; }
+static efi_handle_t *efi_get_handles_by_proto(efi_guid_t *guid) +{ + efi_handle_t *handles = NULL; + efi_uintn_t size = 0; + efi_status_t ret; + + if (guid) { + ret = bs->locate_handle(BY_PROTOCOL, guid, NULL, &size, + handles); + if (ret == EFI_BUFFER_TOO_SMALL) { + handles = calloc(1, size); + if (!handles) + return NULL; + + ret = bs->locate_handle(BY_PROTOCOL, guid, NULL, &size, + handles); + } + if (ret != EFI_SUCCESS) { + free(handles); + return NULL; + } + } else { + ret = bs->locate_handle(ALL_HANDLES, NULL, NULL, &size, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) { + handles = calloc(1, size); + if (!handles) + return NULL; + + ret = bs->locate_handle(ALL_HANDLES, NULL, NULL, &size, + handles); + } + if (ret != EFI_SUCCESS) { + free(handles); + return NULL; + } + } + + return handles; +} + +static int efi_get_device_handle_info(efi_handle_t handle, u16 **name) +{ + struct efi_device_path *dp; + efi_status_t ret; + + ret = bs->open_protocol(handle, &efi_guid_device_path, + (void **)&dp, NULL /* FIXME */, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret == EFI_SUCCESS) { + *name = efi_dp_str(dp); + return 0; + } else { + return -1; + } +} + +static int do_efi_show_devices(int argc, char * const argv[]) +{ + efi_handle_t *handles = NULL, *handle; + u16 *devname; + + handles = efi_get_handles_by_proto(NULL); + if (!handles) + return CMD_RET_SUCCESS; + + printf("Device Name\n"); + printf("============================================\n"); + for (handle = handles; *handle; handle++) { + if (!efi_get_device_handle_info(*handle, &devname)) { + printf("%ls\n", devname); + efi_free_pool(devname); + } + handle++; + } + + return CMD_RET_SUCCESS; +} + static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -625,6 +705,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, if (argc < 2) return CMD_RET_USAGE;
+ bs = systab.boottime; + argc--; argv++; command = argv[0];
@@ -642,6 +724,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_dump_var(argc, argv); else if (!strcmp(command, "setvar")) return do_efi_set_var(argc, argv); + else if (!strcmp(command, "devices")) + return do_efi_show_devices(argc, argv); else return CMD_RET_USAGE; } @@ -663,7 +747,9 @@ static char efishell_help_text[] = " - get uefi variable's value\n" "efishell setvar <name> [<value>]\n" " - set/delete uefi variable's value\n" - " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n"; + " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n" + "efishell devices\n" + " - show uefi devices\n"; #endif
U_BOOT_CMD(
"drivers" command prints all the uefi drivers on the system. => efi drivers Driver Name Image Path ============================================ (unknown) <NULL>+(not found) guid: 18a031ab-b443-4d1a-a5c0-0c09261e9f71
Currently, no useful information can be printed.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/efishell.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 929b6343b1b2..ed8de9e0355d 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -343,6 +343,95 @@ static int do_efi_show_devices(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static int efi_get_driver_handle_info(efi_handle_t handle, u16 **name, + u16 **devname, u16 **filename) +{ + struct efi_driver_binding_protocol *binding; + struct efi_loaded_image *image; + efi_status_t ret; + + ret = bs->open_protocol(handle, &efi_guid_driver_binding_protocol, + (void **)&binding, NULL, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + return -1; + + ret = bs->open_protocol(binding->image_handle, &efi_guid_loaded_image, + (void **)&image, NULL /* FIXME */, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (ret != EFI_SUCCESS) + goto e_out; + + /* + * TODO: + * handle image->device_handle, + * use append_device_path() + */ + *devname = NULL; + *filename = efi_dp_str(image->file_path); + + return 0; + +e_out: + *devname = NULL; + *filename = NULL; + + return 0; +} + +static int do_efi_show_drivers(int argc, char * const argv[]) +{ + efi_handle_t *handles = NULL, *handle; + efi_uintn_t size = 0; + u16 *drvname, *devname, *filename; + efi_status_t ret; + int i; + + ret = bs->locate_handle(BY_PROTOCOL, &efi_guid_driver_binding_protocol, + NULL, &size, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) { + handles = calloc(1, size); + if (!handles) + return CMD_RET_FAILURE; + + ret = bs->locate_handle(BY_PROTOCOL, + &efi_guid_driver_binding_protocol, + NULL, &size, handles); + } + if (ret != EFI_SUCCESS) { + free(handles); + return CMD_RET_FAILURE; + } + + printf("Driver Name Image Path\n"); + printf("============================================\n"); + handle = handles; + for (i = 0; i < size / sizeof(*handle); i++) { + if (!efi_get_driver_handle_info(*handle, &drvname, &devname, + &filename)) { + printf("%-16s%ls+%ls\n", + "(unknown)", devname, filename); + efi_free_pool(devname); + efi_free_pool(filename); + + /* TODO: no other info */ + struct efi_object *efiobj; + struct list_head *lhandle; + struct efi_handler *protocol; + + efiobj = efi_search_obj(*handle); + list_for_each(lhandle, &efiobj->protocols) { + protocol = list_entry(lhandle, + struct efi_handler, link); + printf(" guid: %pUl\n", protocol->guid); + } + } + handle++; + } + + return CMD_RET_SUCCESS; +} + static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -726,6 +815,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_set_var(argc, argv); else if (!strcmp(command, "devices")) return do_efi_show_devices(argc, argv); + else if (!strcmp(command, "drivers")) + return do_efi_show_drivers(argc, argv); else return CMD_RET_USAGE; } @@ -749,7 +840,9 @@ static char efishell_help_text[] = " - set/delete uefi variable's value\n" " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n" "efishell devices\n" - " - show uefi devices\n"; + " - show uefi devices\n" + "efishell drivers\n" + " - show uefi drivers\n"; #endif
U_BOOT_CMD(
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"drivers" command prints all the uefi drivers on the system. => efi drivers Driver Name Image Path ============================================ (unknown) <NULL>+(not found) guid: 18a031ab-b443-4d1a-a5c0-0c09261e9f71
Currently, no useful information can be printed.
Why? We have lib/efi_driver/efi_block_device.c. So you should be able to test the output.
Best regards
Heirnich
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efishell.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 929b6343b1b2..ed8de9e0355d 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -343,6 +343,95 @@ static int do_efi_show_devices(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static int efi_get_driver_handle_info(efi_handle_t handle, u16 **name,
u16 **devname, u16 **filename)+{
- struct efi_driver_binding_protocol *binding;
- struct efi_loaded_image *image;
- efi_status_t ret;
- ret = bs->open_protocol(handle, &efi_guid_driver_binding_protocol,
(void **)&binding, NULL, NULL,EFI_OPEN_PROTOCOL_GET_PROTOCOL);- if (ret != EFI_SUCCESS)
return -1;- ret = bs->open_protocol(binding->image_handle, &efi_guid_loaded_image,
(void **)&image, NULL /* FIXME */, NULL,EFI_OPEN_PROTOCOL_GET_PROTOCOL);- if (ret != EFI_SUCCESS)
goto e_out;- /*
* TODO:* handle image->device_handle,* use append_device_path()*/- *devname = NULL;
- *filename = efi_dp_str(image->file_path);
- return 0;
+e_out:
- *devname = NULL;
- *filename = NULL;
- return 0;
+}
+static int do_efi_show_drivers(int argc, char * const argv[]) +{
- efi_handle_t *handles = NULL, *handle;
- efi_uintn_t size = 0;
- u16 *drvname, *devname, *filename;
- efi_status_t ret;
- int i;
- ret = bs->locate_handle(BY_PROTOCOL, &efi_guid_driver_binding_protocol,
NULL, &size, NULL);- if (ret == EFI_BUFFER_TOO_SMALL) {
handles = calloc(1, size);if (!handles)return CMD_RET_FAILURE;ret = bs->locate_handle(BY_PROTOCOL,&efi_guid_driver_binding_protocol,NULL, &size, handles);- }
- if (ret != EFI_SUCCESS) {
free(handles);return CMD_RET_FAILURE;- }
- printf("Driver Name Image Path\n");
- printf("============================================\n");
- handle = handles;
- for (i = 0; i < size / sizeof(*handle); i++) {
if (!efi_get_driver_handle_info(*handle, &drvname, &devname,&filename)) {printf("%-16s%ls+%ls\n","(unknown)", devname, filename);efi_free_pool(devname);efi_free_pool(filename);/* TODO: no other info */struct efi_object *efiobj;struct list_head *lhandle;struct efi_handler *protocol;efiobj = efi_search_obj(*handle);list_for_each(lhandle, &efiobj->protocols) {protocol = list_entry(lhandle,struct efi_handler, link);printf(" guid: %pUl\n", protocol->guid);}}handle++;- }
- return CMD_RET_SUCCESS;
+}
static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -726,6 +815,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_set_var(argc, argv); else if (!strcmp(command, "devices")) return do_efi_show_devices(argc, argv);
- else if (!strcmp(command, "drivers"))
return do_efi_show_drivers(argc, argv);} @@ -749,7 +840,9 @@ static char efishell_help_text[] = " - set/delete uefi variable's value\n" " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n" "efishell devices\n"
- " - show uefi devices\n";
- " - show uefi devices\n"
- "efishell drivers\n"
- " - show uefi drivers\n";
#endif
U_BOOT_CMD(
On Thu, Dec 20, 2018 at 08:51:34AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"drivers" command prints all the uefi drivers on the system. => efi drivers Driver Name Image Path ============================================ (unknown) <NULL>+(not found) guid: 18a031ab-b443-4d1a-a5c0-0c09261e9f71
Currently, no useful information can be printed.
Why? We have lib/efi_driver/efi_block_device.c. So you should be able to test the output.
There are a couple of reasons that I think there are no useful information: 1. EFI driver doesn't have any printable name. A corresponding u-boot driver, efi_block in efi_block_device.c, has a name, but there is not direct link between efi driver and this u-boot driver. So it's not easy to find a name for efi driver. 2. I will fix this by adding a "name" field to efi_driver_binding_extended_protocol, but if even so, we can only say "EFI block driver." We have no chance to know about controller -specific, say USB, SCSI or anything, information.
3. More importantly, efi block devices can be defined in two ways, via efi_uclass/efi_block_device and efi_disk. The latter supports BLOCK_IO_PROTOCOL binding having any associated driver/controller. It seems to me that it's quite confusion.
Thanks, -Takahiro Akashi
Best regards
Heirnich
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efishell.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 929b6343b1b2..ed8de9e0355d 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -343,6 +343,95 @@ static int do_efi_show_devices(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static int efi_get_driver_handle_info(efi_handle_t handle, u16 **name,
u16 **devname, u16 **filename)+{
- struct efi_driver_binding_protocol *binding;
- struct efi_loaded_image *image;
- efi_status_t ret;
- ret = bs->open_protocol(handle, &efi_guid_driver_binding_protocol,
(void **)&binding, NULL, NULL,EFI_OPEN_PROTOCOL_GET_PROTOCOL);- if (ret != EFI_SUCCESS)
return -1;- ret = bs->open_protocol(binding->image_handle, &efi_guid_loaded_image,
(void **)&image, NULL /* FIXME */, NULL,EFI_OPEN_PROTOCOL_GET_PROTOCOL);- if (ret != EFI_SUCCESS)
goto e_out;- /*
* TODO:* handle image->device_handle,* use append_device_path()*/- *devname = NULL;
- *filename = efi_dp_str(image->file_path);
- return 0;
+e_out:
- *devname = NULL;
- *filename = NULL;
- return 0;
+}
+static int do_efi_show_drivers(int argc, char * const argv[]) +{
- efi_handle_t *handles = NULL, *handle;
- efi_uintn_t size = 0;
- u16 *drvname, *devname, *filename;
- efi_status_t ret;
- int i;
- ret = bs->locate_handle(BY_PROTOCOL, &efi_guid_driver_binding_protocol,
NULL, &size, NULL);- if (ret == EFI_BUFFER_TOO_SMALL) {
handles = calloc(1, size);if (!handles)return CMD_RET_FAILURE;ret = bs->locate_handle(BY_PROTOCOL,&efi_guid_driver_binding_protocol,NULL, &size, handles);- }
- if (ret != EFI_SUCCESS) {
free(handles);return CMD_RET_FAILURE;- }
- printf("Driver Name Image Path\n");
- printf("============================================\n");
- handle = handles;
- for (i = 0; i < size / sizeof(*handle); i++) {
if (!efi_get_driver_handle_info(*handle, &drvname, &devname,&filename)) {printf("%-16s%ls+%ls\n","(unknown)", devname, filename);efi_free_pool(devname);efi_free_pool(filename);/* TODO: no other info */struct efi_object *efiobj;struct list_head *lhandle;struct efi_handler *protocol;efiobj = efi_search_obj(*handle);list_for_each(lhandle, &efiobj->protocols) {protocol = list_entry(lhandle,struct efi_handler, link);printf(" guid: %pUl\n", protocol->guid);}}handle++;- }
- return CMD_RET_SUCCESS;
+}
static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -726,6 +815,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_set_var(argc, argv); else if (!strcmp(command, "devices")) return do_efi_show_devices(argc, argv);
- else if (!strcmp(command, "drivers"))
return do_efi_show_drivers(argc, argv);} @@ -749,7 +840,9 @@ static char efishell_help_text[] = " - set/delete uefi variable's value\n" " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n" "efishell devices\n"
- " - show uefi devices\n";
- " - show uefi devices\n"
- "efishell drivers\n"
- " - show uefi drivers\n";
#endif
U_BOOT_CMD(
On 12/25/18 8:22 AM, AKASHI Takahiro wrote:
On Thu, Dec 20, 2018 at 08:51:34AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"drivers" command prints all the uefi drivers on the system. => efi drivers Driver Name Image Path ============================================ (unknown) <NULL>+(not found) guid: 18a031ab-b443-4d1a-a5c0-0c09261e9f71
Currently, no useful information can be printed.
Why? We have lib/efi_driver/efi_block_device.c. So you should be able to test the output.
There are a couple of reasons that I think there are no useful information:
- EFI driver doesn't have any printable name. A corresponding u-boot driver, efi_block in efi_block_device.c, has a name, but there is not direct link between efi driver and this u-boot driver. So it's not easy to find a name for efi driver.
- I will fix this by adding a "name" field to efi_driver_binding_extended_protocol, but if even so, we can only say "EFI block driver." We have no chance to know about controller -specific, say USB, SCSI or anything, information.
I would just stick to the standard. The following information can be provided in the standard:
Loop over all handles implementing the driver binding protocol and output:
- address of driver handle - device path (if installed on the handle)
- More importantly, efi block devices can be defined in two ways, via efi_uclass/efi_block_device and efi_disk. The latter supports BLOCK_IO_PROTOCOL binding having any associated driver/controller. It seems to me that it's quite confusion.
lib/efi_driver/efi_block_device.c is used when an EFI binary creates a new block device. E.g. when iPXE connects to an iSCSI drive it creates a handle with the the block io protocol and call ConnectControllers. Now our efi_block_device binds to the handle, identifies the partitions, and provides the simple file system protocol.
Unfortunately the migration of all block devices to the driver model has been delayed to July 2019 (cf. doc/driver-model/MIGRATION.txt). Once it is completed we can clean up the code.
Best regards
Heinrich
Thanks, -Takahiro Akashi
Best regards
Heirnich
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efishell.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 929b6343b1b2..ed8de9e0355d 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -343,6 +343,95 @@ static int do_efi_show_devices(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static int efi_get_driver_handle_info(efi_handle_t handle, u16 **name,
u16 **devname, u16 **filename)+{
- struct efi_driver_binding_protocol *binding;
- struct efi_loaded_image *image;
- efi_status_t ret;
- ret = bs->open_protocol(handle, &efi_guid_driver_binding_protocol,
(void **)&binding, NULL, NULL,EFI_OPEN_PROTOCOL_GET_PROTOCOL);- if (ret != EFI_SUCCESS)
return -1;- ret = bs->open_protocol(binding->image_handle, &efi_guid_loaded_image,
(void **)&image, NULL /* FIXME */, NULL,EFI_OPEN_PROTOCOL_GET_PROTOCOL);- if (ret != EFI_SUCCESS)
goto e_out;- /*
* TODO:* handle image->device_handle,* use append_device_path()*/- *devname = NULL;
- *filename = efi_dp_str(image->file_path);
- return 0;
+e_out:
- *devname = NULL;
- *filename = NULL;
- return 0;
+}
+static int do_efi_show_drivers(int argc, char * const argv[]) +{
- efi_handle_t *handles = NULL, *handle;
- efi_uintn_t size = 0;
- u16 *drvname, *devname, *filename;
- efi_status_t ret;
- int i;
- ret = bs->locate_handle(BY_PROTOCOL, &efi_guid_driver_binding_protocol,
NULL, &size, NULL);- if (ret == EFI_BUFFER_TOO_SMALL) {
handles = calloc(1, size);if (!handles)return CMD_RET_FAILURE;ret = bs->locate_handle(BY_PROTOCOL,&efi_guid_driver_binding_protocol,NULL, &size, handles);- }
- if (ret != EFI_SUCCESS) {
free(handles);return CMD_RET_FAILURE;- }
- printf("Driver Name Image Path\n");
- printf("============================================\n");
- handle = handles;
- for (i = 0; i < size / sizeof(*handle); i++) {
if (!efi_get_driver_handle_info(*handle, &drvname, &devname,&filename)) {printf("%-16s%ls+%ls\n","(unknown)", devname, filename);efi_free_pool(devname);efi_free_pool(filename);/* TODO: no other info */struct efi_object *efiobj;struct list_head *lhandle;struct efi_handler *protocol;efiobj = efi_search_obj(*handle);list_for_each(lhandle, &efiobj->protocols) {protocol = list_entry(lhandle,struct efi_handler, link);printf(" guid: %pUl\n", protocol->guid);}}handle++;- }
- return CMD_RET_SUCCESS;
+}
static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -726,6 +815,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_set_var(argc, argv); else if (!strcmp(command, "devices")) return do_efi_show_devices(argc, argv);
- else if (!strcmp(command, "drivers"))
return do_efi_show_drivers(argc, argv);} @@ -749,7 +840,9 @@ static char efishell_help_text[] = " - set/delete uefi variable's value\n" " <value> may be "="..."", "=0x..." (set) or "=" (delete)\n" "efishell devices\n"
- " - show uefi devices\n";
- " - show uefi devices\n"
- "efishell drivers\n"
- " - show uefi drivers\n";
#endif
U_BOOT_CMD(
"images" command prints loaded images-related information.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/efishell.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index ed8de9e0355d..010870df63c8 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -432,6 +432,13 @@ static int do_efi_show_drivers(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static int do_efi_show_images(int argc, char * const argv[]) +{ + efi_print_image_infos(NULL); + + return CMD_RET_SUCCESS; +} + static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -817,6 +824,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_show_devices(argc, argv); else if (!strcmp(command, "drivers")) return do_efi_show_drivers(argc, argv); + else if (!strcmp(command, "images")) + return do_efi_show_images(argc, argv); else return CMD_RET_USAGE; } @@ -842,7 +851,9 @@ static char efishell_help_text[] = "efishell devices\n" " - show uefi devices\n" "efishell drivers\n" - " - show uefi drivers\n"; + " - show uefi drivers\n" + "efishell images\n" + " - show loaded images\n"; #endif
U_BOOT_CMD(
"memmap" command prints uefi-specific memory map information. => efi memmap Type Start End Attributes ================ ================ ================ ========== CONVENTIONAL 0000000040000000-000000007de27000 WB RUNTIME DATA 000000007de27000-000000007de28000 WB|RT RESERVED 000000007de28000-000000007de2a000 WB RUNTIME DATA 000000007de2a000-000000007de2b000 WB|RT RESERVED 000000007de2b000-000000007de2c000 WB RUNTIME DATA 000000007de2c000-000000007de2d000 WB|RT LOADER DATA 000000007de2d000-000000007ff37000 WB RUNTIME CODE 000000007ff37000-000000007ff38000 WB|RT LOADER DATA 000000007ff38000-0000000080000000 WB
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/efishell.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 010870df63c8..5a81a627d616 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -439,6 +439,78 @@ static int do_efi_show_images(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static const char * const efi_mem_type_string[] = { + [EFI_RESERVED_MEMORY_TYPE] = "RESERVED", + [EFI_LOADER_CODE] = "LOADER CODE", + [EFI_LOADER_DATA] = "LOADER DATA", + [EFI_BOOT_SERVICES_CODE] = "BOOT CODE", + [EFI_BOOT_SERVICES_DATA] = "BOOT DATA", + [EFI_RUNTIME_SERVICES_CODE] = "RUNTIME CODE", + [EFI_RUNTIME_SERVICES_DATA] = "RUNTIME DATA", + [EFI_CONVENTIONAL_MEMORY] = "CONVENTIONAL", + [EFI_UNUSABLE_MEMORY] = "UNUSABLE MEM", + [EFI_ACPI_RECLAIM_MEMORY] = "ACPI RECLAIM MEM", + [EFI_ACPI_MEMORY_NVS] = "ACPI NVS", + [EFI_MMAP_IO] = "IO", + [EFI_MMAP_IO_PORT] = "IO PORT", + [EFI_PAL_CODE] = "PAL", +}; + +#define EFI_MEM_ATTR(attribute, bit, string) \ + if ((attribute) & (bit)) { \ + if (sep) \ + putc('|'); \ + sep = 1; \ + printf(string); \ + } + +static int do_efi_show_memmap(int argc, char * const argv[]) +{ + struct efi_mem_desc *map = NULL; + efi_uintn_t map_size = 0; + int i, sep; + efi_status_t ret; + + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) { + map = malloc(map_size); + if (!map) + return CMD_RET_FAILURE; + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL); + } + if (ret != EFI_SUCCESS) { + free(map); + return CMD_RET_FAILURE; + } + + printf("Type Start End Attributes\n"); + printf("================ ================ ================ ==========\n"); + for (i = 0; i < map_size / sizeof(*map); map++, i++) { + sep = 0; + printf("%-16s %016llx-%016llx ", + efi_mem_type_string[map->type], + map->physical_start, + map->physical_start + map->num_pages * EFI_PAGE_SIZE); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UC, "UC"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WC, "WC"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WT, "WT"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WB, "WB"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UCE, "UCE"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WP, "WP"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RP, "RP"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_XP, "WP"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_NV, "NV"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_MORE_RELIABLE, "REL"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RO, "RO"); + EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RUNTIME, "RT"); + putc('\n'); + } + + free(map); + + return CMD_RET_SUCCESS; +} + static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -826,6 +898,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_show_drivers(argc, argv); else if (!strcmp(command, "images")) return do_efi_show_images(argc, argv); + else if (!strcmp(command, "memmap")) + return do_efi_show_memmap(argc, argv); else return CMD_RET_USAGE; } @@ -853,7 +927,9 @@ static char efishell_help_text[] = "efishell drivers\n" " - show uefi drivers\n" "efishell images\n" - " - show loaded images\n"; + " - show loaded images\n" + "efishell memmap\n" + " - show uefi memory map\n"; #endif
U_BOOT_CMD(
"dh" command prints all the uefi handles used in the system. => efishell dh (T.B.D.) 0: (protocol info not available) 1: (protocol info not available) 2: (protocol info not available) 3: (protocol info not available) 4: (protocol info not available) 5: (protocol info not available) 6: (protocol info not available) 7: (protocol info not available) 8: (protocol info not available) 9: (protocol info not available) 10: (protocol info not available) 11: (protocol info not available) 12: (protocol info not available) 13: (protocol info not available) 14: (protocol info not available) 15: (protocol info not available)
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/efishell.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 5a81a627d616..47ad77606062 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -511,6 +511,33 @@ static int do_efi_show_memmap(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static char *efi_get_proto_info(efi_handle_t handle) +{ + return strdup("(protocol info not available)"); +} + +static int do_efi_show_handles(int argc, char * const argv[]) +{ + efi_handle_t *handles = NULL, *handle; + char *info; + int i; + + handles = efi_get_handles_by_proto(NULL); + if (!handles) + return CMD_RET_SUCCESS; + + for (handle = handles, i = 0; *handle; handle++, i++) { + /* TODO: depends on protocols */ + info = efi_get_proto_info(*handle); + printf("%d: %s\n", i, info ?: ""); + free(info); + } + + free(handles); + + return CMD_RET_SUCCESS; +} + static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -900,6 +927,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_show_images(argc, argv); else if (!strcmp(command, "memmap")) return do_efi_show_memmap(argc, argv); + else if (!strcmp(command, "dh")) + return do_efi_show_handles(argc, argv); else return CMD_RET_USAGE; } @@ -929,7 +958,9 @@ static char efishell_help_text[] = "efishell images\n" " - show loaded images\n" "efishell memmap\n" - " - show uefi memory map\n"; + " - show uefi memory map\n" + "efishell dh\n" + " - show uefi handles\n"; #endif
U_BOOT_CMD(
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"dh" command prints all the uefi handles used in the system. => efishell dh (T.B.D.) 0: (protocol info not available) 1: (protocol info not available) 2: (protocol info not available) 3: (protocol info not available) 4: (protocol info not available) 5: (protocol info not available) 6: (protocol info not available) 7: (protocol info not available) 8: (protocol info not available) 9: (protocol info not available) 10: (protocol info not available) 11: (protocol info not available) 12: (protocol info not available) 13: (protocol info not available) 14: (protocol info not available) 15: (protocol info not available)
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efishell.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 5a81a627d616..47ad77606062 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -511,6 +511,33 @@ static int do_efi_show_memmap(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static char *efi_get_proto_info(efi_handle_t handle) +{
- return strdup("(protocol info not available)");
Shouldn't this enumerate all protocol GUIDs installed on the handles by calling ProtocolsPerHandle()?
Also instances of installed drivers identifiable by the driver binding protocol might be interesting.
Best regards
Heinrich
+}
+static int do_efi_show_handles(int argc, char * const argv[]) +{
- efi_handle_t *handles = NULL, *handle;
- char *info;
- int i;
- handles = efi_get_handles_by_proto(NULL);
- if (!handles)
return CMD_RET_SUCCESS;- for (handle = handles, i = 0; *handle; handle++, i++) {
/* TODO: depends on protocols */info = efi_get_proto_info(*handle);printf("%d: %s\n", i, info ?: "");free(info);- }
- free(handles);
- return CMD_RET_SUCCESS;
+}
static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -900,6 +927,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_show_images(argc, argv); else if (!strcmp(command, "memmap")) return do_efi_show_memmap(argc, argv);
- else if (!strcmp(command, "dh"))
return do_efi_show_handles(argc, argv);} @@ -929,7 +958,9 @@ static char efishell_help_text[] = "efishell images\n" " - show loaded images\n" "efishell memmap\n"
- " - show uefi memory map\n";
- " - show uefi memory map\n"
- "efishell dh\n"
- " - show uefi handles\n";
#endif
U_BOOT_CMD(
On Thu, Dec 20, 2018 at 08:49:25AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"dh" command prints all the uefi handles used in the system. => efishell dh (T.B.D.) 0: (protocol info not available) 1: (protocol info not available) 2: (protocol info not available) 3: (protocol info not available) 4: (protocol info not available) 5: (protocol info not available) 6: (protocol info not available) 7: (protocol info not available) 8: (protocol info not available) 9: (protocol info not available) 10: (protocol info not available) 11: (protocol info not available) 12: (protocol info not available) 13: (protocol info not available) 14: (protocol info not available) 15: (protocol info not available)
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efishell.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 5a81a627d616..47ad77606062 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -511,6 +511,33 @@ static int do_efi_show_memmap(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+static char *efi_get_proto_info(efi_handle_t handle) +{
- return strdup("(protocol info not available)");
Shouldn't this enumerate all protocol GUIDs installed on the handles by calling ProtocolsPerHandle()?
Okay.
Also instances of installed drivers identifiable by the driver binding protocol might be interesting.
Getting a driver name from driver binding protocol can be a bit hard. See my reply to your comment on "efishell drivers."
Thanks, -Takahiro Akashi
Best regards
Heinrich
+}
+static int do_efi_show_handles(int argc, char * const argv[]) +{
- efi_handle_t *handles = NULL, *handle;
- char *info;
- int i;
- handles = efi_get_handles_by_proto(NULL);
- if (!handles)
return CMD_RET_SUCCESS;- for (handle = handles, i = 0; *handle; handle++, i++) {
/* TODO: depends on protocols */info = efi_get_proto_info(*handle);printf("%d: %s\n", i, info ?: "");free(info);- }
- free(handles);
- return CMD_RET_SUCCESS;
+}
static int do_efi_boot_add(int argc, char * const argv[]) { int id; @@ -900,6 +927,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, return do_efi_show_images(argc, argv); else if (!strcmp(command, "memmap")) return do_efi_show_memmap(argc, argv);
- else if (!strcmp(command, "dh"))
return do_efi_show_handles(argc, argv);} @@ -929,7 +958,9 @@ static char efishell_help_text[] = "efishell images\n" " - show loaded images\n" "efishell memmap\n"
- " - show uefi memory map\n";
- " - show uefi memory map\n"
- "efishell dh\n"
- " - show uefi handles\n";
#endif
U_BOOT_CMD(
Those function will be used for integration with 'env' command so as to handle uefi variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/efishell.c | 38 ++++++++++++++++++++++++++++++++++---- include/command.h | 4 ++++ 2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c index 47ad77606062..42a29de7617c 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -65,7 +65,7 @@ static void dump_var_data(char *data, unsigned long len) * * efi_$guid_$varname = {attributes}(type)value */ -static int do_efi_dump_var(int argc, char * const argv[]) +static int _do_efi_dump_var(int argc, char * const argv[]) { char regex[256]; char * const regexlist[] = {regex}; @@ -128,6 +128,21 @@ static int do_efi_dump_var(int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+int do_efi_dump_var(int argc, char * const argv[]) +{ + efi_status_t r; + + /* Initialize EFI drivers */ + r = efi_init_obj_list(); + if (r != EFI_SUCCESS) { + printf("Error: Cannot set up EFI drivers, r = %lu\n", + r & ~EFI_ERROR_MASK); + return CMD_RET_FAILURE; + } + + return _do_efi_dump_var(argc, argv); +} + static int append_value(char **bufp, unsigned long *sizep, char *data) { char *tmp_buf = NULL, *new_buf = NULL, *value; @@ -225,7 +240,7 @@ out: return 0; }
-static int do_efi_set_var(int argc, char * const argv[]) +static int _do_efi_set_var(int argc, char * const argv[]) { char *var_name, *value = NULL; unsigned long size = 0; @@ -265,6 +280,21 @@ out: return ret; }
+int do_efi_set_var(int argc, char * const argv[]) +{ + efi_status_t r; + + /* Initialize EFI drivers */ + r = efi_init_obj_list(); + if (r != EFI_SUCCESS) { + printf("Error: Cannot set up EFI drivers, r = %lu\n", + r & ~EFI_ERROR_MASK); + return CMD_RET_FAILURE; + } + + return _do_efi_set_var(argc, argv); +} + static efi_handle_t *efi_get_handles_by_proto(efi_guid_t *guid) { efi_handle_t *handles = NULL; @@ -916,9 +946,9 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag, if (!strcmp(command, "boot")) return do_efi_boot_opt(argc, argv); else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore")) - return do_efi_dump_var(argc, argv); + return _do_efi_dump_var(argc, argv); else if (!strcmp(command, "setvar")) - return do_efi_set_var(argc, argv); + return _do_efi_set_var(argc, argv); else if (!strcmp(command, "devices")) return do_efi_show_devices(argc, argv); else if (!strcmp(command, "drivers")) diff --git a/include/command.h b/include/command.h index 9b7b876585d9..5b081ae94cf3 100644 --- a/include/command.h +++ b/include/command.h @@ -51,6 +51,10 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); #if defined(CONFIG_CMD_BOOTEFI) int do_bootefi_bootmgr_exec(int boot_id); #endif +#if defined(CONFIG_CMD_EFISHELL) +int do_efi_dump_var(int argc, char * const argv[]); +int do_efi_set_var(int argc, char * const argv[]); +#endif
/* common/command.c */ int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
"env [print|set] -e" allows for handling uefi variables without knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/nvedit.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index c0facabfc4fe..8168c963ac9d 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -27,6 +27,8 @@ #include <cli.h> #include <command.h> #include <console.h> +#include <efi.h> +#include <efi_loader.h> #include <environment.h> #include <search.h> #include <errno.h> @@ -119,6 +121,25 @@ static int do_env_print(cmd_tbl_t *cmdtp, int flag, int argc, int rcode = 0; int env_flag = H_HIDE_DOT;
+#if defined(CONFIG_CMD_EFISHELL) + if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') { + efi_status_t r; + + argc--; + argv++; + + /* Initialize EFI drivers */ + r = efi_init_obj_list(); + if (r != EFI_SUCCESS) { + printf("Error: Cannot set up EFI drivers, r = %lu\n", + r & ~EFI_ERROR_MASK); + return CMD_RET_FAILURE; + } + + return do_efi_dump_var(argc, argv); + } +#endif + if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'a') { argc--; argv++; @@ -216,6 +237,26 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag) ENTRY e, *ep;
debug("Initial value for argc=%d\n", argc); + +#if defined(CONFIG_CMD_EFISHELL) + if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') { + efi_status_t r; + + argc--; + argv++; + + /* Initialize EFI drivers */ + r = efi_init_obj_list(); + if (r != EFI_SUCCESS) { + printf("Error: Cannot set up EFI drivers, r = %lu\n", + r & ~EFI_ERROR_MASK); + return CMD_RET_FAILURE; + } + + return do_efi_set_var(argc, argv); + } +#endif + while (argc > 1 && **(argv + 1) == '-') { char *arg = *++argv;
@@ -1262,15 +1303,23 @@ static char env_help_text[] = #if defined(CONFIG_CMD_IMPORTENV) "env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL) + "env print [-a | -e [name] | name ...] - print environment\n" +#else "env print [-a | name ...] - print environment\n" +#endif #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n" #endif #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE) "env save - save environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL) + "env set [-e | -f] name [arg ...]\n"; +#else "env set [-f] name [arg ...]\n"; #endif +#endif
U_BOOT_CMD( env, CONFIG_SYS_MAXARGS, 1, do_env, @@ -1295,6 +1344,10 @@ U_BOOT_CMD_COMPLETE( printenv, CONFIG_SYS_MAXARGS, 1, do_env_print, "print environment variables", "[-a]\n - print [all] values of all environment variables\n" +#if defined(CONFIG_CMD_EFISHELL) + "printenv -e [<name>]\n" + " - print UEFI variable 'name' or all the variables\n" +#endif "printenv name ...\n" " - print value of environment variable 'name'", var_complete @@ -1322,7 +1375,11 @@ U_BOOT_CMD_COMPLETE( U_BOOT_CMD_COMPLETE( setenv, CONFIG_SYS_MAXARGS, 0, do_env_set, "set environment variables", - "[-f] name value ...\n" +#if defined(CONFIG_CMD_EFISHELL) + "-e <name> [<value>]\n" + " - set UEFI variable 'name' to 'value' ...'\n" +#endif + "setenv [-f] name value ...\n" " - [forcibly] set environment variable 'name' to 'value ...'\n" "setenv [-f] name\n" " - [forcibly] delete environment variable 'name'", @@ -1343,7 +1400,7 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", -#if defined(CONFIG_CMD_BOOTEFI) +#if defined(CONFIG_CMD_EFISHELL) "var -e [BootXXXX]\n" " - load and run UEFI app based on 'BootXXXX' UEFI variable", #else
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"env [print|set] -e" allows for handling uefi variables without knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Hello Takahiro,
in several patch series you are implementing multiple interactive commands that concern
- handling of EFI variables - executing EFI binaries - managing boot sequence
I very much appreciate your effort to provide an independent UEFI shell implementation. What I am worried about is that your current patches make it part of the monolithic U-Boot binary.
This design has multiple drawbacks:
The memory size available for U-Boot is very limited for many devices. We already had to disable EFI_LOADER for some boards due to this limitations. Hence we want to keep everything out of the U-Boot binary that does not serve the primary goal of loading and executing the next binary.
The UEFI forum has published a UEFI Shell specification which is very extensive. We still have a lot of deficiencies in U-Boot's UEFI API implementation. By merging in parts of an UEFI shell implementation our project looses focus. There is an EDK2 implementation of said specification. If we fix the remaining bugs of the EFI API implementation in U-Boot we could simply run the EDK2 shell which provides all that is needed for interactive work.
With you monolithic approach your UEFI shell implementation can neither be used with other UEFI API implementations than U-Boot nor can it be tested against other API implementations.
Due to these considerations I suggest that you build your UEFI shell implementation as a separate UEFI binary (like helloworld.efi). You may offer an embedding of the binary (like the bootefi hello command) into the finally linked U-Boot binary via a configuration variable. Please, put the shell implementation into a separate directory. You may want to designate yourself as maintainer (in file MAINTAINERS).
Best regards
Heinrich
cmd/nvedit.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index c0facabfc4fe..8168c963ac9d 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -27,6 +27,8 @@ #include <cli.h> #include <command.h> #include <console.h> +#include <efi.h> +#include <efi_loader.h> #include <environment.h> #include <search.h> #include <errno.h> @@ -119,6 +121,25 @@ static int do_env_print(cmd_tbl_t *cmdtp, int flag, int argc, int rcode = 0; int env_flag = H_HIDE_DOT;
+#if defined(CONFIG_CMD_EFISHELL)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
efi_status_t r;argc--;argv++;/* Initialize EFI drivers */r = efi_init_obj_list();if (r != EFI_SUCCESS) {printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;}return do_efi_dump_var(argc, argv);- }
+#endif
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'a') { argc--; argv++;
@@ -216,6 +237,26 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag) ENTRY e, *ep;
debug("Initial value for argc=%d\n", argc);
+#if defined(CONFIG_CMD_EFISHELL)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
efi_status_t r;argc--;argv++;/* Initialize EFI drivers */r = efi_init_obj_list();if (r != EFI_SUCCESS) {printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;}return do_efi_set_var(argc, argv);- }
+#endif
- while (argc > 1 && **(argv + 1) == '-') { char *arg = *++argv;
@@ -1262,15 +1303,23 @@ static char env_help_text[] = #if defined(CONFIG_CMD_IMPORTENV) "env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL)
- "env print [-a | -e [name] | name ...] - print environment\n"
+#else "env print [-a | name ...] - print environment\n" +#endif #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n" #endif #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE) "env save - save environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL)
- "env set [-e | -f] name [arg ...]\n";
+#else "env set [-f] name [arg ...]\n"; #endif +#endif
U_BOOT_CMD( env, CONFIG_SYS_MAXARGS, 1, do_env, @@ -1295,6 +1344,10 @@ U_BOOT_CMD_COMPLETE( printenv, CONFIG_SYS_MAXARGS, 1, do_env_print, "print environment variables", "[-a]\n - print [all] values of all environment variables\n" +#if defined(CONFIG_CMD_EFISHELL)
- "printenv -e [<name>]\n"
- " - print UEFI variable 'name' or all the variables\n"
+#endif "printenv name ...\n" " - print value of environment variable 'name'", var_complete @@ -1322,7 +1375,11 @@ U_BOOT_CMD_COMPLETE( U_BOOT_CMD_COMPLETE( setenv, CONFIG_SYS_MAXARGS, 0, do_env_set, "set environment variables",
- "[-f] name value ...\n"
+#if defined(CONFIG_CMD_EFISHELL)
- "-e <name> [<value>]\n"
- " - set UEFI variable 'name' to 'value' ...'\n"
+#endif
- "setenv [-f] name value ...\n" " - [forcibly] set environment variable 'name' to 'value ...'\n" "setenv [-f] name\n" " - [forcibly] delete environment variable 'name'",
@@ -1343,7 +1400,7 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", -#if defined(CONFIG_CMD_BOOTEFI) +#if defined(CONFIG_CMD_EFISHELL) "var -e [BootXXXX]\n" " - load and run UEFI app based on 'BootXXXX' UEFI variable", #else
Heinrich,
On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"env [print|set] -e" allows for handling uefi variables without knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Hello Takahiro,
in several patch series you are implementing multiple interactive commands that concern
- handling of EFI variables
- executing EFI binaries
- managing boot sequence
I very much appreciate your effort to provide an independent UEFI shell implementation. What I am worried about is that your current patches make it part of the monolithic U-Boot binary.
First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's comment on v2. So you can disable efishell command if you don't want it.
Are you still worried?
This design has multiple drawbacks:
The memory size available for U-Boot is very limited for many devices. We already had to disable EFI_LOADER for some boards due to this limitations. Hence we want to keep everything out of the U-Boot binary that does not serve the primary goal of loading and executing the next binary.
I don't know your point here. If EFI_LOADER is disabled, efishell will never be compiled in.
The UEFI forum has published a UEFI Shell specification which is very extensive. We still have a lot of deficiencies in U-Boot's UEFI API implementation. By merging in parts of an UEFI shell implementation our project looses focus.
What is "our project?" What is "focus?" I'm just asking as I want to share that information with you.
There is an EDK2 implementation of said specification. If we fix the remaining bugs of the EFI API implementation in U-Boot we could simply run the EDK2 shell which provides all that is needed for interactive work.
With you monolithic approach your UEFI shell implementation can neither be used with other UEFI API implementations than U-Boot nor can it be tested against other API implementations.
Let me explain my stance. My efishell is basically something like a pursuit as well as a debug/test tool which was and is still quite useful for me. Without it, I would have completed (most of) my efi-related work so far. So I believe that it will also be useful for other people who may want to get involved and play with u-boot's efi environment.
I have never intended to fully implement a shell which is to be compliant with UEFI specification while I'm trying to mimick some command interfaces for convenience. UEFI shell, as you know, provides plenty of "protocols" on which some UEFI applications, including UEFI SCT, reply. I will never implement it with my efishell.
I hope that my efishell is a quick and easy way of learning more about u-boot's uefi environment. I will be even happier if more people get involved there.
Due to these considerations I suggest that you build your UEFI shell implementation as a separate UEFI binary (like helloworld.efi). You may offer an embedding of the binary (like the bootefi hello command) into the finally linked U-Boot binary via a configuration variable. Please, put the shell implementation into a separate directory. You may want to designate yourself as maintainer (in file MAINTAINERS).
Yeah, your suggestion is reasonable and I have thought of it before. There are, however, several reasons that I haven't done so; particularly, efishell is implemented not only with boottime services but also other helper functions, say, from device path utilities. Exporting them as libraries is possible but I don't think that it would be so valuable.
Even if efishell is a separate application, it will not contribute to reduce the total footprint if it is embedded along with u-boot binary.
Thanks, -Takahiro Akashi
Best regards
Heinrich
cmd/nvedit.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index c0facabfc4fe..8168c963ac9d 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -27,6 +27,8 @@ #include <cli.h> #include <command.h> #include <console.h> +#include <efi.h> +#include <efi_loader.h> #include <environment.h> #include <search.h> #include <errno.h> @@ -119,6 +121,25 @@ static int do_env_print(cmd_tbl_t *cmdtp, int flag, int argc, int rcode = 0; int env_flag = H_HIDE_DOT;
+#if defined(CONFIG_CMD_EFISHELL)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
efi_status_t r;argc--;argv++;/* Initialize EFI drivers */r = efi_init_obj_list();if (r != EFI_SUCCESS) {printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;}return do_efi_dump_var(argc, argv);- }
+#endif
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'a') { argc--; argv++;
@@ -216,6 +237,26 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag) ENTRY e, *ep;
debug("Initial value for argc=%d\n", argc);
+#if defined(CONFIG_CMD_EFISHELL)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
efi_status_t r;argc--;argv++;/* Initialize EFI drivers */r = efi_init_obj_list();if (r != EFI_SUCCESS) {printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;}return do_efi_set_var(argc, argv);- }
+#endif
- while (argc > 1 && **(argv + 1) == '-') { char *arg = *++argv;
@@ -1262,15 +1303,23 @@ static char env_help_text[] = #if defined(CONFIG_CMD_IMPORTENV) "env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL)
- "env print [-a | -e [name] | name ...] - print environment\n"
+#else "env print [-a | name ...] - print environment\n" +#endif #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n" #endif #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE) "env save - save environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL)
- "env set [-e | -f] name [arg ...]\n";
+#else "env set [-f] name [arg ...]\n"; #endif +#endif
U_BOOT_CMD( env, CONFIG_SYS_MAXARGS, 1, do_env, @@ -1295,6 +1344,10 @@ U_BOOT_CMD_COMPLETE( printenv, CONFIG_SYS_MAXARGS, 1, do_env_print, "print environment variables", "[-a]\n - print [all] values of all environment variables\n" +#if defined(CONFIG_CMD_EFISHELL)
- "printenv -e [<name>]\n"
- " - print UEFI variable 'name' or all the variables\n"
+#endif "printenv name ...\n" " - print value of environment variable 'name'", var_complete @@ -1322,7 +1375,11 @@ U_BOOT_CMD_COMPLETE( U_BOOT_CMD_COMPLETE( setenv, CONFIG_SYS_MAXARGS, 0, do_env_set, "set environment variables",
- "[-f] name value ...\n"
+#if defined(CONFIG_CMD_EFISHELL)
- "-e <name> [<value>]\n"
- " - set UEFI variable 'name' to 'value' ...'\n"
+#endif
- "setenv [-f] name value ...\n" " - [forcibly] set environment variable 'name' to 'value ...'\n" "setenv [-f] name\n" " - [forcibly] delete environment variable 'name'",
@@ -1343,7 +1400,7 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", -#if defined(CONFIG_CMD_BOOTEFI) +#if defined(CONFIG_CMD_EFISHELL) "var -e [BootXXXX]\n" " - load and run UEFI app based on 'BootXXXX' UEFI variable", #else
On 12/19/18 2:49 AM, AKASHI Takahiro wrote:
Heinrich,
On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"env [print|set] -e" allows for handling uefi variables without knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Hello Takahiro,
in several patch series you are implementing multiple interactive commands that concern
- handling of EFI variables
- executing EFI binaries
- managing boot sequence
I very much appreciate your effort to provide an independent UEFI shell implementation. What I am worried about is that your current patches make it part of the monolithic U-Boot binary.
First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's comment on v2. So you can disable efishell command if you don't want it.
Are you still worried?
This design has multiple drawbacks:
The memory size available for U-Boot is very limited for many devices. We already had to disable EFI_LOADER for some boards due to this limitations. Hence we want to keep everything out of the U-Boot binary that does not serve the primary goal of loading and executing the next binary.
I don't know your point here. If EFI_LOADER is disabled, efishell will never be compiled in.
The UEFI forum has published a UEFI Shell specification which is very extensive. We still have a lot of deficiencies in U-Boot's UEFI API implementation. By merging in parts of an UEFI shell implementation our project looses focus.
What is "our project?" What is "focus?" I'm just asking as I want to share that information with you.
There is an EDK2 implementation of said specification. If we fix the remaining bugs of the EFI API implementation in U-Boot we could simply run the EDK2 shell which provides all that is needed for interactive work.
With you monolithic approach your UEFI shell implementation can neither be used with other UEFI API implementations than U-Boot nor can it be tested against other API implementations.
Let me explain my stance. My efishell is basically something like a pursuit as well as a debug/test tool which was and is still quite useful for me. Without it, I would have completed (most of) my efi-related work so far. So I believe that it will also be useful for other people who may want to get involved and play with u-boot's efi environment.
On SD-Cards U-Boot is installed between the MBR and the first partition. On other devices it is put into a very small ROM. Both ways the maximum size is rather limited.
U-Boot provides all that is needed to load and execute an EFI binary. So you can put your efishell as file into the EFI partition like you would install the EDK2 shell.
The only hardshift this approach brings is that you have to implement your own printf because UEFI does not offer formatted output. But this can be copied from lib/efi_selftest/efi_selftest_console.c.
The same decision I took for booting from iSCSI. I did not try to put an iSCSI driver into U-Boot instead I use iPXE as an executable that is loaded from the EFI partition.
I have never intended to fully implement a shell which is to be compliant with UEFI specification while I'm trying to mimick some command interfaces for convenience. UEFI shell, as you know, provides plenty of "protocols" on which some UEFI applications, including UEFI SCT, reply. I will never implement it with my efishell.
I hope that my efishell is a quick and easy way of learning more about u-boot's uefi environment. I will be even happier if more people get involved there.
Due to these considerations I suggest that you build your UEFI shell implementation as a separate UEFI binary (like helloworld.efi). You may offer an embedding of the binary (like the bootefi hello command) into the finally linked U-Boot binary via a configuration variable. Please, put the shell implementation into a separate directory. You may want to designate yourself as maintainer (in file MAINTAINERS).
Yeah, your suggestion is reasonable and I have thought of it before. There are, however, several reasons that I haven't done so; particularly, efishell is implemented not only with boottime services but also other helper functions, say, from device path utilities. Exporting them as libraries is possible but I don't think that it would be so valuable.
Even if efishell is a separate application, it will not contribute to reduce the total footprint if it is embedded along with u-boot binary.
That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into the U-Boot binary - is default no. Same I would do for efishell.efi.
Best regards
Heinrich
Thanks, -Takahiro Akashi
Best regards
Heinrich
cmd/nvedit.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index c0facabfc4fe..8168c963ac9d 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -27,6 +27,8 @@ #include <cli.h> #include <command.h> #include <console.h> +#include <efi.h> +#include <efi_loader.h> #include <environment.h> #include <search.h> #include <errno.h> @@ -119,6 +121,25 @@ static int do_env_print(cmd_tbl_t *cmdtp, int flag, int argc, int rcode = 0; int env_flag = H_HIDE_DOT;
+#if defined(CONFIG_CMD_EFISHELL)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
efi_status_t r;argc--;argv++;/* Initialize EFI drivers */r = efi_init_obj_list();if (r != EFI_SUCCESS) {printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;}return do_efi_dump_var(argc, argv);- }
+#endif
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'a') { argc--; argv++;
@@ -216,6 +237,26 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag) ENTRY e, *ep;
debug("Initial value for argc=%d\n", argc);
+#if defined(CONFIG_CMD_EFISHELL)
- if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
efi_status_t r;argc--;argv++;/* Initialize EFI drivers */r = efi_init_obj_list();if (r != EFI_SUCCESS) {printf("Error: Cannot set up EFI drivers, r = %lu\n",r & ~EFI_ERROR_MASK);return CMD_RET_FAILURE;}return do_efi_set_var(argc, argv);- }
+#endif
- while (argc > 1 && **(argv + 1) == '-') { char *arg = *++argv;
@@ -1262,15 +1303,23 @@ static char env_help_text[] = #if defined(CONFIG_CMD_IMPORTENV) "env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL)
- "env print [-a | -e [name] | name ...] - print environment\n"
+#else "env print [-a | name ...] - print environment\n" +#endif #if defined(CONFIG_CMD_RUN) "env run var [...] - run commands in an environment variable\n" #endif #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE) "env save - save environment\n" #endif +#if defined(CONFIG_CMD_EFISHELL)
- "env set [-e | -f] name [arg ...]\n";
+#else "env set [-f] name [arg ...]\n"; #endif +#endif
U_BOOT_CMD( env, CONFIG_SYS_MAXARGS, 1, do_env, @@ -1295,6 +1344,10 @@ U_BOOT_CMD_COMPLETE( printenv, CONFIG_SYS_MAXARGS, 1, do_env_print, "print environment variables", "[-a]\n - print [all] values of all environment variables\n" +#if defined(CONFIG_CMD_EFISHELL)
- "printenv -e [<name>]\n"
- " - print UEFI variable 'name' or all the variables\n"
+#endif "printenv name ...\n" " - print value of environment variable 'name'", var_complete @@ -1322,7 +1375,11 @@ U_BOOT_CMD_COMPLETE( U_BOOT_CMD_COMPLETE( setenv, CONFIG_SYS_MAXARGS, 0, do_env_set, "set environment variables",
- "[-f] name value ...\n"
+#if defined(CONFIG_CMD_EFISHELL)
- "-e <name> [<value>]\n"
- " - set UEFI variable 'name' to 'value' ...'\n"
+#endif
- "setenv [-f] name value ...\n" " - [forcibly] set environment variable 'name' to 'value ...'\n" "setenv [-f] name\n" " - [forcibly] delete environment variable 'name'",
@@ -1343,7 +1400,7 @@ U_BOOT_CMD( U_BOOT_CMD_COMPLETE( run, CONFIG_SYS_MAXARGS, 1, do_run, "run commands in an environment variable", -#if defined(CONFIG_CMD_BOOTEFI) +#if defined(CONFIG_CMD_EFISHELL) "var -e [BootXXXX]\n" " - load and run UEFI app based on 'BootXXXX' UEFI variable", #else
On 19.12.18 13:23, Heinrich Schuchardt wrote:
On 12/19/18 2:49 AM, AKASHI Takahiro wrote:
Heinrich,
On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"env [print|set] -e" allows for handling uefi variables without knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Hello Takahiro,
in several patch series you are implementing multiple interactive commands that concern
- handling of EFI variables
- executing EFI binaries
- managing boot sequence
I very much appreciate your effort to provide an independent UEFI shell implementation. What I am worried about is that your current patches make it part of the monolithic U-Boot binary.
First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's comment on v2. So you can disable efishell command if you don't want it.
Are you still worried?
This design has multiple drawbacks:
The memory size available for U-Boot is very limited for many devices. We already had to disable EFI_LOADER for some boards due to this limitations. Hence we want to keep everything out of the U-Boot binary that does not serve the primary goal of loading and executing the next binary.
I don't know your point here. If EFI_LOADER is disabled, efishell will never be compiled in.
The UEFI forum has published a UEFI Shell specification which is very extensive. We still have a lot of deficiencies in U-Boot's UEFI API implementation. By merging in parts of an UEFI shell implementation our project looses focus.
What is "our project?" What is "focus?" I'm just asking as I want to share that information with you.
There is an EDK2 implementation of said specification. If we fix the remaining bugs of the EFI API implementation in U-Boot we could simply run the EDK2 shell which provides all that is needed for interactive work.
With you monolithic approach your UEFI shell implementation can neither be used with other UEFI API implementations than U-Boot nor can it be tested against other API implementations.
Let me explain my stance. My efishell is basically something like a pursuit as well as a debug/test tool which was and is still quite useful for me. Without it, I would have completed (most of) my efi-related work so far. So I believe that it will also be useful for other people who may want to get involved and play with u-boot's efi environment.
On SD-Cards U-Boot is installed between the MBR and the first partition. On other devices it is put into a very small ROM. Both ways the maximum size is rather limited.
U-Boot provides all that is needed to load and execute an EFI binary. So you can put your efishell as file into the EFI partition like you would install the EDK2 shell.
The only hardshift this approach brings is that you have to implement your own printf because UEFI does not offer formatted output. But this can be copied from lib/efi_selftest/efi_selftest_console.c.
The same decision I took for booting from iSCSI. I did not try to put an iSCSI driver into U-Boot instead I use iPXE as an executable that is loaded from the EFI partition.
I have never intended to fully implement a shell which is to be compliant with UEFI specification while I'm trying to mimick some command interfaces for convenience. UEFI shell, as you know, provides plenty of "protocols" on which some UEFI applications, including UEFI SCT, reply. I will never implement it with my efishell.
I hope that my efishell is a quick and easy way of learning more about u-boot's uefi environment. I will be even happier if more people get involved there.
Due to these considerations I suggest that you build your UEFI shell implementation as a separate UEFI binary (like helloworld.efi). You may offer an embedding of the binary (like the bootefi hello command) into the finally linked U-Boot binary via a configuration variable. Please, put the shell implementation into a separate directory. You may want to designate yourself as maintainer (in file MAINTAINERS).
Yeah, your suggestion is reasonable and I have thought of it before. There are, however, several reasons that I haven't done so; particularly, efishell is implemented not only with boottime services but also other helper functions, say, from device path utilities. Exporting them as libraries is possible but I don't think that it would be so valuable.
Even if efishell is a separate application, it will not contribute to reduce the total footprint if it is embedded along with u-boot binary.
That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into the U-Boot binary - is default no. Same I would do for efishell.efi.
One big drawback with a separate binary is the missing command line integration. It becomes quite awkward to execute efi debug commands then, since you'll have to run them through a special bootefi subcommand.
If you really want to have a "uefi shell", I think the sanest option is to just provide a built-in copy of the edk2 uefi shell, similar to the hello world binary. The big benefit of this patch set however, is not that we get a shell - it's that we get quick and tiny debug introspectability into efi_loader data structures.
I think the biggest problem here really is the name of the code. Why don't we just call it "debugefi"? It would be default N except for debug targets (just like bootefi_hello).
That way when someone wants to just quickly introspect internal data structures, they can. I also hope that if the name contains debug, nobody will expect command line compatibility going forward, so we have much more freedom to change internals (which is my biggest concern).
So in my opinion, if you fix the 2 other comments from Heinrich and rename everything from "efishell" to "debugefi" (so it aligns with bootefi), we should be good.
Alex
On Sun, Dec 23, 2018 at 02:56:40AM +0100, Alexander Graf wrote:
On 19.12.18 13:23, Heinrich Schuchardt wrote:
On 12/19/18 2:49 AM, AKASHI Takahiro wrote:
Heinrich,
On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"env [print|set] -e" allows for handling uefi variables without knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Hello Takahiro,
in several patch series you are implementing multiple interactive commands that concern
- handling of EFI variables
- executing EFI binaries
- managing boot sequence
I very much appreciate your effort to provide an independent UEFI shell implementation. What I am worried about is that your current patches make it part of the monolithic U-Boot binary.
First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's comment on v2. So you can disable efishell command if you don't want it.
Are you still worried?
This design has multiple drawbacks:
The memory size available for U-Boot is very limited for many devices. We already had to disable EFI_LOADER for some boards due to this limitations. Hence we want to keep everything out of the U-Boot binary that does not serve the primary goal of loading and executing the next binary.
I don't know your point here. If EFI_LOADER is disabled, efishell will never be compiled in.
The UEFI forum has published a UEFI Shell specification which is very extensive. We still have a lot of deficiencies in U-Boot's UEFI API implementation. By merging in parts of an UEFI shell implementation our project looses focus.
What is "our project?" What is "focus?" I'm just asking as I want to share that information with you.
There is an EDK2 implementation of said specification. If we fix the remaining bugs of the EFI API implementation in U-Boot we could simply run the EDK2 shell which provides all that is needed for interactive work.
With you monolithic approach your UEFI shell implementation can neither be used with other UEFI API implementations than U-Boot nor can it be tested against other API implementations.
Let me explain my stance. My efishell is basically something like a pursuit as well as a debug/test tool which was and is still quite useful for me. Without it, I would have completed (most of) my efi-related work so far. So I believe that it will also be useful for other people who may want to get involved and play with u-boot's efi environment.
On SD-Cards U-Boot is installed between the MBR and the first partition. On other devices it is put into a very small ROM. Both ways the maximum size is rather limited.
U-Boot provides all that is needed to load and execute an EFI binary. So you can put your efishell as file into the EFI partition like you would install the EDK2 shell.
The only hardshift this approach brings is that you have to implement your own printf because UEFI does not offer formatted output. But this can be copied from lib/efi_selftest/efi_selftest_console.c.
The same decision I took for booting from iSCSI. I did not try to put an iSCSI driver into U-Boot instead I use iPXE as an executable that is loaded from the EFI partition.
I have never intended to fully implement a shell which is to be compliant with UEFI specification while I'm trying to mimick some command interfaces for convenience. UEFI shell, as you know, provides plenty of "protocols" on which some UEFI applications, including UEFI SCT, reply. I will never implement it with my efishell.
I hope that my efishell is a quick and easy way of learning more about u-boot's uefi environment. I will be even happier if more people get involved there.
Due to these considerations I suggest that you build your UEFI shell implementation as a separate UEFI binary (like helloworld.efi). You may offer an embedding of the binary (like the bootefi hello command) into the finally linked U-Boot binary via a configuration variable. Please, put the shell implementation into a separate directory. You may want to designate yourself as maintainer (in file MAINTAINERS).
Yeah, your suggestion is reasonable and I have thought of it before. There are, however, several reasons that I haven't done so; particularly, efishell is implemented not only with boottime services but also other helper functions, say, from device path utilities. Exporting them as libraries is possible but I don't think that it would be so valuable.
Even if efishell is a separate application, it will not contribute to reduce the total footprint if it is embedded along with u-boot binary.
That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into the U-Boot binary - is default no. Same I would do for efishell.efi.
One big drawback with a separate binary is the missing command line integration. It becomes quite awkward to execute efi debug commands then, since you'll have to run them through a special bootefi subcommand.
If you really want to have a "uefi shell", I think the sanest option is to just provide a built-in copy of the edk2 uefi shell, similar to the hello world binary. The big benefit of this patch set however, is not that we get a shell - it's that we get quick and tiny debug introspectability into efi_loader data structures.
And my command can be used for simple testing.
I think the biggest problem here really is the name of the code. Why don't we just call it "debugefi"? It would be default N except for debug targets (just like bootefi_hello).
That way when someone wants to just quickly introspect internal data structures, they can. I also hope that if the name contains debug, nobody will expect command line compatibility going forward, so we have much more freedom to change internals (which is my biggest concern).
So in my opinion, if you fix the 2 other comments from Heinrich and rename everything from "efishell" to "debugefi" (so it aligns with bootefi), we should be good.
If Heinrich agrees, I will fix the name although I'm not a super fan of this new name :)
Thanks, -Takahiro Akashi
Alex
On 25.12.18 09:44, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 02:56:40AM +0100, Alexander Graf wrote:
On 19.12.18 13:23, Heinrich Schuchardt wrote:
On 12/19/18 2:49 AM, AKASHI Takahiro wrote:
Heinrich,
On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
"env [print|set] -e" allows for handling uefi variables without knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Hello Takahiro,
in several patch series you are implementing multiple interactive commands that concern
- handling of EFI variables
- executing EFI binaries
- managing boot sequence
I very much appreciate your effort to provide an independent UEFI shell implementation. What I am worried about is that your current patches make it part of the monolithic U-Boot binary.
First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's comment on v2. So you can disable efishell command if you don't want it.
Are you still worried?
This design has multiple drawbacks:
The memory size available for U-Boot is very limited for many devices. We already had to disable EFI_LOADER for some boards due to this limitations. Hence we want to keep everything out of the U-Boot binary that does not serve the primary goal of loading and executing the next binary.
I don't know your point here. If EFI_LOADER is disabled, efishell will never be compiled in.
The UEFI forum has published a UEFI Shell specification which is very extensive. We still have a lot of deficiencies in U-Boot's UEFI API implementation. By merging in parts of an UEFI shell implementation our project looses focus.
What is "our project?" What is "focus?" I'm just asking as I want to share that information with you.
There is an EDK2 implementation of said specification. If we fix the remaining bugs of the EFI API implementation in U-Boot we could simply run the EDK2 shell which provides all that is needed for interactive work.
With you monolithic approach your UEFI shell implementation can neither be used with other UEFI API implementations than U-Boot nor can it be tested against other API implementations.
Let me explain my stance. My efishell is basically something like a pursuit as well as a debug/test tool which was and is still quite useful for me. Without it, I would have completed (most of) my efi-related work so far. So I believe that it will also be useful for other people who may want to get involved and play with u-boot's efi environment.
On SD-Cards U-Boot is installed between the MBR and the first partition. On other devices it is put into a very small ROM. Both ways the maximum size is rather limited.
U-Boot provides all that is needed to load and execute an EFI binary. So you can put your efishell as file into the EFI partition like you would install the EDK2 shell.
The only hardshift this approach brings is that you have to implement your own printf because UEFI does not offer formatted output. But this can be copied from lib/efi_selftest/efi_selftest_console.c.
The same decision I took for booting from iSCSI. I did not try to put an iSCSI driver into U-Boot instead I use iPXE as an executable that is loaded from the EFI partition.
I have never intended to fully implement a shell which is to be compliant with UEFI specification while I'm trying to mimick some command interfaces for convenience. UEFI shell, as you know, provides plenty of "protocols" on which some UEFI applications, including UEFI SCT, reply. I will never implement it with my efishell.
I hope that my efishell is a quick and easy way of learning more about u-boot's uefi environment. I will be even happier if more people get involved there.
Due to these considerations I suggest that you build your UEFI shell implementation as a separate UEFI binary (like helloworld.efi). You may offer an embedding of the binary (like the bootefi hello command) into the finally linked U-Boot binary via a configuration variable. Please, put the shell implementation into a separate directory. You may want to designate yourself as maintainer (in file MAINTAINERS).
Yeah, your suggestion is reasonable and I have thought of it before. There are, however, several reasons that I haven't done so; particularly, efishell is implemented not only with boottime services but also other helper functions, say, from device path utilities. Exporting them as libraries is possible but I don't think that it would be so valuable.
Even if efishell is a separate application, it will not contribute to reduce the total footprint if it is embedded along with u-boot binary.
That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into the U-Boot binary - is default no. Same I would do for efishell.efi.
One big drawback with a separate binary is the missing command line integration. It becomes quite awkward to execute efi debug commands then, since you'll have to run them through a special bootefi subcommand.
If you really want to have a "uefi shell", I think the sanest option is to just provide a built-in copy of the edk2 uefi shell, similar to the hello world binary. The big benefit of this patch set however, is not that we get a shell - it's that we get quick and tiny debug introspectability into efi_loader data structures.
And my command can be used for simple testing.
Exactly, that would give us the best of both worlds.
I think the biggest problem here really is the name of the code. Why don't we just call it "debugefi"? It would be default N except for debug targets (just like bootefi_hello).
That way when someone wants to just quickly introspect internal data structures, they can. I also hope that if the name contains debug, nobody will expect command line compatibility going forward, so we have much more freedom to change internals (which is my biggest concern).
So in my opinion, if you fix the 2 other comments from Heinrich and rename everything from "efishell" to "debugefi" (so it aligns with bootefi), we should be good.
If Heinrich agrees, I will fix the name although I'm not a super fan of this new name :)
Well, feel free to come up with a new one, but it definitely must have a ring to it that it's a tiny, debug only feature that is not intended for normal use ;).
For normal operation, we need to come up with mechanisms that integrate much deeper into U-Boot's generic command structure.
Alex
Heinrich,
On Wed, Dec 26, 2018 at 10:20:32PM +0100, Alexander Graf wrote:
On 25.12.18 09:44, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 02:56:40AM +0100, Alexander Graf wrote:
On 19.12.18 13:23, Heinrich Schuchardt wrote:
On 12/19/18 2:49 AM, AKASHI Takahiro wrote:
Heinrich,
On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote:
On 12/18/18 6:05 AM, AKASHI Takahiro wrote: > "env [print|set] -e" allows for handling uefi variables without > knowing details about mapping to corresponding u-boot variables. > > Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Hello Takahiro,
in several patch series you are implementing multiple interactive commands that concern
- handling of EFI variables
- executing EFI binaries
- managing boot sequence
I very much appreciate your effort to provide an independent UEFI shell implementation. What I am worried about is that your current patches make it part of the monolithic U-Boot binary.
First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's comment on v2. So you can disable efishell command if you don't want it.
Are you still worried?
This design has multiple drawbacks:
The memory size available for U-Boot is very limited for many devices. We already had to disable EFI_LOADER for some boards due to this limitations. Hence we want to keep everything out of the U-Boot binary that does not serve the primary goal of loading and executing the next binary.
I don't know your point here. If EFI_LOADER is disabled, efishell will never be compiled in.
The UEFI forum has published a UEFI Shell specification which is very extensive. We still have a lot of deficiencies in U-Boot's UEFI API implementation. By merging in parts of an UEFI shell implementation our project looses focus.
What is "our project?" What is "focus?" I'm just asking as I want to share that information with you.
There is an EDK2 implementation of said specification. If we fix the remaining bugs of the EFI API implementation in U-Boot we could simply run the EDK2 shell which provides all that is needed for interactive work.
With you monolithic approach your UEFI shell implementation can neither be used with other UEFI API implementations than U-Boot nor can it be tested against other API implementations.
Let me explain my stance. My efishell is basically something like a pursuit as well as a debug/test tool which was and is still quite useful for me. Without it, I would have completed (most of) my efi-related work so far. So I believe that it will also be useful for other people who may want to get involved and play with u-boot's efi environment.
On SD-Cards U-Boot is installed between the MBR and the first partition. On other devices it is put into a very small ROM. Both ways the maximum size is rather limited.
U-Boot provides all that is needed to load and execute an EFI binary. So you can put your efishell as file into the EFI partition like you would install the EDK2 shell.
The only hardshift this approach brings is that you have to implement your own printf because UEFI does not offer formatted output. But this can be copied from lib/efi_selftest/efi_selftest_console.c.
The same decision I took for booting from iSCSI. I did not try to put an iSCSI driver into U-Boot instead I use iPXE as an executable that is loaded from the EFI partition.
I have never intended to fully implement a shell which is to be compliant with UEFI specification while I'm trying to mimick some command interfaces for convenience. UEFI shell, as you know, provides plenty of "protocols" on which some UEFI applications, including UEFI SCT, reply. I will never implement it with my efishell.
I hope that my efishell is a quick and easy way of learning more about u-boot's uefi environment. I will be even happier if more people get involved there.
Due to these considerations I suggest that you build your UEFI shell implementation as a separate UEFI binary (like helloworld.efi). You may offer an embedding of the binary (like the bootefi hello command) into the finally linked U-Boot binary via a configuration variable. Please, put the shell implementation into a separate directory. You may want to designate yourself as maintainer (in file MAINTAINERS).
Yeah, your suggestion is reasonable and I have thought of it before. There are, however, several reasons that I haven't done so; particularly, efishell is implemented not only with boottime services but also other helper functions, say, from device path utilities. Exporting them as libraries is possible but I don't think that it would be so valuable.
Even if efishell is a separate application, it will not contribute to reduce the total footprint if it is embedded along with u-boot binary.
That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into the U-Boot binary - is default no. Same I would do for efishell.efi.
One big drawback with a separate binary is the missing command line integration. It becomes quite awkward to execute efi debug commands then, since you'll have to run them through a special bootefi subcommand.
If you really want to have a "uefi shell", I think the sanest option is to just provide a built-in copy of the edk2 uefi shell, similar to the hello world binary. The big benefit of this patch set however, is not that we get a shell - it's that we get quick and tiny debug introspectability into efi_loader data structures.
And my command can be used for simple testing.
Exactly, that would give us the best of both worlds.
I think the biggest problem here really is the name of the code. Why don't we just call it "debugefi"? It would be default N except for debug targets (just like bootefi_hello).
That way when someone wants to just quickly introspect internal data structures, they can. I also hope that if the name contains debug, nobody will expect command line compatibility going forward, so we have much more freedom to change internals (which is my biggest concern).
So in my opinion, if you fix the 2 other comments from Heinrich and rename everything from "efishell" to "debugefi" (so it aligns with bootefi), we should be good.
If Heinrich agrees, I will fix the name although I'm not a super fan of this new name :)
Well, feel free to come up with a new one, but it definitely must have a ring to it that it's a tiny, debug only feature that is not intended for normal use ;).
Do you have any idea/preference about this command's name?
-Takahiro Akashi
For normal operation, we need to come up with mechanisms that integrate much deeper into U-Boot's generic command structure.
Alex
On Mon, Jan 07, 2019 at 04:47:13PM +0900, AKASHI Takahiro wrote:
Heinrich,
On Wed, Dec 26, 2018 at 10:20:32PM +0100, Alexander Graf wrote:
On 25.12.18 09:44, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 02:56:40AM +0100, Alexander Graf wrote:
On 19.12.18 13:23, Heinrich Schuchardt wrote:
On 12/19/18 2:49 AM, AKASHI Takahiro wrote:
Heinrich,
On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote: > On 12/18/18 6:05 AM, AKASHI Takahiro wrote: >> "env [print|set] -e" allows for handling uefi variables without >> knowing details about mapping to corresponding u-boot variables. >> >> Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org > > Hello Takahiro, > > in several patch series you are implementing multiple interactive > commands that concern > > - handling of EFI variables > - executing EFI binaries > - managing boot sequence > > I very much appreciate your effort to provide an independent UEFI shell > implementation. What I am worried about is that your current patches > make it part of the monolithic U-Boot binary.
First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's comment on v2. So you can disable efishell command if you don't want it.
Are you still worried?
> This design has multiple drawbacks: > > The memory size available for U-Boot is very limited for many devices. > We already had to disable EFI_LOADER for some boards due to this > limitations. Hence we want to keep everything out of the U-Boot binary > that does not serve the primary goal of loading and executing the next > binary.
I don't know your point here. If EFI_LOADER is disabled, efishell will never be compiled in.
> The UEFI forum has published a UEFI Shell specification which is very > extensive. We still have a lot of deficiencies in U-Boot's UEFI API > implementation. By merging in parts of an UEFI shell implementation our > project looses focus.
What is "our project?" What is "focus?" I'm just asking as I want to share that information with you.
> There is an EDK2 implementation of said > specification. If we fix the remaining bugs of the EFI API > implementation in U-Boot we could simply run the EDK2 shell which > provides all that is needed for interactive work. > > With you monolithic approach your UEFI shell implementation can neither > be used with other UEFI API implementations than U-Boot nor can it be > tested against other API implementations.
Let me explain my stance. My efishell is basically something like a pursuit as well as a debug/test tool which was and is still quite useful for me. Without it, I would have completed (most of) my efi-related work so far. So I believe that it will also be useful for other people who may want to get involved and play with u-boot's efi environment.
On SD-Cards U-Boot is installed between the MBR and the first partition. On other devices it is put into a very small ROM. Both ways the maximum size is rather limited.
U-Boot provides all that is needed to load and execute an EFI binary. So you can put your efishell as file into the EFI partition like you would install the EDK2 shell.
The only hardshift this approach brings is that you have to implement your own printf because UEFI does not offer formatted output. But this can be copied from lib/efi_selftest/efi_selftest_console.c.
The same decision I took for booting from iSCSI. I did not try to put an iSCSI driver into U-Boot instead I use iPXE as an executable that is loaded from the EFI partition.
I have never intended to fully implement a shell which is to be compliant with UEFI specification while I'm trying to mimick some command interfaces for convenience. UEFI shell, as you know, provides plenty of "protocols" on which some UEFI applications, including UEFI SCT, reply. I will never implement it with my efishell.
I hope that my efishell is a quick and easy way of learning more about u-boot's uefi environment. I will be even happier if more people get involved there.
> Due to these considerations I suggest that you build your UEFI shell > implementation as a separate UEFI binary (like helloworld.efi). You may > offer an embedding of the binary (like the bootefi hello command) into > the finally linked U-Boot binary via a configuration variable. Please, > put the shell implementation into a separate directory. You may want to > designate yourself as maintainer (in file MAINTAINERS).
Yeah, your suggestion is reasonable and I have thought of it before. There are, however, several reasons that I haven't done so; particularly, efishell is implemented not only with boottime services but also other helper functions, say, from device path utilities. Exporting them as libraries is possible but I don't think that it would be so valuable.
Even if efishell is a separate application, it will not contribute to reduce the total footprint if it is embedded along with u-boot binary.
That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into the U-Boot binary - is default no. Same I would do for efishell.efi.
One big drawback with a separate binary is the missing command line integration. It becomes quite awkward to execute efi debug commands then, since you'll have to run them through a special bootefi subcommand.
If you really want to have a "uefi shell", I think the sanest option is to just provide a built-in copy of the edk2 uefi shell, similar to the hello world binary. The big benefit of this patch set however, is not that we get a shell - it's that we get quick and tiny debug introspectability into efi_loader data structures.
And my command can be used for simple testing.
Exactly, that would give us the best of both worlds.
I think the biggest problem here really is the name of the code. Why don't we just call it "debugefi"? It would be default N except for debug targets (just like bootefi_hello).
That way when someone wants to just quickly introspect internal data structures, they can. I also hope that if the name contains debug, nobody will expect command line compatibility going forward, so we have much more freedom to change internals (which is my biggest concern).
So in my opinion, if you fix the 2 other comments from Heinrich and rename everything from "efishell" to "debugefi" (so it aligns with bootefi), we should be good.
If Heinrich agrees, I will fix the name although I'm not a super fan of this new name :)
Well, feel free to come up with a new one, but it definitely must have a ring to it that it's a tiny, debug only feature that is not intended for normal use ;).
Do you have any idea/preference about this command's name?
I prefer efidebug/efidbg or efitool so that we can use a shorthand name, efi, at command line in most cases.
-Takahiro Akashi
-Takahiro Akashi
For normal operation, we need to come up with mechanisms that integrate much deeper into U-Boot's generic command structure.
Alex
On 08.01.19 08:29, AKASHI Takahiro wrote:
On Mon, Jan 07, 2019 at 04:47:13PM +0900, AKASHI Takahiro wrote:
Heinrich,
On Wed, Dec 26, 2018 at 10:20:32PM +0100, Alexander Graf wrote:
On 25.12.18 09:44, AKASHI Takahiro wrote:
On Sun, Dec 23, 2018 at 02:56:40AM +0100, Alexander Graf wrote:
On 19.12.18 13:23, Heinrich Schuchardt wrote:
On 12/19/18 2:49 AM, AKASHI Takahiro wrote: > Heinrich, > > On Tue, Dec 18, 2018 at 07:07:02AM +0100, Heinrich Schuchardt wrote: >> On 12/18/18 6:05 AM, AKASHI Takahiro wrote: >>> "env [print|set] -e" allows for handling uefi variables without >>> knowing details about mapping to corresponding u-boot variables. >>> >>> Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org >> >> Hello Takahiro, >> >> in several patch series you are implementing multiple interactive >> commands that concern >> >> - handling of EFI variables >> - executing EFI binaries >> - managing boot sequence >> >> I very much appreciate your effort to provide an independent UEFI shell >> implementation. What I am worried about is that your current patches >> make it part of the monolithic U-Boot binary. > > First of all, in v3, CONFIG_CMD_EFISHELL was introduced after Alex's > comment on v2. So you can disable efishell command if you don't want it. > > Are you still worried? > >> This design has multiple drawbacks: >> >> The memory size available for U-Boot is very limited for many devices. >> We already had to disable EFI_LOADER for some boards due to this >> limitations. Hence we want to keep everything out of the U-Boot binary >> that does not serve the primary goal of loading and executing the next >> binary. > > I don't know your point here. If EFI_LOADER is disabled, efishell > will never be compiled in. > >> The UEFI forum has published a UEFI Shell specification which is very >> extensive. We still have a lot of deficiencies in U-Boot's UEFI API >> implementation. By merging in parts of an UEFI shell implementation our >> project looses focus. > > What is "our project?" What is "focus?" > I'm just asking as I want to share that information with you. > >> There is an EDK2 implementation of said >> specification. If we fix the remaining bugs of the EFI API >> implementation in U-Boot we could simply run the EDK2 shell which >> provides all that is needed for interactive work. >> >> With you monolithic approach your UEFI shell implementation can neither >> be used with other UEFI API implementations than U-Boot nor can it be >> tested against other API implementations. > > Let me explain my stance. > My efishell is basically something like a pursuit as well as > a debug/test tool which was and is still quite useful for me. > Without it, I would have completed (most of) my efi-related work so far. > So I believe that it will also be useful for other people who may want > to get involved and play with u-boot's efi environment.
On SD-Cards U-Boot is installed between the MBR and the first partition. On other devices it is put into a very small ROM. Both ways the maximum size is rather limited.
U-Boot provides all that is needed to load and execute an EFI binary. So you can put your efishell as file into the EFI partition like you would install the EDK2 shell.
The only hardshift this approach brings is that you have to implement your own printf because UEFI does not offer formatted output. But this can be copied from lib/efi_selftest/efi_selftest_console.c.
The same decision I took for booting from iSCSI. I did not try to put an iSCSI driver into U-Boot instead I use iPXE as an executable that is loaded from the EFI partition.
> > I have never intended to fully implement a shell which is to be compliant > with UEFI specification while I'm trying to mimick some command > interfaces for convenience. UEFI shell, as you know, provides plenty > of "protocols" on which some UEFI applications, including UEFI SCT, > reply. I will never implement it with my efishell. > > I hope that my efishell is a quick and easy way of learning more about > u-boot's uefi environment. I will be even happier if more people > get involved there. > >> Due to these considerations I suggest that you build your UEFI shell >> implementation as a separate UEFI binary (like helloworld.efi). You may >> offer an embedding of the binary (like the bootefi hello command) into >> the finally linked U-Boot binary via a configuration variable. Please, >> put the shell implementation into a separate directory. You may want to >> designate yourself as maintainer (in file MAINTAINERS). > > Yeah, your suggestion is reasonable and I have thought of it before. > There are, however, several reasons that I haven't done so; particularly, > efishell is implemented not only with boottime services but also > other helper functions, say, from device path utilities. Exporting them > as libraries is possible but I don't think that it would be so valuable. > > Even if efishell is a separate application, it will not contribute to > reduce the total footprint if it is embedded along with u-boot binary.
That is why CONFIG_CMD_BOOTEFI_HELLO - which embeds helloworld.efi into the U-Boot binary - is default no. Same I would do for efishell.efi.
One big drawback with a separate binary is the missing command line integration. It becomes quite awkward to execute efi debug commands then, since you'll have to run them through a special bootefi subcommand.
If you really want to have a "uefi shell", I think the sanest option is to just provide a built-in copy of the edk2 uefi shell, similar to the hello world binary. The big benefit of this patch set however, is not that we get a shell - it's that we get quick and tiny debug introspectability into efi_loader data structures.
And my command can be used for simple testing.
Exactly, that would give us the best of both worlds.
I think the biggest problem here really is the name of the code. Why don't we just call it "debugefi"? It would be default N except for debug targets (just like bootefi_hello).
That way when someone wants to just quickly introspect internal data structures, they can. I also hope that if the name contains debug, nobody will expect command line compatibility going forward, so we have much more freedom to change internals (which is my biggest concern).
So in my opinion, if you fix the 2 other comments from Heinrich and rename everything from "efishell" to "debugefi" (so it aligns with bootefi), we should be good.
If Heinrich agrees, I will fix the name although I'm not a super fan of this new name :)
Well, feel free to come up with a new one, but it definitely must have a ring to it that it's a tiny, debug only feature that is not intended for normal use ;).
Do you have any idea/preference about this command's name?
I prefer efidebug/efidbg or efitool so that we can use a shorthand name, efi, at command line in most cases.
That definitely works for me as well, yes.
Alex
participants (3)
-
AKASHI Takahiro -
Alexander Graf -
Heinrich Schuchardt