[U-Boot] [PATCH v5 1/7] cmd: add efidebug command
Alexander Graf
agraf at suse.de
Tue Jan 22 09:18:58 UTC 2019
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.
> 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.
Alex
More information about the U-Boot
mailing list