efi: make efi and bootmgr more usable

With an extra argument, efi_bootmgr_load() can now load an efi binary based on a "BootXXXX" variable specified.

Signed-off-by AKASHI Takahiro <[email protected]>

---
 include/efi_loader.h         | 3 ++-
 lib/efi_loader/efi_bootmgr.c | 8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

include/efi_loader.h +2 -1

index d83de906fbce..ce0f420b5004 100644
@@ -540,7 +540,8 @@ struct efi_load_option {
 
 void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
 unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+		       struct efi_device_path **device_path,
 		       struct efi_device_path **file_path);
 
 #else /* CONFIG_IS_ENABLED(EFI_LOADER) */

lib/efi_loader/efi_bootmgr.c +2 -1

index a095df3f540b..74eb832a8c92 100644
@@ -170,7 +170,8 @@ error:
  * available load-options, finding and returning the first one that can
  * be loaded successfully.
  */
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+		       struct efi_device_path **device_path,
 		       struct efi_device_path **file_path)
 {
 	uint16_t *bootorder;

lib/efi_loader/efi_bootmgr.c +2 -0

+	}
+
 	bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
 	if (!bootorder)
 		goto error;

Quoted 4 lines · AKASHI Takahiro

With an extra argument, efi_bootmgr_load() can now load an efi binary based on a "BootXXXX" variable specified.

Signed-off-by AKASHI Takahiro <[email protected]>

I don't see you changing the caller, so this hunk won't compile on its own?

Please make sure that every single step in your patch set compiles.

Quoted 37 lines

---
 include/efi_loader.h         | 3 ++-
 lib/efi_loader/efi_bootmgr.c | 8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

include/efi_loader.h +2 -1

index d83de906fbce..ce0f420b5004 100644
@@ -540,7 +540,8 @@ struct efi_load_option {
 
 void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
 unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+		       struct efi_device_path **device_path,
 		       struct efi_device_path **file_path);
 
 #else /* CONFIG_IS_ENABLED(EFI_LOADER) */

lib/efi_loader/efi_bootmgr.c +2 -1

index a095df3f540b..74eb832a8c92 100644
@@ -170,7 +170,8 @@ error:
  * available load-options, finding and returning the first one that can
  * be loaded successfully.
  */
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+		       struct efi_device_path **device_path,
 		       struct efi_device_path **file_path)
 {
 	uint16_t *bootorder;

@@ -183,6 +184,11 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
 	bs = systab.boottime;
 	rs = systab.runtime;
 

lib/efi_loader/efi_bootmgr.c void *efi_bootmgr_load(struct efi_device_path **device_path, · line 189

Quoted 3 lines

lib/efi_loader/efi_bootmgr.c +3 -0

+	if (boot_id != -1) {
+		image = try_load_entry(boot_id, device_path, file_path);
+		goto error;

I think at this point it might be better to split the function, no?

You're not actually reusing any code from efi_bootmgr_load with the 2 instantiation types (explicit boot id, loop through boot order).

Alex

Quoted 6 lines

lib/efi_loader/efi_bootmgr.c +2 -0

+	}
+
 	bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
 	if (!bootorder)
 		goto error;

Quoted 9 lines · Alexander Graf

Quoted 4 lines · AKASHI Takahiro

With an extra argument, efi_bootmgr_load() can now load an efi binary based on a "BootXXXX" variable specified.

Signed-off-by AKASHI Takahiro <[email protected]>

I don't see you changing the caller, so this hunk won't compile on its own?

You're correct.

Quoted 1 line

Please make sure that every single step in your patch set compiles.

I will try to double-check in the future.

Quoted 37 lines

Quoted 37 lines

---
 include/efi_loader.h         | 3 ++-
 lib/efi_loader/efi_bootmgr.c | 8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

include/efi_loader.h +2 -1

index d83de906fbce..ce0f420b5004 100644
@@ -540,7 +540,8 @@ struct efi_load_option {
 
 void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
 unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+		       struct efi_device_path **device_path,
 		       struct efi_device_path **file_path);
 
 #else /* CONFIG_IS_ENABLED(EFI_LOADER) */

lib/efi_loader/efi_bootmgr.c +2 -1

index a095df3f540b..74eb832a8c92 100644
@@ -170,7 +170,8 @@ error:
  * available load-options, finding and returning the first one that can
  * be loaded successfully.
  */
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+		       struct efi_device_path **device_path,
 		       struct efi_device_path **file_path)
 {
 	uint16_t *bootorder;

@@ -183,6 +184,11 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
 	bs = systab.boottime;
 	rs = systab.runtime;
 

Quoted 5 lines

lib/efi_loader/efi_bootmgr.c void *efi_bootmgr_load(struct efi_device_path **device_path, · line 189

Quoted 3 lines

lib/efi_loader/efi_bootmgr.c +3 -0

+	if (boot_id != -1) {
+		image = try_load_entry(boot_id, device_path, file_path);
+		goto error;

I think at this point it might be better to split the function, no?

It's one way to fix, but I want simply to fix the issue by modifying the caller, do_bootefi_bootmgr_exec() because

Quoted 2 lines

You're not actually reusing any code from efi_bootmgr_load with the 2 instantiation types (explicit boot id, loop through boot order).

I will add "BootNext" support to efi_bootmgr_load() so that it will be a single point of loading. See my relevant patch:
https://lists.denx.de/pipermail/u-boot/2018-November/349281.html

Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.

The new command, efishell, helps address these issues and give us more friendly interfaces:
* efishell boot add: add BootXXXX variable
* efishell boot rm: remove BootXXXX variable
* efishell boot dump: display all BootXXXX variables
* efishell boot order: set/display a boot order (BootOrder)
* efishell setvar: set an UEFI variable (with limited functionality)
* efishell dumpvar: display all UEFI variables

As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/Makefile   |   2 +-
 cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
 cmd/efishell.h |   5 +
 3 files changed, 679 insertions(+), 1 deletion(-)
 create mode 100644 cmd/efishell.c
 create mode 100644 cmd/efishell.h

cmd/Makefile +0 -0

index d9cdaf6064b8..bd531d505a8e 100644

cmd/Makefile +0 -0

 obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
 obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
 obj-$(CONFIG_CMD_BOOTZ) += bootz.o

cmd/efishell.c new file +12 -0

new file mode 100644
index 000000000000..abc8216c7bd6
@@ -0,0 +1,673 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  EFI Shell-like command
+ *
+ *  Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
+ */
+
+#include <charset.h>
+#include <common.h>
+#include <command.h>
+#include <efi_loader.h>
+#include <environment.h>

cmd/efishell.c new file +303 -0

+
+static void dump_var_data(char *data, unsigned long len)
+{
+	char *start, *end, *p;
+	unsigned long pos, count;
+	char hex[3], line[9];
+	int i;
+
+	end = data + len;
+	for (start = data, pos = 0; start < end; start += count, pos += count) {
+		count = end - start;
+		if (count > 16)
+			count = 16;
+
+		/* count should be multiple of two */
+		printf("%08lx: ", pos);
+
+		/* in hex format */
+		p = start;
+		for (i = 0; i < count / 2; p += 2, i++)
+			printf(" %c%c", *p, *(p + 1));
+		for (; i < 8; i++)
+			printf("   ");
+
+		/* in character format */
+		p = start;
+		hex[2] = '\0';
+		for (i = 0; i < count / 2; i++) {
+			hex[0] = *p++;
+			hex[1] = *p++;
+			line[i] = simple_strtoul(hex, 0, 16);
+			if (line[i] < 0x20 || line[i] > 0x7f)
+				line[i] = '.';
+		}
+		line[i] = '\0';
+		printf("  %s\n", line);
+	}
+}
+
+/*
+ * From efi_variable.c,
+ *
+ * Mapping between EFI variables and u-boot variables:
+ *
+ *   efi_$guid_$varname = {attributes}(type)value
+ */
+static int do_efi_dump_var(int argc, char * const argv[])
+{
+	char regex[256];
+	char * const regexlist[] = {regex};
+	char *res = NULL, *start, *end;
+	int len;
+
+	if (argc > 2)
+		return CMD_RET_USAGE;
+
+	if (argc == 2)
+		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
+	else
+		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
+	debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
+
+	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+			&res, 0, 1, regexlist);
+
+	if (len < 0)
+		return CMD_RET_FAILURE;
+
+	if (len > 0) {
+		end = res;
+		while (true) {
+			/* variable name */
+			start = strchr(end, '_');
+			if (!start)
+				break;
+			start = strchr(++start, '_');
+			if (!start)
+				break;
+			end = strchr(++start, '=');
+			if (!end)
+				break;
+			printf("%.*s:", (int)(end - start), start);
+			end++;
+
+			/* value */
+			start = end;
+			end = strstr(start, "(blob)");
+			if (!end) {
+				putc('\n');
+				break;
+			}
+			end += 6;
+			printf(" %.*s\n", (int)(end - start), start);
+
+			start = end;
+			end = strchr(start, '\n');
+			if (!end)
+				break;
+			dump_var_data(start,  end - start);
+		}
+		free(res);
+
+		if (len < 2 && argc == 2)
+			printf("%s: not found\n", argv[1]);
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static int append_value(char **bufp, unsigned long *sizep, char *data)
+{
+	char *tmp_buf = NULL, *new_buf = NULL, *value;
+	unsigned long len = 0;
+
+	if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
+		union {
+			u8 u8;
+			u16 u16;
+			u32 u32;
+			u64 u64;
+		} tmp_data;
+		unsigned long hex_value;
+		void *hex_ptr;
+
+		data += 3;
+		len = strlen(data);
+		if ((len & 0x1)) /* not multiple of two */
+			return -1;
+
+		len /= 2;
+		if (len > 8)
+			return -1;
+		else if (len > 4)
+			len = 8;
+		else if (len > 2)
+			len = 4;
+
+		/* convert hex hexadecimal number */
+		if (strict_strtoul(data, 16, &hex_value) < 0)
+			return -1;
+
+		tmp_buf = malloc(len);
+		if (!tmp_buf)
+			return -1;
+
+		if (len == 1) {
+			tmp_data.u8 = hex_value;
+			hex_ptr = &tmp_data.u8;
+		} else if (len == 2) {
+			tmp_data.u16 = hex_value;
+			hex_ptr = &tmp_data.u16;
+		} else if (len == 4) {
+			tmp_data.u32 = hex_value;
+			hex_ptr = &tmp_data.u32;
+		} else {
+			tmp_data.u64 = hex_value;
+			hex_ptr = &tmp_data.u64;
+		}
+		memcpy(tmp_buf, hex_ptr, len);
+		value = tmp_buf;
+
+	} else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
+		data += 2;
+		len = strlen(data);
+		if (len & 0x1) /* not multiple of two */
+			return -1;
+
+		len /= 2;
+		tmp_buf = malloc(len);
+		if (!tmp_buf)
+			return -1;
+
+		if (hex2bin((u8 *)tmp_buf, data, len) < 0)
+			return -1;
+
+		value = tmp_buf;
+	} else { /* string */
+		if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
+			if (data[1] == '"')
+				data += 2;
+			else
+				data += 3;
+			value = data;
+			len = strlen(data) - 1;
+			if (data[len] != '"')
+				return -1;
+		} else {
+			value = data;
+			len = strlen(data);
+		}
+	}
+
+	new_buf = realloc(*bufp, *sizep + len);
+	if (!new_buf)
+		goto out;
+
+	memcpy(new_buf + *sizep, value, len);
+	*bufp = new_buf;
+	*sizep += len;
+
+out:
+	free(tmp_buf);
+
+	return 0;
+}
+
+static int do_efi_set_var(int argc, char * const argv[])
+{
+	char *var_name, *value = NULL;
+	unsigned long size = 0;
+	u16 *var_name16, *p;
+	efi_guid_t guid;
+	efi_status_t ret;
+
+	if (argc == 1)
+		return CMD_RET_SUCCESS;
+
+	var_name = argv[1];
+	if (argc == 2) {
+		/* remove */
+		value = NULL;
+		size = 0;
+	} else { /* set */
+		argc -= 2;
+		argv += 2;
+
+		for ( ; argc > 0; argc--, argv++)
+			if (append_value(&value, &size, argv[0]) < 0) {
+				ret = CMD_RET_FAILURE;
+				goto out;
+			}
+	}
+
+	var_name16 = malloc((strlen(var_name) + 1) * 2);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
+
+	guid = efi_global_variable_guid;
+	ret = efi_set_variable(var_name16, &guid,
+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, value);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	return ret;
+}
+
+static int do_efi_boot_add(int argc, char * const argv[])
+{
+	int id;
+	char *endp;
+	char var_name[9];
+	u16 var_name16[9], *p;
+	efi_guid_t guid;
+	size_t label_len, label_len16;
+	u16 *label;
+	struct efi_device_path *device_path = NULL, *file_path = NULL;
+	struct efi_load_option lo;
+	void *data = NULL;
+	unsigned long size;
+	int ret;
+
+	if (argc < 6 || argc > 7)
+		return CMD_RET_USAGE;
+
+	id = (int)simple_strtoul(argv[1], &endp, 0);
+	if (*endp != '\0' || id > 0xffff)
+		return CMD_RET_FAILURE;
+
+	sprintf(var_name, "Boot%04X", id);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, 9);
+
+	guid = efi_global_variable_guid;
+
+	/* attributes */
+	lo.attributes = 0x1; /* always ACTIVE */
+
+	/* label */
+	label_len = strlen(argv[2]);
+	label_len16 = utf8_utf16_strnlen(argv[2], label_len);
+	label = malloc((label_len16 + 1) * sizeof(u16));
+	if (!label)
+		return CMD_RET_FAILURE;
+	lo.label = label; /* label will be changed below */
+	utf8_utf16_strncpy(&label, argv[2], label_len);
+
+	/* file path */
+	ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
+			       &file_path);
+	if (ret != EFI_SUCCESS) {
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+	lo.file_path = file_path;
+	lo.file_path_length = efi_dp_size(file_path)
+				+ sizeof(struct efi_device_path); /* for END */
+
+	/* optional data */
+	lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
+
+	size = efi_serialize_load_option(&lo, (u8 **)&data);
+	if (!size) {
+		ret = CMD_RET_FAILURE;

cmd/efishell.c new file +92 -0

+	free(device_path);
+	free(file_path);
+#endif
+	free(lo.label);
+	return ret;
+}
+
+static int do_efi_boot_rm(int argc, char * const argv[])
+{
+	efi_guid_t guid;
+	int id, i;
+	char *endp;
+	char var_name[9];
+	u16 var_name16[9];
+	efi_status_t ret;
+
+	if (argc == 1)
+		return CMD_RET_USAGE;
+
+	guid = efi_global_variable_guid;
+	for (i = 1; i < argc; i++, argv++) {
+		id = (int)simple_strtoul(argv[1], &endp, 0);
+		if (*endp != '\0' || id > 0xffff)
+			return CMD_RET_FAILURE;
+
+		sprintf(var_name, "Boot%04X", id);
+		utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
+
+		ret = efi_set_variable(var_name16, &guid,
+				       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				       EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
+		if (ret) {
+			printf("cannot remove Boot%04X", id);
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static void show_efi_boot_opt_data(int id, void *data)
+{
+	struct efi_load_option lo;
+	char *label, *p;
+	size_t label_len16, label_len;
+	u16 *dp_str;
+
+	efi_deserialize_load_option(&lo, data);
+
+	label_len16 = u16_strlen(lo.label);
+	label_len = utf16_utf8_strnlen(lo.label, label_len16);
+	label = malloc(label_len + 1);
+	if (!label)
+		return;
+	p = label;
+	utf16_utf8_strncpy(&p, lo.label, label_len16);
+
+	printf("Boot%04X:\n", id);
+	printf("\tattributes: %c%c%c (0x%08x)\n",
+	       /* ACTIVE */
+	       lo.attributes & 0x1 ? 'A' : '-',
+	       /* FORCE RECONNECT */
+	       lo.attributes & 0x2 ? 'R' : '-',
+	       /* HIDDEN */
+	       lo.attributes & 0x8 ? 'H' : '-',
+	       lo.attributes);
+	printf("\tlabel: %s\n", label);
+
+	dp_str = efi_dp_str(lo.file_path);
+	printf("\tfile_path: %ls\n", dp_str);
+	efi_free_pool(dp_str);
+
+	printf("\tdata: %s\n", lo.optional_data);
+
+	free(label);
+}
+
+static void show_efi_boot_opt(int id)
+{
+	char var_name[9];
+	u16 var_name16[9], *p;
+	efi_guid_t guid;
+	void *data = NULL;
+	unsigned long size;
+	int ret;
+
+	sprintf(var_name, "Boot%04X", id);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, 9);
+	guid = efi_global_variable_guid;
+
+	size = 0;

cmd/efishell.c new file +54 -0

+	}
+}
+
+static int do_efi_boot_dump(int argc, char * const argv[])
+{
+	char regex[256];
+	char * const regexlist[] = {regex};
+	char *variables = NULL, *boot, *value;
+	int len;
+	int id;
+
+	if (argc > 1)
+		return CMD_RET_USAGE;
+
+	snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
+
+	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+			&variables, 0, 1, regexlist);
+
+	if (!len)
+		return CMD_RET_SUCCESS;
+
+	if (len < 0)
+		return CMD_RET_FAILURE;
+
+	boot = variables;
+	while (*boot) {
+		value = strstr(boot, "Boot") + 4;
+		id = (int)simple_strtoul(value, NULL, 16);
+		show_efi_boot_opt(id);
+		boot = strchr(boot, '\n');
+		if (!*boot)
+			break;
+		boot++;
+	}
+	free(variables);
+
+	return CMD_RET_SUCCESS;
+}
+
+static int show_efi_boot_order(void)
+{
+	efi_guid_t guid;
+	u16 *bootorder = NULL;
+	unsigned long size;
+	int num, i;
+	char var_name[9];
+	u16 var_name16[9], *p16;
+	void *data;
+	struct efi_load_option lo;
+	char *label, *p;
+	size_t label_len16, label_len;
+	efi_status_t ret = CMD_RET_SUCCESS;
+

cmd/efishell.c new file +17 -0

+		return CMD_RET_SUCCESS;
+	}
+
+	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;
+		}
+

cmd/efishell.c new file +145 -0

+		}
+
+		efi_deserialize_load_option(&lo, data);
+
+		label_len16 = u16_strlen(lo.label);
+		label_len = utf16_utf8_strnlen(lo.label, label_len16);
+		label = malloc(label_len + 1);
+		if (!label) {
+			free(data);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+		p = label;
+		utf16_utf8_strncpy(&p, lo.label, label_len16);
+		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
+		free(label);
+
+		free(data);
+	}
+out:
+	free(bootorder);
+
+	return ret;
+}
+
+static int do_efi_boot_order(int argc, char * const argv[])
+{
+	u16 *bootorder = NULL;
+	unsigned long size;
+	int id, i;
+	char *endp;
+	efi_guid_t guid;
+	efi_status_t ret;
+
+	if (argc == 1)
+		return show_efi_boot_order();
+
+	argc--;
+	argv++;
+
+	size = argc * sizeof(u16);
+	bootorder = malloc(size);
+	if (!bootorder)
+		return CMD_RET_FAILURE;
+
+	for (i = 0; i < argc; i++) {
+		id = (int)simple_strtoul(argv[i], &endp, 0);
+		if (*endp != '\0' || id > 0xffff) {
+			printf("invalid value: %s\n", argv[i]);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+
+		bootorder[i] = (u16)id;
+	}
+
+	guid = efi_global_variable_guid;
+	ret = efi_set_variable(L"BootOrder", &guid,
+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	free(bootorder);
+
+	return ret;
+}
+
+static int do_efi_boot_opt(int argc, char * const argv[])
+{
+	char *sub_command;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--; argv++;
+	sub_command = argv[0];
+
+	if (!strcmp(sub_command, "add"))
+		return do_efi_boot_add(argc, argv);
+	else if (!strcmp(sub_command, "rm"))
+		return do_efi_boot_rm(argc, argv);
+	else if (!strcmp(sub_command, "dump"))
+		return do_efi_boot_dump(argc, argv);
+	else if (!strcmp(sub_command, "order"))
+		return do_efi_boot_order(argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+
+/* Interpreter command to configure EFI environment */
+static int do_efishell(cmd_tbl_t *cmdtp, int flag,
+		       int argc, char * const argv[])
+{
+	char *command;
+	efi_status_t r;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--; argv++;
+	command = argv[0];
+
+	/* Initialize EFI drivers */
+	r = efi_init_obj_list();
+	if (r != EFI_SUCCESS) {
+		printf("Error: Cannot set up EFI drivers, r = %lu\n",
+		       r & ~EFI_ERROR_MASK);
+		return CMD_RET_FAILURE;
+	}
+
+	if (!strcmp(command, "boot"))
+		return do_efi_boot_opt(argc, argv);
+	else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
+		return do_efi_dump_var(argc, argv);
+	else if (!strcmp(command, "setvar"))
+		return do_efi_set_var(argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char efishell_help_text[] =
+	"  - EFI Shell-like interface to configure EFI environment\n"
+	"\n"
+	"efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
+	"  - set uefi's BOOT variable\n"
+	"efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
+	"  - set/display uefi boot order\n"
+	"efishell boot dump\n"
+	"  - display all uefi's BOOT variables\n"
+	"efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
+	"  - set/display uefi boot order\n"
+	"\n"
+	"efishell dumpvar [<name>]\n"
+	"  - get uefi variable's value\n"
+	"efishell setvar <name> [<value>]\n"
+	"  - set/delete uefi variable's value\n"
+	"    <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
+#endif
+
+U_BOOT_CMD(
+	efishell, 10, 0, do_efishell,
+	"Configure EFI environment",
+	efishell_help_text
+);

cmd/efishell.h new file +0 -0

new file mode 100644
index 000000000000..6f70b402b26b

Quoted 26 lines · AKASHI Takahiro

Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.

The new command, efishell, helps address these issues and give us more friendly interfaces:
* efishell boot add: add BootXXXX variable
* efishell boot rm: remove BootXXXX variable
* efishell boot dump: display all BootXXXX variables
* efishell boot order: set/display a boot order (BootOrder)
* efishell setvar: set an UEFI variable (with limited functionality)
* efishell dumpvar: display all UEFI variables

As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/Makefile   |   2 +-
 cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
 cmd/efishell.h |   5 +
 3 files changed, 679 insertions(+), 1 deletion(-)
 create mode 100644 cmd/efishell.c
 create mode 100644 cmd/efishell.h

cmd/Makefile obj-$(CONFIG_CMD_BINOP) += binop.o · line 27

Quoted 10 lines

cmd/Makefile +1 -1

index d9cdaf6064b8..bd531d505a8e 100644
@@ -24,7 +24,7 @@ obj-$(CONFIG_CMD_BINOP) += binop.o
 obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
 obj-$(CONFIG_CMD_BMP) += bmp.o
 obj-$(CONFIG_CMD_BOOTCOUNT) += bootcount.o
-obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o
+obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o efishell.o

Please create a separate command line option for efishell and make it disabled by default.

I think it's a really really useful debugging aid, but in a normal environment people should only need to run efi binaries (plus bootmgr) and modify efi variables, no?

Quoted 27 lines

cmd/Makefile +0 -0

 obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
 obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
 obj-$(CONFIG_CMD_BOOTZ) += bootz.o

cmd/efishell.c new file +16 -0

new file mode 100644
index 000000000000..abc8216c7bd6
@@ -0,0 +1,673 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  EFI Shell-like command
+ *
+ *  Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
+ */
+
+#include <charset.h>
+#include <common.h>
+#include <command.h>
+#include <efi_loader.h>
+#include <environment.h>
+#include <errno.h>
+#include <exports.h>
+#include <hexdump.h>
+#include <malloc.h>

cmd/efishell.c line 22

Quoted 6 lines

cmd/efishell.c new file +6 -0

+#include <search.h>
+#include <linux/ctype.h>
+#include <asm/global_data.h>
+#include "efishell.h"
+
+DECLARE_GLOBAL_DATA_PTR;

Where in this file do you need gd?

Quoted 307 lines

cmd/efishell.c new file +307 -0

+
+static void dump_var_data(char *data, unsigned long len)
+{
+	char *start, *end, *p;
+	unsigned long pos, count;
+	char hex[3], line[9];
+	int i;
+
+	end = data + len;
+	for (start = data, pos = 0; start < end; start += count, pos += count) {
+		count = end - start;
+		if (count > 16)
+			count = 16;
+
+		/* count should be multiple of two */
+		printf("%08lx: ", pos);
+
+		/* in hex format */
+		p = start;
+		for (i = 0; i < count / 2; p += 2, i++)
+			printf(" %c%c", *p, *(p + 1));
+		for (; i < 8; i++)
+			printf("   ");
+
+		/* in character format */
+		p = start;
+		hex[2] = '\0';
+		for (i = 0; i < count / 2; i++) {
+			hex[0] = *p++;
+			hex[1] = *p++;
+			line[i] = simple_strtoul(hex, 0, 16);
+			if (line[i] < 0x20 || line[i] > 0x7f)
+				line[i] = '.';
+		}
+		line[i] = '\0';
+		printf("  %s\n", line);
+	}
+}
+
+/*
+ * From efi_variable.c,
+ *
+ * Mapping between EFI variables and u-boot variables:
+ *
+ *   efi_$guid_$varname = {attributes}(type)value
+ */
+static int do_efi_dump_var(int argc, char * const argv[])
+{
+	char regex[256];
+	char * const regexlist[] = {regex};
+	char *res = NULL, *start, *end;
+	int len;
+
+	if (argc > 2)
+		return CMD_RET_USAGE;
+
+	if (argc == 2)
+		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
+	else
+		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
+	debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
+
+	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+			&res, 0, 1, regexlist);
+
+	if (len < 0)
+		return CMD_RET_FAILURE;
+
+	if (len > 0) {
+		end = res;
+		while (true) {
+			/* variable name */
+			start = strchr(end, '_');
+			if (!start)
+				break;
+			start = strchr(++start, '_');
+			if (!start)
+				break;
+			end = strchr(++start, '=');
+			if (!end)
+				break;
+			printf("%.*s:", (int)(end - start), start);
+			end++;
+
+			/* value */
+			start = end;
+			end = strstr(start, "(blob)");
+			if (!end) {
+				putc('\n');
+				break;
+			}
+			end += 6;
+			printf(" %.*s\n", (int)(end - start), start);
+
+			start = end;
+			end = strchr(start, '\n');
+			if (!end)
+				break;
+			dump_var_data(start,  end - start);
+		}
+		free(res);
+
+		if (len < 2 && argc == 2)
+			printf("%s: not found\n", argv[1]);
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static int append_value(char **bufp, unsigned long *sizep, char *data)
+{
+	char *tmp_buf = NULL, *new_buf = NULL, *value;
+	unsigned long len = 0;
+
+	if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
+		union {
+			u8 u8;
+			u16 u16;
+			u32 u32;
+			u64 u64;
+		} tmp_data;
+		unsigned long hex_value;
+		void *hex_ptr;
+
+		data += 3;
+		len = strlen(data);
+		if ((len & 0x1)) /* not multiple of two */
+			return -1;
+
+		len /= 2;
+		if (len > 8)
+			return -1;
+		else if (len > 4)
+			len = 8;
+		else if (len > 2)
+			len = 4;
+
+		/* convert hex hexadecimal number */
+		if (strict_strtoul(data, 16, &hex_value) < 0)
+			return -1;
+
+		tmp_buf = malloc(len);
+		if (!tmp_buf)
+			return -1;
+
+		if (len == 1) {
+			tmp_data.u8 = hex_value;
+			hex_ptr = &tmp_data.u8;
+		} else if (len == 2) {
+			tmp_data.u16 = hex_value;
+			hex_ptr = &tmp_data.u16;
+		} else if (len == 4) {
+			tmp_data.u32 = hex_value;
+			hex_ptr = &tmp_data.u32;
+		} else {
+			tmp_data.u64 = hex_value;
+			hex_ptr = &tmp_data.u64;
+		}
+		memcpy(tmp_buf, hex_ptr, len);
+		value = tmp_buf;
+
+	} else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
+		data += 2;
+		len = strlen(data);
+		if (len & 0x1) /* not multiple of two */
+			return -1;
+
+		len /= 2;
+		tmp_buf = malloc(len);
+		if (!tmp_buf)
+			return -1;
+
+		if (hex2bin((u8 *)tmp_buf, data, len) < 0)
+			return -1;
+
+		value = tmp_buf;
+	} else { /* string */
+		if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
+			if (data[1] == '"')
+				data += 2;
+			else
+				data += 3;
+			value = data;
+			len = strlen(data) - 1;
+			if (data[len] != '"')
+				return -1;
+		} else {
+			value = data;
+			len = strlen(data);
+		}
+	}
+
+	new_buf = realloc(*bufp, *sizep + len);
+	if (!new_buf)
+		goto out;
+
+	memcpy(new_buf + *sizep, value, len);
+	*bufp = new_buf;
+	*sizep += len;
+
+out:
+	free(tmp_buf);
+
+	return 0;
+}
+
+static int do_efi_set_var(int argc, char * const argv[])
+{
+	char *var_name, *value = NULL;
+	unsigned long size = 0;
+	u16 *var_name16, *p;
+	efi_guid_t guid;
+	efi_status_t ret;
+
+	if (argc == 1)
+		return CMD_RET_SUCCESS;
+
+	var_name = argv[1];
+	if (argc == 2) {
+		/* remove */
+		value = NULL;
+		size = 0;
+	} else { /* set */
+		argc -= 2;
+		argv += 2;
+
+		for ( ; argc > 0; argc--, argv++)
+			if (append_value(&value, &size, argv[0]) < 0) {
+				ret = CMD_RET_FAILURE;
+				goto out;
+			}
+	}
+
+	var_name16 = malloc((strlen(var_name) + 1) * 2);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
+
+	guid = efi_global_variable_guid;
+	ret = efi_set_variable(var_name16, &guid,
+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, value);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	return ret;
+}
+
+static int do_efi_boot_add(int argc, char * const argv[])
+{
+	int id;
+	char *endp;
+	char var_name[9];
+	u16 var_name16[9], *p;
+	efi_guid_t guid;
+	size_t label_len, label_len16;
+	u16 *label;
+	struct efi_device_path *device_path = NULL, *file_path = NULL;
+	struct efi_load_option lo;
+	void *data = NULL;
+	unsigned long size;
+	int ret;
+
+	if (argc < 6 || argc > 7)
+		return CMD_RET_USAGE;
+
+	id = (int)simple_strtoul(argv[1], &endp, 0);
+	if (*endp != '\0' || id > 0xffff)
+		return CMD_RET_FAILURE;
+
+	sprintf(var_name, "Boot%04X", id);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, 9);
+
+	guid = efi_global_variable_guid;
+
+	/* attributes */
+	lo.attributes = 0x1; /* always ACTIVE */
+
+	/* label */
+	label_len = strlen(argv[2]);
+	label_len16 = utf8_utf16_strnlen(argv[2], label_len);
+	label = malloc((label_len16 + 1) * sizeof(u16));
+	if (!label)
+		return CMD_RET_FAILURE;
+	lo.label = label; /* label will be changed below */
+	utf8_utf16_strncpy(&label, argv[2], label_len);
+
+	/* file path */
+	ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
+			       &file_path);
+	if (ret != EFI_SUCCESS) {
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+	lo.file_path = file_path;
+	lo.file_path_length = efi_dp_size(file_path)
+				+ sizeof(struct efi_device_path); /* for END */
+
+	/* optional data */
+	lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
+
+	size = efi_serialize_load_option(&lo, (u8 **)&data);
+	if (!size) {
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+
+	ret = efi_set_variable(var_name16, &guid,

cmd/efishell.c line 335

Quoted 6 lines

cmd/efishell.c new file +6 -0

+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, data);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	free(data);
+#if 0 /* FIXME */

Eh, what? :)

Quoted 96 lines

cmd/efishell.c new file +96 -0

+	free(device_path);
+	free(file_path);
+#endif
+	free(lo.label);
+	return ret;
+}
+
+static int do_efi_boot_rm(int argc, char * const argv[])
+{
+	efi_guid_t guid;
+	int id, i;
+	char *endp;
+	char var_name[9];
+	u16 var_name16[9];
+	efi_status_t ret;
+
+	if (argc == 1)
+		return CMD_RET_USAGE;
+
+	guid = efi_global_variable_guid;
+	for (i = 1; i < argc; i++, argv++) {
+		id = (int)simple_strtoul(argv[1], &endp, 0);
+		if (*endp != '\0' || id > 0xffff)
+			return CMD_RET_FAILURE;
+
+		sprintf(var_name, "Boot%04X", id);
+		utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
+
+		ret = efi_set_variable(var_name16, &guid,
+				       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				       EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
+		if (ret) {
+			printf("cannot remove Boot%04X", id);
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static void show_efi_boot_opt_data(int id, void *data)
+{
+	struct efi_load_option lo;
+	char *label, *p;
+	size_t label_len16, label_len;
+	u16 *dp_str;
+
+	efi_deserialize_load_option(&lo, data);
+
+	label_len16 = u16_strlen(lo.label);
+	label_len = utf16_utf8_strnlen(lo.label, label_len16);
+	label = malloc(label_len + 1);
+	if (!label)
+		return;
+	p = label;
+	utf16_utf8_strncpy(&p, lo.label, label_len16);
+
+	printf("Boot%04X:\n", id);
+	printf("\tattributes: %c%c%c (0x%08x)\n",
+	       /* ACTIVE */
+	       lo.attributes & 0x1 ? 'A' : '-',
+	       /* FORCE RECONNECT */
+	       lo.attributes & 0x2 ? 'R' : '-',
+	       /* HIDDEN */
+	       lo.attributes & 0x8 ? 'H' : '-',
+	       lo.attributes);
+	printf("\tlabel: %s\n", label);
+
+	dp_str = efi_dp_str(lo.file_path);
+	printf("\tfile_path: %ls\n", dp_str);
+	efi_free_pool(dp_str);
+
+	printf("\tdata: %s\n", lo.optional_data);
+
+	free(label);
+}
+
+static void show_efi_boot_opt(int id)
+{
+	char var_name[9];
+	u16 var_name16[9], *p;
+	efi_guid_t guid;
+	void *data = NULL;
+	unsigned long size;
+	int ret;
+
+	sprintf(var_name, "Boot%04X", id);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, 9);
+	guid = efi_global_variable_guid;
+
+	size = 0;
+	ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
+	if (ret == (int)EFI_BUFFER_TOO_SMALL) {
+		data = malloc(size);
+		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);

cmd/efishell.c line 437

Quoted 6 lines

cmd/efishell.c new file +6 -0

+	}
+	if (ret == EFI_SUCCESS) {
+		show_efi_boot_opt_data(id, data);
+		free(data);
+	} else if (ret == EFI_NOT_FOUND) {
+		printf("Boot%04X: not found\n", id);

Missing free?

Quoted 58 lines

cmd/efishell.c new file +58 -0

+	}
+}
+
+static int do_efi_boot_dump(int argc, char * const argv[])
+{
+	char regex[256];
+	char * const regexlist[] = {regex};
+	char *variables = NULL, *boot, *value;
+	int len;
+	int id;
+
+	if (argc > 1)
+		return CMD_RET_USAGE;
+
+	snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
+
+	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+			&variables, 0, 1, regexlist);
+
+	if (!len)
+		return CMD_RET_SUCCESS;
+
+	if (len < 0)
+		return CMD_RET_FAILURE;
+
+	boot = variables;
+	while (*boot) {
+		value = strstr(boot, "Boot") + 4;
+		id = (int)simple_strtoul(value, NULL, 16);
+		show_efi_boot_opt(id);
+		boot = strchr(boot, '\n');
+		if (!*boot)
+			break;
+		boot++;
+	}
+	free(variables);
+
+	return CMD_RET_SUCCESS;
+}
+
+static int show_efi_boot_order(void)
+{
+	efi_guid_t guid;
+	u16 *bootorder = NULL;
+	unsigned long size;
+	int num, i;
+	char var_name[9];
+	u16 var_name16[9], *p16;
+	void *data;
+	struct efi_load_option lo;
+	char *label, *p;
+	size_t label_len16, label_len;
+	efi_status_t ret = CMD_RET_SUCCESS;
+
+	guid = efi_global_variable_guid;
+	size = 0;
+	ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
+	if (ret == EFI_BUFFER_TOO_SMALL) {

cmd/efishell.c line 501

Quoted 6 lines

cmd/efishell.c new file +6 -0

+		bootorder = malloc(size);
+		ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,
+				       bootorder);
+	}
+	if (ret != EFI_SUCCESS) {
+		printf("BootOrder not defined\n");

Missing free(bootorder)

Quoted 21 lines

cmd/efishell.c new file +21 -0

+		return CMD_RET_SUCCESS;
+	}
+
+	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;

cmd/efishell.c line 528

Quoted 6 lines

cmd/efishell.c new file +6 -0

+		}
+		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
+		if (ret != EFI_SUCCESS) {
+			free(data);
+			ret = CMD_RET_FAILURE;
+			goto out;

Probably better to free(data) at out:, no?

Quoted 150 lines

cmd/efishell.c new file +145 -0

+		}
+
+		efi_deserialize_load_option(&lo, data);
+
+		label_len16 = u16_strlen(lo.label);
+		label_len = utf16_utf8_strnlen(lo.label, label_len16);
+		label = malloc(label_len + 1);
+		if (!label) {
+			free(data);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+		p = label;
+		utf16_utf8_strncpy(&p, lo.label, label_len16);
+		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
+		free(label);
+
+		free(data);
+	}
+out:
+	free(bootorder);
+
+	return ret;
+}
+
+static int do_efi_boot_order(int argc, char * const argv[])
+{
+	u16 *bootorder = NULL;
+	unsigned long size;
+	int id, i;
+	char *endp;
+	efi_guid_t guid;
+	efi_status_t ret;
+
+	if (argc == 1)
+		return show_efi_boot_order();
+
+	argc--;
+	argv++;
+
+	size = argc * sizeof(u16);
+	bootorder = malloc(size);
+	if (!bootorder)
+		return CMD_RET_FAILURE;
+
+	for (i = 0; i < argc; i++) {
+		id = (int)simple_strtoul(argv[i], &endp, 0);
+		if (*endp != '\0' || id > 0xffff) {
+			printf("invalid value: %s\n", argv[i]);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+
+		bootorder[i] = (u16)id;
+	}
+
+	guid = efi_global_variable_guid;
+	ret = efi_set_variable(L"BootOrder", &guid,
+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	free(bootorder);
+
+	return ret;
+}
+
+static int do_efi_boot_opt(int argc, char * const argv[])
+{
+	char *sub_command;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--; argv++;
+	sub_command = argv[0];
+
+	if (!strcmp(sub_command, "add"))
+		return do_efi_boot_add(argc, argv);
+	else if (!strcmp(sub_command, "rm"))
+		return do_efi_boot_rm(argc, argv);
+	else if (!strcmp(sub_command, "dump"))
+		return do_efi_boot_dump(argc, argv);
+	else if (!strcmp(sub_command, "order"))
+		return do_efi_boot_order(argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+
+/* Interpreter command to configure EFI environment */
+static int do_efishell(cmd_tbl_t *cmdtp, int flag,
+		       int argc, char * const argv[])
+{
+	char *command;
+	efi_status_t r;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--; argv++;
+	command = argv[0];
+
+	/* Initialize EFI drivers */
+	r = efi_init_obj_list();
+	if (r != EFI_SUCCESS) {
+		printf("Error: Cannot set up EFI drivers, r = %lu\n",
+		       r & ~EFI_ERROR_MASK);
+		return CMD_RET_FAILURE;
+	}
+
+	if (!strcmp(command, "boot"))
+		return do_efi_boot_opt(argc, argv);
+	else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
+		return do_efi_dump_var(argc, argv);
+	else if (!strcmp(command, "setvar"))
+		return do_efi_set_var(argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char efishell_help_text[] =
+	"  - EFI Shell-like interface to configure EFI environment\n"
+	"\n"
+	"efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
+	"  - set uefi's BOOT variable\n"
+	"efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
+	"  - set/display uefi boot order\n"
+	"efishell boot dump\n"
+	"  - display all uefi's BOOT variables\n"
+	"efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
+	"  - set/display uefi boot order\n"
+	"\n"
+	"efishell dumpvar [<name>]\n"
+	"  - get uefi variable's value\n"
+	"efishell setvar <name> [<value>]\n"
+	"  - set/delete uefi variable's value\n"
+	"    <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
+#endif
+
+U_BOOT_CMD(
+	efishell, 10, 0, do_efishell,
+	"Configure EFI environment",
+	efishell_help_text
+);

cmd/efishell.h new file +0 -0

new file mode 100644
index 000000000000..6f70b402b26b

cmd/efishell.h line 5

Quoted 6 lines

cmd/efishell.h new file +5 -0

@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <efi.h>
+
+efi_status_t efi_init_obj_list(void);

Why isn't this in efi_loader.h? That's the subsystem that exports it, no?

Alex

Quoted 29 lines · Alexander Graf

Quoted 26 lines · AKASHI Takahiro

Currently, there is no easy way to add or modify UEFI variables. In particular, bootmgr supports BootOrder/BootXXXX variables, it is quite hard to define them as u-boot variables because they are represented in a complicated and encoded format.

The new command, efishell, helps address these issues and give us more friendly interfaces:
* efishell boot add: add BootXXXX variable
* efishell boot rm: remove BootXXXX variable
* efishell boot dump: display all BootXXXX variables
* efishell boot order: set/display a boot order (BootOrder)
* efishell setvar: set an UEFI variable (with limited functionality)
* efishell dumpvar: display all UEFI variables

As the name suggests, this command basically provides a subset fo UEFI shell commands with simplified functionality.

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/Makefile   |   2 +-
 cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
 cmd/efishell.h |   5 +
 3 files changed, 679 insertions(+), 1 deletion(-)
 create mode 100644 cmd/efishell.c
 create mode 100644 cmd/efishell.h

Quoted 13 lines

cmd/Makefile obj-$(CONFIG_CMD_BINOP) += binop.o · line 27

Quoted 10 lines

cmd/Makefile +1 -1

index d9cdaf6064b8..bd531d505a8e 100644
@@ -24,7 +24,7 @@ obj-$(CONFIG_CMD_BINOP) += binop.o
 obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
 obj-$(CONFIG_CMD_BMP) += bmp.o
 obj-$(CONFIG_CMD_BOOTCOUNT) += bootcount.o
-obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o
+obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o efishell.o

Please create a separate command line option for efishell and make it disabled by default.

OK.

Quoted 3 lines

I think it's a really really useful debugging aid, but in a normal environment people should only need to run efi binaries (plus bootmgr) and modify efi variables, no?

The ultimate goal would be to provide this command as a separate application. In this case, however, it won't much different from uefi shell itself :)

Quoted 27 lines

Quoted 27 lines

cmd/Makefile +0 -0

 obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
 obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
 obj-$(CONFIG_CMD_BOOTZ) += bootz.o

cmd/efishell.c new file +16 -0

new file mode 100644
index 000000000000..abc8216c7bd6
@@ -0,0 +1,673 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  EFI Shell-like command
+ *
+ *  Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
+ */
+
+#include <charset.h>
+#include <common.h>
+#include <command.h>
+#include <efi_loader.h>
+#include <environment.h>
+#include <errno.h>
+#include <exports.h>
+#include <hexdump.h>
+#include <malloc.h>

Quoted 8 lines

cmd/efishell.c line 22

Quoted 6 lines

cmd/efishell.c new file +6 -0

+#include <search.h>
+#include <linux/ctype.h>
+#include <asm/global_data.h>
+#include "efishell.h"
+
+DECLARE_GLOBAL_DATA_PTR;

Where in this file do you need gd?

OK.
# I thought this should always be declared in any file by default.

Quoted 10 lines

Quoted 10 lines

cmd/efishell.c new file +10 -0

+	/* 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,

Quoted 8 lines

cmd/efishell.c line 335

Quoted 6 lines

cmd/efishell.c new file +6 -0

+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, data);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	free(data);
+#if 0 /* FIXME */

Eh, what? :)

Well, in the past, I saw u-boot hang-out if free()'s were enabled. Now I found that we must free data with efi_free_pool() instead of free().

Quoted 8 lines

Quoted 8 lines

cmd/efishell.c new file +7 -0

+	free(device_path);
+	free(file_path);
+#endif
+	free(lo.label);
+	return ret;
+}
+

Quoted 5 lines

Quoted 5 lines

cmd/efishell.h new file +0 -0

new file mode 100644
index 000000000000..6f70b402b26b

Quoted 8 lines

cmd/efishell.h line 5

Quoted 6 lines

cmd/efishell.h new file +5 -0

@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <efi.h>
+
+efi_status_t efi_init_obj_list(void);

Why isn't this in efi_loader.h? That's the subsystem that exports it, no?

Agree. Moreover, I think of refactoring the code and moving efi_init_obj_list() to a new file, lib/efi_loader/efi_setup.c, so that we can add more features directly as part of the initialization code.

Features may include USB removable disk support for which I already posted a patch set[1] and yet-coming capsule-on-disk support.

Actually, I have this refactoring patch in my local dev branch. May I submit it as a separate one?

[1] https://lists.denx.de/pipermail/u-boot/2018-November/347493.html

Thanks,
-Takahiro Akashi

Quoted 2 lines

Alex

"devices" command prints all the uefi variables on the system. => efishell devices
Device Name
============================================
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(2,MBR,0x086246ba,0x40800,0x3f800)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(1,MBR,0x086246ba,0x800,0x40000)

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

cmd/efishell.c +0 -0

index abc8216c7bd6..f4fa3fdf28a7 100644

Quoted 22 lines · AKASHI Takahiro

"devices" command prints all the uefi variables on the system. => efishell devices
Device Name
============================================
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(2,MBR,0x086246ba,0x40800,0x3f800)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(1,MBR,0x086246ba,0x800,0x40000)

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

cmd/efishell.c +0 -0

index abc8216c7bd6..f4fa3fdf28a7 100644
@@ -21,6 +21,8 @@
 

cmd/efishell.c line 24

Quoted 3 lines

cmd/efishell.c +1 -0

 DECLARE_GLOBAL_DATA_PTR;
 
+static const struct efi_boot_services *bs;

Why do you need a local copy of this?

Alex

Quoted 25 lines · Alexander Graf

Quoted 22 lines · AKASHI Takahiro

"devices" command prints all the uefi variables on the system. => efishell devices
Device Name
============================================
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(2,MBR,0x086246ba,0x40800,0x3f800)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(1,MBR,0x086246ba,0x800,0x40000)

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

cmd/efishell.c +0 -0

index abc8216c7bd6..f4fa3fdf28a7 100644
@@ -21,6 +21,8 @@
 

Quoted 5 lines

cmd/efishell.c line 24

Quoted 3 lines

cmd/efishell.c +1 -0

 DECLARE_GLOBAL_DATA_PTR;
 
+static const struct efi_boot_services *bs;

Why do you need a local copy of this?

Good point. It's because I followed the way boot manager does :)

I think that it would be good to do so since either boot manager or efishell should ultimately be an independent efi application in its nature.

What do you think?

Quoted 32 lines · AKASHI Takahiro

Quoted 30 lines · Alexander Graf

Quoted 21 lines · AKASHI Takahiro

"devices" command prints all the uefi variables on the system. => efishell devices
Device Name
============================================
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(2,MBR,0x086246ba,0x40800,0x3f800)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(1,MBR,0x086246ba,0x800,0x40000)

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

cmd/efishell.c +0 -0

index abc8216c7bd6..f4fa3fdf28a7 100644

cmd/efishell.c line 24

Quoted 4 lines

cmd/efishell.c +1 -0

@@ -21,6 +21,8 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static const struct efi_boot_services *bs;

Why do you need a local copy of this?

Quoted 7 lines

Good point. It's because I followed the way boot manager does :)

I think that it would be good to do so since either boot manager or efishell should ultimately be an independent efi application in its nature.

What do you think?

As mentioned in the other email thread, I think that we should definitely evaluate to add the edk2 shell as built-in option.

That way for the "full-fledged" shell experience, your built-in code is not needed. But I still believe it would be useful for quick and built-in debugging.

Alex

Quoted 41 lines · Alexander Graf

Quoted 38 lines · AKASHI Takahiro

Quoted 25 lines · Alexander Graf

Quoted 22 lines · AKASHI Takahiro

"devices" command prints all the uefi variables on the system. => efishell devices
Device Name
============================================
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(2,MBR,0x086246ba,0x40800,0x3f800)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(1,MBR,0x086246ba,0x800,0x40000)

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

cmd/efishell.c +0 -0

index abc8216c7bd6..f4fa3fdf28a7 100644
@@ -21,6 +21,8 @@
 

Quoted 5 lines

cmd/efishell.c line 24

Quoted 3 lines

cmd/efishell.c +1 -0

 DECLARE_GLOBAL_DATA_PTR;
 
+static const struct efi_boot_services *bs;

Why do you need a local copy of this?

Good point. It's because I followed the way boot manager does :)

I think that it would be good to do so since either boot manager or efishell should ultimately be an independent efi application in its nature.

Quoted 1 line

Quoted 1 line

What do you think?

Back to your original comment: Why do you need a local copy of this?

Do you think we should use systab.boottime,runtime instead?

Quoted 2 lines

As mentioned in the other email thread, I think that we should definitely evaluate to add the edk2 shell as built-in option.

Do you expect that I will add a new config, say, CONFIG_EFI_SHELL_PATH?

Quoted 44 lines · AKASHI Takahiro

Quoted 42 lines · Alexander Graf

Quoted 39 lines · AKASHI Takahiro

Quoted 25 lines · Alexander Graf

Quoted 22 lines · AKASHI Takahiro

"devices" command prints all the uefi variables on the system. => efishell devices
Device Name
============================================
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(2,MBR,0x086246ba,0x40800,0x3f800)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
				HD(1,MBR,0x086246ba,0x800,0x40000)

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

cmd/efishell.c +0 -0

index abc8216c7bd6..f4fa3fdf28a7 100644
@@ -21,6 +21,8 @@
 

Quoted 5 lines

cmd/efishell.c line 24

Quoted 3 lines

cmd/efishell.c +1 -0

 DECLARE_GLOBAL_DATA_PTR;
 
+static const struct efi_boot_services *bs;

Why do you need a local copy of this?

Good point. It's because I followed the way boot manager does :)

I think that it would be good to do so since either boot manager or efishell should ultimately be an independent efi application in its nature.

What do you think?

Quoted 3 lines

Back to your original comment: Why do you need a local copy of this?

Do you think we should use systab.boottime,runtime instead?

Sure, that's one thing. Or you could make it a local variable. Or you could even do a #define in the file. But that static const here will most likely waste space in .bss and does not take into account when someone patches the systab.

Quoted 4 lines

Quoted 2 lines

As mentioned in the other email thread, I think that we should definitely evaluate to add the edk2 shell as built-in option.

Do you expect that I will add a new config, say, CONFIG_EFI_SHELL_PATH?

Doesn't have to be you, but it might be useful to have something like that, yes.

CONFIG_CMD_BOOTEFI_SHELL=y
CONFIG_CMD_BOOTEFI_SHELL_PATH=shell.efi

which then would work the same as CONFIG_CMD_BOOTEFI_HELLO, but simply execute a precompiled version of the UEFI shell. IIRC the UEFI shell also supports passing arguments via the command line (load_options -> "bootargs" variable).

So with that you should be able to run

U-Boot# setenv bootargs memmap; bootefi shell

and get a list of the UEFI memory map.

Alex

"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 <[email protected]>

---
 cmd/efishell.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

cmd/efishell.c +25 -0

index b5050d63fa22..3e1118a407ff 100644
@@ -442,6 +442,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);		\
+	}
+

cmd/efishell.c +42 -1

+		if (!map)
+			return CMD_RET_FAILURE;
+		ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+	}
+	if (ret != EFI_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+	if (ret != EFI_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	printf("Type             Start            End              Attributes\n");
+	printf("================ ================ ================ ==========\n");
+	for (i = 0; i < map_size / sizeof(*map); map++, i++) {
+		sep = 0;
+		printf("%-16s %016llx-%016llx ",
+		       efi_mem_type_string[map->type],
+		       map->physical_start,
+		       map->physical_start + map->num_pages * EFI_PAGE_SIZE);
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UC, "UC");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WC, "WC");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WT, "WT");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WB, "WB");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UCE, "UCE");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WP, "WP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RP, "RP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_XP, "WP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_NV, "NV");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_MORE_RELIABLE, "REL");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RO, "RO");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RUNTIME, "RT");
+		putc('\n');
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
 static int do_efi_boot_add(int argc, char * const argv[])
 {
 	int id;

@@ -826,6 +898,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
 		return do_efi_show_drivers(argc, argv);
 	else if (!strcmp(command, "images"))
 		return do_efi_show_images(argc, argv);
+	else if (!strcmp(command, "memmap"))
+		return do_efi_show_memmap(argc, argv);
 	else
 		return CMD_RET_USAGE;
 }

@@ -853,7 +927,9 @@ static char efishell_help_text[] =
 	"efishell drivers\n"
 	"  - show uefi drivers\n"
 	"efishell images\n"
-	"  - show loaded images\n";
+	"  - show loaded images\n"
+	"efishell memmap\n"
+	"  - show uefi memory map\n";
 #endif
 
 U_BOOT_CMD(

Quoted 56 lines · AKASHI Takahiro

"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 <[email protected]>

---
 cmd/efishell.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

cmd/efishell.c +29 -0

index b5050d63fa22..3e1118a407ff 100644
@@ -442,6 +442,78 @@ static int do_efi_show_images(int argc, char * const argv[])
 	return CMD_RET_SUCCESS;
 }
 
+static const char * const efi_mem_type_string[] = {
+	[EFI_RESERVED_MEMORY_TYPE] = "RESERVED",
+	[EFI_LOADER_CODE] = "LOADER CODE",
+	[EFI_LOADER_DATA] = "LOADER DATA",
+	[EFI_BOOT_SERVICES_CODE] = "BOOT CODE",
+	[EFI_BOOT_SERVICES_DATA] = "BOOT DATA",
+	[EFI_RUNTIME_SERVICES_CODE] = "RUNTIME CODE",
+	[EFI_RUNTIME_SERVICES_DATA] = "RUNTIME DATA",
+	[EFI_CONVENTIONAL_MEMORY] = "CONVENTIONAL",
+	[EFI_UNUSABLE_MEMORY] = "UNUSABLE MEM",
+	[EFI_ACPI_RECLAIM_MEMORY] = "ACPI RECLAIM MEM",
+	[EFI_ACPI_MEMORY_NVS] = "ACPI NVS",
+	[EFI_MMAP_IO] = "IO",
+	[EFI_MMAP_IO_PORT] = "IO PORT",
+	[EFI_PAL_CODE] = "PAL",
+};
+
+#define EFI_MEM_ATTR(attribute, bit, string) \
+	if ((attribute) & (bit)) {	\
+		if (sep)		\
+			putc('|');	\
+		sep = 1;		\
+		printf(string);		\
+	}
+
+static int do_efi_show_memmap(int argc, char * const argv[])
+{
+	struct efi_mem_desc *map = NULL;
+	efi_uintn_t map_size = 0;

cmd/efishell.c static int do_efi_show_images(int argc, char * const argv[]) · line 479

Quoted 6 lines

cmd/efishell.c +6 -0

+	int i, sep;
+	efi_status_t ret;
+
+	ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+	if (ret == EFI_BUFFER_TOO_SMALL) {
+		map = malloc(map_size);

Where is the free() to this malloc()?

Alex

Quoted 61 lines

cmd/efishell.c +42 -1

+		if (!map)
+			return CMD_RET_FAILURE;
+		ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+	}
+	if (ret != EFI_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+	if (ret != EFI_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	printf("Type             Start            End              Attributes\n");
+	printf("================ ================ ================ ==========\n");
+	for (i = 0; i < map_size / sizeof(*map); map++, i++) {
+		sep = 0;
+		printf("%-16s %016llx-%016llx ",
+		       efi_mem_type_string[map->type],
+		       map->physical_start,
+		       map->physical_start + map->num_pages * EFI_PAGE_SIZE);
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UC, "UC");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WC, "WC");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WT, "WT");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WB, "WB");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UCE, "UCE");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WP, "WP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RP, "RP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_XP, "WP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_NV, "NV");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_MORE_RELIABLE, "REL");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RO, "RO");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RUNTIME, "RT");
+		putc('\n');
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
 static int do_efi_boot_add(int argc, char * const argv[])
 {
 	int id;

@@ -826,6 +898,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
 		return do_efi_show_drivers(argc, argv);
 	else if (!strcmp(command, "images"))
 		return do_efi_show_images(argc, argv);
+	else if (!strcmp(command, "memmap"))
+		return do_efi_show_memmap(argc, argv);
 	else
 		return CMD_RET_USAGE;
 }

@@ -853,7 +927,9 @@ static char efishell_help_text[] =
 	"efishell drivers\n"
 	"  - show uefi drivers\n"
 	"efishell images\n"
-	"  - show loaded images\n";
+	"  - show loaded images\n"
+	"efishell memmap\n"
+	"  - show uefi memory map\n";
 #endif
 
 U_BOOT_CMD(

The current way how command parameters, particularly "fdt addr," are handled makes it a bit complicated to add a subcommand-specific parameter. So just refactor the code and extract efi_handle_fdt().

This commit is a preparatory change for enhancing bootmgr sub-command.

Signed-off-by AKASHI Takahiro <[email protected]>

---
 cmd/bootefi.c  | 51 +++++++++++++++++++++++++++++++++-----------------
 cmd/efishell.h |  1 +
 2 files changed, 35 insertions(+), 17 deletions(-)

cmd/bootefi.c +34 -17

index 5d61819f1f5c..3aedf5a09f93 100644
@@ -356,6 +356,31 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
 	return ret;
 }
 
+int efi_handle_fdt(char *fdt_opt)
+{
+	unsigned long fdt_addr;
+	efi_status_t r;
+
+	if (fdt_opt) {
+		fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
+		if (!fdt_addr && *fdt_opt != '0')
+			return CMD_RET_USAGE;
+
+		/* Install device tree */
+		r = efi_install_fdt(fdt_addr);
+		if (r != EFI_SUCCESS) {
+			printf("ERROR: failed to install device tree\n");
+			return CMD_RET_FAILURE;
+		}
+	} else {
+		/* Remove device tree. EFI_NOT_FOUND can be ignored here */
+		efi_install_configuration_table(&efi_guid_fdt, NULL);
+		printf("WARNING: booting without device tree\n");
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
 /**
  * do_bootefi_exec() - execute EFI binary
  *

@@ -511,7 +536,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	unsigned long addr;
 	char *saddr;
 	efi_status_t r;
-	unsigned long fdt_addr;
 
 	/* Allow unaligned memory access */
 	allow_unaligned();

@@ -527,21 +551,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
-	if (argc > 2) {
-		fdt_addr = simple_strtoul(argv[2], NULL, 16);
-		if (!fdt_addr && *argv[2] != '0')
-			return CMD_RET_USAGE;
-		/* Install device tree */
-		r = efi_install_fdt(fdt_addr);
-		if (r != EFI_SUCCESS) {
-			printf("ERROR: failed to install device tree\n");
-			return CMD_RET_FAILURE;
-		}
-	} else {
-		/* Remove device tree. EFI_NOT_FOUND can be ignored here */
-		efi_install_configuration_table(&efi_guid_fdt, NULL);
-		printf("WARNING: booting without device tree\n");
-	}
 #ifdef CONFIG_CMD_BOOTEFI_HELLO
 	if (!strcmp(argv[1], "hello")) {
 		ulong size = __efi_helloworld_end - __efi_helloworld_begin;

@@ -559,6 +568,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		struct efi_loaded_image_obj *image_obj;
 		struct efi_loaded_image *loaded_image_info;
 
+		if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
+			return CMD_RET_FAILURE;
+
 		/* Construct a dummy device path. */
 		bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
 						      (uintptr_t)&efi_selftest,

@@ -583,7 +595,10 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	} else
 #endif
 	if (!strcmp(argv[1], "bootmgr")) {
-		return do_bootefi_bootmgr_exec();
+		if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
+			return CMD_RET_FAILURE;
+
+		return do_bootefi_bootmgr_exec(boot_id);
 	} else {
 		saddr = argv[1];
 

@@ -592,6 +607,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		if (!addr && *saddr != '0')
 			return CMD_RET_USAGE;
 
+		if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
+			return CMD_RET_FAILURE;
 	}
 
 	printf("## Starting EFI application at %08lx ...\n", addr);

cmd/efishell.h +0 -0

index 6f70b402b26b..1e5764a8a38d 100644