Re: [PATCH v6 2/3] Boot var automatic management for removable medias

2023-05-29 Thread Raymond Mao
Hi Heinrich,

On Sat, 27 May 2023 at 17:54, Heinrich Schuchardt  wrote:
>
> On 5/26/23 23:36, 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 
> > ---
[]
> > 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)
>
> efi_bootmgr_update_media_device_boot_option() returns EFI_SUCCESS if
> calloc fails(). It fails with EFI_NOT_FOUND if there is no handle with a
> simple file protocol.
>
> Why would we return 0 here if we are out of memory?
> It would be preferable to let
> efi_bootmgr_update_media_device_boot_option() return EFI_SUCCESS if
> there is no partition with a file system.
>
It is an original bug of the function
'eficonfig_generate_media_device_boot_option()'.
I can add the fix to return EFI_OUT_OF_RESOURCES with this patch.
Also, I can add below lines at the end of the function to return EFI_SUCCESS for
EFI_NOT_FOUND if you agree.
---
if (ret == EFI_NOT_FOUND)
return  EFI_SUCCESS;
return ret;
---

Thanks and regards.
Raymond


Re: [PATCH v6 2/3] Boot var automatic management for removable medias

2023-05-27 Thread Heinrich Schuchardt

On 5/26/23 23:36, 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 
---
Changes in v3
- Split the patch into moving and renaming functions and
   individual patches for each changed functionality
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
   from efi_init_variables() to efi_init_obj_list()
Changes in v6
- Revert unrelated changes

  lib/efi_loader/efi_disk.c  | 7 +++
  lib/efi_loader/efi_setup.c | 5 +
  2 files changed, 12 insertions(+)

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)


efi_bootmgr_update_media_device_boot_option() returns EFI_SUCCESS if
calloc fails(). It fails with EFI_NOT_FOUND if there is no handle with a
simple file protocol.

Why would we return 0 here if we are out of memory?
It would be preferable to let
efi_bootmgr_update_media_device_boot_option() return EFI_SUCCESS if
there is no partition with a file system.


+   return -1;
+   }
+
return 0;
  }

diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 58d4e13402..4686374995 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -245,6 +245,11 @@ efi_status_t efi_init_obj_list(void)
if (ret != EFI_SUCCESS)
goto out;

+   /* update boot option after variable service initialized */
+   ret = efi_bootmgr_update_media_device_boot_option();
+   if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
+   goto out;


Why is a failing calloc() in
efi_bootmgr_update_media_device_boot_option() ok here?

There is a patch missing to fix
efi_bootmgr_update_media_device_boot_option().

Best regards

Heinrich


+
/* Define supported languages */
ret = efi_init_platform_lang();
if (ret != EFI_SUCCESS)




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

2023-05-26 Thread Raymond Mao
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 
---
Changes in v3
- Split the patch into moving and renaming functions and
  individual patches for each changed functionality
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
  from efi_init_variables() to efi_init_obj_list()
Changes in v6
- Revert unrelated changes

 lib/efi_loader/efi_disk.c  | 7 +++
 lib/efi_loader/efi_setup.c | 5 +
 2 files changed, 12 insertions(+)

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_setup.c b/lib/efi_loader/efi_setup.c
index 58d4e13402..4686374995 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -245,6 +245,11 @@ efi_status_t efi_init_obj_list(void)
if (ret != EFI_SUCCESS)
goto out;
 
+   /* update boot option after variable service initialized */
+   ret = efi_bootmgr_update_media_device_boot_option();
+   if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND)
+   goto out;
+
/* Define supported languages */
ret = efi_init_platform_lang();
if (ret != EFI_SUCCESS)
-- 
2.25.1