[PATCH v4 2/3] Boot var automatic management for removable medias

[Ilias Apalodimas]

On Thu, 25 May 2023 at 19:42, Raymond Mao <raymond.mao at linaro.org> wrote:
>
>
>
> On Thu, 25 May 2023 at 09:55, Ilias Apalodimas <ilias.apalodimas at linaro.org> wrote:
>>
>> On Tue, May 23, 2023 at 12:18:20PM -0700, Raymond Mao wrote:
>> > Changes for complying to EFI spec §3.5.1.1
>> > 'Removable Media Boot Behavior'.
>> > Boot variables can be automatically generated during a removable
>> > media is probed. At the same time, unused boot variables will be
>> > detected and removed.
>> >
>> > Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
>> > ---
>> > Changes in v2
>> > - Ignore EFI_NOT_FOUND returned from
>> >   efi_bootmgr_update_media_device_boot_option which means no boot
>> >   options scanned.
>> > Changes in v3
>> > - Split the patch into moving and renaming functions and
>> >   individual patches for each changed functionality
>> > Changes in v4
>> > - Revert the change of introducing a bool parameter when updating
>> >   the boot option. Use short-form of device path by default
>> >
>> >  lib/efi_loader/efi_disk.c         |  7 +++++++
>> >  lib/efi_loader/efi_variable.c     | 10 +++++++++-
>> >  lib/efi_loader/efi_variable_tee.c |  5 +++++
>> >  3 files changed, 21 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> > index d2256713a8..ca5f07f2ec 100644
>> > --- a/lib/efi_loader/efi_disk.c
>> > +++ b/lib/efi_loader/efi_disk.c
>> > @@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event)
>> >                       return -1;
>> >       }
>> >
>> > +     /* only do the boot option management when UEFI sub-system is initialized */
>> > +     if (efi_obj_list_initialized == EFI_SUCCESS) {
>> > +             ret = efi_bootmgr_update_media_device_boot_option();
>> > +             if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
>> > +                     return -1;
>> > +     }
>> > +
>> >       return 0;
>> >  }
>> >
>> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
>> > index be95ed44e6..fe71144358 100644
>> > --- a/lib/efi_loader/efi_variable.c
>> > +++ b/lib/efi_loader/efi_variable.c
>> > @@ -476,6 +476,14 @@ efi_status_t efi_init_variables(void)
>> >                       log_err("Invalid EFI variable seed\n");
>> >       }
>> >
>> > +     ret = efi_init_secure_state();
>> > +     if (ret != EFI_SUCCESS)
>> > +             return ret;
>> >
>> > -     return efi_init_secure_state();
>> > +     /* update boot option management after variable service initialized */
>> > +     ret = efi_bootmgr_update_media_device_boot_option();
>> > +     if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
>> > +             return ret;
>> > +
>> > +     return EFI_SUCCESS;
>> >  }
>> > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
>> > index dfef18435d..2995d4a583 100644
>> > --- a/lib/efi_loader/efi_variable_tee.c
>> > +++ b/lib/efi_loader/efi_variable_tee.c
>> > @@ -748,5 +748,10 @@ efi_status_t efi_init_variables(void)
>> >       if (ret != EFI_SUCCESS)
>> >               return ret;
>> >
>> > +     /* update boot option management after variable service initialized */
>> > +     ret = efi_bootmgr_update_media_device_boot_option();
>> > +     if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
>> > +             return ret;
>> > +
>> >       return EFI_SUCCESS;
>>
>> Instead of having to add identical code in the variable init code why dont
>> we add this in efi_setup.c, right under the efi_init_variables() call?
>
> [RM] Because I think the purpose of the function efi_bootmgr_update_media_device_boot_option
> is to update the boot option which belongs to part of variable initialization.
> So it is better to be inside efi_init_variables() other than efi_init_obj_list().

We also have efi_init_platform_lang(), efi_init_os_indications() which
also 'just' initialize variables. So I think having it in the
efi_setup() makes sense.

Thanks
/Ilias
>>
>>
>> Thanks
>> /Ilias
>> >  }
>> > --
>> > 2.25.1
>> >