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.apalodi...@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.apalodi...@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 >