[U-Boot] [PATCH v5 1/7] cmd: add efidebug command

[AKASHI Takahiro]

Alex,

On Mon, Jan 21, 2019 at 02:07:31PM +0100, Alexander Graf wrote:
> On 01/21/2019 08:49 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, efidebug, helps address these issues and give us
> >more friendly interfaces:
> >  * efidebug boot add: add BootXXXX variable
> >  * efidebug boot rm: remove BootXXXX variable
> >  * efidebug boot dump: display all BootXXXX variables
> >  * efidebug boot next: set BootNext variable
> >  * efidebug boot order: set/display a boot order (BootOrder)
> >  * efidebug setvar: set an UEFI variable (with limited functionality)
> >  * efidebug dumpvar: display all UEFI variables
> >
> >Please note that the file, efidebug.c, will be compiled under
> >CONFIG_EFI_LOADER because some helper functions will be used
> >to enable "env -e" command in a later patch whether or not
> >the command is compiled in.
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> >---
> >  MAINTAINERS       |   1 +
> >  cmd/Kconfig       |  10 +
> >  cmd/Makefile      |   1 +
> >  cmd/efidebug.c    | 755 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/command.h |   6 +
> >  5 files changed, 773 insertions(+)
> >  create mode 100644 cmd/efidebug.c
> >
> >diff --git a/MAINTAINERS b/MAINTAINERS
> >index ae825014bda9..301c5c69ea25 100644
> >--- a/MAINTAINERS
> >+++ b/MAINTAINERS
> >@@ -438,6 +438,7 @@ F:	lib/efi*/
> >  F:	test/py/tests/test_efi*
> >  F:	test/unicode_ut.c
> >  F:	cmd/bootefi.c
> >+F:	cmd/efidebug.c
> >  F:	tools/file2include.c
> >  FPGA
> >diff --git a/cmd/Kconfig b/cmd/Kconfig
> >index ea1a325eb301..d9cab3cc0c49 100644
> >--- a/cmd/Kconfig
> >+++ b/cmd/Kconfig
> >@@ -1397,6 +1397,16 @@ config CMD_DISPLAY
> >  	  displayed on a simple board-specific display. Implement
> >  	  display_putc() to use it.
> >+config CMD_EFIDEBUG
> >+	bool "efidebug - display/customize UEFI environment"
> >+	depends on EFI_LOADER
> >+	default n
> >+	help
> >+	  Enable the 'efidebug' command which provides a subset of UEFI
> >+	  shell utility with simplified functionality. It will be useful
> >+	  particularly for managing boot parameters as  well as examining
> >+	  various EFI status for debugging.
> >+
> >  config CMD_LED
> >  	bool "led"
> >  	default y if LED
> >diff --git a/cmd/Makefile b/cmd/Makefile
> >index 15ae4d250f50..e48d34c394ee 100644
> >--- a/cmd/Makefile
> >+++ b/cmd/Makefile
> >@@ -51,6 +51,7 @@ obj-$(CONFIG_CMD_ECHO) += echo.o
> >  obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
> >  obj-$(CONFIG_CMD_EEPROM) += eeprom.o
> >  obj-$(CONFIG_EFI_STUB) += efi.o
> >+obj-$(CONFIG_EFI_LOADER) += efidebug.o
> >  obj-$(CONFIG_CMD_ELF) += elf.o
> >  obj-$(CONFIG_HUSH_PARSER) += exit.o
> >  obj-$(CONFIG_CMD_EXT4) += ext4.o
> >diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> >new file mode 100644
> >index 000000000000..c54fb6cfa101
> >--- /dev/null
> >+++ b/cmd/efidebug.c
> >@@ -0,0 +1,755 @@
> >+// SPDX-License-Identifier: GPL-2.0+
> >+/*
> >+ *  UEFI Shell-like command
> >+ *
> >+ *  Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
> >+ */
> >+
> >+#include <charset.h>
> >+#include <common.h>
> >+#include <command.h>
> >+#include <efi_loader.h>
> >+#include <environment.h>
> >+#include <errno.h>
> >+#include <exports.h>
> >+#include <hexdump.h>
> >+#include <malloc.h>
> >+#include <search.h>
> >+#include <linux/ctype.h>
> >+#include <asm/global_data.h>
> >+
> >+static void dump_var_data(char *data, unsigned long len)
> >+{
> >+	char *start, *end, *p;
> >+	unsigned long pos, count;
> >+	char hex[3], line[9];
> >+	int i;
> >+
> >+	end = data + len;
> >+	for (start = data, pos = 0; start < end; start += count, pos += count) {
> >+		count = end - start;
> >+		if (count > 16)
> >+			count = 16;
> >+
> >+		/* count should be multiple of two */
> >+		printf("%08lx: ", pos);
> >+
> >+		/* in hex format */
> >+		p = start;
> >+		for (i = 0; i < count / 2; p += 2, i++)
> >+			printf(" %c%c", *p, *(p + 1));
> >+		for (; i < 8; i++)
> >+			printf("   ");
> >+
> >+		/* in character format */
> >+		p = start;
> >+		hex[2] = '\0';
> >+		for (i = 0; i < count / 2; i++) {
> >+			hex[0] = *p++;
> >+			hex[1] = *p++;
> >+			line[i] = (char)simple_strtoul(hex, 0, 16);
> >+			if (line[i] < 0x20 || line[i] > 0x7f)
> >+				line[i] = '.';
> >+		}
> >+		line[i] = '\0';
> >+		printf("  %s\n", line);
> >+	}
> >+}
> 
> Is this print_hex_dump() reimplemented?

Actually, no.
A UEFI variable on u-boot is encoded as ascii representation of binary data.
That means, for example,

        => env set -e PlatformLang en
        => env print -e
        PlatformLang: {boot,run}(blob)
        00000000:  65 6e                    en
        => env print
        ...
        efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLang={boot,run}(blob)656e
        ...

the value of "PlatformLang" as a u-boot variable here is "656e", not "en."
So if we want to use print_hex_dump(), we first have to convert that
string to a binary. But then print_hex_dump() converts the binary to
a string. It's just rediculuous, isn't it?

You might think that the value in this case should be
        {boot, run}(utf8)en
                    ^^^^
It's possible, but it depends on a variable and
currently my do_set_efi_var() doesn't support it anyway.

Thanks,
-Takahiro Akashi

> 
> Alex
>