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

[Sughosh Ganu]

hi Heinrich,

On Wed, 1 Jun 2022 at 11:33, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> 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.

Okay. I get your point. Will make the change to not return
EFI_NOT_FOUND when the OsIndications variable is not set with
CONFIG_EFI_IGNORE_OSINDICATIONS=y. The subsequent code will clear the
EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED bit if the variable
is found irrespective of the value of the above config.

>
> 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?

I believe that can be done by enabling CONFIG_EFI_CAPSULE_ON_DISK_EARLY.

-sughosh

>
> 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;
> >       }