[U-Boot] [PATCH v5 1/7] cmd: add efidebug command
AKASHI Takahiro
takahiro.akashi at linaro.org
Thu Jan 24 01:02:48 UTC 2019
On Wed, Jan 23, 2019 at 10:52:38AM +0100, Alexander Graf wrote:
> On 01/23/2019 05:47 AM, AKASHI Takahiro wrote:
> >On Tue, Jan 22, 2019 at 10:18:58AM +0100, Alexander Graf wrote:
> >>
> >>On 22.01.19 02:02, AKASHI Takahiro wrote:
> >>>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?
> >>I actually think I would prefer that. This way we consolidate everything
> >>through a single API which means we then know that the results are
> >>always the same. If we implement parsing multiple times, there's a good
> >>chance we'll have a bug in one of them which gives us different results
> >>which then again makes debugging harder rather than easier.
> >Agree, but please note we have to add an apparently weird dependency of
> >CONFIG_HEXDUMP to EFI_LOADER as do_efi_dump_var_ext() needs to be compiled in
> >whether CMD_EFIDEBUG is enabled or not.
>
> You can just not print hex dumps of variables if CONFIG_HEXDUMP is not set?
Yes, print_hex_dump() just prints nothing if !CONFIG_HEXDUMP.
Adding "imply HEXDUMP" would be enough.
> >
> >>>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.
> >>One more reason to use only a single path to funnel things through :).
> >>
> >>So yes, can you maybe reuse RTS->get_variable() here? Same question on
> >>writing variables I suppose.
> >Well, we need GetNextVariableName API if we want to use GetVariable
> >(for listing). I have held off merging my another patch for this.
> >Is it time to do it?
>
> I thought that was in the tree now?
Not GetNextVariableName patch itself, but a patch of modifying do_efi_dump_var()
using GetNextVariableName.
-Takahiro Akashi
>
> Alex
>
More information about the U-Boot
mailing list