[v3,1/8] cmd: add efishell command

Message ID 20181218050510.20308-2-takahiro.akashi@linaro.org
State New
Headers show
Series
  • cmd: add efishell for efi environment
Related show

Commit Message

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

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

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

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/Kconfig    |  10 +
 cmd/Makefile   |   1 +
 cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 684 insertions(+)
 create mode 100644 cmd/efishell.c

Comments

Heinrich Schuchardt Dec. 30, 2018, 3:44 p.m. | #1
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
> Currently, there is no easy way to add or modify UEFI variables.
> In particular, bootmgr supports BootOrder/BootXXXX variables, it is
> quite hard to define them as u-boot variables because they are represented
> in a complicated and encoded format.
> 
> The new command, efishell, helps address these issues and give us
> more friendly interfaces:
>  * efishell boot add: add BootXXXX variable
>  * efishell boot rm: remove BootXXXX variable
>  * efishell boot dump: display all BootXXXX variables
>  * efishell boot order: set/display a boot order (BootOrder)
>  * efishell setvar: set an UEFI variable (with limited functionality)
>  * efishell dumpvar: display all UEFI variables
> 
> As the name suggests, this command basically provides a subset fo UEFI
> shell commands with simplified functionality.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  cmd/Kconfig    |  10 +
>  cmd/Makefile   |   1 +
>  cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 684 insertions(+)
>  create mode 100644 cmd/efishell.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index f2f3b5e2b76b..a8a4bf7db45e 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1390,6 +1390,16 @@ config CMD_DISPLAY
>  	  displayed on a simple board-specific display. Implement
>  	  display_putc() to use it.
>  
> +config CMD_EFISHELL
> +	bool "Enable the 'efishell' command for EFI environment"
> +	depends on EFI_LOADER
> +	default n
> +	help
> +	  Enable the 'efishell' command which provides a subset of UEFI
> +	  shell utility with simplified functionality. It will be useful
> +	  particularly for managing boot parameters as  well as examining
> +	  various EFI status for debugging.
> +
>  config CMD_LED
>  	bool "led"
>  	default y if LED
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 5ec2f9e8ebfd..0258d8a373b1 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_CMD_ECHO) += echo.o
>  obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
>  obj-$(CONFIG_CMD_EEPROM) += eeprom.o
>  obj-$(CONFIG_EFI_STUB) += efi.o
> +obj-$(CONFIG_CMD_EFISHELL) += efishell.o
>  obj-$(CONFIG_CMD_ELF) += elf.o
>  obj-$(CONFIG_HUSH_PARSER) += exit.o
>  obj-$(CONFIG_CMD_EXT4) += ext4.o
> diff --git a/cmd/efishell.c b/cmd/efishell.c
> new file mode 100644
> index 000000000000..5819e52cf575
> --- /dev/null
> +++ b/cmd/efishell.c
> @@ -0,0 +1,673 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  EFI Shell-like command
> + *
> + *  Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
> + */
> +
> +#include <charset.h>
> +#include <common.h>
> +#include <command.h>
> +#include <efi_loader.h>
> +#include <environment.h>
> +#include <errno.h>
> +#include <exports.h>
> +#include <hexdump.h>
> +#include <malloc.h>
> +#include <search.h>
> +#include <linux/ctype.h>
> +#include <asm/global_data.h>
> +
> +static void dump_var_data(char *data, unsigned long len)
> +{
> +	char *start, *end, *p;
> +	unsigned long pos, count;
> +	char hex[3], line[9];
> +	int i;
> +
> +	end = data + len;
> +	for (start = data, pos = 0; start < end; start += count, pos += count) {
> +		count = end - start;
> +		if (count > 16)
> +			count = 16;
> +
> +		/* count should be multiple of two */
> +		printf("%08lx: ", pos);
> +
> +		/* in hex format */
> +		p = start;
> +		for (i = 0; i < count / 2; p += 2, i++)
> +			printf(" %c%c", *p, *(p + 1));
> +		for (; i < 8; i++)
> +			printf("   ");
> +
> +		/* in character format */
> +		p = start;
> +		hex[2] = '\0';
> +		for (i = 0; i < count / 2; i++) {
> +			hex[0] = *p++;
> +			hex[1] = *p++;
> +			line[i] = simple_strtoul(hex, 0, 16);
> +			if (line[i] < 0x20 || line[i] > 0x7f)
> +				line[i] = '.';
> +		}
> +		line[i] = '\0';
> +		printf("  %s\n", line);
> +	}
> +}
> +
> +/*
> + * From efi_variable.c,
> + *
> + * Mapping between EFI variables and u-boot variables:
> + *
> + *   efi_$guid_$varname = {attributes}(type)value
> + */
> +static int do_efi_dump_var(int argc, char * const argv[])
> +{
> +	char regex[256];
> +	char * const regexlist[] = {regex};
> +	char *res = NULL, *start, *end;
> +	int len;
> +
> +	if (argc > 2)
> +		return CMD_RET_USAGE;
> +
> +	if (argc == 2)
> +		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
> +	else
> +		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> +	debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
> +
> +	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> +			&res, 0, 1, regexlist);
> +
> +	if (len < 0)
> +		return CMD_RET_FAILURE;
> +
> +	if (len > 0) {
> +		end = res;
> +		while (true) {
> +			/* variable name */
> +			start = strchr(end, '_');
> +			if (!start)
> +				break;
> +			start = strchr(++start, '_');
> +			if (!start)
> +				break;
> +			end = strchr(++start, '=');
> +			if (!end)
> +				break;
> +			printf("%.*s:", (int)(end - start), start);
> +			end++;
> +
> +			/* value */
> +			start = end;
> +			end = strstr(start, "(blob)");
> +			if (!end) {
> +				putc('\n');
> +				break;
> +			}
> +			end += 6;
> +			printf(" %.*s\n", (int)(end - start), start);
> +
> +			start = end;
> +			end = strchr(start, '\n');
> +			if (!end)
> +				break;
> +			dump_var_data(start,  end - start);
> +		}
> +		free(res);
> +
> +		if (len < 2 && argc == 2)
> +			printf("%s: not found\n", argv[1]);
> +	}
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
> +static int append_value(char **bufp, unsigned long *sizep, char *data)
> +{
> +	char *tmp_buf = NULL, *new_buf = NULL, *value;
> +	unsigned long len = 0;
> +
> +	if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
> +		union {
> +			u8 u8;
> +			u16 u16;
> +			u32 u32;
> +			u64 u64;
> +		} tmp_data;
> +		unsigned long hex_value;
> +		void *hex_ptr;
> +
> +		data += 3;
> +		len = strlen(data);
> +		if ((len & 0x1)) /* not multiple of two */
> +			return -1;
> +
> +		len /= 2;
> +		if (len > 8)
> +			return -1;
> +		else if (len > 4)
> +			len = 8;
> +		else if (len > 2)
> +			len = 4;
> +
> +		/* convert hex hexadecimal number */
> +		if (strict_strtoul(data, 16, &hex_value) < 0)
> +			return -1;
> +
> +		tmp_buf = malloc(len);
> +		if (!tmp_buf)
> +			return -1;
> +
> +		if (len == 1) {
> +			tmp_data.u8 = hex_value;
> +			hex_ptr = &tmp_data.u8;
> +		} else if (len == 2) {
> +			tmp_data.u16 = hex_value;
> +			hex_ptr = &tmp_data.u16;
> +		} else if (len == 4) {
> +			tmp_data.u32 = hex_value;
> +			hex_ptr = &tmp_data.u32;
> +		} else {
> +			tmp_data.u64 = hex_value;
> +			hex_ptr = &tmp_data.u64;
> +		}
> +		memcpy(tmp_buf, hex_ptr, len);
> +		value = tmp_buf;
> +
> +	} else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
> +		data += 2;
> +		len = strlen(data);
> +		if (len & 0x1) /* not multiple of two */
> +			return -1;
> +
> +		len /= 2;
> +		tmp_buf = malloc(len);
> +		if (!tmp_buf)
> +			return -1;
> +
> +		if (hex2bin((u8 *)tmp_buf, data, len) < 0)
> +			return -1;
> +
> +		value = tmp_buf;
> +	} else { /* string */
> +		if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
> +			if (data[1] == '"')
> +				data += 2;
> +			else
> +				data += 3;
> +			value = data;
> +			len = strlen(data) - 1;
> +			if (data[len] != '"')
> +				return -1;
> +		} else {
> +			value = data;
> +			len = strlen(data);
> +		}
> +	}
> +
> +	new_buf = realloc(*bufp, *sizep + len);
> +	if (!new_buf)
> +		goto out;
> +
> +	memcpy(new_buf + *sizep, value, len);
> +	*bufp = new_buf;
> +	*sizep += len;
> +
> +out:
> +	free(tmp_buf);
> +
> +	return 0;
> +}
> +
> +static int do_efi_set_var(int argc, char * const argv[])
> +{
> +	char *var_name, *value = NULL;
> +	unsigned long size = 0;
> +	u16 *var_name16, *p;
> +	efi_guid_t guid;
> +	efi_status_t ret;
> +
> +	if (argc == 1)
> +		return CMD_RET_SUCCESS;
> +
> +	var_name = argv[1];
> +	if (argc == 2) {
> +		/* remove */
> +		value = NULL;
> +		size = 0;
> +	} else { /* set */
> +		argc -= 2;
> +		argv += 2;
> +
> +		for ( ; argc > 0; argc--, argv++)
> +			if (append_value(&value, &size, argv[0]) < 0) {
> +				ret = CMD_RET_FAILURE;
> +				goto out;
> +			}
> +	}
> +
> +	var_name16 = malloc((strlen(var_name) + 1) * 2);
> +	p = var_name16;
> +	utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
> +
> +	guid = efi_global_variable_guid;
> +	ret = efi_set_variable(var_name16, &guid,
> +			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +			       EFI_VARIABLE_RUNTIME_ACCESS, size, value);
> +	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> +out:
> +	return ret;
> +}
> +
> +static int do_efi_boot_add(int argc, char * const argv[])
> +{
> +	int id;
> +	char *endp;
> +	char var_name[9];
> +	u16 var_name16[9], *p;
> +	efi_guid_t guid;
> +	size_t label_len, label_len16;
> +	u16 *label;
> +	struct efi_device_path *device_path = NULL, *file_path = NULL;
> +	struct efi_load_option lo;
> +	void *data = NULL;
> +	unsigned long size;
> +	int ret;
> +
> +	if (argc < 6 || argc > 7)
> +		return CMD_RET_USAGE;
> +
> +	id = (int)simple_strtoul(argv[1], &endp, 0);
> +	if (*endp != '\0' || id > 0xffff)
> +		return CMD_RET_FAILURE;
> +
> +	sprintf(var_name, "Boot%04X", id);
> +	p = var_name16;
> +	utf8_utf16_strncpy(&p, var_name, 9);
> +
> +	guid = efi_global_variable_guid;
> +
> +	/* attributes */
> +	lo.attributes = 0x1; /* always ACTIVE */
> +
> +	/* label */
> +	label_len = strlen(argv[2]);
> +	label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> +	label = malloc((label_len16 + 1) * sizeof(u16));
> +	if (!label)
> +		return CMD_RET_FAILURE;
> +	lo.label = label; /* label will be changed below */
> +	utf8_utf16_strncpy(&label, argv[2], label_len);
> +
> +	/* file path */
> +	ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> +			       &file_path);
> +	if (ret != EFI_SUCCESS) {
> +		ret = CMD_RET_FAILURE;
> +		goto out;
> +	}
> +	lo.file_path = file_path;
> +	lo.file_path_length = efi_dp_size(file_path)
> +				+ sizeof(struct efi_device_path); /* for END */
> +
> +	/* optional data */
> +	lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
> +
> +	size = efi_serialize_load_option(&lo, (u8 **)&data);

The load options should be converted to a UTF-16 string before calling
efi_serialize_load_option so that we can copy them to the loaded image
protocol.

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

'size' must be of type efi_uint_t. Otherwise you get warnings like this one:

cmd/efishell.c:729:50: warning: passing argument 4 of ‘efi_get_variable’
from incompatible pointer type [-Wincompatible-pointer-types]
  ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);


> +	int ret;
> +
> +	sprintf(var_name, "Boot%04X", id);
> +	p = var_name16;
> +	utf8_utf16_strncpy(&p, var_name, 9);
> +	guid = efi_global_variable_guid;
> +
> +	size = 0;
> +	ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> +	if (ret == (int)EFI_BUFFER_TOO_SMALL) {
> +		data = malloc(size);
> +		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> +	}
> +	if (ret == EFI_SUCCESS)
> +		show_efi_boot_opt_data(id, data);
> +	else if (ret == EFI_NOT_FOUND)
> +		printf("Boot%04X: not found\n", id);
> +
> +	free(data);
> +}
> +
> +static int do_efi_boot_dump(int argc, char * const argv[])
> +{
> +	char regex[256];
> +	char * const regexlist[] = {regex};
> +	char *variables = NULL, *boot, *value;
> +	int len;
> +	int id;
> +
> +	if (argc > 1)
> +		return CMD_RET_USAGE;
> +
> +	snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
> +
> +	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> +			&variables, 0, 1, regexlist);
> +
> +	if (!len)
> +		return CMD_RET_SUCCESS;
> +
> +	if (len < 0)
> +		return CMD_RET_FAILURE;
> +
> +	boot = variables;
> +	while (*boot) {
> +		value = strstr(boot, "Boot") + 4;
> +		id = (int)simple_strtoul(value, NULL, 16);
> +		show_efi_boot_opt(id);
> +		boot = strchr(boot, '\n');
> +		if (!*boot)
> +			break;
> +		boot++;
> +	}
> +	free(variables);
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
> +static int show_efi_boot_order(void)
> +{
> +	efi_guid_t guid;
> +	u16 *bootorder = NULL;
> +	unsigned long size;
> +	int num, i;
> +	char var_name[9];
> +	u16 var_name16[9], *p16;
> +	void *data;
> +	struct efi_load_option lo;
> +	char *label, *p;
> +	size_t label_len16, label_len;
> +	efi_status_t ret;
> +
> +	guid = efi_global_variable_guid;
> +	size = 0;
> +	ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
> +	if (ret == EFI_BUFFER_TOO_SMALL) {
> +		bootorder = malloc(size);
> +		ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,
> +				       bootorder);
> +	}
> +	if (ret == EFI_NOT_FOUND) {
> +		printf("BootOrder not defined\n");
> +		ret = CMD_RET_SUCCESS;
> +		goto out;
> +	} else if (ret != EFI_SUCCESS) {
> +		ret = CMD_RET_FAILURE;
> +		goto out;
> +	}
> +
> +	num = size / sizeof(u16);
> +	for (i = 0; i < num; i++) {
> +		sprintf(var_name, "Boot%04X", bootorder[i]);
> +		p16 = var_name16;
> +		utf8_utf16_strncpy(&p16, var_name, 9);
> +
> +		size = 0;
> +		ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> +		if (ret != EFI_BUFFER_TOO_SMALL) {
> +			printf("%2d: Boot%04X: (not defined)\n",
> +			       i + 1, bootorder[i]);
> +			continue;
> +		}
> +
> +		data = malloc(size);
> +		if (!data) {
> +			ret = CMD_RET_FAILURE;
> +			goto out;
> +		}
> +		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> +		if (ret != EFI_SUCCESS) {
> +			free(data);
> +			ret = CMD_RET_FAILURE;
> +			goto out;
> +		}
> +
> +		efi_deserialize_load_option(&lo, data);
> +
> +		label_len16 = u16_strlen(lo.label);
> +		label_len = utf16_utf8_strnlen(lo.label, label_len16);
> +		label = malloc(label_len + 1);
> +		if (!label) {
> +			free(data);
> +			ret = CMD_RET_FAILURE;
> +			goto out;
> +		}
> +		p = label;
> +		utf16_utf8_strncpy(&p, lo.label, label_len16);
> +		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
> +		free(label);
> +
> +		free(data);
> +	}
> +out:
> +	free(bootorder);
> +
> +	return ret;
> +}
> +
> +static int do_efi_boot_order(int argc, char * const argv[])
> +{
> +	u16 *bootorder = NULL;
> +	unsigned long size;
> +	int id, i;
> +	char *endp;
> +	efi_guid_t guid;
> +	efi_status_t ret;
> +
> +	if (argc == 1)
> +		return show_efi_boot_order();
> +
> +	argc--;
> +	argv++;
> +
> +	size = argc * sizeof(u16);
> +	bootorder = malloc(size);
> +	if (!bootorder)
> +		return CMD_RET_FAILURE;
> +
> +	for (i = 0; i < argc; i++) {
> +		id = (int)simple_strtoul(argv[i], &endp, 0);
> +		if (*endp != '\0' || id > 0xffff) {
> +			printf("invalid value: %s\n", argv[i]);
> +			ret = CMD_RET_FAILURE;
> +			goto out;
> +		}
> +
> +		bootorder[i] = (u16)id;
> +	}
> +
> +	guid = efi_global_variable_guid;
> +	ret = efi_set_variable(L"BootOrder", &guid,
> +			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +			       EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
> +	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> +out:
> +	free(bootorder);
> +
> +	return ret;
> +}
> +
> +static int do_efi_boot_opt(int argc, char * const argv[])
> +{
> +	char *sub_command;
> +
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	argc--; argv++;
> +	sub_command = argv[0];
> +
> +	if (!strcmp(sub_command, "add"))
> +		return do_efi_boot_add(argc, argv);
> +	else if (!strcmp(sub_command, "rm"))
> +		return do_efi_boot_rm(argc, argv);
> +	else if (!strcmp(sub_command, "dump"))
> +		return do_efi_boot_dump(argc, argv);
> +	else if (!strcmp(sub_command, "order"))
> +		return do_efi_boot_order(argc, argv);
> +	else
> +		return CMD_RET_USAGE;
> +}
> +
> +/* Interpreter command to configure EFI environment */
> +static int do_efishell(cmd_tbl_t *cmdtp, int flag,
> +		       int argc, char * const argv[])
> +{
> +	char *command;
> +	efi_status_t r;
> +
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	argc--; argv++;
> +	command = argv[0];
> +
> +	/* Initialize EFI drivers */
> +	r = efi_init_obj_list();
> +	if (r != EFI_SUCCESS) {
> +		printf("Error: Cannot set up EFI drivers, r = %lu\n",
> +		       r & ~EFI_ERROR_MASK);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	if (!strcmp(command, "boot"))
> +		return do_efi_boot_opt(argc, argv);
> +	else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
> +		return do_efi_dump_var(argc, argv);
> +	else if (!strcmp(command, "setvar"))
> +		return do_efi_set_var(argc, argv);
> +	else
> +		return CMD_RET_USAGE;
> +}
> +
> +#ifdef CONFIG_SYS_LONGHELP
> +static char efishell_help_text[] =
> +	"  - EFI Shell-like interface to configure EFI environment\n"
> +	"\n"
> +	"efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"

The 'option' parameter is optional. Please, add brackets ([]).

The parameter 'option' is misleading. Please call it 'load_options'.

Please, add a text explaining that 'load_options' is meant to be the
command line string passed to the EFI binary.

Best regards

Heinrich

> +	"  - set uefi's BOOT variable\n"
> +	"efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
> +	"  - set/display uefi boot order\n"
> +	"efishell boot dump\n"
> +	"  - display all uefi's BOOT variables\n"
> +	"efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
> +	"  - set/display uefi boot order\n"
> +	"\n"
> +	"efishell dumpvar [<name>]\n"
> +	"  - get uefi variable's value\n"
> +	"efishell setvar <name> [<value>]\n"
> +	"  - set/delete uefi variable's value\n"
> +	"    <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
> +#endif
> +
> +U_BOOT_CMD(
> +	efishell, 10, 0, do_efishell,
> +	"Configure EFI environment",
> +	efishell_help_text
> +);
>
Heinrich Schuchardt Dec. 30, 2018, 5:10 p.m. | #2
On 12/30/18 4:44 PM, Heinrich Schuchardt wrote:
> On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
>> Currently, there is no easy way to add or modify UEFI variables.
>> In particular, bootmgr supports BootOrder/BootXXXX variables, it is
>> quite hard to define them as u-boot variables because they are represented
>> in a complicated and encoded format.
>>
>> The new command, efishell, helps address these issues and give us
>> more friendly interfaces:
>>  * efishell boot add: add BootXXXX variable
>>  * efishell boot rm: remove BootXXXX variable
>>  * efishell boot dump: display all BootXXXX variables
>>  * efishell boot order: set/display a boot order (BootOrder)
>>  * efishell setvar: set an UEFI variable (with limited functionality)
>>  * efishell dumpvar: display all UEFI variables
>>
>> As the name suggests, this command basically provides a subset fo UEFI
>> shell commands with simplified functionality.
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

The behavior is a bit unexpected:

=> efishell boot order 200
=> efishell boot order
 1: Boot00C8: (not defined)
exit not allowed from main input shell.

I would expect 'efishell boot order' to take a 4 digit hexadecimal
number and to do no conversion from decimal to hexadecimal.

I was also surprised to see 'exit not allowed from main input shell.'

Best regards

Heinrich
Heinrich Schuchardt Dec. 30, 2018, 11:47 p.m. | #3
On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
> Currently, there is no easy way to add or modify UEFI variables.
> In particular, bootmgr supports BootOrder/BootXXXX variables, it is
> quite hard to define them as u-boot variables because they are represented
> in a complicated and encoded format.
> 
> The new command, efishell, helps address these issues and give us
> more friendly interfaces:
>  * efishell boot add: add BootXXXX variable
>  * efishell boot rm: remove BootXXXX variable
>  * efishell boot dump: display all BootXXXX variables
>  * efishell boot order: set/display a boot order (BootOrder)
>  * efishell setvar: set an UEFI variable (with limited functionality)
>  * efishell dumpvar: display all UEFI variables
> 
> As the name suggests, this command basically provides a subset fo UEFI
> shell commands with simplified functionality.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  cmd/Kconfig    |  10 +
>  cmd/Makefile   |   1 +
>  cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 684 insertions(+)
>  create mode 100644 cmd/efishell.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index f2f3b5e2b76b..a8a4bf7db45e 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1390,6 +1390,16 @@ config CMD_DISPLAY
>  	  displayed on a simple board-specific display. Implement
>  	  display_putc() to use it.
>  

<snip>

> +static int do_efi_boot_add(int argc, char * const argv[])
> +{
> +	int id;
> +	char *endp;
> +	char var_name[9];
> +	u16 var_name16[9], *p;
> +	efi_guid_t guid;
> +	size_t label_len, label_len16;
> +	u16 *label;
> +	struct efi_device_path *device_path = NULL, *file_path = NULL;
> +	struct efi_load_option lo;
> +	void *data = NULL;
> +	unsigned long size;
> +	int ret;
> +
> +	if (argc < 6 || argc > 7)
> +		return CMD_RET_USAGE;
> +
> +	id = (int)simple_strtoul(argv[1], &endp, 0);
> +	if (*endp != '\0' || id > 0xffff)
> +		return CMD_RET_FAILURE;
> +
> +	sprintf(var_name, "Boot%04X", id);
> +	p = var_name16;
> +	utf8_utf16_strncpy(&p, var_name, 9);
> +
> +	guid = efi_global_variable_guid;
> +
> +	/* attributes */
> +	lo.attributes = 0x1; /* always ACTIVE */
> +
> +	/* label */
> +	label_len = strlen(argv[2]);
> +	label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> +	label = malloc((label_len16 + 1) * sizeof(u16));
> +	if (!label)
> +		return CMD_RET_FAILURE;
> +	lo.label = label; /* label will be changed below */
> +	utf8_utf16_strncpy(&label, argv[2], label_len);
> +
> +	/* file path */
> +	ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> +			       &file_path);

This will create a full device path like

/VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f)/\<file_path>

This is unlike what the Linux program efibootmgr would do. Efibootmgr
will create a shortened device path where the first node is the
partition, e.g.

HD(1,MBR,0xd1535d21,0x1,0x7f)/\<file_path>

The advantage of this shortened device path is that it only depends on
the disk content and not on the firmware. With a full device path
approach adding a node (e.g. the disk controller) in front of the
partition would invalidate the boot entry. Furthermore the operating
system will not be aware of the full device path.

EDK2 uses the following logic in the boot manager to expand the device
path (BmGetNextLoadOptionDevicePath()):

Check if the file path is a full device path.
Check if the device path matches a partition on any drive.
Check if the file path matches a file on any partition.

I think in U-Boot we will have to support shortened device paths to
collaborate with operating systems.

Best regards

Heinrich
AKASHI Takahiro Jan. 7, 2019, 5:06 a.m. | #4
On Sun, Dec 30, 2018 at 04:44:53PM +0100, Heinrich Schuchardt wrote:
> On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
> > Currently, there is no easy way to add or modify UEFI variables.
> > In particular, bootmgr supports BootOrder/BootXXXX variables, it is
> > quite hard to define them as u-boot variables because they are represented
> > in a complicated and encoded format.
> > 
> > The new command, efishell, helps address these issues and give us
> > more friendly interfaces:
> >  * efishell boot add: add BootXXXX variable
> >  * efishell boot rm: remove BootXXXX variable
> >  * efishell boot dump: display all BootXXXX variables
> >  * efishell boot order: set/display a boot order (BootOrder)
> >  * efishell setvar: set an UEFI variable (with limited functionality)
> >  * efishell dumpvar: display all UEFI variables
> > 
> > As the name suggests, this command basically provides a subset fo UEFI
> > shell commands with simplified functionality.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  cmd/Kconfig    |  10 +
> >  cmd/Makefile   |   1 +
> >  cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 684 insertions(+)
> >  create mode 100644 cmd/efishell.c
> > 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index f2f3b5e2b76b..a8a4bf7db45e 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -1390,6 +1390,16 @@ config CMD_DISPLAY
> >  	  displayed on a simple board-specific display. Implement
> >  	  display_putc() to use it.
> >  
> > +config CMD_EFISHELL
> > +	bool "Enable the 'efishell' command for EFI environment"
> > +	depends on EFI_LOADER
> > +	default n
> > +	help
> > +	  Enable the 'efishell' command which provides a subset of UEFI
> > +	  shell utility with simplified functionality. It will be useful
> > +	  particularly for managing boot parameters as  well as examining
> > +	  various EFI status for debugging.
> > +
> >  config CMD_LED
> >  	bool "led"
> >  	default y if LED
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 5ec2f9e8ebfd..0258d8a373b1 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -49,6 +49,7 @@ obj-$(CONFIG_CMD_ECHO) += echo.o
> >  obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
> >  obj-$(CONFIG_CMD_EEPROM) += eeprom.o
> >  obj-$(CONFIG_EFI_STUB) += efi.o
> > +obj-$(CONFIG_CMD_EFISHELL) += efishell.o
> >  obj-$(CONFIG_CMD_ELF) += elf.o
> >  obj-$(CONFIG_HUSH_PARSER) += exit.o
> >  obj-$(CONFIG_CMD_EXT4) += ext4.o
> > diff --git a/cmd/efishell.c b/cmd/efishell.c
> > new file mode 100644
> > index 000000000000..5819e52cf575
> > --- /dev/null
> > +++ b/cmd/efishell.c
> > @@ -0,0 +1,673 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  EFI Shell-like command
> > + *
> > + *  Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
> > + */
> > +
> > +#include <charset.h>
> > +#include <common.h>
> > +#include <command.h>
> > +#include <efi_loader.h>
> > +#include <environment.h>
> > +#include <errno.h>
> > +#include <exports.h>
> > +#include <hexdump.h>
> > +#include <malloc.h>
> > +#include <search.h>
> > +#include <linux/ctype.h>
> > +#include <asm/global_data.h>
> > +
> > +static void dump_var_data(char *data, unsigned long len)
> > +{
> > +	char *start, *end, *p;
> > +	unsigned long pos, count;
> > +	char hex[3], line[9];
> > +	int i;
> > +
> > +	end = data + len;
> > +	for (start = data, pos = 0; start < end; start += count, pos += count) {
> > +		count = end - start;
> > +		if (count > 16)
> > +			count = 16;
> > +
> > +		/* count should be multiple of two */
> > +		printf("%08lx: ", pos);
> > +
> > +		/* in hex format */
> > +		p = start;
> > +		for (i = 0; i < count / 2; p += 2, i++)
> > +			printf(" %c%c", *p, *(p + 1));
> > +		for (; i < 8; i++)
> > +			printf("   ");
> > +
> > +		/* in character format */
> > +		p = start;
> > +		hex[2] = '\0';
> > +		for (i = 0; i < count / 2; i++) {
> > +			hex[0] = *p++;
> > +			hex[1] = *p++;
> > +			line[i] = simple_strtoul(hex, 0, 16);
> > +			if (line[i] < 0x20 || line[i] > 0x7f)
> > +				line[i] = '.';
> > +		}
> > +		line[i] = '\0';
> > +		printf("  %s\n", line);
> > +	}
> > +}
> > +
> > +/*
> > + * From efi_variable.c,
> > + *
> > + * Mapping between EFI variables and u-boot variables:
> > + *
> > + *   efi_$guid_$varname = {attributes}(type)value
> > + */
> > +static int do_efi_dump_var(int argc, char * const argv[])
> > +{
> > +	char regex[256];
> > +	char * const regexlist[] = {regex};
> > +	char *res = NULL, *start, *end;
> > +	int len;
> > +
> > +	if (argc > 2)
> > +		return CMD_RET_USAGE;
> > +
> > +	if (argc == 2)
> > +		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
> > +	else
> > +		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> > +	debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
> > +
> > +	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> > +			&res, 0, 1, regexlist);
> > +
> > +	if (len < 0)
> > +		return CMD_RET_FAILURE;
> > +
> > +	if (len > 0) {
> > +		end = res;
> > +		while (true) {
> > +			/* variable name */
> > +			start = strchr(end, '_');
> > +			if (!start)
> > +				break;
> > +			start = strchr(++start, '_');
> > +			if (!start)
> > +				break;
> > +			end = strchr(++start, '=');
> > +			if (!end)
> > +				break;
> > +			printf("%.*s:", (int)(end - start), start);
> > +			end++;
> > +
> > +			/* value */
> > +			start = end;
> > +			end = strstr(start, "(blob)");
> > +			if (!end) {
> > +				putc('\n');
> > +				break;
> > +			}
> > +			end += 6;
> > +			printf(" %.*s\n", (int)(end - start), start);
> > +
> > +			start = end;
> > +			end = strchr(start, '\n');
> > +			if (!end)
> > +				break;
> > +			dump_var_data(start,  end - start);
> > +		}
> > +		free(res);
> > +
> > +		if (len < 2 && argc == 2)
> > +			printf("%s: not found\n", argv[1]);
> > +	}
> > +
> > +	return CMD_RET_SUCCESS;
> > +}
> > +
> > +static int append_value(char **bufp, unsigned long *sizep, char *data)
> > +{
> > +	char *tmp_buf = NULL, *new_buf = NULL, *value;
> > +	unsigned long len = 0;
> > +
> > +	if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
> > +		union {
> > +			u8 u8;
> > +			u16 u16;
> > +			u32 u32;
> > +			u64 u64;
> > +		} tmp_data;
> > +		unsigned long hex_value;
> > +		void *hex_ptr;
> > +
> > +		data += 3;
> > +		len = strlen(data);
> > +		if ((len & 0x1)) /* not multiple of two */
> > +			return -1;
> > +
> > +		len /= 2;
> > +		if (len > 8)
> > +			return -1;
> > +		else if (len > 4)
> > +			len = 8;
> > +		else if (len > 2)
> > +			len = 4;
> > +
> > +		/* convert hex hexadecimal number */
> > +		if (strict_strtoul(data, 16, &hex_value) < 0)
> > +			return -1;
> > +
> > +		tmp_buf = malloc(len);
> > +		if (!tmp_buf)
> > +			return -1;
> > +
> > +		if (len == 1) {
> > +			tmp_data.u8 = hex_value;
> > +			hex_ptr = &tmp_data.u8;
> > +		} else if (len == 2) {
> > +			tmp_data.u16 = hex_value;
> > +			hex_ptr = &tmp_data.u16;
> > +		} else if (len == 4) {
> > +			tmp_data.u32 = hex_value;
> > +			hex_ptr = &tmp_data.u32;
> > +		} else {
> > +			tmp_data.u64 = hex_value;
> > +			hex_ptr = &tmp_data.u64;
> > +		}
> > +		memcpy(tmp_buf, hex_ptr, len);
> > +		value = tmp_buf;
> > +
> > +	} else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
> > +		data += 2;
> > +		len = strlen(data);
> > +		if (len & 0x1) /* not multiple of two */
> > +			return -1;
> > +
> > +		len /= 2;
> > +		tmp_buf = malloc(len);
> > +		if (!tmp_buf)
> > +			return -1;
> > +
> > +		if (hex2bin((u8 *)tmp_buf, data, len) < 0)
> > +			return -1;
> > +
> > +		value = tmp_buf;
> > +	} else { /* string */
> > +		if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
> > +			if (data[1] == '"')
> > +				data += 2;
> > +			else
> > +				data += 3;
> > +			value = data;
> > +			len = strlen(data) - 1;
> > +			if (data[len] != '"')
> > +				return -1;
> > +		} else {
> > +			value = data;
> > +			len = strlen(data);
> > +		}
> > +	}
> > +
> > +	new_buf = realloc(*bufp, *sizep + len);
> > +	if (!new_buf)
> > +		goto out;
> > +
> > +	memcpy(new_buf + *sizep, value, len);
> > +	*bufp = new_buf;
> > +	*sizep += len;
> > +
> > +out:
> > +	free(tmp_buf);
> > +
> > +	return 0;
> > +}
> > +
> > +static int do_efi_set_var(int argc, char * const argv[])
> > +{
> > +	char *var_name, *value = NULL;
> > +	unsigned long size = 0;
> > +	u16 *var_name16, *p;
> > +	efi_guid_t guid;
> > +	efi_status_t ret;
> > +
> > +	if (argc == 1)
> > +		return CMD_RET_SUCCESS;
> > +
> > +	var_name = argv[1];
> > +	if (argc == 2) {
> > +		/* remove */
> > +		value = NULL;
> > +		size = 0;
> > +	} else { /* set */
> > +		argc -= 2;
> > +		argv += 2;
> > +
> > +		for ( ; argc > 0; argc--, argv++)
> > +			if (append_value(&value, &size, argv[0]) < 0) {
> > +				ret = CMD_RET_FAILURE;
> > +				goto out;
> > +			}
> > +	}
> > +
> > +	var_name16 = malloc((strlen(var_name) + 1) * 2);
> > +	p = var_name16;
> > +	utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
> > +
> > +	guid = efi_global_variable_guid;
> > +	ret = efi_set_variable(var_name16, &guid,
> > +			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +			       EFI_VARIABLE_RUNTIME_ACCESS, size, value);
> > +	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> > +out:
> > +	return ret;
> > +}
> > +
> > +static int do_efi_boot_add(int argc, char * const argv[])
> > +{
> > +	int id;
> > +	char *endp;
> > +	char var_name[9];
> > +	u16 var_name16[9], *p;
> > +	efi_guid_t guid;
> > +	size_t label_len, label_len16;
> > +	u16 *label;
> > +	struct efi_device_path *device_path = NULL, *file_path = NULL;
> > +	struct efi_load_option lo;
> > +	void *data = NULL;
> > +	unsigned long size;
> > +	int ret;
> > +
> > +	if (argc < 6 || argc > 7)
> > +		return CMD_RET_USAGE;
> > +
> > +	id = (int)simple_strtoul(argv[1], &endp, 0);
> > +	if (*endp != '\0' || id > 0xffff)
> > +		return CMD_RET_FAILURE;
> > +
> > +	sprintf(var_name, "Boot%04X", id);
> > +	p = var_name16;
> > +	utf8_utf16_strncpy(&p, var_name, 9);
> > +
> > +	guid = efi_global_variable_guid;
> > +
> > +	/* attributes */
> > +	lo.attributes = 0x1; /* always ACTIVE */
> > +
> > +	/* label */
> > +	label_len = strlen(argv[2]);
> > +	label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> > +	label = malloc((label_len16 + 1) * sizeof(u16));
> > +	if (!label)
> > +		return CMD_RET_FAILURE;
> > +	lo.label = label; /* label will be changed below */
> > +	utf8_utf16_strncpy(&label, argv[2], label_len);
> > +
> > +	/* file path */
> > +	ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> > +			       &file_path);
> > +	if (ret != EFI_SUCCESS) {
> > +		ret = CMD_RET_FAILURE;
> > +		goto out;
> > +	}
> > +	lo.file_path = file_path;
> > +	lo.file_path_length = efi_dp_size(file_path)
> > +				+ sizeof(struct efi_device_path); /* for END */
> > +
> > +	/* optional data */
> > +	lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
> > +
> > +	size = efi_serialize_load_option(&lo, (u8 **)&data);
> 
> The load options should be converted to a UTF-16 string before calling
> efi_serialize_load_option so that we can copy them to the loaded image
> protocol.

I don't get your point.
What do you mean by "load options," lo or optional_data?
The latter is defined as a u8 string in efi_loader.h, which was
originally located in efi_bootmgr.c.

> > +	if (!size) {
> > +		ret = CMD_RET_FAILURE;
> > +		goto out;
> > +	}
> > +
> > +	ret = efi_set_variable(var_name16, &guid,
> > +			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +			       EFI_VARIABLE_RUNTIME_ACCESS, size, data);
> > +	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> > +out:
> > +	free(data);
> > +	efi_free_pool(device_path);
> > +	efi_free_pool(file_path);
> > +	free(lo.label);
> > +
> > +	return ret;
> > +}
> > +
> > +static int do_efi_boot_rm(int argc, char * const argv[])
> > +{
> > +	efi_guid_t guid;
> > +	int id, i;
> > +	char *endp;
> > +	char var_name[9];
> > +	u16 var_name16[9];
> > +	efi_status_t ret;
> > +
> > +	if (argc == 1)
> > +		return CMD_RET_USAGE;
> > +
> > +	guid = efi_global_variable_guid;
> > +	for (i = 1; i < argc; i++, argv++) {
> > +		id = (int)simple_strtoul(argv[1], &endp, 0);
> > +		if (*endp != '\0' || id > 0xffff)
> > +			return CMD_RET_FAILURE;
> > +
> > +		sprintf(var_name, "Boot%04X", id);
> > +		utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
> > +
> > +		ret = efi_set_variable(var_name16, &guid,
> > +				       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +				       EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
> > +		if (ret) {
> > +			printf("cannot remove Boot%04X", id);
> > +			return CMD_RET_FAILURE;
> > +		}
> > +	}
> > +
> > +	return CMD_RET_SUCCESS;
> > +}
> > +
> > +static void show_efi_boot_opt_data(int id, void *data)
> > +{
> > +	struct efi_load_option lo;
> > +	char *label, *p;
> > +	size_t label_len16, label_len;
> > +	u16 *dp_str;
> > +
> > +	efi_deserialize_load_option(&lo, data);
> > +
> > +	label_len16 = u16_strlen(lo.label);
> > +	label_len = utf16_utf8_strnlen(lo.label, label_len16);
> > +	label = malloc(label_len + 1);
> > +	if (!label)
> > +		return;
> > +	p = label;
> > +	utf16_utf8_strncpy(&p, lo.label, label_len16);
> > +
> > +	printf("Boot%04X:\n", id);
> > +	printf("\tattributes: %c%c%c (0x%08x)\n",
> > +	       /* ACTIVE */
> > +	       lo.attributes & 0x1 ? 'A' : '-',
> > +	       /* FORCE RECONNECT */
> > +	       lo.attributes & 0x2 ? 'R' : '-',
> > +	       /* HIDDEN */
> > +	       lo.attributes & 0x8 ? 'H' : '-',
> > +	       lo.attributes);
> > +	printf("\tlabel: %s\n", label);
> > +
> > +	dp_str = efi_dp_str(lo.file_path);
> > +	printf("\tfile_path: %ls\n", dp_str);
> > +	efi_free_pool(dp_str);
> > +
> > +	printf("\tdata: %s\n", lo.optional_data);
> > +
> > +	free(label);
> > +}
> > +
> > +static void show_efi_boot_opt(int id)
> > +{
> > +	char var_name[9];
> > +	u16 var_name16[9], *p;
> > +	efi_guid_t guid;
> > +	void *data = NULL;
> > +	unsigned long size;
> 
> 'size' must be of type efi_uint_t. Otherwise you get warnings like this one:
> 
> cmd/efishell.c:729:50: warning: passing argument 4 of ‘efi_get_variable’
> from incompatible pointer type [-Wincompatible-pointer-types]
>   ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);

OK.

> 
> > +	int ret;
> > +
> > +	sprintf(var_name, "Boot%04X", id);
> > +	p = var_name16;
> > +	utf8_utf16_strncpy(&p, var_name, 9);
> > +	guid = efi_global_variable_guid;
> > +
> > +	size = 0;
> > +	ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> > +	if (ret == (int)EFI_BUFFER_TOO_SMALL) {
> > +		data = malloc(size);
> > +		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> > +	}
> > +	if (ret == EFI_SUCCESS)
> > +		show_efi_boot_opt_data(id, data);
> > +	else if (ret == EFI_NOT_FOUND)
> > +		printf("Boot%04X: not found\n", id);
> > +
> > +	free(data);
> > +}
> > +
> > +static int do_efi_boot_dump(int argc, char * const argv[])
> > +{
> > +	char regex[256];
> > +	char * const regexlist[] = {regex};
> > +	char *variables = NULL, *boot, *value;
> > +	int len;
> > +	int id;
> > +
> > +	if (argc > 1)
> > +		return CMD_RET_USAGE;
> > +
> > +	snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
> > +
> > +	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> > +			&variables, 0, 1, regexlist);
> > +
> > +	if (!len)
> > +		return CMD_RET_SUCCESS;
> > +
> > +	if (len < 0)
> > +		return CMD_RET_FAILURE;
> > +
> > +	boot = variables;
> > +	while (*boot) {
> > +		value = strstr(boot, "Boot") + 4;
> > +		id = (int)simple_strtoul(value, NULL, 16);
> > +		show_efi_boot_opt(id);
> > +		boot = strchr(boot, '\n');
> > +		if (!*boot)
> > +			break;
> > +		boot++;
> > +	}
> > +	free(variables);
> > +
> > +	return CMD_RET_SUCCESS;
> > +}
> > +
> > +static int show_efi_boot_order(void)
> > +{
> > +	efi_guid_t guid;
> > +	u16 *bootorder = NULL;
> > +	unsigned long size;
> > +	int num, i;
> > +	char var_name[9];
> > +	u16 var_name16[9], *p16;
> > +	void *data;
> > +	struct efi_load_option lo;
> > +	char *label, *p;
> > +	size_t label_len16, label_len;
> > +	efi_status_t ret;
> > +
> > +	guid = efi_global_variable_guid;
> > +	size = 0;
> > +	ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
> > +	if (ret == EFI_BUFFER_TOO_SMALL) {
> > +		bootorder = malloc(size);
> > +		ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,
> > +				       bootorder);
> > +	}
> > +	if (ret == EFI_NOT_FOUND) {
> > +		printf("BootOrder not defined\n");
> > +		ret = CMD_RET_SUCCESS;
> > +		goto out;
> > +	} else if (ret != EFI_SUCCESS) {
> > +		ret = CMD_RET_FAILURE;
> > +		goto out;
> > +	}
> > +
> > +	num = size / sizeof(u16);
> > +	for (i = 0; i < num; i++) {
> > +		sprintf(var_name, "Boot%04X", bootorder[i]);
> > +		p16 = var_name16;
> > +		utf8_utf16_strncpy(&p16, var_name, 9);
> > +
> > +		size = 0;
> > +		ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> > +		if (ret != EFI_BUFFER_TOO_SMALL) {
> > +			printf("%2d: Boot%04X: (not defined)\n",
> > +			       i + 1, bootorder[i]);
> > +			continue;
> > +		}
> > +
> > +		data = malloc(size);
> > +		if (!data) {
> > +			ret = CMD_RET_FAILURE;
> > +			goto out;
> > +		}
> > +		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> > +		if (ret != EFI_SUCCESS) {
> > +			free(data);
> > +			ret = CMD_RET_FAILURE;
> > +			goto out;
> > +		}
> > +
> > +		efi_deserialize_load_option(&lo, data);
> > +
> > +		label_len16 = u16_strlen(lo.label);
> > +		label_len = utf16_utf8_strnlen(lo.label, label_len16);
> > +		label = malloc(label_len + 1);
> > +		if (!label) {
> > +			free(data);
> > +			ret = CMD_RET_FAILURE;
> > +			goto out;
> > +		}
> > +		p = label;
> > +		utf16_utf8_strncpy(&p, lo.label, label_len16);
> > +		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
> > +		free(label);
> > +
> > +		free(data);
> > +	}
> > +out:
> > +	free(bootorder);
> > +
> > +	return ret;
> > +}
> > +
> > +static int do_efi_boot_order(int argc, char * const argv[])
> > +{
> > +	u16 *bootorder = NULL;
> > +	unsigned long size;
> > +	int id, i;
> > +	char *endp;
> > +	efi_guid_t guid;
> > +	efi_status_t ret;
> > +
> > +	if (argc == 1)
> > +		return show_efi_boot_order();
> > +
> > +	argc--;
> > +	argv++;
> > +
> > +	size = argc * sizeof(u16);
> > +	bootorder = malloc(size);
> > +	if (!bootorder)
> > +		return CMD_RET_FAILURE;
> > +
> > +	for (i = 0; i < argc; i++) {
> > +		id = (int)simple_strtoul(argv[i], &endp, 0);
> > +		if (*endp != '\0' || id > 0xffff) {
> > +			printf("invalid value: %s\n", argv[i]);
> > +			ret = CMD_RET_FAILURE;
> > +			goto out;
> > +		}
> > +
> > +		bootorder[i] = (u16)id;
> > +	}
> > +
> > +	guid = efi_global_variable_guid;
> > +	ret = efi_set_variable(L"BootOrder", &guid,
> > +			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +			       EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
> > +	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> > +out:
> > +	free(bootorder);
> > +
> > +	return ret;
> > +}
> > +
> > +static int do_efi_boot_opt(int argc, char * const argv[])
> > +{
> > +	char *sub_command;
> > +
> > +	if (argc < 2)
> > +		return CMD_RET_USAGE;
> > +
> > +	argc--; argv++;
> > +	sub_command = argv[0];
> > +
> > +	if (!strcmp(sub_command, "add"))
> > +		return do_efi_boot_add(argc, argv);
> > +	else if (!strcmp(sub_command, "rm"))
> > +		return do_efi_boot_rm(argc, argv);
> > +	else if (!strcmp(sub_command, "dump"))
> > +		return do_efi_boot_dump(argc, argv);
> > +	else if (!strcmp(sub_command, "order"))
> > +		return do_efi_boot_order(argc, argv);
> > +	else
> > +		return CMD_RET_USAGE;
> > +}
> > +
> > +/* Interpreter command to configure EFI environment */
> > +static int do_efishell(cmd_tbl_t *cmdtp, int flag,
> > +		       int argc, char * const argv[])
> > +{
> > +	char *command;
> > +	efi_status_t r;
> > +
> > +	if (argc < 2)
> > +		return CMD_RET_USAGE;
> > +
> > +	argc--; argv++;
> > +	command = argv[0];
> > +
> > +	/* Initialize EFI drivers */
> > +	r = efi_init_obj_list();
> > +	if (r != EFI_SUCCESS) {
> > +		printf("Error: Cannot set up EFI drivers, r = %lu\n",
> > +		       r & ~EFI_ERROR_MASK);
> > +		return CMD_RET_FAILURE;
> > +	}
> > +
> > +	if (!strcmp(command, "boot"))
> > +		return do_efi_boot_opt(argc, argv);
> > +	else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
> > +		return do_efi_dump_var(argc, argv);
> > +	else if (!strcmp(command, "setvar"))
> > +		return do_efi_set_var(argc, argv);
> > +	else
> > +		return CMD_RET_USAGE;
> > +}
> > +
> > +#ifdef CONFIG_SYS_LONGHELP
> > +static char efishell_help_text[] =
> > +	"  - EFI Shell-like interface to configure EFI environment\n"
> > +	"\n"
> > +	"efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
> 
> The 'option' parameter is optional. Please, add brackets ([]).

OK.

> The parameter 'option' is misleading. Please call it 'load_options'.

OK.

> Please, add a text explaining that 'load_options' is meant to be the
> command line string passed to the EFI binary.

OK.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > +	"  - set uefi's BOOT variable\n"
> > +	"efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
> > +	"  - set/display uefi boot order\n"
> > +	"efishell boot dump\n"
> > +	"  - display all uefi's BOOT variables\n"
> > +	"efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
> > +	"  - set/display uefi boot order\n"
> > +	"\n"
> > +	"efishell dumpvar [<name>]\n"
> > +	"  - get uefi variable's value\n"
> > +	"efishell setvar <name> [<value>]\n"
> > +	"  - set/delete uefi variable's value\n"
> > +	"    <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
> > +#endif
> > +
> > +U_BOOT_CMD(
> > +	efishell, 10, 0, do_efishell,
> > +	"Configure EFI environment",
> > +	efishell_help_text
> > +);
> > 
>
AKASHI Takahiro Jan. 7, 2019, 5:08 a.m. | #5
On Sun, Dec 30, 2018 at 06:10:51PM +0100, Heinrich Schuchardt wrote:
> On 12/30/18 4:44 PM, Heinrich Schuchardt wrote:
> > On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
> >> Currently, there is no easy way to add or modify UEFI variables.
> >> In particular, bootmgr supports BootOrder/BootXXXX variables, it is
> >> quite hard to define them as u-boot variables because they are represented
> >> in a complicated and encoded format.
> >>
> >> The new command, efishell, helps address these issues and give us
> >> more friendly interfaces:
> >>  * efishell boot add: add BootXXXX variable
> >>  * efishell boot rm: remove BootXXXX variable
> >>  * efishell boot dump: display all BootXXXX variables
> >>  * efishell boot order: set/display a boot order (BootOrder)
> >>  * efishell setvar: set an UEFI variable (with limited functionality)
> >>  * efishell dumpvar: display all UEFI variables
> >>
> >> As the name suggests, this command basically provides a subset fo UEFI
> >> shell commands with simplified functionality.
> >>
> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> The behavior is a bit unexpected:
> 
> => efishell boot order 200
> => efishell boot order
>  1: Boot00C8: (not defined)
> exit not allowed from main input shell.
> 
> I would expect 'efishell boot order' to take a 4 digit hexadecimal
> number and to do no conversion from decimal to hexadecimal.

OK, but we should allow a less-than-4-digit number. 

> I was also surprised to see 'exit not allowed from main input shell.'

I cannot reproduce this problem.

-Takahiro Akashi

> Best regards
> 
> Heinrich
AKASHI Takahiro Jan. 7, 2019, 5:13 a.m. | #6
On Mon, Dec 31, 2018 at 12:47:07AM +0100, Heinrich Schuchardt wrote:
> On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
> > Currently, there is no easy way to add or modify UEFI variables.
> > In particular, bootmgr supports BootOrder/BootXXXX variables, it is
> > quite hard to define them as u-boot variables because they are represented
> > in a complicated and encoded format.
> > 
> > The new command, efishell, helps address these issues and give us
> > more friendly interfaces:
> >  * efishell boot add: add BootXXXX variable
> >  * efishell boot rm: remove BootXXXX variable
> >  * efishell boot dump: display all BootXXXX variables
> >  * efishell boot order: set/display a boot order (BootOrder)
> >  * efishell setvar: set an UEFI variable (with limited functionality)
> >  * efishell dumpvar: display all UEFI variables
> > 
> > As the name suggests, this command basically provides a subset fo UEFI
> > shell commands with simplified functionality.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  cmd/Kconfig    |  10 +
> >  cmd/Makefile   |   1 +
> >  cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 684 insertions(+)
> >  create mode 100644 cmd/efishell.c
> > 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index f2f3b5e2b76b..a8a4bf7db45e 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -1390,6 +1390,16 @@ config CMD_DISPLAY
> >  	  displayed on a simple board-specific display. Implement
> >  	  display_putc() to use it.
> >  
> 
> <snip>
> 
> > +static int do_efi_boot_add(int argc, char * const argv[])
> > +{
> > +	int id;
> > +	char *endp;
> > +	char var_name[9];
> > +	u16 var_name16[9], *p;
> > +	efi_guid_t guid;
> > +	size_t label_len, label_len16;
> > +	u16 *label;
> > +	struct efi_device_path *device_path = NULL, *file_path = NULL;
> > +	struct efi_load_option lo;
> > +	void *data = NULL;
> > +	unsigned long size;
> > +	int ret;
> > +
> > +	if (argc < 6 || argc > 7)
> > +		return CMD_RET_USAGE;
> > +
> > +	id = (int)simple_strtoul(argv[1], &endp, 0);
> > +	if (*endp != '\0' || id > 0xffff)
> > +		return CMD_RET_FAILURE;
> > +
> > +	sprintf(var_name, "Boot%04X", id);
> > +	p = var_name16;
> > +	utf8_utf16_strncpy(&p, var_name, 9);
> > +
> > +	guid = efi_global_variable_guid;
> > +
> > +	/* attributes */
> > +	lo.attributes = 0x1; /* always ACTIVE */
> > +
> > +	/* label */
> > +	label_len = strlen(argv[2]);
> > +	label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> > +	label = malloc((label_len16 + 1) * sizeof(u16));
> > +	if (!label)
> > +		return CMD_RET_FAILURE;
> > +	lo.label = label; /* label will be changed below */
> > +	utf8_utf16_strncpy(&label, argv[2], label_len);
> > +
> > +	/* file path */
> > +	ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> > +			       &file_path);
> 
> This will create a full device path like
> 
> /VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f)/\<file_path>
> 
> This is unlike what the Linux program efibootmgr would do. Efibootmgr
> will create a shortened device path where the first node is the
> partition, e.g.
> 
> HD(1,MBR,0xd1535d21,0x1,0x7f)/\<file_path>
> 
> The advantage of this shortened device path is that it only depends on
> the disk content and not on the firmware. With a full device path
> approach adding a node (e.g. the disk controller) in front of the
> partition would invalidate the boot entry. Furthermore the operating
> system will not be aware of the full device path.
> 
> EDK2 uses the following logic in the boot manager to expand the device
> path (BmGetNextLoadOptionDevicePath()):
> 
> Check if the file path is a full device path.
> Check if the device path matches a partition on any drive.
> Check if the file path matches a file on any partition.
> 
> I think in U-Boot we will have to support shortened device paths to
> collaborate with operating systems.

So I assume that your comment here is not on my patch, but u-boot's
bootmgr. Once bootmgr is modified, I will be able to make my code
aligned.

-Takahiro Akashi

> Best regards
> 
> Heinrich
Alexander Graf Jan. 8, 2019, 9:57 a.m. | #7
On 07.01.19 06:08, AKASHI Takahiro wrote:
> On Sun, Dec 30, 2018 at 06:10:51PM +0100, Heinrich Schuchardt wrote:
>> On 12/30/18 4:44 PM, Heinrich Schuchardt wrote:
>>> On 12/18/18 6:05 AM, AKASHI Takahiro wrote:
>>>> Currently, there is no easy way to add or modify UEFI variables.
>>>> In particular, bootmgr supports BootOrder/BootXXXX variables, it is
>>>> quite hard to define them as u-boot variables because they are represented
>>>> in a complicated and encoded format.
>>>>
>>>> The new command, efishell, helps address these issues and give us
>>>> more friendly interfaces:
>>>>  * efishell boot add: add BootXXXX variable
>>>>  * efishell boot rm: remove BootXXXX variable
>>>>  * efishell boot dump: display all BootXXXX variables
>>>>  * efishell boot order: set/display a boot order (BootOrder)
>>>>  * efishell setvar: set an UEFI variable (with limited functionality)
>>>>  * efishell dumpvar: display all UEFI variables
>>>>
>>>> As the name suggests, this command basically provides a subset fo UEFI
>>>> shell commands with simplified functionality.
>>>>
>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>
>> The behavior is a bit unexpected:
>>
>> => efishell boot order 200
>> => efishell boot order
>>  1: Boot00C8: (not defined)
>> exit not allowed from main input shell.
>>
>> I would expect 'efishell boot order' to take a 4 digit hexadecimal
>> number and to do no conversion from decimal to hexadecimal.
> 
> OK, but we should allow a less-than-4-digit number. 

Yes, but definitely stick to Hex regardless. Hex is the default number
space in U-Boot - and it just happens to fit quite well with the
variable definition too :).


Alex

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index f2f3b5e2b76b..a8a4bf7db45e 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1390,6 +1390,16 @@  config CMD_DISPLAY
 	  displayed on a simple board-specific display. Implement
 	  display_putc() to use it.
 
+config CMD_EFISHELL
+	bool "Enable the 'efishell' command for EFI environment"
+	depends on EFI_LOADER
+	default n
+	help
+	  Enable the 'efishell' command which provides a subset of UEFI
+	  shell utility with simplified functionality. It will be useful
+	  particularly for managing boot parameters as  well as examining
+	  various EFI status for debugging.
+
 config CMD_LED
 	bool "led"
 	default y if LED
diff --git a/cmd/Makefile b/cmd/Makefile
index 5ec2f9e8ebfd..0258d8a373b1 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -49,6 +49,7 @@  obj-$(CONFIG_CMD_ECHO) += echo.o
 obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
 obj-$(CONFIG_CMD_EEPROM) += eeprom.o
 obj-$(CONFIG_EFI_STUB) += efi.o
+obj-$(CONFIG_CMD_EFISHELL) += efishell.o
 obj-$(CONFIG_CMD_ELF) += elf.o
 obj-$(CONFIG_HUSH_PARSER) += exit.o
 obj-$(CONFIG_CMD_EXT4) += ext4.o
diff --git a/cmd/efishell.c b/cmd/efishell.c
new file mode 100644
index 000000000000..5819e52cf575
--- /dev/null
+++ b/cmd/efishell.c
@@ -0,0 +1,673 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  EFI Shell-like command
+ *
+ *  Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
+ */
+
+#include <charset.h>
+#include <common.h>
+#include <command.h>
+#include <efi_loader.h>
+#include <environment.h>
+#include <errno.h>
+#include <exports.h>
+#include <hexdump.h>
+#include <malloc.h>
+#include <search.h>
+#include <linux/ctype.h>
+#include <asm/global_data.h>
+
+static void dump_var_data(char *data, unsigned long len)
+{
+	char *start, *end, *p;
+	unsigned long pos, count;
+	char hex[3], line[9];
+	int i;
+
+	end = data + len;
+	for (start = data, pos = 0; start < end; start += count, pos += count) {
+		count = end - start;
+		if (count > 16)
+			count = 16;
+
+		/* count should be multiple of two */
+		printf("%08lx: ", pos);
+
+		/* in hex format */
+		p = start;
+		for (i = 0; i < count / 2; p += 2, i++)
+			printf(" %c%c", *p, *(p + 1));
+		for (; i < 8; i++)
+			printf("   ");
+
+		/* in character format */
+		p = start;
+		hex[2] = '\0';
+		for (i = 0; i < count / 2; i++) {
+			hex[0] = *p++;
+			hex[1] = *p++;
+			line[i] = simple_strtoul(hex, 0, 16);
+			if (line[i] < 0x20 || line[i] > 0x7f)
+				line[i] = '.';
+		}
+		line[i] = '\0';
+		printf("  %s\n", line);
+	}
+}
+
+/*
+ * From efi_variable.c,
+ *
+ * Mapping between EFI variables and u-boot variables:
+ *
+ *   efi_$guid_$varname = {attributes}(type)value
+ */
+static int do_efi_dump_var(int argc, char * const argv[])
+{
+	char regex[256];
+	char * const regexlist[] = {regex};
+	char *res = NULL, *start, *end;
+	int len;
+
+	if (argc > 2)
+		return CMD_RET_USAGE;
+
+	if (argc == 2)
+		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
+	else
+		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
+	debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
+
+	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+			&res, 0, 1, regexlist);
+
+	if (len < 0)
+		return CMD_RET_FAILURE;
+
+	if (len > 0) {
+		end = res;
+		while (true) {
+			/* variable name */
+			start = strchr(end, '_');
+			if (!start)
+				break;
+			start = strchr(++start, '_');
+			if (!start)
+				break;
+			end = strchr(++start, '=');
+			if (!end)
+				break;
+			printf("%.*s:", (int)(end - start), start);
+			end++;
+
+			/* value */
+			start = end;
+			end = strstr(start, "(blob)");
+			if (!end) {
+				putc('\n');
+				break;
+			}
+			end += 6;
+			printf(" %.*s\n", (int)(end - start), start);
+
+			start = end;
+			end = strchr(start, '\n');
+			if (!end)
+				break;
+			dump_var_data(start,  end - start);
+		}
+		free(res);
+
+		if (len < 2 && argc == 2)
+			printf("%s: not found\n", argv[1]);
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static int append_value(char **bufp, unsigned long *sizep, char *data)
+{
+	char *tmp_buf = NULL, *new_buf = NULL, *value;
+	unsigned long len = 0;
+
+	if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
+		union {
+			u8 u8;
+			u16 u16;
+			u32 u32;
+			u64 u64;
+		} tmp_data;
+		unsigned long hex_value;
+		void *hex_ptr;
+
+		data += 3;
+		len = strlen(data);
+		if ((len & 0x1)) /* not multiple of two */
+			return -1;
+
+		len /= 2;
+		if (len > 8)
+			return -1;
+		else if (len > 4)
+			len = 8;
+		else if (len > 2)
+			len = 4;
+
+		/* convert hex hexadecimal number */
+		if (strict_strtoul(data, 16, &hex_value) < 0)
+			return -1;
+
+		tmp_buf = malloc(len);
+		if (!tmp_buf)
+			return -1;
+
+		if (len == 1) {
+			tmp_data.u8 = hex_value;
+			hex_ptr = &tmp_data.u8;
+		} else if (len == 2) {
+			tmp_data.u16 = hex_value;
+			hex_ptr = &tmp_data.u16;
+		} else if (len == 4) {
+			tmp_data.u32 = hex_value;
+			hex_ptr = &tmp_data.u32;
+		} else {
+			tmp_data.u64 = hex_value;
+			hex_ptr = &tmp_data.u64;
+		}
+		memcpy(tmp_buf, hex_ptr, len);
+		value = tmp_buf;
+
+	} else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
+		data += 2;
+		len = strlen(data);
+		if (len & 0x1) /* not multiple of two */
+			return -1;
+
+		len /= 2;
+		tmp_buf = malloc(len);
+		if (!tmp_buf)
+			return -1;
+
+		if (hex2bin((u8 *)tmp_buf, data, len) < 0)
+			return -1;
+
+		value = tmp_buf;
+	} else { /* string */
+		if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
+			if (data[1] == '"')
+				data += 2;
+			else
+				data += 3;
+			value = data;
+			len = strlen(data) - 1;
+			if (data[len] != '"')
+				return -1;
+		} else {
+			value = data;
+			len = strlen(data);
+		}
+	}
+
+	new_buf = realloc(*bufp, *sizep + len);
+	if (!new_buf)
+		goto out;
+
+	memcpy(new_buf + *sizep, value, len);
+	*bufp = new_buf;
+	*sizep += len;
+
+out:
+	free(tmp_buf);
+
+	return 0;
+}
+
+static int do_efi_set_var(int argc, char * const argv[])
+{
+	char *var_name, *value = NULL;
+	unsigned long size = 0;
+	u16 *var_name16, *p;
+	efi_guid_t guid;
+	efi_status_t ret;
+
+	if (argc == 1)
+		return CMD_RET_SUCCESS;
+
+	var_name = argv[1];
+	if (argc == 2) {
+		/* remove */
+		value = NULL;
+		size = 0;
+	} else { /* set */
+		argc -= 2;
+		argv += 2;
+
+		for ( ; argc > 0; argc--, argv++)
+			if (append_value(&value, &size, argv[0]) < 0) {
+				ret = CMD_RET_FAILURE;
+				goto out;
+			}
+	}
+
+	var_name16 = malloc((strlen(var_name) + 1) * 2);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
+
+	guid = efi_global_variable_guid;
+	ret = efi_set_variable(var_name16, &guid,
+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, value);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	return ret;
+}
+
+static int do_efi_boot_add(int argc, char * const argv[])
+{
+	int id;
+	char *endp;
+	char var_name[9];
+	u16 var_name16[9], *p;
+	efi_guid_t guid;
+	size_t label_len, label_len16;
+	u16 *label;
+	struct efi_device_path *device_path = NULL, *file_path = NULL;
+	struct efi_load_option lo;
+	void *data = NULL;
+	unsigned long size;
+	int ret;
+
+	if (argc < 6 || argc > 7)
+		return CMD_RET_USAGE;
+
+	id = (int)simple_strtoul(argv[1], &endp, 0);
+	if (*endp != '\0' || id > 0xffff)
+		return CMD_RET_FAILURE;
+
+	sprintf(var_name, "Boot%04X", id);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, 9);
+
+	guid = efi_global_variable_guid;
+
+	/* attributes */
+	lo.attributes = 0x1; /* always ACTIVE */
+
+	/* label */
+	label_len = strlen(argv[2]);
+	label_len16 = utf8_utf16_strnlen(argv[2], label_len);
+	label = malloc((label_len16 + 1) * sizeof(u16));
+	if (!label)
+		return CMD_RET_FAILURE;
+	lo.label = label; /* label will be changed below */
+	utf8_utf16_strncpy(&label, argv[2], label_len);
+
+	/* file path */
+	ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
+			       &file_path);
+	if (ret != EFI_SUCCESS) {
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+	lo.file_path = file_path;
+	lo.file_path_length = efi_dp_size(file_path)
+				+ sizeof(struct efi_device_path); /* for END */
+
+	/* optional data */
+	lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
+
+	size = efi_serialize_load_option(&lo, (u8 **)&data);
+	if (!size) {
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+
+	ret = efi_set_variable(var_name16, &guid,
+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, data);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	free(data);
+	efi_free_pool(device_path);
+	efi_free_pool(file_path);
+	free(lo.label);
+
+	return ret;
+}
+
+static int do_efi_boot_rm(int argc, char * const argv[])
+{
+	efi_guid_t guid;
+	int id, i;
+	char *endp;
+	char var_name[9];
+	u16 var_name16[9];
+	efi_status_t ret;
+
+	if (argc == 1)
+		return CMD_RET_USAGE;
+
+	guid = efi_global_variable_guid;
+	for (i = 1; i < argc; i++, argv++) {
+		id = (int)simple_strtoul(argv[1], &endp, 0);
+		if (*endp != '\0' || id > 0xffff)
+			return CMD_RET_FAILURE;
+
+		sprintf(var_name, "Boot%04X", id);
+		utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
+
+		ret = efi_set_variable(var_name16, &guid,
+				       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+				       EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
+		if (ret) {
+			printf("cannot remove Boot%04X", id);
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static void show_efi_boot_opt_data(int id, void *data)
+{
+	struct efi_load_option lo;
+	char *label, *p;
+	size_t label_len16, label_len;
+	u16 *dp_str;
+
+	efi_deserialize_load_option(&lo, data);
+
+	label_len16 = u16_strlen(lo.label);
+	label_len = utf16_utf8_strnlen(lo.label, label_len16);
+	label = malloc(label_len + 1);
+	if (!label)
+		return;
+	p = label;
+	utf16_utf8_strncpy(&p, lo.label, label_len16);
+
+	printf("Boot%04X:\n", id);
+	printf("\tattributes: %c%c%c (0x%08x)\n",
+	       /* ACTIVE */
+	       lo.attributes & 0x1 ? 'A' : '-',
+	       /* FORCE RECONNECT */
+	       lo.attributes & 0x2 ? 'R' : '-',
+	       /* HIDDEN */
+	       lo.attributes & 0x8 ? 'H' : '-',
+	       lo.attributes);
+	printf("\tlabel: %s\n", label);
+
+	dp_str = efi_dp_str(lo.file_path);
+	printf("\tfile_path: %ls\n", dp_str);
+	efi_free_pool(dp_str);
+
+	printf("\tdata: %s\n", lo.optional_data);
+
+	free(label);
+}
+
+static void show_efi_boot_opt(int id)
+{
+	char var_name[9];
+	u16 var_name16[9], *p;
+	efi_guid_t guid;
+	void *data = NULL;
+	unsigned long size;
+	int ret;
+
+	sprintf(var_name, "Boot%04X", id);
+	p = var_name16;
+	utf8_utf16_strncpy(&p, var_name, 9);
+	guid = efi_global_variable_guid;
+
+	size = 0;
+	ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
+	if (ret == (int)EFI_BUFFER_TOO_SMALL) {
+		data = malloc(size);
+		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
+	}
+	if (ret == EFI_SUCCESS)
+		show_efi_boot_opt_data(id, data);
+	else if (ret == EFI_NOT_FOUND)
+		printf("Boot%04X: not found\n", id);
+
+	free(data);
+}
+
+static int do_efi_boot_dump(int argc, char * const argv[])
+{
+	char regex[256];
+	char * const regexlist[] = {regex};
+	char *variables = NULL, *boot, *value;
+	int len;
+	int id;
+
+	if (argc > 1)
+		return CMD_RET_USAGE;
+
+	snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
+
+	len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+			&variables, 0, 1, regexlist);
+
+	if (!len)
+		return CMD_RET_SUCCESS;
+
+	if (len < 0)
+		return CMD_RET_FAILURE;
+
+	boot = variables;
+	while (*boot) {
+		value = strstr(boot, "Boot") + 4;
+		id = (int)simple_strtoul(value, NULL, 16);
+		show_efi_boot_opt(id);
+		boot = strchr(boot, '\n');
+		if (!*boot)
+			break;
+		boot++;
+	}
+	free(variables);
+
+	return CMD_RET_SUCCESS;
+}
+
+static int show_efi_boot_order(void)
+{
+	efi_guid_t guid;
+	u16 *bootorder = NULL;
+	unsigned long size;
+	int num, i;
+	char var_name[9];
+	u16 var_name16[9], *p16;
+	void *data;
+	struct efi_load_option lo;
+	char *label, *p;
+	size_t label_len16, label_len;
+	efi_status_t ret;
+
+	guid = efi_global_variable_guid;
+	size = 0;
+	ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
+	if (ret == EFI_BUFFER_TOO_SMALL) {
+		bootorder = malloc(size);
+		ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,
+				       bootorder);
+	}
+	if (ret == EFI_NOT_FOUND) {
+		printf("BootOrder not defined\n");
+		ret = CMD_RET_SUCCESS;
+		goto out;
+	} else if (ret != EFI_SUCCESS) {
+		ret = CMD_RET_FAILURE;
+		goto out;
+	}
+
+	num = size / sizeof(u16);
+	for (i = 0; i < num; i++) {
+		sprintf(var_name, "Boot%04X", bootorder[i]);
+		p16 = var_name16;
+		utf8_utf16_strncpy(&p16, var_name, 9);
+
+		size = 0;
+		ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
+		if (ret != EFI_BUFFER_TOO_SMALL) {
+			printf("%2d: Boot%04X: (not defined)\n",
+			       i + 1, bootorder[i]);
+			continue;
+		}
+
+		data = malloc(size);
+		if (!data) {
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+		ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
+		if (ret != EFI_SUCCESS) {
+			free(data);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+
+		efi_deserialize_load_option(&lo, data);
+
+		label_len16 = u16_strlen(lo.label);
+		label_len = utf16_utf8_strnlen(lo.label, label_len16);
+		label = malloc(label_len + 1);
+		if (!label) {
+			free(data);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+		p = label;
+		utf16_utf8_strncpy(&p, lo.label, label_len16);
+		printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
+		free(label);
+
+		free(data);
+	}
+out:
+	free(bootorder);
+
+	return ret;
+}
+
+static int do_efi_boot_order(int argc, char * const argv[])
+{
+	u16 *bootorder = NULL;
+	unsigned long size;
+	int id, i;
+	char *endp;
+	efi_guid_t guid;
+	efi_status_t ret;
+
+	if (argc == 1)
+		return show_efi_boot_order();
+
+	argc--;
+	argv++;
+
+	size = argc * sizeof(u16);
+	bootorder = malloc(size);
+	if (!bootorder)
+		return CMD_RET_FAILURE;
+
+	for (i = 0; i < argc; i++) {
+		id = (int)simple_strtoul(argv[i], &endp, 0);
+		if (*endp != '\0' || id > 0xffff) {
+			printf("invalid value: %s\n", argv[i]);
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+
+		bootorder[i] = (u16)id;
+	}
+
+	guid = efi_global_variable_guid;
+	ret = efi_set_variable(L"BootOrder", &guid,
+			       EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			       EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
+	ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+	free(bootorder);
+
+	return ret;
+}
+
+static int do_efi_boot_opt(int argc, char * const argv[])
+{
+	char *sub_command;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--; argv++;
+	sub_command = argv[0];
+
+	if (!strcmp(sub_command, "add"))
+		return do_efi_boot_add(argc, argv);
+	else if (!strcmp(sub_command, "rm"))
+		return do_efi_boot_rm(argc, argv);
+	else if (!strcmp(sub_command, "dump"))
+		return do_efi_boot_dump(argc, argv);
+	else if (!strcmp(sub_command, "order"))
+		return do_efi_boot_order(argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+
+/* Interpreter command to configure EFI environment */
+static int do_efishell(cmd_tbl_t *cmdtp, int flag,
+		       int argc, char * const argv[])
+{
+	char *command;
+	efi_status_t r;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--; argv++;
+	command = argv[0];
+
+	/* Initialize EFI drivers */
+	r = efi_init_obj_list();
+	if (r != EFI_SUCCESS) {
+		printf("Error: Cannot set up EFI drivers, r = %lu\n",
+		       r & ~EFI_ERROR_MASK);
+		return CMD_RET_FAILURE;
+	}
+
+	if (!strcmp(command, "boot"))
+		return do_efi_boot_opt(argc, argv);
+	else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
+		return do_efi_dump_var(argc, argv);
+	else if (!strcmp(command, "setvar"))
+		return do_efi_set_var(argc, argv);
+	else
+		return CMD_RET_USAGE;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char efishell_help_text[] =
+	"  - EFI Shell-like interface to configure EFI environment\n"
+	"\n"
+	"efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
+	"  - set uefi's BOOT variable\n"
+	"efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
+	"  - set/display uefi boot order\n"
+	"efishell boot dump\n"
+	"  - display all uefi's BOOT variables\n"
+	"efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
+	"  - set/display uefi boot order\n"
+	"\n"
+	"efishell dumpvar [<name>]\n"
+	"  - get uefi variable's value\n"
+	"efishell setvar <name> [<value>]\n"
+	"  - set/delete uefi variable's value\n"
+	"    <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
+#endif
+
+U_BOOT_CMD(
+	efishell, 10, 0, do_efishell,
+	"Configure EFI environment",
+	efishell_help_text
+);