Am 20. Januar 2023 13:31:19 MEZ schrieb Ilias Apalodimas 
<ilias.apalodi...@linaro.org>:
>Up to now the EFI subsystem was left out of the main U-Boot init
>process.  This has led to various hacks over the years, with the most
>notable one being sprinkling around the efi init call to various places
>such as U-Boot commands, the early boot code etc.
>
>Since EFI has it's own Kconfig option and people can remove it, let's
>wire up the EFI init call on an event for EVT_MAIN_LOOP.
>
>This will also get rid of ad-hoc code in the main event loop, which was
>trying to initialize the subsystem early and perform capsule updates.
>
>TODO:
>- The efi_tcg protocol implicitly initializes the TPM, as a result
>  some of the tpm selftests will fail with the RFC.  If everyone
>  agrees that this is a good idea, I'll clean up the TPM hacks as well
>- We  still need to run capsule updates on the main_loop() code since
>  in some cases (e.g sandbox) we need preboot commands.
>- wider tests, I've only run QEMU for now
>
>Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>


In case of efi_init_obj_list() failing we should still reach the U-Boot console 
but each of the EFI commands should abort early.

Please, put the Kconfig related capsule change into a separate patch.

Otherwise looks good to me.

Best regards

Heinrich 



>---
> boot/bootm_os.c            |  8 --------
> cmd/bootefi.c              |  8 --------
> cmd/bootmenu.c             |  9 ---------
> cmd/eficonfig.c            |  8 --------
> cmd/efidebug.c             |  9 ---------
> cmd/nvedit_efi.c           | 17 -----------------
> common/main.c              |  8 +++-----
> include/efi_loader.h       |  2 --
> lib/efi_loader/Kconfig     | 10 ----------
> lib/efi_loader/efi_setup.c | 25 ++++++++++++++++++-------
> lib/fwu_updates/Kconfig    |  1 -
> lib/fwu_updates/fwu.c      |  3 ---
> 12 files changed, 21 insertions(+), 87 deletions(-)
>
>diff --git a/boot/bootm_os.c b/boot/bootm_os.c
>index 99ff0e6c02d2..4775cecb4e64 100644
>--- a/boot/bootm_os.c
>+++ b/boot/bootm_os.c
>@@ -505,14 +505,6 @@ static int do_bootm_efi(int flag, int argc, char *const 
>argv[],
>       if (ret)
>               return ret;
>
>-      /* Initialize EFI drivers */
>-      efi_ret = efi_init_obj_list();
>-      if (efi_ret != EFI_SUCCESS) {
>-              printf("## Failed to initialize UEFI sub-system: r = %lu\n",
>-                     efi_ret & ~EFI_ERROR_MASK);
>-              return 1;
>-      }
>-
>       /* Install device tree */
>       efi_ret = efi_install_fdt(images->ft_len
>                                 ? images->ft_addr : EFI_FDT_USE_INTERNAL);
>diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>index 2a7d42925d65..5d56448ee6ce 100644
>--- a/cmd/bootefi.c
>+++ b/cmd/bootefi.c
>@@ -668,14 +668,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, 
>int argc,
>       if (argc < 2)
>               return CMD_RET_USAGE;
>
>-      /* 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;
>-      }
>-
>       if (argc > 2) {
>               uintptr_t fdt_addr;
>
>diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
>index 3236ca5d7994..1c604c79e01d 100644
>--- a/cmd/bootmenu.c
>+++ b/cmd/bootmenu.c
>@@ -449,15 +449,6 @@ static void handle_uefi_bootnext(void)
>       efi_status_t ret;
>       efi_uintn_t size;
>
>-      /* 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;
>-      }
>-
>       /* If UEFI BootNext variable is set, boot the BootNext load option */
>       size = sizeof(u16);
>       ret = efi_get_variable_int(u"BootNext",
>diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
>index d830e4af53b8..16025d451901 100644
>--- a/cmd/eficonfig.c
>+++ b/cmd/eficonfig.c
>@@ -2545,14 +2545,6 @@ static int do_eficonfig(struct cmd_tbl *cmdtp, int 
>flag, int argc, char *const a
>       if (argc > 1)
>               return CMD_RET_USAGE;
>
>-      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 = eficonfig_init();
>       if (ret != EFI_SUCCESS)
>               return CMD_RET_FAILURE;
>diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>index e6959ede930f..9ad253da91b2 100644
>--- a/cmd/efidebug.c
>+++ b/cmd/efidebug.c
>@@ -1464,21 +1464,12 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag,
>                      int argc, char *const argv[])
> {
>       struct cmd_tbl *cp;
>-      efi_status_t r;
>
>       if (argc < 2)
>               return CMD_RET_USAGE;
>
>       argc--; argv++;
>
>-      /* Initialize UEFI drivers */
>-      r = efi_init_obj_list();
>-      if (r != EFI_SUCCESS) {
>-              printf("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>-                     r & ~EFI_ERROR_MASK);
>-              return CMD_RET_FAILURE;
>-      }
>-
>       cp = find_cmd_tbl(argv[0], cmd_efidebug_sub,
>                         ARRAY_SIZE(cmd_efidebug_sub));
>       if (!cp)
>diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c
>index 24944ab81e23..1ac1114d4c89 100644
>--- a/cmd/nvedit_efi.c
>+++ b/cmd/nvedit_efi.c
>@@ -210,15 +210,6 @@ int do_env_print_efi(struct cmd_tbl *cmdtp, int flag, int 
>argc,
>       const efi_guid_t *guid_p = NULL;
>       efi_guid_t guid;
>       bool verbose = true;
>-      efi_status_t ret;
>-
>-      /* Initialize EFI drivers */
>-      ret = efi_init_obj_list();
>-      if (ret != EFI_SUCCESS) {
>-              printf("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>-                     ret & ~EFI_ERROR_MASK);
>-              return CMD_RET_FAILURE;
>-      }
>
>       for (argc--, argv++; argc > 0 && argv[0][0] == '-'; argc--, argv++) {
>               if (!strcmp(argv[0], "-guid")) {
>@@ -388,14 +379,6 @@ int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int 
>argc,
>       if (argc == 1)
>               return CMD_RET_USAGE;
>
>-      /* Initialize EFI drivers */
>-      ret = efi_init_obj_list();
>-      if (ret != EFI_SUCCESS) {
>-              printf("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>-                     ret & ~EFI_ERROR_MASK);
>-              return CMD_RET_FAILURE;
>-      }
>-
>       /*
>        * attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
>        *           EFI_VARIABLE_RUNTIME_ACCESS;
>diff --git a/common/main.c b/common/main.c
>index 682f3359ea38..edb5f4c20682 100644
>--- a/common/main.c
>+++ b/common/main.c
>@@ -54,11 +54,9 @@ void main_loop(void)
>       if (IS_ENABLED(CONFIG_UPDATE_TFTP))
>               update_tftp(0UL, NULL, NULL);
>
>-      if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY)) {
>-              /* efi_init_early() already called */
>-              if (efi_init_obj_list() == EFI_SUCCESS)
>-                      efi_launch_capsules();
>-      }
>+      /* EFI subsystem will be initialized from an event */
>+      if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK))
>+              efi_launch_capsules();
>
>       s = bootdelay_process();
>       if (cli_process_fdt(&s))
>diff --git a/include/efi_loader.h b/include/efi_loader.h
>index f9e427f09059..3a801cbbc4c1 100644
>--- a/include/efi_loader.h
>+++ b/include/efi_loader.h
>@@ -513,8 +513,6 @@ extern struct list_head efi_register_notify_events;
>
> /* called at pre-initialization */
> int efi_init_early(void);
>-/* Initialize efi execution environment */
>-efi_status_t efi_init_obj_list(void);
> /* Set up console modes */
> void efi_setup_console_size(void);
> /* Install device tree */
>diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>index b630d88ef9e2..f3aaa51bb36a 100644
>--- a/lib/efi_loader/Kconfig
>+++ b/lib/efi_loader/Kconfig
>@@ -153,16 +153,6 @@ config EFI_IGNORE_OSINDICATIONS
>         without setting the EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
>         flag in variable OsIndications.
>
>-config EFI_CAPSULE_ON_DISK_EARLY
>-      bool "Initiate capsule-on-disk at U-Boot boottime"
>-      depends on EFI_CAPSULE_ON_DISK
>-      help
>-        Normally, without this option enabled, capsules will be
>-        executed only at the first time of invoking one of efi command.
>-        If this option is enabled, capsules will be enforced to be
>-        executed as part of U-Boot initialisation so that they will
>-        surely take place whatever is set to distro_bootcmd.
>-
> config EFI_CAPSULE_FIRMWARE
>       bool
>
>diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>index f0f01d3b1d5a..e2317aed1ec2 100644
>--- a/lib/efi_loader/efi_setup.c
>+++ b/lib/efi_loader/efi_setup.c
>@@ -215,7 +215,7 @@ out:
>  *
>  * Return:    status code
>  */
>-efi_status_t efi_init_obj_list(void)
>+static efi_status_t efi_init_obj_list(void)
> {
>       efi_status_t ret = EFI_SUCCESS;
>
>@@ -335,14 +335,25 @@ efi_status_t efi_init_obj_list(void)
>
>       /* Initialize EFI runtime services */
>       ret = efi_reset_system_init();
>-      if (ret != EFI_SUCCESS)
>-              goto out;
>
>-      /* Execute capsules after reboot */
>-      if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) &&
>-          !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
>-              ret = efi_launch_capsules();
> out:
>       efi_obj_list_initialized = ret;
>       return ret;
> }
>+
>+static int efi_evt_int(void *ctx, struct event *event)
>+{
>+      efi_status_t ret;
>+      int r = 0;
>+
>+      ret = efi_init_obj_list();
>+      if (ret != EFI_SUCCESS) {
>+              log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
>+                      ret & ~EFI_ERROR_MASK);
>+              r = -1;
>+      }
>+
>+      return r;
>+}
>+
>+EVENT_SPY(EVT_MAIN_LOOP, efi_evt_int);
>diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig
>index 78759e6618df..6ff577cb53b3 100644
>--- a/lib/fwu_updates/Kconfig
>+++ b/lib/fwu_updates/Kconfig
>@@ -3,7 +3,6 @@ config FWU_MULTI_BANK_UPDATE
>       depends on EFI_CAPSULE_ON_DISK
>       select PARTITION_TYPE_GUID
>       select EFI_SETUP_EARLY
>-      imply EFI_CAPSULE_ON_DISK_EARLY
>       select EVENT
>       help
>         Feature for updating firmware images on platforms having
>diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
>index 5313d0730202..e4256bea7a2c 100644
>--- a/lib/fwu_updates/fwu.c
>+++ b/lib/fwu_updates/fwu.c
>@@ -700,9 +700,6 @@ static int fwu_boottime_checks(void *ctx, struct event 
>*event)
>               return 0;
>       }
>
>-      if (efi_init_obj_list() != EFI_SUCCESS)
>-              return 0;
>-
>       ret = fwu_get_dev_mdata(&dev, &mdata);
>       if (ret)
>               return ret;
>--
>2.38.1
>

Reply via email to