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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

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