On Thu, 25 May 2023 at 19:42, Raymond Mao <raymond....@linaro.org> wrote: > > > > On Thu, 25 May 2023 at 09:55, Ilias Apalodimas <ilias.apalodi...@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....@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 >> >