Re: [PATCH v2 03/12] cmd: bootefi: carve out EFI boot manager interface

2023-11-20 Thread AKASHI Takahiro
On Tue, Nov 21, 2023 at 04:38:12AM +0100, Heinrich Schuchardt wrote:
> On 11/21/23 02:29, AKASHI Takahiro wrote:
> > Carve EFI boot manager related code out of do_bootefi_image() in order
> > to move boot manager specific code into library directory in the later
> > commit.
> > 
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >   cmd/bootefi.c | 43 ---
> >   1 file changed, 24 insertions(+), 19 deletions(-)
> > 
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index e9e5ab67a1f5..87910c42333a 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -413,28 +413,40 @@ out:
> >   }
> > 
> >   /**
> > - * do_efibootmgr() - execute EFI boot manager
> > + * efi_bootmgr_run() - execute EFI boot manager
> > + * fdt:Flat device tree
> > + *
> > + * Invoke EFI boot manager and execute a binary depending on
> > + * boot options. If @fdt is not NULL, it will be passed to
> > + * the executed binary.
> 
> How about the fallback to the control device-tree?

efi_install_fdt() will take care of that if fdt == EFI_FDT_INTERNAL_USE.
I didn't change any semantics.
I will add some description here to clarify it.

> How about booting with ACPI?

Not sure what is your concern, but
I didn't change any semantics in bootmgr use-case.

-Takahiro Akashi

> 
> Best regards
> 
> Heinrich
> 
> >*
> >* Return:status code
> >*/
> > -static int do_efibootmgr(void)
> > +static efi_status_t efi_bootmgr_run(void *fdt)
> >   {
> > efi_handle_t handle;
> > -   efi_status_t ret;
> > void *load_options;
> > +   efi_status_t ret;
> > 
> > -   ret = efi_bootmgr_load(, _options);
> > +   /* Initialize EFI drivers */
> > +   ret = efi_init_obj_list();
> > if (ret != EFI_SUCCESS) {
> > -   log_notice("EFI boot manager: Cannot load any image\n");
> > +   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > +   ret & ~EFI_ERROR_MASK);
> > return CMD_RET_FAILURE;
> > }
> > 
> > -   ret = do_bootefi_exec(handle, load_options);
> > -
> > +   ret = efi_install_fdt(fdt);
> > if (ret != EFI_SUCCESS)
> > -   return CMD_RET_FAILURE;
> > +   return ret;
> > 
> > -   return CMD_RET_SUCCESS;
> > +   ret = efi_bootmgr_load(, _options);
> > +   if (ret != EFI_SUCCESS) {
> > +   log_notice("EFI boot manager: Cannot load any image\n");
> > +   return ret;
> > +   }
> > +
> > +   return do_bootefi_exec(handle, load_options);
> >   }
> > 
> >   /**
> > @@ -624,21 +636,14 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int 
> > flag, int argc,
> > 
> > if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> > !strcmp(argv[1], "bootmgr")) {
> > -   /* Initialize EFI drivers */
> > -   ret = efi_init_obj_list();
> > -   if (ret != EFI_SUCCESS) {
> > -   log_err("Error: Cannot initialize UEFI sub-system, r = 
> > %lu\n",
> > -   ret & ~EFI_ERROR_MASK);
> > -   return CMD_RET_FAILURE;
> > -   }
> > +   ret = efi_bootmgr_run(fdt);
> > 
> > -   ret = efi_install_fdt(fdt);
> > if (ret == EFI_INVALID_PARAMETER)
> > return CMD_RET_USAGE;
> > -   else if (ret != EFI_SUCCESS)
> > +   else if (ret)
> > return CMD_RET_FAILURE;
> > 
> > -   return do_efibootmgr();
> > +   return CMD_RET_SUCCESS;
> > }
> > 
> > if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> 


Re: [PATCH v2 03/12] cmd: bootefi: carve out EFI boot manager interface

2023-11-20 Thread Heinrich Schuchardt

On 11/21/23 02:29, AKASHI Takahiro wrote:

Carve EFI boot manager related code out of do_bootefi_image() in order
to move boot manager specific code into library directory in the later
commit.

Signed-off-by: AKASHI Takahiro 
---
  cmd/bootefi.c | 43 ---
  1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index e9e5ab67a1f5..87910c42333a 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -413,28 +413,40 @@ out:
  }

  /**
- * do_efibootmgr() - execute EFI boot manager
+ * efi_bootmgr_run() - execute EFI boot manager
+ * fdt:Flat device tree
+ *
+ * Invoke EFI boot manager and execute a binary depending on
+ * boot options. If @fdt is not NULL, it will be passed to
+ * the executed binary.


How about the fallback to the control device-tree?
How about booting with ACPI?

Best regards

Heinrich


   *
   * Return:status code
   */
-static int do_efibootmgr(void)
+static efi_status_t efi_bootmgr_run(void *fdt)
  {
efi_handle_t handle;
-   efi_status_t ret;
void *load_options;
+   efi_status_t ret;

-   ret = efi_bootmgr_load(, _options);
+   /* Initialize EFI drivers */
+   ret = efi_init_obj_list();
if (ret != EFI_SUCCESS) {
-   log_notice("EFI boot manager: Cannot load any image\n");
+   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+   ret & ~EFI_ERROR_MASK);
return CMD_RET_FAILURE;
}

-   ret = do_bootefi_exec(handle, load_options);
-
+   ret = efi_install_fdt(fdt);
if (ret != EFI_SUCCESS)
-   return CMD_RET_FAILURE;
+   return ret;

-   return CMD_RET_SUCCESS;
+   ret = efi_bootmgr_load(, _options);
+   if (ret != EFI_SUCCESS) {
+   log_notice("EFI boot manager: Cannot load any image\n");
+   return ret;
+   }
+
+   return do_bootefi_exec(handle, load_options);
  }

  /**
@@ -624,21 +636,14 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, 
int argc,

if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
!strcmp(argv[1], "bootmgr")) {
-   /* Initialize EFI drivers */
-   ret = efi_init_obj_list();
-   if (ret != EFI_SUCCESS) {
-   log_err("Error: Cannot initialize UEFI sub-system, r = 
%lu\n",
-   ret & ~EFI_ERROR_MASK);
-   return CMD_RET_FAILURE;
-   }
+   ret = efi_bootmgr_run(fdt);

-   ret = efi_install_fdt(fdt);
if (ret == EFI_INVALID_PARAMETER)
return CMD_RET_USAGE;
-   else if (ret != EFI_SUCCESS)
+   else if (ret)
return CMD_RET_FAILURE;

-   return do_efibootmgr();
+   return CMD_RET_SUCCESS;
}

if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&




[PATCH v2 03/12] cmd: bootefi: carve out EFI boot manager interface

2023-11-20 Thread AKASHI Takahiro
Carve EFI boot manager related code out of do_bootefi_image() in order
to move boot manager specific code into library directory in the later
commit.

Signed-off-by: AKASHI Takahiro 
---
 cmd/bootefi.c | 43 ---
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index e9e5ab67a1f5..87910c42333a 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -413,28 +413,40 @@ out:
 }
 
 /**
- * do_efibootmgr() - execute EFI boot manager
+ * efi_bootmgr_run() - execute EFI boot manager
+ * fdt:Flat device tree
+ *
+ * Invoke EFI boot manager and execute a binary depending on
+ * boot options. If @fdt is not NULL, it will be passed to
+ * the executed binary.
  *
  * Return: status code
  */
-static int do_efibootmgr(void)
+static efi_status_t efi_bootmgr_run(void *fdt)
 {
efi_handle_t handle;
-   efi_status_t ret;
void *load_options;
+   efi_status_t ret;
 
-   ret = efi_bootmgr_load(, _options);
+   /* Initialize EFI drivers */
+   ret = efi_init_obj_list();
if (ret != EFI_SUCCESS) {
-   log_notice("EFI boot manager: Cannot load any image\n");
+   log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+   ret & ~EFI_ERROR_MASK);
return CMD_RET_FAILURE;
}
 
-   ret = do_bootefi_exec(handle, load_options);
-
+   ret = efi_install_fdt(fdt);
if (ret != EFI_SUCCESS)
-   return CMD_RET_FAILURE;
+   return ret;
 
-   return CMD_RET_SUCCESS;
+   ret = efi_bootmgr_load(, _options);
+   if (ret != EFI_SUCCESS) {
+   log_notice("EFI boot manager: Cannot load any image\n");
+   return ret;
+   }
+
+   return do_bootefi_exec(handle, load_options);
 }
 
 /**
@@ -624,21 +636,14 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, 
int argc,
 
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
!strcmp(argv[1], "bootmgr")) {
-   /* Initialize EFI drivers */
-   ret = efi_init_obj_list();
-   if (ret != EFI_SUCCESS) {
-   log_err("Error: Cannot initialize UEFI sub-system, r = 
%lu\n",
-   ret & ~EFI_ERROR_MASK);
-   return CMD_RET_FAILURE;
-   }
+   ret = efi_bootmgr_run(fdt);
 
-   ret = efi_install_fdt(fdt);
if (ret == EFI_INVALID_PARAMETER)
return CMD_RET_USAGE;
-   else if (ret != EFI_SUCCESS)
+   else if (ret)
return CMD_RET_FAILURE;
 
-   return do_efibootmgr();
+   return CMD_RET_SUCCESS;
}
 
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
-- 
2.34.1