[RFC PATCH 1/1] efi_loader: get rid of ad-hoc EFI subsystem init
Heinrich Schuchardt
xypron.glpk at gmx.de
Fri Jan 20 13:58:21 CET 2023
Am 20. Januar 2023 13:31:19 MEZ schrieb Ilias Apalodimas <ilias.apalodimas at 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.apalodimas at 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
>
More information about the U-Boot
mailing list