[PATCH] efi_loader: FMP cleanups
Ilias Apalodimas
ilias.apalodimas at linaro.org
Tue Jun 15 05:55:50 CEST 2021
Akashi-san,
On Tue, Jun 15, 2021 at 10:51:01AM +0900, AKASHI Takahiro wrote:
> Ilias,
>
> In this patch, you are trying to address a couple of independent
> issues in a single commit.
> Please split.
> (Heinrich doesn't like that.)
>
> On Mon, Jun 14, 2021 at 06:10:14PM +0300, Ilias Apalodimas 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.
>
> I'd like to say nak in some respects:
> - Although I do understand the UEFI requirement that you mentioned above,
> FIT and RAW FMP drivers can handle *different* firmware even though
> they share the same dfu_alt_info.
How ?
> We should not impose unnecessary restriction if we manage to have some
> workaround to meet the requirement.
It's not the updating part only. It's that the .get_image_info also relies on
the same env variable. Specifically in the fwupd case on an RPI4 with the
dfu set at 'dfu_alt_info=u-boot.bin fat 0 1;' although 2 ERSTs entries were
populated only one was reported. Probably because this really does give you
2 ways of updating the same flash.
> (I still believe that the firmware definition for ESRT should exist
> elsewhere other than in FMP themselves.)
That's a whole different story, and if we have that, then .get_image_info
should change as well instead of using the DFU information. Because right
now we enabled security (or think we have), while allowing users to set an env
variable which is not authenticated, and completely change what the
firmware reports (or updates). We can always add a huge warning saying
something along the lines of "If you really care this should come with a
CONFIG_ENV_IS_NOWHERE and a boot timeout set to -1".
The spec is pretty clear and we allow users to *break* it with a config
option. Arguably this is fine, since the code continues to work fine and
you can perform the updates, but in essence RAW and FITs are used to update
the same medium right now. You can't have a capsule with half it's contents
describing something RAW and the other half being a FIT. You have a FIT based
capsule or a RAW based capsule.
> - We should allow users to add their own FMP drivers and so not call
> [arch_]efi_load_capsule_drivers() unconditionally
> even if you don't like "__weak" attribute.
I am fine with the __weak attribute. On the other hand I consider the
current code the defacto way users would use to update their firmware. That's
why I removed the __weak attribute. If a hardware vendor was to update
their special PCI option ROM or a flash that lives on the secure world they
should install their FMPs on a different handle and leave the current code
as is.
> - Selecting only one of FIT and RAW FMPs in sandbox*_defconfig will
> leave some test cases in pytest skipped.
Yea that's unfortunate, but maybe we can just add an extra config on the
sandbox?
Thanks
/Ilias
>
> -Takahiro Akashi
>
> > 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 at 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
> >
More information about the U-Boot
mailing list