[U-Boot] [PATCH 1/5] efi_loader: bootmgr: support BootNext and BootCurrent variable behavior

[Heinrich Schuchardt]

On 12/18/18 6:02 AM, AKASHI Takahiro wrote:
> See UEFI v2.7, section 3.1.2 for details of the specification.
> 
> With my efishell command[1], you can try as the following:
>   => efi boot add 1 SHELL ...
>   => efi boot add 2 HELLO ...
>   => efi boot order 1 2
>   => efi bootmgr
>      (starting SHELL ...)
>   => efi setvar BootNext =H0200

If you already have an "efi boot" sub-command wouldn't the following
syntax make more sense?

    efi boot next 2



Best regards

Heinrich

>   => efi bootmgr
>      (starting HELLO ...)
>   => efi dumpvar
>   <snip ...>
>   BootCurrent: {boot,run}(blob)
>   00000000:  02 00                    ..
>   BootOrder: {boot,run}(blob)
>   00000000:  01 00 02 00              ....
> 
> Using "run -e" would be more human-friendly, though.
> 
> [1] https://lists.denx.de/pipermail/u-boot/2018-November/346450.html
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  lib/efi_loader/efi_bootmgr.c | 37 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index a095df3f540b..a54ae28343ce 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -145,11 +145,21 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
>  	efi_deserialize_load_option(&lo, load_option);
>  
>  	if (lo.attributes & LOAD_OPTION_ACTIVE) {
> +		u32 attributes;
>  		efi_status_t ret;
>  
>  		debug("%s: trying to load \"%ls\" from %pD\n",
>  		      __func__, lo.label, lo.file_path);
>  
> +		attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +			     EFI_VARIABLE_RUNTIME_ACCESS;
> +		size = sizeof(n);
> +		ret = rs->set_variable(L"BootCurrent",
> +				       (efi_guid_t *)&efi_global_variable_guid,
> +				       attributes, size, &n);
> +		if (ret != EFI_SUCCESS)
> +			goto error;
> +
>  		ret = efi_load_image_from_path(lo.file_path, &image);
>  
>  		if (ret != EFI_SUCCESS)
> @@ -173,16 +183,41 @@ error:
>  void *efi_bootmgr_load(struct efi_device_path **device_path,
>  		       struct efi_device_path **file_path)
>  {
> -	uint16_t *bootorder;
> +	u16 bootnext, *bootorder;
> +	u32 attributes;
>  	efi_uintn_t size;
>  	void *image = NULL;
>  	int i, num;
> +	efi_status_t ret;
>  
>  	__efi_entry_check();
>  
>  	bs = systab.boottime;
>  	rs = systab.runtime;
>  
> +	/* BootNext */
> +	size = sizeof(bootnext);
> +	ret = rs->get_variable(L"BootNext",
> +			       (efi_guid_t *)&efi_global_variable_guid,
> +			       NULL, &size, &bootnext);
> +	if (!bootnext)
> +		goto run_list;
> +
> +	attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +		     EFI_VARIABLE_RUNTIME_ACCESS;

According to the UEFI spec you do not need any attributes when deleting
a variable.

Please, add a comment describing what you are doing here:

/* Delete BootNext variable */

Best regards

Heinrich

> +	size = 0;
> +	ret = rs->set_variable(L"BootNext",
> +			       (efi_guid_t *)&efi_global_variable_guid,
> +			       attributes, size, &bootnext);
> +	if (ret != EFI_SUCCESS)
> +		goto error;
> +
> +	image = try_load_entry(bootnext, device_path, file_path);
> +	if (image)
> +		goto error;
> +
> +run_list:
> +	/* BootOrder */
>  	bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
>  	if (!bootorder)
>  		goto error;
>