Re: [PATCH] efi_loader: FMP cleanups

Ilias Apalodimas
2021-06-15T03:56:58Z

On Tue, Jun 15, 2021 at 09:49:58AM +0900, Masami Hiramatsu wrote:
> 2021年6月15日(火) 0:10 Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>
> >
> > 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
> >
>
> This looks good to me, and this covers one patch which I sent before.
> https://lists.denx.de/pipermail/u-boot/2021-June/451401.html
>
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

Thanks Masami. I'll send a v2 regardless since I am missing an IS_ENABLED
option when trying to install the FMPs

Cheers
/Ilias
>
> Thank you!
>
> > 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
> >
>
>
> --
> Masami Hiramatsu