Re: [PATCH] efi_loader: FMP cleanups

Ilias Apalodimas
2021-06-14T15:14:20Z

Too fast on the trigger.

The efi_load_capsule_drivers() must go into an IS_ENABLED. I'll wait
for any other comments and send a V2

On Mon, 14 Jun 2021 at 18:10, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Right now we allow both of the FMPs (RAW and FIT based) to be installed at
> the same time. Moreover we only install those if a CapsuleUpdate is
> requested. Since we now have an ESRT table, it makes more sense to
> unconditionally install the FMP, so any userspace applications (e.g fwupd)
> can make use of them and trigger an update.
>
> While at it clean up the FMP installation as well. Chapter 23 of the EFI
> spec (rev 2.9) says:
> "A specific updatable hardware firmware store must be represented by
> exactly one FMP instance".
> This is not the case for us, since both of our FMP protocols can be
> installed at the same time and are controlled by a single 'dfu_alt_info'
> env variable.
> So make the config option a choice and allow the user to install one
> of them at any given time.
>
> The overall changes show up in fwupd
>
> pre-patch:
> fwupdmgr get-devices
> No detected devices
>
> post-patch (with FIT FMP installed):
> fwupdmgr get-devices
> WARNING: Required efivarfs filesystem was not found
> See https://github.com/fwupd/fwupd/wiki/PluginFlag:efivar-not-mounted for more information.
> Unknown Product
> │
> └─Unknown Firmware:
> Device ID: 605080e08f71dabb86d0781c28f7d923edabf7d6
> Current version: 0
> Vendor: DMI:U-Boot
> Update Error: Not updatable as efivarfs was not found
> GUIDs: ae13ff2d-9ad4-4e25-9ac8-6d80b3b22147
> 230c8b18-8d9b-53ec-838b-6cfc0383493a ← main-system-firmware
> 1a1da7d4-0a24-51b5-8a1a-1e3274328094 ← UEFI\RES_{AE13FF2D-9AD4-4E25-9AC8-6D80B3B22147}
> Device Flags: • Internal device
> • System requires external power source
> • Needs a reboot after installation
> • Device is usable for the duration of the update
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> configs/sandbox64_defconfig | 1 -
> configs/sandbox_defconfig | 1 -
> configs/xilinx_zynqmp_virt_defconfig | 1 -
> include/efi_loader.h | 1 +
> lib/efi_loader/Kconfig | 48 +++++++++++++++-------------
> lib/efi_loader/efi_capsule.c | 22 ++++---------
> lib/efi_loader/efi_setup.c | 4 +++
> 7 files changed, 37 insertions(+), 41 deletions(-)
>
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index 9a373bab6fe3..af18b6c7826e 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -233,7 +233,6 @@ CONFIG_LZ4=y
> CONFIG_ERRNO_STR=y
> CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> CONFIG_EFI_SECURE_BOOT=y
> CONFIG_TEST_FDTDEC=y
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index bdbf714e2bd9..24313fdfa53d 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -280,7 +280,6 @@ CONFIG_LZ4=y
> CONFIG_ERRNO_STR=y
> CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> CONFIG_EFI_SECURE_BOOT=y
> CONFIG_TEST_FDTDEC=y
> diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig
> index e939b04ef6a5..0c2d1a70a5a1 100644
> --- a/configs/xilinx_zynqmp_virt_defconfig
> +++ b/configs/xilinx_zynqmp_virt_defconfig
> @@ -188,5 +188,4 @@ CONFIG_EFI_SET_TIME=y
> CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> CONFIG_EFI_CAPSULE_ON_DISK_EARLY=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 0a9c82a257e1..b81180cfda8b 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -972,4 +972,5 @@ efi_status_t efi_esrt_register(void);
> * - error code otherwise.
> */
> efi_status_t efi_esrt_populate(void);
> +efi_status_t efi_load_capsule_drivers(void);
> #endif /* _EFI_LOADER_H */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 6242caceb7f9..da6f5faf5adb 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -161,6 +161,31 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT
> Select this option if you want to enable capsule-based
> firmware update using Firmware Management Protocol.
>
> +choice EFI_CAPSULE_TYPE
> + prompt "Capsule type (RAW/FIT)"
> + depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> +
> +config EFI_CAPSULE_FIRMWARE_FIT
> + bool "FMP driver for FIT images"
> + depends on FIT
> + select UPDATE_FIT
> + select DFU
> + select EFI_CAPSULE_FIRMWARE
> + help
> + Select this option if you want to enable firmware management protocol
> + driver for FIT image
> +
> +config EFI_CAPSULE_FIRMWARE_RAW
> + bool "FMP driver for raw images"
> + select DFU_WRITE_ALT
> + select DFU
> + select EFI_CAPSULE_FIRMWARE
> + help
> + Select this option if you want to enable firmware management protocol
> + driver for raw image
> +
> +endchoice
> +
> config EFI_CAPSULE_AUTHENTICATE
> bool "Update Capsule authentication"
> depends on EFI_CAPSULE_FIRMWARE
> @@ -181,29 +206,6 @@ config EFI_CAPSULE_AUTHENTICATE
> Select this option if you want to enable capsule
> authentication
>
> -config EFI_CAPSULE_FIRMWARE_FIT
> - bool "FMP driver for FIT image"
> - depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> - depends on FIT
> - select UPDATE_FIT
> - select DFU
> - select EFI_CAPSULE_FIRMWARE
> - default n
> - help
> - Select this option if you want to enable firmware management protocol
> - driver for FIT image
> -
> -config EFI_CAPSULE_FIRMWARE_RAW
> - bool "FMP driver for raw image"
> - depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> - select DFU
> - select DFU_WRITE_ALT
> - select EFI_CAPSULE_FIRMWARE
> - default n
> - help
> - Select this option if you want to enable firmware management protocol
> - driver for raw image
> -
> config EFI_DEVICE_PATH_TO_TEXT
> bool "Device path to text protocol"
> default y
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 9ead0d2c7816..3b4214a8d4cc 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -918,31 +918,27 @@ static void efi_capsule_scan_done(void)
> }
>
> /**
> - * arch_efi_load_capsule_drivers - initialize capsule drivers
> + * efi_load_capsule_drivers - initialize capsule drivers
> *
> - * Architecture or board specific initialization routine
> + * Generic FMP drivers backed by DFU
> *
> * Return: status code
> */
> -efi_status_t __weak arch_efi_load_capsule_drivers(void)
> +efi_status_t efi_load_capsule_drivers(void)
> {
> - __maybe_unused efi_handle_t handle;
> + __maybe_unused efi_handle_t handle = NULL;
> efi_status_t ret = EFI_SUCCESS;
>
> - if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)) {
> - handle = NULL;
> + if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_FIT))
> ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
> &handle, &efi_guid_firmware_management_protocol,
> &efi_fmp_fit, NULL));
> - }
>
> - if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) {
> - handle = NULL;
> + if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW))
> ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
> - &efi_root,
> + &handle,
> &efi_guid_firmware_management_protocol,
> &efi_fmp_raw, NULL));
> - }
>
> return ret;
> }
> @@ -975,10 +971,6 @@ efi_status_t efi_launch_capsules(void)
>
> index = get_last_capsule();
>
> - /* Load capsule drivers */
> - ret = arch_efi_load_capsule_drivers();
> - if (ret != EFI_SUCCESS)
> - return ret;
>
> /*
> * Find capsules on disk.
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 3c5cf9a4357e..0106efdc161b 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -254,6 +254,10 @@ efi_status_t efi_init_obj_list(void)
> if (ret != EFI_SUCCESS)
> goto out;
>
> + ret = efi_load_capsule_drivers();
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> #if defined(CONFIG_LCD) || defined(CONFIG_DM_VIDEO)
> ret = efi_gop_register();
> if (ret != EFI_SUCCESS)
> --
> 2.32.0.rc0
>