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 >