* [U-Boot] [PATCH v4 1/9] cmd: add efitool command
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 3:31 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 2/9] cmd: efitool: add devices command AKASHI Takahiro
` (8 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
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, efitool, helps address these issues and give us
more friendly interfaces:
* efitool boot add: add BootXXXX variable
* efitool boot rm: remove BootXXXX variable
* efitool boot dump: display all BootXXXX variables
* efitool boot order: set/display a boot order (BootOrder)
* efitool setvar: set an UEFI variable (with limited functionality)
* efitool 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/efitool.c | 707 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 718 insertions(+)
create mode 100644 cmd/efitool.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index ea1a325eb301..4a721b8bb5b8 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1397,6 +1397,16 @@ config CMD_DISPLAY
displayed on a simple board-specific display. Implement
display_putc() to use it.
+config CMD_EFITOOL
+ bool "efitool - display/customize UEFI environment"
+ depends on EFI_LOADER
+ default n
+ help
+ Enable the 'efitool' 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 15ae4d250f50..f103d2e0fbb6 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -51,6 +51,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_EFITOOL) += efitool.o
obj-$(CONFIG_CMD_ELF) += elf.o
obj-$(CONFIG_HUSH_PARSER) += exit.o
obj-$(CONFIG_CMD_EXT4) += ext4.o
diff --git a/cmd/efitool.c b/cmd/efitool.c
new file mode 100644
index 000000000000..c7dc2c11f2e7
--- /dev/null
+++ b/cmd/efitool.c
@@ -0,0 +1,707 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * UEFI 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] = (char)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 UEFI 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;
+ efi_uintn_t size = 0;
+ u16 *var_name16 = NULL, *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:
+ free(value);
+ free(var_name16);
+
+ 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;
+ efi_uintn_t size;
+ int ret;
+
+ if (argc < 6 || argc > 7)
+ return CMD_RET_USAGE;
+
+ id = (int)simple_strtoul(argv[1], &endp, 16);
+ 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, 16);
+ 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, 0, 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;
+ efi_uintn_t 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;
+ efi_uintn_t 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_next(int argc, char * const argv[])
+{
+ u16 bootnext;
+ efi_uintn_t size;
+ char *endp;
+ efi_guid_t guid;
+ efi_status_t ret;
+
+ if (argc != 2)
+ return CMD_RET_USAGE;
+
+ bootnext = (u16)simple_strtoul(argv[1], &endp, 16);
+ if (*endp != '\0' || bootnext > 0xffff) {
+ printf("invalid value: %s\n", argv[1]);
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+
+ guid = efi_global_variable_guid;
+ size = sizeof(u16);
+ ret = efi_set_variable(L"BootNext", &guid,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS, size, &bootnext);
+ ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+ return ret;
+}
+
+static int do_efi_boot_order(int argc, char * const argv[])
+{
+ u16 *bootorder = NULL;
+ efi_uintn_t 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, 16);
+ 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, "next"))
+ return do_efi_boot_next(argc, argv);
+ else if (!strcmp(sub_command, "order"))
+ return do_efi_boot_order(argc, argv);
+ else
+ return CMD_RET_USAGE;
+}
+
+/* Interpreter command to configure UEFI environment */
+static int do_efitool(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 UEFI drivers */
+ r = efi_init_obj_list();
+ if (r != EFI_SUCCESS) {
+ printf("Error: Cannot set up UEFI 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 efitool_help_text[] =
+ " - UEFI Shell-like interface to configure UEFI environment\n"
+ "\n"
+ "efitool boot add <bootid> <label> <interface> <device>[:<part>] <file path> [<load options>]\n"
+ " - set UEFI BootXXXX variable\n"
+ " <load options> will be passed to UEFI application\n"
+ "efitool boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
+ " - delete UEFI BootXXXX variables\n"
+ "efitool boot dump\n"
+ " - show all UEFI BootXXXX variables\n"
+ "efitool boot next <bootid>\n"
+ " - set UEFI BootNext variable\n"
+ "efitool boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
+ " - set/show UEFI boot order\n"
+ "\n"
+ "efitool dumpvar [<name>]\n"
+ " - show UEFI variable's value\n"
+ "efitool setvar <name> [<value>]\n"
+ " - set/delete uefi variable's value\n"
+ " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n";
+#endif
+
+U_BOOT_CMD(
+ efitool, 10, 0, do_efitool,
+ "Configure UEFI environment",
+ efitool_help_text
+);
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 1/9] cmd: add efitool command
2019-01-15 2:55 ` [U-Boot] [PATCH v4 1/9] cmd: add efitool command AKASHI Takahiro
@ 2019-01-15 3:31 ` Heinrich Schuchardt
2019-01-17 4:27 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 3:31 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 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, efitool, helps address these issues and give us
> more friendly interfaces:
> * efitool boot add: add BootXXXX variable
> * efitool boot rm: remove BootXXXX variable
> * efitool boot dump: display all BootXXXX variables
> * efitool boot order: set/display a boot order (BootOrder)
> * efitool setvar: set an UEFI variable (with limited functionality)
> * efitool 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>
Hello Takahiro,
thanks a lot for your patch series. The additional command is really useful.
Unfortunately the implementation of sub-commands does not follow the
coding style of other sub-commands. Could you please have a look at the
current doc/README.commands
http://git.denx.de/?p=u-boot.git;a=blob;f=doc/README.commands
I think it should be easy to convert your code to follow the
implementation style suggested there and used all over U-Boot.
Sorry that I did not put this into an earlier comment. I only learnt
about this recently by a review from Simon and then updated the README.
Please, add the new file in MAINTAINERS to the EFI PAYLOAD section.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 1/9] cmd: add efitool command
2019-01-15 3:31 ` Heinrich Schuchardt
@ 2019-01-17 4:27 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-17 4:27 UTC (permalink / raw)
To: u-boot
On Tue, Jan 15, 2019 at 04:31:35AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 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, efitool, helps address these issues and give us
> > more friendly interfaces:
> > * efitool boot add: add BootXXXX variable
> > * efitool boot rm: remove BootXXXX variable
> > * efitool boot dump: display all BootXXXX variables
> > * efitool boot order: set/display a boot order (BootOrder)
> > * efitool setvar: set an UEFI variable (with limited functionality)
> > * efitool 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>
>
> Hello Takahiro,
>
> thanks a lot for your patch series. The additional command is really useful.
>
> Unfortunately the implementation of sub-commands does not follow the
> coding style of other sub-commands. Could you please have a look at the
> current doc/README.commands
>
> http://git.denx.de/?p=u-boot.git;a=blob;f=doc/README.commands
OK.
> I think it should be easy to convert your code to follow the
> implementation style suggested there and used all over U-Boot.
>
> Sorry that I did not put this into an earlier comment. I only learnt
> about this recently by a review from Simon and then updated the README.
>
> Please, add the new file in MAINTAINERS to the EFI PAYLOAD section.
Sure
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 2/9] cmd: efitool: add devices command
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
2019-01-15 2:55 ` [U-Boot] [PATCH v4 1/9] cmd: add efitool command AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 5:09 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol AKASHI Takahiro
` (7 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
"devices" command prints all the uefi variables on the system.
=> efi devices
Scanning disk ahci_scsi.id0lun0...
Scanning disk ahci_scsi.id1lun0...
Found 4 disks
D
e
v Device Path
================ ====================
7ef3bfa0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
7ef3cca0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)
7ef3d070 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)
7ef3d1b0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(1,MBR,0x086246ba,0x800,0x40000)
7ef3d3e0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efitool.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 97 insertions(+), 1 deletion(-)
diff --git a/cmd/efitool.c b/cmd/efitool.c
index c7dc2c11f2e7..6b2df1beedb5 100644
--- a/cmd/efitool.c
+++ b/cmd/efitool.c
@@ -18,6 +18,8 @@
#include <linux/ctype.h>
#include <asm/global_data.h>
+#define BS systab.boottime
+
static void dump_var_data(char *data, unsigned long len)
{
char *start, *end, *p;
@@ -266,6 +268,96 @@ out:
return ret;
}
+static int efi_get_handles_by_proto(efi_guid_t *guid, efi_handle_t **handlesp,
+ int *num)
+{
+ 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 -1;
+
+ ret = BS->locate_handle(BY_PROTOCOL, guid, NULL, &size,
+ handles);
+ }
+ if (ret != EFI_SUCCESS) {
+ free(handles);
+ return -1;
+ }
+ } else {
+ ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ handles = calloc(1, size);
+ if (!handles)
+ return -1;
+
+ ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size,
+ handles);
+ }
+ if (ret != EFI_SUCCESS) {
+ free(handles);
+ return -1;
+ }
+ }
+
+ *handlesp = handles;
+ *num = size / sizeof(efi_handle_t);
+
+ return 0;
+}
+
+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;
+ u16 *devname;
+ int num, i;
+
+ handles = NULL;
+ num = 0;
+ if (efi_get_handles_by_proto(NULL, &handles, &num))
+ return CMD_RET_FAILURE;
+
+ if (!num)
+ return CMD_RET_SUCCESS;
+
+ printf("D\n");
+ printf("e\n");
+ printf("v Device Path\n");
+ printf("================ ====================\n");
+ for (i = 0; i < num; i++) {
+ if (!efi_get_device_handle_info(handles[i], &devname)) {
+ printf("%16p %ls\n", handles[i], devname);
+ efi_free_pool(devname);
+ }
+ }
+
+ free(handles);
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -673,6 +765,8 @@ static int do_efitool(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;
}
@@ -697,7 +791,9 @@ static char efitool_help_text[] =
" - show UEFI variable's value\n"
"efitool setvar <name> [<value>]\n"
" - set/delete uefi variable's value\n"
- " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n";
+ " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n"
+ "efitool devices\n"
+ " - show uefi devices\n";
#endif
U_BOOT_CMD(
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 2/9] cmd: efitool: add devices command
2019-01-15 2:55 ` [U-Boot] [PATCH v4 2/9] cmd: efitool: add devices command AKASHI Takahiro
@ 2019-01-15 5:09 ` Heinrich Schuchardt
2019-01-17 4:48 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 5:09 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> "devices" command prints all the uefi variables on the system.
>
> => efi devices
> Scanning disk ahci_scsi.id0lun0...
> Scanning disk ahci_scsi.id1lun0...
> Found 4 disks
> D
> e
> v Device Path
> ================ ====================
> 7ef3bfa0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> 7ef3cca0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)
> 7ef3d070 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)
> 7ef3d1b0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(1,MBR,0x086246ba,0x800,0x40000)
> 7ef3d3e0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/efitool.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efitool.c b/cmd/efitool.c
> index c7dc2c11f2e7..6b2df1beedb5 100644
> --- a/cmd/efitool.c
> +++ b/cmd/efitool.c
> @@ -18,6 +18,8 @@
> #include <linux/ctype.h>
> #include <asm/global_data.h>
>
> +#define BS systab.boottime
> +
> static void dump_var_data(char *data, unsigned long len)
> {
> char *start, *end, *p;
> @@ -266,6 +268,96 @@ out:
> return ret;
> }
>
> +static int efi_get_handles_by_proto(efi_guid_t *guid, efi_handle_t **handlesp,
> + int *num)
> +{
> + 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 -1;
> +
> + ret = BS->locate_handle(BY_PROTOCOL, guid, NULL, &size,
> + handles);
> + }
> + if (ret != EFI_SUCCESS) {
> + free(handles);
> + return -1;
> + }
> + } else {
> + ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + handles = calloc(1, size);
> + if (!handles)
> + return -1;
> +
> + ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size,
> + handles);
> + }
> + if (ret != EFI_SUCCESS) {
> + free(handles);
> + return -1;
> + }
> + }
> +
> + *handlesp = handles;
> + *num = size / sizeof(efi_handle_t);
> +
> + return 0;
> +}
> +
> +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;
> + u16 *devname;
This is the device path. So why not call it dev_path?
> + int num, i;
> +
> + handles = NULL;
> + num = 0;
> + if (efi_get_handles_by_proto(NULL, &handles, &num))
> + return CMD_RET_FAILURE;
> +
> + if (!num)
> + return CMD_RET_SUCCESS;
> +
> + printf("D\n");
> + printf("e\n");
> + printf("v Device Path\n");
> + printf("================ ====================\n");
> + for (i = 0; i < num; i++) {
> + if (!efi_get_device_handle_info(handles[i], &devname)) {
> + printf("%16p %ls\n", handles[i], devname);
> + efi_free_pool(devname);
> + }
> + }
> +
> + free(handles);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> static int do_efi_boot_add(int argc, char * const argv[])
> {
Please, use the standard way of adding sub-commands, cf.
doc/README.commands.
Thanks a lot for your contribution.
Regards
Heinrich
> int id;
> @@ -673,6 +765,8 @@ static int do_efitool(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;
> }
> @@ -697,7 +791,9 @@ static char efitool_help_text[] =
> " - show UEFI variable's value\n"
> "efitool setvar <name> [<value>]\n"
> " - set/delete uefi variable's value\n"
> - " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n";
> + " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n"
> + "efitool devices\n"
> + " - show uefi devices\n";
> #endif
>
> U_BOOT_CMD(
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 2/9] cmd: efitool: add devices command
2019-01-15 5:09 ` Heinrich Schuchardt
@ 2019-01-17 4:48 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-17 4:48 UTC (permalink / raw)
To: u-boot
On Tue, Jan 15, 2019 at 06:09:31AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> > "devices" command prints all the uefi variables on the system.
> >
> > => efi devices
> > Scanning disk ahci_scsi.id0lun0...
> > Scanning disk ahci_scsi.id1lun0...
> > Found 4 disks
> > D
> > e
> > v Device Path
> > ================ ====================
> > 7ef3bfa0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > 7ef3cca0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)
> > 7ef3d070 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)
> > 7ef3d1b0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(1,MBR,0x086246ba,0x800,0x40000)
> > 7ef3d3e0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/efitool.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 97 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/efitool.c b/cmd/efitool.c
> > index c7dc2c11f2e7..6b2df1beedb5 100644
> > --- a/cmd/efitool.c
> > +++ b/cmd/efitool.c
> > @@ -18,6 +18,8 @@
> > #include <linux/ctype.h>
> > #include <asm/global_data.h>
> >
> > +#define BS systab.boottime
> > +
> > static void dump_var_data(char *data, unsigned long len)
> > {
> > char *start, *end, *p;
> > @@ -266,6 +268,96 @@ out:
> > return ret;
> > }
> >
> > +static int efi_get_handles_by_proto(efi_guid_t *guid, efi_handle_t **handlesp,
> > + int *num)
> > +{
> > + 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 -1;
> > +
> > + ret = BS->locate_handle(BY_PROTOCOL, guid, NULL, &size,
> > + handles);
> > + }
> > + if (ret != EFI_SUCCESS) {
> > + free(handles);
> > + return -1;
> > + }
> > + } else {
> > + ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size, NULL);
> > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > + handles = calloc(1, size);
> > + if (!handles)
> > + return -1;
> > +
> > + ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size,
> > + handles);
> > + }
> > + if (ret != EFI_SUCCESS) {
> > + free(handles);
> > + return -1;
> > + }
> > + }
> > +
> > + *handlesp = handles;
> > + *num = size / sizeof(efi_handle_t);
> > +
> > + return 0;
> > +}
> > +
> > +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;
> > + u16 *devname;
>
> This is the device path. So why not call it dev_path?
dev_path is normally used for a device path object itself.
I prefer dp_text.
> > + int num, i;
> > +
> > + handles = NULL;
> > + num = 0;
> > + if (efi_get_handles_by_proto(NULL, &handles, &num))
> > + return CMD_RET_FAILURE;
> > +
> > + if (!num)
> > + return CMD_RET_SUCCESS;
> > +
> > + printf("D\n");
> > + printf("e\n");
> > + printf("v Device Path\n");
> > + printf("================ ====================\n");
> > + for (i = 0; i < num; i++) {
> > + if (!efi_get_device_handle_info(handles[i], &devname)) {
> > + printf("%16p %ls\n", handles[i], devname);
> > + efi_free_pool(devname);
> > + }
> > + }
> > +
> > + free(handles);
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > static int do_efi_boot_add(int argc, char * const argv[])
> > {
>
> Please, use the standard way of adding sub-commands, cf.
> doc/README.commands.
OK
Thanks,
-Takahiro Akashi
> Thanks a lot for your contribution.
>
> Regards
>
> Heinrich
>
> > int id;
> > @@ -673,6 +765,8 @@ static int do_efitool(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;
> > }
> > @@ -697,7 +791,9 @@ static char efitool_help_text[] =
> > " - show UEFI variable's value\n"
> > "efitool setvar <name> [<value>]\n"
> > " - set/delete uefi variable's value\n"
> > - " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n";
> > + " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n"
> > + "efitool devices\n"
> > + " - show uefi devices\n";
> > #endif
> >
> > U_BOOT_CMD(
> >
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
2019-01-15 2:55 ` [U-Boot] [PATCH v4 1/9] cmd: add efitool command AKASHI Takahiro
2019-01-15 2:55 ` [U-Boot] [PATCH v4 2/9] cmd: efitool: add devices command AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 3:41 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 4/9] cmd: efitool: add drivers command AKASHI Takahiro
` (6 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
This new field will be shown as a driver's name in "efitool drivers"
command.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
include/efi_driver.h | 1 +
lib/efi_driver/efi_uclass.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/efi_driver.h b/include/efi_driver.h
index 840483a416a4..ee8867816094 100644
--- a/include/efi_driver.h
+++ b/include/efi_driver.h
@@ -34,6 +34,7 @@ struct efi_driver_ops {
* This structure adds internal fields to the driver binding protocol.
*/
struct efi_driver_binding_extended_protocol {
+ const char *name;
struct efi_driver_binding_protocol bp;
const struct efi_driver_ops *ops;
};
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index bb86ffd399c3..8bbaa02d490e 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -271,6 +271,7 @@ static efi_status_t efi_add_driver(struct driver *drv)
bp->bp.stop = efi_uc_stop;
bp->bp.version = 0xffffffff;
bp->ops = drv->ops;
+ bp->name = drv->name;
ret = efi_create_handle(&bp->bp.driver_binding_handle);
if (ret != EFI_SUCCESS) {
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol
2019-01-15 2:55 ` [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol AKASHI Takahiro
@ 2019-01-15 3:41 ` Heinrich Schuchardt
2019-01-17 5:33 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 3:41 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> This new field will be shown as a driver's name in "efitool drivers"
> command.
We can have drivers supplied by U-Boot and drivers supplied by an EFI
binary that we recently installed via the bootefi command.
A driver installed via the bootefi command will not have allocated
memory for the extra fields.
So you cannot use the name field in your "efitool drivers" command.
Best regards
Heinrich
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> include/efi_driver.h | 1 +
> lib/efi_driver/efi_uclass.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/include/efi_driver.h b/include/efi_driver.h
> index 840483a416a4..ee8867816094 100644
> --- a/include/efi_driver.h
> +++ b/include/efi_driver.h
> @@ -34,6 +34,7 @@ struct efi_driver_ops {
> * This structure adds internal fields to the driver binding protocol.
> */
> struct efi_driver_binding_extended_protocol {
> + const char *name;
> struct efi_driver_binding_protocol bp;
> const struct efi_driver_ops *ops;
> };
> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> index bb86ffd399c3..8bbaa02d490e 100644
> --- a/lib/efi_driver/efi_uclass.c
> +++ b/lib/efi_driver/efi_uclass.c
> @@ -271,6 +271,7 @@ static efi_status_t efi_add_driver(struct driver *drv)
> bp->bp.stop = efi_uc_stop;
> bp->bp.version = 0xffffffff;
> bp->ops = drv->ops;
> + bp->name = drv->name;
>
> ret = efi_create_handle(&bp->bp.driver_binding_handle);
> if (ret != EFI_SUCCESS) {
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol
2019-01-15 3:41 ` Heinrich Schuchardt
@ 2019-01-17 5:33 ` AKASHI Takahiro
2019-01-17 6:58 ` Heinrich Schuchardt
0 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-17 5:33 UTC (permalink / raw)
To: u-boot
You raised a couple of questions to me.
On Tue, Jan 15, 2019 at 04:41:08AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> > This new field will be shown as a driver's name in "efitool drivers"
> > command.
>
> We can have drivers supplied by U-Boot
I assume that what you mention here is a UCLASS_EFI driver.
What's the problem is;
efi_add_driver() adds EFI_DRIVER_BINDING_PROTOCOL with
*efi_driver_binding_extended_protocol* interface, which is NOT compatible
with EFI_DRIVER_BINDING_PROTOCOL.
On the other hand, for example, in your efi_selftest_controller test
a test driver is set up by installing EFI_DRIVER_BINDING_PROTOCOL
with EFI_DRIVER_BINDING_PROTOCOL interface.
So we have no way to distinguish the two cases(handles) and cannot
deal with them properly.
> and drivers supplied by an EFI
> binary that we recently installed via the bootefi command.
>
> A driver installed via the bootefi command will not have allocated
> memory for the extra fields.
There is no good example of driver of such kind.
I don't know how we can retrieve a meaningful "driver name."
> So you cannot use the name field in your "efitool drivers" command.
Any suggestion?
Thanks,
-Takahiro Akashi
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > include/efi_driver.h | 1 +
> > lib/efi_driver/efi_uclass.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/include/efi_driver.h b/include/efi_driver.h
> > index 840483a416a4..ee8867816094 100644
> > --- a/include/efi_driver.h
> > +++ b/include/efi_driver.h
> > @@ -34,6 +34,7 @@ struct efi_driver_ops {
> > * This structure adds internal fields to the driver binding protocol.
> > */
> > struct efi_driver_binding_extended_protocol {
> > + const char *name;
> > struct efi_driver_binding_protocol bp;
> > const struct efi_driver_ops *ops;
> > };
> > diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> > index bb86ffd399c3..8bbaa02d490e 100644
> > --- a/lib/efi_driver/efi_uclass.c
> > +++ b/lib/efi_driver/efi_uclass.c
> > @@ -271,6 +271,7 @@ static efi_status_t efi_add_driver(struct driver *drv)
> > bp->bp.stop = efi_uc_stop;
> > bp->bp.version = 0xffffffff;
> > bp->ops = drv->ops;
> > + bp->name = drv->name;
> >
> > ret = efi_create_handle(&bp->bp.driver_binding_handle);
> > if (ret != EFI_SUCCESS) {
> >
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol
2019-01-17 5:33 ` AKASHI Takahiro
@ 2019-01-17 6:58 ` Heinrich Schuchardt
2019-01-21 7:47 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-17 6:58 UTC (permalink / raw)
To: u-boot
On 1/17/19 6:33 AM, AKASHI Takahiro wrote:
> You raised a couple of questions to me.
>
> On Tue, Jan 15, 2019 at 04:41:08AM +0100, Heinrich Schuchardt wrote:
>> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
>>> This new field will be shown as a driver's name in "efitool drivers"
>>> command.
>>
>> We can have drivers supplied by U-Boot
>
> I assume that what you mention here is a UCLASS_EFI driver.
>
> What's the problem is;
> efi_add_driver() adds EFI_DRIVER_BINDING_PROTOCOL with
> *efi_driver_binding_extended_protocol* interface, which is NOT compatible
> with EFI_DRIVER_BINDING_PROTOCOL.
> On the other hand, for example, in your efi_selftest_controller test
> a test driver is set up by installing EFI_DRIVER_BINDING_PROTOCOL
> with EFI_DRIVER_BINDING_PROTOCOL interface.
>
> So we have no way to distinguish the two cases(handles) and cannot
> deal with them properly.
Correct. It is allowable to add private fields to a protocol. EDK2 does
the same. But we cannot make any assumptions about the private fields here.
>
>> and drivers supplied by an EFI
>> binary that we recently installed via the bootefi command.
>>
>> A driver installed via the bootefi command will not have allocated
>> memory for the extra fields.
>
> There is no good example of driver of such kind.
> I don't know how we can retrieve a meaningful "driver name."
The UEFI spec does not foresee any name field.
The interesting information is: on which handle did the driver install
which protocol.
This information could be gathered by looping over all protocols on all
handles and looking for an OpenProtocolInformation where the driver is
the controller.
For me it would be fine if we leave that to a later patch.
Best regards
Heinrich
>
>> So you cannot use the name field in your "efitool drivers" command.
>
> Any suggestion?
>
> Thanks,
> -Takahiro Akashi
>
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>> include/efi_driver.h | 1 +
>>> lib/efi_driver/efi_uclass.c | 1 +
>>> 2 files changed, 2 insertions(+)
>>>
>>> diff --git a/include/efi_driver.h b/include/efi_driver.h
>>> index 840483a416a4..ee8867816094 100644
>>> --- a/include/efi_driver.h
>>> +++ b/include/efi_driver.h
>>> @@ -34,6 +34,7 @@ struct efi_driver_ops {
>>> * This structure adds internal fields to the driver binding protocol.
>>> */
>>> struct efi_driver_binding_extended_protocol {
>>> + const char *name;
>>> struct efi_driver_binding_protocol bp;
>>> const struct efi_driver_ops *ops;
>>> };
>>> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
>>> index bb86ffd399c3..8bbaa02d490e 100644
>>> --- a/lib/efi_driver/efi_uclass.c
>>> +++ b/lib/efi_driver/efi_uclass.c
>>> @@ -271,6 +271,7 @@ static efi_status_t efi_add_driver(struct driver *drv)
>>> bp->bp.stop = efi_uc_stop;
>>> bp->bp.version = 0xffffffff;
>>> bp->ops = drv->ops;
>>> + bp->name = drv->name;
>>>
>>> ret = efi_create_handle(&bp->bp.driver_binding_handle);
>>> if (ret != EFI_SUCCESS) {
>>>
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol
2019-01-17 6:58 ` Heinrich Schuchardt
@ 2019-01-21 7:47 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-21 7:47 UTC (permalink / raw)
To: u-boot
On Thu, Jan 17, 2019 at 07:58:17AM +0100, Heinrich Schuchardt wrote:
> On 1/17/19 6:33 AM, AKASHI Takahiro wrote:
> > You raised a couple of questions to me.
> >
> > On Tue, Jan 15, 2019 at 04:41:08AM +0100, Heinrich Schuchardt wrote:
> >> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> >>> This new field will be shown as a driver's name in "efitool drivers"
> >>> command.
> >>
> >> We can have drivers supplied by U-Boot
> >
> > I assume that what you mention here is a UCLASS_EFI driver.
> >
> > What's the problem is;
> > efi_add_driver() adds EFI_DRIVER_BINDING_PROTOCOL with
> > *efi_driver_binding_extended_protocol* interface, which is NOT compatible
> > with EFI_DRIVER_BINDING_PROTOCOL.
> > On the other hand, for example, in your efi_selftest_controller test
> > a test driver is set up by installing EFI_DRIVER_BINDING_PROTOCOL
> > with EFI_DRIVER_BINDING_PROTOCOL interface.
> >
> > So we have no way to distinguish the two cases(handles) and cannot
> > deal with them properly.
>
> Correct. It is allowable to add private fields to a protocol. EDK2 does
> the same. But we cannot make any assumptions about the private fields here.
I misunderstood something here, but anyhow,
> >
> >> and drivers supplied by an EFI
> >> binary that we recently installed via the bootefi command.
> >>
> >> A driver installed via the bootefi command will not have allocated
> >> memory for the extra fields.
> >
> > There is no good example of driver of such kind.
> > I don't know how we can retrieve a meaningful "driver name."
>
> The UEFI spec does not foresee any name field.
Edk2's shell does get a driver's name by using COMPONENT_NAME2_PROTOCOL,
and we'd better do the same way.
So "<NULL>" will be printed with my patch for now.
-Takahiro Akashi
> The interesting information is: on which handle did the driver install
> which protocol.
>
> This information could be gathered by looping over all protocols on all
> handles and looking for an OpenProtocolInformation where the driver is
> the controller.
>
> For me it would be fine if we leave that to a later patch.
>
> Best regards
>
> Heinrich
>
> >
> >> So you cannot use the name field in your "efitool drivers" command.
> >
> > Any suggestion?
> >
> > Thanks,
> > -Takahiro Akashi
> >
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>
> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>> ---
> >>> include/efi_driver.h | 1 +
> >>> lib/efi_driver/efi_uclass.c | 1 +
> >>> 2 files changed, 2 insertions(+)
> >>>
> >>> diff --git a/include/efi_driver.h b/include/efi_driver.h
> >>> index 840483a416a4..ee8867816094 100644
> >>> --- a/include/efi_driver.h
> >>> +++ b/include/efi_driver.h
> >>> @@ -34,6 +34,7 @@ struct efi_driver_ops {
> >>> * This structure adds internal fields to the driver binding protocol.
> >>> */
> >>> struct efi_driver_binding_extended_protocol {
> >>> + const char *name;
> >>> struct efi_driver_binding_protocol bp;
> >>> const struct efi_driver_ops *ops;
> >>> };
> >>> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> >>> index bb86ffd399c3..8bbaa02d490e 100644
> >>> --- a/lib/efi_driver/efi_uclass.c
> >>> +++ b/lib/efi_driver/efi_uclass.c
> >>> @@ -271,6 +271,7 @@ static efi_status_t efi_add_driver(struct driver *drv)
> >>> bp->bp.stop = efi_uc_stop;
> >>> bp->bp.version = 0xffffffff;
> >>> bp->ops = drv->ops;
> >>> + bp->name = drv->name;
> >>>
> >>> ret = efi_create_handle(&bp->bp.driver_binding_handle);
> >>> if (ret != EFI_SUCCESS) {
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 4/9] cmd: efitool: add drivers command
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
` (2 preceding siblings ...)
2019-01-15 2:55 ` [U-Boot] [PATCH v4 3/9] efi_driver: add name to driver binding protocol AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 3:39 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 5/9] cmd: efitool: add dh command AKASHI Takahiro
` (5 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
"drivers" command prints all the uefi drivers on the system.
=> efi drivers
D
r
v Driver Name Image Path
================ ==================== ==========
7ef31d30 EFI block driver <built-in>
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efitool.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/cmd/efitool.c b/cmd/efitool.c
index 6b2df1beedb5..4d46721fbf91 100644
--- a/cmd/efitool.c
+++ b/cmd/efitool.c
@@ -8,6 +8,7 @@
#include <charset.h>
#include <common.h>
#include <command.h>
+#include <efi_driver.h>
#include <efi_loader.h>
#include <environment.h>
#include <errno.h>
@@ -16,6 +17,7 @@
#include <malloc.h>
#include <search.h>
#include <linux/ctype.h>
+#include <linux/kernel.h>
#include <asm/global_data.h>
#define BS systab.boottime
@@ -358,6 +360,82 @@ 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_handler *handler;
+ struct efi_driver_binding_extended_protocol *bp;
+ efi_status_t ret;
+
+ ret = efi_search_protocol(handle, &efi_guid_driver_binding_protocol,
+ &handler);
+ if (ret != EFI_SUCCESS)
+ return -1;
+ bp = handler->protocol_interface;
+
+ *name = malloc((strlen(bp->name) + 1) * sizeof(u16));
+ if (*name)
+ ascii2unicode(*name, bp->name);
+
+ /*
+ * TODO:
+ * handle image->device_handle,
+ * use append_device_path()
+ */
+ *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("D\n");
+ printf("r\n");
+ printf("v 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)) {
+ if (filename)
+ printf("%16p %-20ls %ls/%ls\n",
+ *handle, drvname, devname, filename);
+ else
+ printf("%16p %-20ls <built-in>\n",
+ *handle, drvname);
+ free(drvname);
+ free(devname);
+ free(filename);
+ }
+ handle++;
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -767,6 +845,8 @@ static int do_efitool(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;
}
@@ -793,7 +873,9 @@ static char efitool_help_text[] =
" - set/delete uefi variable's value\n"
" <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n"
"efitool devices\n"
- " - show uefi devices\n";
+ " - show uefi devices\n"
+ "efitool drivers\n"
+ " - show uefi drivers\n";
#endif
U_BOOT_CMD(
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 4/9] cmd: efitool: add drivers command
2019-01-15 2:55 ` [U-Boot] [PATCH v4 4/9] cmd: efitool: add drivers command AKASHI Takahiro
@ 2019-01-15 3:39 ` Heinrich Schuchardt
0 siblings, 0 replies; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 3:39 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> "drivers" command prints all the uefi drivers on the system.
>
> => efi drivers
> D
> r
> v Driver Name Image Path
> ================ ==================== ==========
> 7ef31d30 EFI block driver <built-in>
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/efitool.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efitool.c b/cmd/efitool.c
> index 6b2df1beedb5..4d46721fbf91 100644
> --- a/cmd/efitool.c
> +++ b/cmd/efitool.c
> @@ -8,6 +8,7 @@
> #include <charset.h>
> #include <common.h>
> #include <command.h>
> +#include <efi_driver.h>
> #include <efi_loader.h>
> #include <environment.h>
> #include <errno.h>
> @@ -16,6 +17,7 @@
> #include <malloc.h>
> #include <search.h>
> #include <linux/ctype.h>
> +#include <linux/kernel.h>
> #include <asm/global_data.h>
>
> #define BS systab.boottime
> @@ -358,6 +360,82 @@ 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_handler *handler;
> + struct efi_driver_binding_extended_protocol *bp;
We can have drivers supplied by U-Boot and drivers supplied by an EFI
binary that we recently installed via the bootefi command.
A driver installed via the bootefi command will not have allocated
memory for the extra fields. So the rest of the code accessing the name
field will produce an illegal memory access.
Best regards
Heinrich
> + efi_status_t ret;
> +
> + ret = efi_search_protocol(handle, &efi_guid_driver_binding_protocol,
> + &handler);
> + if (ret != EFI_SUCCESS)
> + return -1;
> + bp = handler->protocol_interface;
> +
> + *name = malloc((strlen(bp->name) + 1) * sizeof(u16));
> + if (*name)
> + ascii2unicode(*name, bp->name);
> +
> + /*
> + * TODO:
> + * handle image->device_handle,
> + * use append_device_path()
> + */
> + *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("D\n");
> + printf("r\n");
> + printf("v 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)) {
> + if (filename)
> + printf("%16p %-20ls %ls/%ls\n",
> + *handle, drvname, devname, filename);
> + else
> + printf("%16p %-20ls <built-in>\n",
> + *handle, drvname);
> + free(drvname);
> + free(devname);
> + free(filename);
> + }
> + handle++;
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> static int do_efi_boot_add(int argc, char * const argv[])
> {
> int id;
> @@ -767,6 +845,8 @@ static int do_efitool(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;
> }
> @@ -793,7 +873,9 @@ static char efitool_help_text[] =
> " - set/delete uefi variable's value\n"
> " <value> may be \"=\"...\"\", \"=0x...\", \"=H...\", (set) or \"=\" (delete)\n"
> "efitool devices\n"
> - " - show uefi devices\n";
> + " - show uefi devices\n"
> + "efitool drivers\n"
> + " - show uefi drivers\n";
> #endif
>
> U_BOOT_CMD(
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 5/9] cmd: efitool: add dh command
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
` (3 preceding siblings ...)
2019-01-15 2:55 ` [U-Boot] [PATCH v4 4/9] cmd: efitool: add drivers command AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 4:55 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 6/9] cmd: efitool: add images command AKASHI Takahiro
` (4 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
"dh" command prints all the uefi handles used in the system.
=> efi dh
7ef3bfa0: Device Path, Device Path To Text, Device Path Utilities,
Unicode Collation 2
7ef31d30: Driver Binding
7ef31da0: Simple Text Output
7ef31e10: Simple Text Input, Simple Text Input Ex
7ef3cca0: Block IO, Device Path
7ef3d070: Block IO, Device Path
7ef3d1b0: Block IO, Device Path, Simple File System
7ef3d3e0: Block IO, Device Path, Simple File System
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efitool.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 102 insertions(+), 1 deletion(-)
diff --git a/cmd/efitool.c b/cmd/efitool.c
index 4d46721fbf91..4bd6367b4bd9 100644
--- a/cmd/efitool.c
+++ b/cmd/efitool.c
@@ -436,6 +436,103 @@ static int do_efi_show_drivers(int argc, char * const argv[])
return CMD_RET_SUCCESS;
}
+static struct {
+ u16 *name;
+ efi_guid_t guid;
+} guid_list[] = {
+ {
+ L"Device Path",
+ DEVICE_PATH_GUID,
+ },
+ {
+ L"Device Path To Text",
+ EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID,
+ },
+ {
+ L"Device Path Utilities",
+ EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID,
+ },
+ {
+ L"Unicode Collation 2",
+ EFI_UNICODE_COLLATION_PROTOCOL2_GUID,
+ },
+ {
+ L"Driver Binding",
+ EFI_DRIVER_BINDING_PROTOCOL_GUID,
+ },
+ {
+ L"Simple Text Input",
+ EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID,
+ },
+ {
+ L"Simple Text Input Ex",
+ EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID,
+ },
+ {
+ L"Simple Text Output",
+ EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID,
+ },
+ {
+ L"Block IO",
+ BLOCK_IO_GUID,
+ },
+ {
+ L"Simple File System",
+ EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
+ },
+ {
+ NULL,
+ {{0,},},
+ },
+};
+
+static int do_efi_show_handles(int argc, char * const argv[])
+{
+ efi_handle_t *handles;
+ efi_guid_t **guid;
+ efi_uintn_t count;
+ int num, i, j, k;
+ efi_status_t ret;
+
+ handles = NULL;
+ num = 0;
+ if (efi_get_handles_by_proto(NULL, &handles, &num))
+ return CMD_RET_FAILURE;
+
+ if (!num)
+ return CMD_RET_SUCCESS;
+
+ for (i = 0; i < num; i++) {
+ printf("%16p:", handles[i]);
+ ret = BS->protocols_per_handle(handles[i], &guid, &count);
+ if (ret || !count) {
+ putc('\n');
+ continue;
+ }
+
+ for (j = 0; j < count; j++) {
+ for (k = 0; guid_list[k].name; k++)
+ if (!guidcmp(&guid_list[k].guid, guid[j]))
+ break;
+
+ if (j)
+ printf(", ");
+ else
+ putc(' ');
+
+ if (guid[j])
+ printf("%ls", guid_list[k].name);
+ else
+ printf("%pUl", guid[j]);
+ }
+ putc('\n');
+ }
+
+ free(handles);
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -847,6 +944,8 @@ static int do_efitool(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, "dh"))
+ return do_efi_show_handles(argc, argv);
else
return CMD_RET_USAGE;
}
@@ -875,7 +974,9 @@ static char efitool_help_text[] =
"efitool devices\n"
" - show uefi devices\n"
"efitool drivers\n"
- " - show uefi drivers\n";
+ " - show uefi drivers\n"
+ "efitool dh\n"
+ " - show uefi handles\n";
#endif
U_BOOT_CMD(
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 5/9] cmd: efitool: add dh command
2019-01-15 2:55 ` [U-Boot] [PATCH v4 5/9] cmd: efitool: add dh command AKASHI Takahiro
@ 2019-01-15 4:55 ` Heinrich Schuchardt
2019-01-17 6:01 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 4:55 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> "dh" command prints all the uefi handles used in the system.
>
> => efi dh
> 7ef3bfa0: Device Path, Device Path To Text, Device Path Utilities,
> Unicode Collation 2
> 7ef31d30: Driver Binding
> 7ef31da0: Simple Text Output
> 7ef31e10: Simple Text Input, Simple Text Input Ex
> 7ef3cca0: Block IO, Device Path
> 7ef3d070: Block IO, Device Path
> 7ef3d1b0: Block IO, Device Path, Simple File System
> 7ef3d3e0: Block IO, Device Path, Simple File System
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/efitool.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efitool.c b/cmd/efitool.c
> index 4d46721fbf91..4bd6367b4bd9 100644
> --- a/cmd/efitool.c
> +++ b/cmd/efitool.c
> @@ -436,6 +436,103 @@ static int do_efi_show_drivers(int argc, char * const argv[])
> return CMD_RET_SUCCESS;
> }
>
> +static struct {
> + u16 *name;
Please, reduce the memory size by using char *.
> + efi_guid_t guid;
> +} guid_list[] = {
> + {
> + L"Device Path",
> + DEVICE_PATH_GUID,
> + },
> + {
> + L"Device Path To Text",
> + EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID,
> + },
> + {
> + L"Device Path Utilities",
> + EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID,
> + },
> + {
> + L"Unicode Collation 2",
> + EFI_UNICODE_COLLATION_PROTOCOL2_GUID,
> + },
> + {
> + L"Driver Binding",
> + EFI_DRIVER_BINDING_PROTOCOL_GUID,
> + },
> + {
> + L"Simple Text Input",
> + EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID,
> + },
> + {
> + L"Simple Text Input Ex",
> + EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID,
> + },
> + {
> + L"Simple Text Output",
> + EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID,
> + },
> + {
> + L"Block IO",
> + BLOCK_IO_GUID,
> + },
> + {
> + L"Simple File System",
> + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
> + },
We implement a few more protocols, e.g.
include/efi_api.h:283:#define LOADED_IMAGE_PROTOCOL_GUID \
include/efi_api.h:467:#define BLOCK_IO_GUID \
include/efi_api.h:700:#define EFI_GOP_GUID \
include/efi_api.h:977:#define EFI_DRIVER_BINDING_PROTOCOL_GUID \
include/efi_api.h:999:#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
> + {
> + NULL,
> + {{0,},},
> + }
You could use the ARRAY_SIZE() macro instead of the NULL entry.
> +};
> +
> +static int do_efi_show_handles(int argc, char * const argv[])
> +{
> + efi_handle_t *handles;
> + efi_guid_t **guid;
> + efi_uintn_t count;
> + int num, i, j, k;
> + efi_status_t ret;
> +
> + handles = NULL;
> + num = 0;
> + if (efi_get_handles_by_proto(NULL, &handles, &num))
> + return CMD_RET_FAILURE;
> +
> + if (!num)
> + return CMD_RET_SUCCESS;
> +
> + for (i = 0; i < num; i++) {
> + printf("%16p:", handles[i]);
> + ret = BS->protocols_per_handle(handles[i], &guid, &count);
> + if (ret || !count) {
> + putc('\n');
> + continue;
> + }
> +
> + for (j = 0; j < count; j++) {
> + for (k = 0; guid_list[k].name; k++)
> + if (!guidcmp(&guid_list[k].guid, guid[j]))
> + break;
Please, put this logic into a separate function returning const char *.
Return NULL if not found.
> +
> + if (j)
> + printf(", ");
> + else
> + putc(' ');
> +
> + if (guid[j])
> + printf("%ls", guid_list[k].name);
> + else
> + printf("%pUl", guid[j]);
> + }
> + putc('\n');
> + }
> +
> + free(handles);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> static int do_efi_boot_add(int argc, char * const argv[])
> {
Please, use the standard way of adding sub-commands. See
doc/README.commands.
Best regards
Heinrich
> int id;
> @@ -847,6 +944,8 @@ static int do_efitool(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, "dh"))
> + return do_efi_show_handles(argc, argv);
> else
> return CMD_RET_USAGE;
> }
> @@ -875,7 +974,9 @@ static char efitool_help_text[] =
> "efitool devices\n"
> " - show uefi devices\n"
> "efitool drivers\n"
> - " - show uefi drivers\n";
> + " - show uefi drivers\n"
> + "efitool dh\n"
> + " - show uefi handles\n";
> #endif
>
> U_BOOT_CMD(
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 5/9] cmd: efitool: add dh command
2019-01-15 4:55 ` Heinrich Schuchardt
@ 2019-01-17 6:01 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-17 6:01 UTC (permalink / raw)
To: u-boot
On Tue, Jan 15, 2019 at 05:55:26AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> > "dh" command prints all the uefi handles used in the system.
> >
> > => efi dh
> > 7ef3bfa0: Device Path, Device Path To Text, Device Path Utilities,
> > Unicode Collation 2
> > 7ef31d30: Driver Binding
> > 7ef31da0: Simple Text Output
> > 7ef31e10: Simple Text Input, Simple Text Input Ex
> > 7ef3cca0: Block IO, Device Path
> > 7ef3d070: Block IO, Device Path
> > 7ef3d1b0: Block IO, Device Path, Simple File System
> > 7ef3d3e0: Block IO, Device Path, Simple File System
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/efitool.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/efitool.c b/cmd/efitool.c
> > index 4d46721fbf91..4bd6367b4bd9 100644
> > --- a/cmd/efitool.c
> > +++ b/cmd/efitool.c
> > @@ -436,6 +436,103 @@ static int do_efi_show_drivers(int argc, char * const argv[])
> > return CMD_RET_SUCCESS;
> > }
> >
> > +static struct {
> > + u16 *name;
>
> Please, reduce the memory size by using char *.
OK
> > + efi_guid_t guid;
> > +} guid_list[] = {
> > + {
> > + L"Device Path",
> > + DEVICE_PATH_GUID,
> > + },
> > + {
> > + L"Device Path To Text",
> > + EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID,
> > + },
> > + {
> > + L"Device Path Utilities",
> > + EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID,
> > + },
> > + {
> > + L"Unicode Collation 2",
> > + EFI_UNICODE_COLLATION_PROTOCOL2_GUID,
> > + },
> > + {
> > + L"Driver Binding",
> > + EFI_DRIVER_BINDING_PROTOCOL_GUID,
> > + },
> > + {
> > + L"Simple Text Input",
> > + EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID,
> > + },
> > + {
> > + L"Simple Text Input Ex",
> > + EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID,
> > + },
> > + {
> > + L"Simple Text Output",
> > + EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID,
> > + },
> > + {
> > + L"Block IO",
> > + BLOCK_IO_GUID,
> > + },
> > + {
> > + L"Simple File System",
> > + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
> > + },
>
> We implement a few more protocols, e.g.
>
> include/efi_api.h:467:#define BLOCK_IO_GUID \
> include/efi_api.h:977:#define EFI_DRIVER_BINDING_PROTOCOL_GUID \
> include/efi_api.h:999:#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
They are already in there.
> include/efi_api.h:283:#define LOADED_IMAGE_PROTOCOL_GUID \
> include/efi_api.h:700:#define EFI_GOP_GUID \
Added.
> > + {
> > + NULL,
> > + {{0,},},
> > + }
> You could use the ARRAY_SIZE() macro instead of the NULL entry.
OK
> > +};
> > +
> > +static int do_efi_show_handles(int argc, char * const argv[])
> > +{
> > + efi_handle_t *handles;
> > + efi_guid_t **guid;
> > + efi_uintn_t count;
> > + int num, i, j, k;
> > + efi_status_t ret;
> > +
> > + handles = NULL;
> > + num = 0;
> > + if (efi_get_handles_by_proto(NULL, &handles, &num))
> > + return CMD_RET_FAILURE;
> > +
> > + if (!num)
> > + return CMD_RET_SUCCESS;
> > +
> > + for (i = 0; i < num; i++) {
> > + printf("%16p:", handles[i]);
> > + ret = BS->protocols_per_handle(handles[i], &guid, &count);
> > + if (ret || !count) {
> > + putc('\n');
> > + continue;
> > + }
> > +
> > + for (j = 0; j < count; j++) {
> > + for (k = 0; guid_list[k].name; k++)
> > + if (!guidcmp(&guid_list[k].guid, guid[j]))
> > + break;
>
> Please, put this logic into a separate function returning const char *.
> Return NULL if not found.
OK
> > +
> > + if (j)
> > + printf(", ");
> > + else
> > + putc(' ');
> > +
> > + if (guid[j])
> > + printf("%ls", guid_list[k].name);
> > + else
> > + printf("%pUl", guid[j]);
> > + }
> > + putc('\n');
> > + }
> > +
> > + free(handles);
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > static int do_efi_boot_add(int argc, char * const argv[])
> > {
>
> Please, use the standard way of adding sub-commands. See
> doc/README.commands.
OK
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > int id;
> > @@ -847,6 +944,8 @@ static int do_efitool(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, "dh"))
> > + return do_efi_show_handles(argc, argv);
> > else
> > return CMD_RET_USAGE;
> > }
> > @@ -875,7 +974,9 @@ static char efitool_help_text[] =
> > "efitool devices\n"
> > " - show uefi devices\n"
> > "efitool drivers\n"
> > - " - show uefi drivers\n";
> > + " - show uefi drivers\n"
> > + "efitool dh\n"
> > + " - show uefi handles\n";
> > #endif
> >
> > U_BOOT_CMD(
> >
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 6/9] cmd: efitool: add images command
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
` (4 preceding siblings ...)
2019-01-15 2:55 ` [U-Boot] [PATCH v4 5/9] cmd: efitool: add dh command AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 4:58 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 7/9] cmd: efitool: add memmap command AKASHI Takahiro
` (3 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
"images" command prints loaded images-related information.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efitool.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/cmd/efitool.c b/cmd/efitool.c
index 4bd6367b4bd9..e77273fc6e1e 100644
--- a/cmd/efitool.c
+++ b/cmd/efitool.c
@@ -533,6 +533,13 @@ static int do_efi_show_handles(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;
@@ -946,6 +953,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
return do_efi_show_drivers(argc, argv);
else if (!strcmp(command, "dh"))
return do_efi_show_handles(argc, argv);
+ else if (!strcmp(command, "images"))
+ return do_efi_show_images(argc, argv);
else
return CMD_RET_USAGE;
}
@@ -976,7 +985,9 @@ static char efitool_help_text[] =
"efitool drivers\n"
" - show uefi drivers\n"
"efitool dh\n"
- " - show uefi handles\n";
+ " - show uefi handles\n"
+ "efitool images\n"
+ " - show loaded images\n";
#endif
U_BOOT_CMD(
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 6/9] cmd: efitool: add images command
2019-01-15 2:55 ` [U-Boot] [PATCH v4 6/9] cmd: efitool: add images command AKASHI Takahiro
@ 2019-01-15 4:58 ` Heinrich Schuchardt
2019-01-17 6:02 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 4:58 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> "images" command prints loaded images-related information.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/efitool.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efitool.c b/cmd/efitool.c
> index 4bd6367b4bd9..e77273fc6e1e 100644
> --- a/cmd/efitool.c
> +++ b/cmd/efitool.c
> @@ -533,6 +533,13 @@ static int do_efi_show_handles(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[])
> {
Please, use the standard way of adding sub-commands. See
doc/README.commands.
Best regards
Heinrich
> int id;
> @@ -946,6 +953,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
> return do_efi_show_drivers(argc, argv);
> else if (!strcmp(command, "dh"))
> return do_efi_show_handles(argc, argv);
> + else if (!strcmp(command, "images"))
> + return do_efi_show_images(argc, argv);
> else
> return CMD_RET_USAGE;
> }
> @@ -976,7 +985,9 @@ static char efitool_help_text[] =
> "efitool drivers\n"
> " - show uefi drivers\n"
> "efitool dh\n"
> - " - show uefi handles\n";
> + " - show uefi handles\n"
> + "efitool images\n"
> + " - show loaded images\n";
> #endif
>
> U_BOOT_CMD(
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 6/9] cmd: efitool: add images command
2019-01-15 4:58 ` Heinrich Schuchardt
@ 2019-01-17 6:02 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-17 6:02 UTC (permalink / raw)
To: u-boot
On Tue, Jan 15, 2019 at 05:58:57AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> > "images" command prints loaded images-related information.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/efitool.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/efitool.c b/cmd/efitool.c
> > index 4bd6367b4bd9..e77273fc6e1e 100644
> > --- a/cmd/efitool.c
> > +++ b/cmd/efitool.c
> > @@ -533,6 +533,13 @@ static int do_efi_show_handles(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[])
> > {
>
> Please, use the standard way of adding sub-commands. See
> doc/README.commands.
OK
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > int id;
> > @@ -946,6 +953,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
> > return do_efi_show_drivers(argc, argv);
> > else if (!strcmp(command, "dh"))
> > return do_efi_show_handles(argc, argv);
> > + else if (!strcmp(command, "images"))
> > + return do_efi_show_images(argc, argv);
> > else
> > return CMD_RET_USAGE;
> > }
> > @@ -976,7 +985,9 @@ static char efitool_help_text[] =
> > "efitool drivers\n"
> > " - show uefi drivers\n"
> > "efitool dh\n"
> > - " - show uefi handles\n";
> > + " - show uefi handles\n"
> > + "efitool images\n"
> > + " - show loaded images\n";
> > #endif
> >
> > U_BOOT_CMD(
> >
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 7/9] cmd: efitool: add memmap command
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
` (5 preceding siblings ...)
2019-01-15 2:55 ` [U-Boot] [PATCH v4 6/9] cmd: efitool: add images command AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 4:26 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 8/9] cmd: efitool: export uefi variable helper functions AKASHI Takahiro
` (2 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
"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/efitool.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 77 insertions(+), 1 deletion(-)
diff --git a/cmd/efitool.c b/cmd/efitool.c
index e77273fc6e1e..f06718ea580d 100644
--- a/cmd/efitool.c
+++ b/cmd/efitool.c
@@ -540,6 +540,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 *memmap = NULL, *map;
+ efi_uintn_t map_size = 0;
+ int i, sep;
+ efi_status_t ret;
+
+ ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ memmap = malloc(map_size);
+ if (!memmap)
+ return CMD_RET_FAILURE;
+ ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
+ }
+ if (ret != EFI_SUCCESS) {
+ free(memmap);
+ return CMD_RET_FAILURE;
+ }
+
+ printf("Type Start End Attributes\n");
+ printf("================ ================ ================ ==========\n");
+ for (i = 0, map = memmap; 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(memmap);
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -955,6 +1027,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
return do_efi_show_handles(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;
}
@@ -987,7 +1061,9 @@ static char efitool_help_text[] =
"efitool dh\n"
" - show uefi handles\n"
"efitool images\n"
- " - show loaded images\n";
+ " - show loaded images\n"
+ "efitool memmap\n"
+ " - show uefi memory map\n";
#endif
U_BOOT_CMD(
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 7/9] cmd: efitool: add memmap command
2019-01-15 2:55 ` [U-Boot] [PATCH v4 7/9] cmd: efitool: add memmap command AKASHI Takahiro
@ 2019-01-15 4:26 ` Heinrich Schuchardt
2019-01-17 7:03 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 4:26 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> "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/efitool.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efitool.c b/cmd/efitool.c
> index e77273fc6e1e..f06718ea580d 100644
> --- a/cmd/efitool.c
> +++ b/cmd/efitool.c
> @@ -540,6 +540,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 *memmap = NULL, *map;
> + efi_uintn_t map_size = 0;
> + int i, sep;
> + efi_status_t ret;
> +
> + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
ret = EFI_SUCCCESS will only occur if the memory has not been setup yet.
So shouldn't we error out if ret != EFI_BUFFER_TOO_SMALL ?
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + memmap = malloc(map_size);
> + if (!memmap)
> + return CMD_RET_FAILURE;
> + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
> + }
> + if (ret != EFI_SUCCESS) {
> + free(memmap);
> + return CMD_RET_FAILURE;
> + }
> +
> + printf("Type Start End Attributes\n");
> + printf("================ ================ ================ ==========\n");
> + for (i = 0, map = memmap; 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");
Please, put that logic into a separate function which returns a const
char *. Then we need only a single printf().
I suggest to use an array for the mapping:
struct memory_attribute_name {
u64 attribute;
const char *short_name;
};
struct memory_attribute_name memory_attribute_names[] = {
{EFI_MEMORY_UC, "UC"},
...
};
For looping you can use ARRAY_SIZE(memory_attribute_names).
> + putc('\n');
> + }
> +
> + free(memmap);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> static int do_efi_boot_add(int argc, char * const argv[])
> {
Please, refer to doc/README.commands for the standard way of adding
sub-commands.
Best regards
Heinrich
> int id;
> @@ -955,6 +1027,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
> return do_efi_show_handles(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;
> }
> @@ -987,7 +1061,9 @@ static char efitool_help_text[] =
> "efitool dh\n"
> " - show uefi handles\n"
> "efitool images\n"
> - " - show loaded images\n";
> + " - show loaded images\n"
> + "efitool memmap\n"
> + " - show uefi memory map\n";
> #endif
>
> U_BOOT_CMD(
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 7/9] cmd: efitool: add memmap command
2019-01-15 4:26 ` Heinrich Schuchardt
@ 2019-01-17 7:03 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-17 7:03 UTC (permalink / raw)
To: u-boot
On Tue, Jan 15, 2019 at 05:26:42AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> > "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/efitool.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 77 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/efitool.c b/cmd/efitool.c
> > index e77273fc6e1e..f06718ea580d 100644
> > --- a/cmd/efitool.c
> > +++ b/cmd/efitool.c
> > @@ -540,6 +540,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 *memmap = NULL, *map;
> > + efi_uintn_t map_size = 0;
> > + int i, sep;
> > + efi_status_t ret;
> > +
> > + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
>
> ret = EFI_SUCCCESS will only occur if the memory has not been setup yet.
I don't think so.
> So shouldn't we error out if ret != EFI_BUFFER_TOO_SMALL ?
>
> > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > + memmap = malloc(map_size);
> > + if (!memmap)
> > + return CMD_RET_FAILURE;
> > + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
> > + }
> > + if (ret != EFI_SUCCESS) {
> > + free(memmap);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + printf("Type Start End Attributes\n");
> > + printf("================ ================ ================ ==========\n");
> > + for (i = 0, map = memmap; 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");
>
> Please, put that logic into a separate function which returns a const
> char *. Then we need only a single printf().
I don't think it's worth generating a one-time string only for this
specific use.
If 'printf' is a matter, I will simply replace it to puts() here.
> I suggest to use an array for the mapping:
>
> struct memory_attribute_name {
> u64 attribute;
> const char *short_name;
> };
>
> struct memory_attribute_name memory_attribute_names[] = {
> {EFI_MEMORY_UC, "UC"},
> ...
> };
>
> For looping you can use ARRAY_SIZE(memory_attribute_names).
>
> > + putc('\n');
> > + }
> > +
> > + free(memmap);
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > static int do_efi_boot_add(int argc, char * const argv[])
> > {
>
> Please, refer to doc/README.commands for the standard way of adding
> sub-commands.
OK
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > int id;
> > @@ -955,6 +1027,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
> > return do_efi_show_handles(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;
> > }
> > @@ -987,7 +1061,9 @@ static char efitool_help_text[] =
> > "efitool dh\n"
> > " - show uefi handles\n"
> > "efitool images\n"
> > - " - show loaded images\n";
> > + " - show loaded images\n"
> > + "efitool memmap\n"
> > + " - show uefi memory map\n";
> > #endif
> >
> > U_BOOT_CMD(
> >
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 8/9] cmd: efitool: export uefi variable helper functions
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
` (6 preceding siblings ...)
2019-01-15 2:55 ` [U-Boot] [PATCH v4 7/9] cmd: efitool: add memmap command AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 5:28 ` Heinrich Schuchardt
2019-01-15 2:55 ` [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables AKASHI Takahiro
2019-01-15 9:16 ` [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment Alexander Graf
9 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
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/efitool.c | 38 ++++++++++++++++++++++++++++++++++----
include/command.h | 4 ++++
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/cmd/efitool.c b/cmd/efitool.c
index f06718ea580d..b8fe28c53aaf 100644
--- a/cmd/efitool.c
+++ b/cmd/efitool.c
@@ -67,7 +67,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};
@@ -130,6 +130,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;
@@ -227,7 +242,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;
efi_uintn_t size = 0;
@@ -270,6 +285,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 int efi_get_handles_by_proto(efi_guid_t *guid, efi_handle_t **handlesp,
int *num)
{
@@ -1016,9 +1046,9 @@ static int do_efitool(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 feddef300ccc..315e4b9aabfb 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)
extern int do_bootefi_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
#endif
+#if defined(CONFIG_CMD_EFITOOL)
+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
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 8/9] cmd: efitool: export uefi variable helper functions
2019-01-15 2:55 ` [U-Boot] [PATCH v4 8/9] cmd: efitool: export uefi variable helper functions AKASHI Takahiro
@ 2019-01-15 5:28 ` Heinrich Schuchardt
2019-01-17 7:30 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 5:28 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> 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/efitool.c | 38 ++++++++++++++++++++++++++++++++++----
> include/command.h | 4 ++++
> 2 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/cmd/efitool.c b/cmd/efitool.c
> index f06718ea580d..b8fe28c53aaf 100644
> --- a/cmd/efitool.c
> +++ b/cmd/efitool.c
> @@ -67,7 +67,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[])
Please, do not use two function names only distinguished by and
underscore. A a reader I will always wonder which does what.
Please, use names that describe what the difference is.
> {
> char regex[256];
> char * const regexlist[] = {regex};
> @@ -130,6 +130,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);
You duplicate this code below.
So, please, put this into a separate function.
That could also help to avoid having separate do_efi_set_var and
_do_efi_set_var.
> + 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;
> @@ -227,7 +242,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;
> efi_uintn_t size = 0;
> @@ -270,6 +285,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",
It is more then drivers that we setup. So perhaps
"Cannot initialize UEFI sub-system."
> + r & ~EFI_ERROR_MASK);
> + return CMD_RET_FAILURE;
> + }
> +
> + return _do_efi_set_var(argc, argv);
> +}
> +
> static int efi_get_handles_by_proto(efi_guid_t *guid, efi_handle_t **handlesp,
> int *num)
> {
> @@ -1016,9 +1046,9 @@ static int do_efitool(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 feddef300ccc..315e4b9aabfb 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)
> extern int do_bootefi_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> #endif
> +#if defined(CONFIG_CMD_EFITOOL)
The definition has not dependencies. No need for an #if here.
> +int do_efi_dump_var(int argc, char * const argv[]);
> +int do_efi_set_var(int argc, char * const argv[]);
> +#endi
Shouldn't we put this into some include/efi*?
Best regards
Heinrich
>
> /* common/command.c */
> int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 8/9] cmd: efitool: export uefi variable helper functions
2019-01-15 5:28 ` Heinrich Schuchardt
@ 2019-01-17 7:30 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-17 7:30 UTC (permalink / raw)
To: u-boot
On Tue, Jan 15, 2019 at 06:28:38AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> > 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/efitool.c | 38 ++++++++++++++++++++++++++++++++++----
> > include/command.h | 4 ++++
> > 2 files changed, 38 insertions(+), 4 deletions(-)
> >
> > diff --git a/cmd/efitool.c b/cmd/efitool.c
> > index f06718ea580d..b8fe28c53aaf 100644
> > --- a/cmd/efitool.c
> > +++ b/cmd/efitool.c
> > @@ -67,7 +67,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[])
>
> Please, do not use two function names only distinguished by and
> underscore. A a reader I will always wonder which does what.
While I believe that "_" and "__" are a very common practice to
show an internal version of function,
> Please, use names that describe what the difference is.
I will revert _do_efi_dump_var() to do_efi_dump_var() and
change do_efi_dump_var() to do_efi_dump_var_ext().
> > {
> > char regex[256];
> > char * const regexlist[] = {regex};
> > @@ -130,6 +130,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);
>
> You duplicate this code below.
Do you want another function for just calling one function?
I don't think it's worth doing so.
> So, please, put this into a separate function.
>
> That could also help to avoid having separate do_efi_set_var and
> _do_efi_set_var.
I don't get your point.
Do you want to call efi_init_obj_list() at every do_efi_xxx()?
It just doesn't make sense.
> > + 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;
> > @@ -227,7 +242,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;
> > efi_uintn_t size = 0;
> > @@ -270,6 +285,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",
>
> It is more then drivers that we setup. So perhaps
>
> "Cannot initialize UEFI sub-system."
I don't mind, but that string directly comes from the original code
in bootefi.c.
> > + r & ~EFI_ERROR_MASK);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + return _do_efi_set_var(argc, argv);
> > +}
> > +
> > static int efi_get_handles_by_proto(efi_guid_t *guid, efi_handle_t **handlesp,
> > int *num)
> > {
> > @@ -1016,9 +1046,9 @@ static int do_efitool(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 feddef300ccc..315e4b9aabfb 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)
> > extern int do_bootefi_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> > #endif
> > +#if defined(CONFIG_CMD_EFITOOL)
>
> The definition has not dependencies. No need for an #if here.
Currently has, but as you and Alex suggested in a separate thread,
we may want to enable it (and do_efi_dump/set_var() as well)
whether or not CMD_BOOTEFI and CMD_EFIDEBUG, respectively, are defined.
Thanks,
-Takahiro Akashi
> > +int do_efi_dump_var(int argc, char * const argv[]);
> > +int do_efi_set_var(int argc, char * const argv[]);
> > +#endi
>
> Shouldn't we put this into some include/efi*?
>
> Best regards
>
> Heinrich
>
> >
> > /* common/command.c */
> > int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
> >
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
` (7 preceding siblings ...)
2019-01-15 2:55 ` [U-Boot] [PATCH v4 8/9] cmd: efitool: export uefi variable helper functions AKASHI Takahiro
@ 2019-01-15 2:55 ` AKASHI Takahiro
2019-01-15 3:47 ` Heinrich Schuchardt
2019-01-15 13:26 ` Alexander Graf
2019-01-15 9:16 ` [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment Alexander Graf
9 siblings, 2 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2019-01-15 2:55 UTC (permalink / raw)
To: u-boot
"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 ce746bbf1b3e..44f6c3759253 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_EFITOOL)
+ 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_EFITOOL)
+ 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_EFITOOL)
+ "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_EFITOOL)
+ "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_EFITOOL)
+ "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_EFITOOL)
+ "-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'",
@@ -1345,7 +1402,7 @@ U_BOOT_CMD_COMPLETE(
"run commands in an environment variable",
"var [...]\n"
" - run the commands in the environment variable(s) 'var'"
-#if defined(CONFIG_CMD_BOOTEFI)
+#if defined(CONFIG_CMD_EFITOOL)
"\n"
"run -e [BootXXXX]\n"
" - load and run UEFI app based on 'BootXXXX' UEFI variable",
--
2.19.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables
2019-01-15 2:55 ` [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables AKASHI Takahiro
@ 2019-01-15 3:47 ` Heinrich Schuchardt
2019-01-15 13:23 ` Alexander Graf
2019-01-15 13:26 ` Alexander Graf
1 sibling, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2019-01-15 3:47 UTC (permalink / raw)
To: u-boot
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> "env [print|set] -e" allows for handling uefi variables without
> knowing details about mapping to corresponding u-boot variables.
Why do we need two alternative ways to achieve the same result?
We already have 'efitool setvar' and 'efitool dumpvar'.
Best regards
Heinrich
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables
2019-01-15 3:47 ` Heinrich Schuchardt
@ 2019-01-15 13:23 ` Alexander Graf
0 siblings, 0 replies; 31+ messages in thread
From: Alexander Graf @ 2019-01-15 13:23 UTC (permalink / raw)
To: u-boot
On 01/15/2019 04:47 AM, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
>> "env [print|set] -e" allows for handling uefi variables without
>> knowing details about mapping to corresponding u-boot variables.
> Why do we need two alternative ways to achieve the same result?
> We already have 'efitool setvar' and 'efitool dumpvar'.
I asked for it.
If it was me, I would just drop the "efitool" versions. The more we can
have in common code / CLI, the better for everyone.
But if Takahiro is passionate about having everything accessible through
a single efi helper command for his debugging, I wouldn't oppose to it :).
Alex
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables
2019-01-15 2:55 ` [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables AKASHI Takahiro
2019-01-15 3:47 ` Heinrich Schuchardt
@ 2019-01-15 13:26 ` Alexander Graf
1 sibling, 0 replies; 31+ messages in thread
From: Alexander Graf @ 2019-01-15 13:26 UTC (permalink / raw)
To: u-boot
On 01/15/2019 03:55 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>
> ---
> cmd/nvedit.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index ce746bbf1b3e..44f6c3759253 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_EFITOOL)
This needs to be available without your EFITOOL (which really is a DEBUG
command). So this needs to be accessible using only CONFIG_EFI_LOADER I
think. Maybe we can add a CONFIG_CMD_EFI_SUBCMD or so that these bits
are guarded by if you think that someone really wants to live without.
Alex
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment
2019-01-15 2:55 [U-Boot] [PATCH v4 0/9] cmd: add efitool for efi environment AKASHI Takahiro
` (8 preceding siblings ...)
2019-01-15 2:55 ` [U-Boot] [PATCH v4 9/9] cmd: env: add "-e" option for handling UEFI variables AKASHI Takahiro
@ 2019-01-15 9:16 ` Alexander Graf
9 siblings, 0 replies; 31+ messages in thread
From: Alexander Graf @ 2019-01-15 9:16 UTC (permalink / raw)
To: u-boot
> Am 15.01.2019 um 03:55 schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
>
> 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.
Thanks a lot again. This is invaluable for efi_loader debugging. I found myself so many times adding printfs to enumarate bits this command would have just told me.
>
> Let's see how it works:
> => efitool boot add 1 SHELL mmc 0:1 /Shell.efi ""
efidebug please :). I thought we agreed that this is an optional command, people can not expect to have around with no command line syntax guarantees? The name must reflect that. Efitool does not.
Alex
^ permalink raw reply [flat|nested] 31+ messages in thread