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

[Heinrich Schuchardt]

On 3/8/19 2:29 AM, AKASHI Takahiro wrote:
> See UEFI v2.7, section 3.1.2 for details of the specification.
> 
> With efidebug command, you can run any EFI boot option as follows:
>   => efi boot add 1 SHELL ...
>   => efi boot add 2 HELLO ...
>   => efi boot order 1 2
>   => efi bootmgr
>      (starting SHELL ...)
> 
>   => efi boot next 2
>   => efi bootmgr
>      (starting HELLO ...)
>   => env print -e
>   <snip ...>
>   BootCurrent: {boot,run}(blob)
>   00000000:  02 00                    ..
>   BootOrder: {boot,run}(blob)
>   00000000:  01 00 02 00              ....
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  lib/efi_loader/efi_bootmgr.c | 54 +++++++++++++++++++++++++++++++++---
>  1 file changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 417016102b48..64fc4449446b 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -141,6 +141,7 @@ 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",
> @@ -151,6 +152,16 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
>  		if (ret != EFI_SUCCESS)
>  			goto error;
>  
> +		attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +			     EFI_VARIABLE_RUNTIME_ACCESS;
> +		size = sizeof(n);
> +		ret = EFI_CALL(efi_set_variable(
> +				L"BootCurrent",
> +				(efi_guid_t *)&efi_global_variable_guid,
> +				attributes, size, &n));
> +		if (ret != EFI_SUCCESS)
> +			goto error;
> +
>  		printf("Booting: %ls\n", lo.label);
>  		efi_dp_split_file_path(lo.file_path, device_path, file_path);
>  	}
> @@ -162,21 +173,56 @@ error:
>  }
>  
>  /*
> - * Attempt to load, in the order specified by BootOrder EFI variable, the
> - * available load-options, finding and returning the first one that can
> - * be loaded successfully.
> + * Attempt to load from BootNext or in the order specified by BootOrder
> + * EFI variable, the available load-options, finding and returning
> + * the first one that can be loaded successfully.
>   */
>  void *efi_bootmgr_load(struct efi_device_path **device_path,
>  		       struct efi_device_path **file_path)
>  {
> -	uint16_t *bootorder;
> +	u16 bootnext, *bootorder;
>  	efi_uintn_t size;
>  	void *image = NULL;
>  	int i, num;
> +	efi_status_t ret;
>  
>  	bs = systab.boottime;
>  	rs = systab.runtime;
>  
> +	/* BootNext */
> +	bootnext = 0;
> +	size = sizeof(bootnext);
> +	ret = EFI_CALL(efi_get_variable(L"BootNext",
> +					(efi_guid_t *)&efi_global_variable_guid,
> +					NULL, &size, &bootnext));
> +	if (ret == EFI_SUCCESS) {

if (ret == EFI_SUCCESS && size == sizeof(u16))

If we do not check the size the user will never be informed that his
system is corrupted.

> +		/* delete BootNext */
> +		ret = EFI_CALL(efi_set_variable(
> +					L"BootNext",
> +					(efi_guid_t *)&efi_global_variable_guid,
> +					0, 0, &bootnext));
> +		if (ret == EFI_SUCCESS) {
> +			image = try_load_entry(bootnext,
> +					       device_path, file_path);
> +			if (image)
> +				return image;
> +		} else {
> +			printf("BootNext note deleted\n");

%s/note/not/

"Deleting BootNext failed\n" would make it clearer that this is an error.


> +		}
> +	} else if (ret != EFI_NOT_FOUND) {
> +		if (ret == EFI_BUFFER_TOO_SMALL)
> +			printf("BootNext must be 16-bit integer\n");
> +
> +		/* delete BootNext */
> +		ret = EFI_CALL(efi_set_variable(
> +					L"BootNext",
> +					(efi_guid_t *)&efi_global_variable_guid,
> +					0, 0, &bootnext));
> +		if (ret != EFI_SUCCESS)
> +			printf("BootNext note deleted\n");

%s/note/not/

Or maybe "Deleting BootNext failed\n"

The same logic can be implemented without duplicate code but with size
check:

	/* BootNext */
	size = sizeof(bootnext);
	ret = EFI_CALL(efi_get_variable(L"BootNext",
					(efi_guid_t *)&efi_global_variable_guid,
					NULL, &size, &bootnext));
	if (ret != EFI_NOT_FOUND) {
		if (ret == EFI_BUFFER_TOO_SMALL || size != sizeof(u16))
			printf("BootNext must be 16-bit integer\n");
		/* delete BootNext */
		ret = EFI_CALL(efi_set_variable(L"BootNext",
						(efi_guid_t *)&efi_global_variable_guid,
						0, 0, &bootnext));
		if (ret != EFI_SUCCESS)
			printf("Deleting BootNext failed\n");
	}
	if (ret == EFI_SUCCESS && size == sizeof(u16)) {
		image = try_load_entry(bootnext,
				       device_path, file_path);
		if (image)
			return image;
	}

Best regards

Heinrich

> +	}
> +
> +	/* BootOrder */
>  	bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
>  	if (!bootorder) {
>  		printf("BootOrder not defined\n");
>