[U-Boot] [PATCH v3 1/8] cmd: add efishell command

AKASHI Takahiro
2019-01-07T05:13:35Z

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