[PATCH 2/3] EFI: Do not read OsIndications variable if EFI_IGNORE_OSINDICATIONS is enabled

[Heinrich Schuchardt]

On 5/31/22 09:15, Sughosh Ganu wrote:
> The EFI_IGNORE_OSINDICATIONS config symbol was introduced as a
> mechanism to have capsule updates work even on platforms where the
> SetVariable runtime service was not supported. The current logic
> requires the OsIndications variable to have been set to a 64 bit value
> even when the EFI_IGNORE_OSINDICATIONS config is enabled. Move the
> check to see if the config symbol is enabled at the beginning of the
> function. If the config is enabled, return a success code without
> checking for the existence of the variable.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>   lib/efi_loader/efi_capsule.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index c76a5f3570..0ea21dc168 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -1062,6 +1062,9 @@ static efi_status_t check_run_capsules(void)
>   	efi_uintn_t size;
>   	efi_status_t r;
>
> +	if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
> +		return EFI_SUCCESS;
> +

If CONFIG_EFI_IGNORE_OSINDICATIONS=y it is correct to ignore a missing
OsIndications variable.

But I think we should stay as close as possible to the UEFI specification:

If the variable exists and the flag
EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED is set, we should
remove the flag even if CONFIG_EFI_IGNORE_OSINDICATIONS=y.

Please, have a look at the configuration symbols
EFI_CAPSULE_ON_DISK_EARLY and EFI_SETUP_EARLY.

As block devices now can be added to the UEFI sub-system after
initialization of the same shouldn't we always initialize the sub-system
early. And shouldn't we always consider capsules before distro-boot,
U-Boot menu, or entering the U-Boot shell?

Best regards

Heinrich

>   	size = sizeof(os_indications);
>   	r = efi_get_variable_int(u"OsIndications", &efi_global_variable_guid,
>   				 NULL, &size, &os_indications, NULL);
> @@ -1082,8 +1085,6 @@ static efi_status_t check_run_capsules(void)
>   		if (r != EFI_SUCCESS)
>   			log_err("Setting %ls failed\n", L"OsIndications");
>   		return EFI_SUCCESS;
> -	} else if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS)) {
> -		return EFI_SUCCESS;
>   	} else  {
>   		return EFI_NOT_FOUND;
>   	}