[PATCH v2 11/17] efi_loader: add firmware management protocol for FIT image

Sughosh Ganu sughosh.ganu at linaro.org
Mon Jun 22 10:38:49 CEST 2020


On Mon, 22 Jun 2020 at 13:37, AKASHI Takahiro <takahiro.akashi at linaro.org>
wrote:

> On Mon, Jun 22, 2020 at 01:28:07PM +0530, Sughosh Ganu wrote:
> > On Mon, 22 Jun 2020 at 06:39, AKASHI Takahiro <
> takahiro.akashi at linaro.org>
> > wrote:
> >
> > > On Sun, Jun 21, 2020 at 12:19:17AM +0530, Sughosh Ganu wrote:
> > > > On Wed, 17 Jun 2020 at 08:26, AKASHI Takahiro <
> > > takahiro.akashi at linaro.org>
> > > > wrote:
> > > >
> > > > > In this commit, a very simple firmware management protocol driver
> > > > > is implemented. It will take a common FIT image firmware in a
> capsule
> > > > > file and apply the data using dfu backend storage drivers via
> > > > > update_fit() interface.
> > > > >
> > > > > So "dfu_alt_info" variable should be properly set to specify a
> device
> > > > > and location to be updated. Please read README.dfu.
> > > > >
> > > > > Fit image is a common file format for firmware update on U-Boot,
> and
> > > > > this protocol works neatly just as a wrapper for one.
> > > > >
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > > > > ---
> > > > >  include/efi_api.h             |   4 +
> > > > >  include/efi_loader.h          |   2 +
> > > > >  lib/efi_loader/Kconfig        |  11 ++
> > > > >  lib/efi_loader/Makefile       |   1 +
> > > > >  lib/efi_loader/efi_capsule.c  |  12 +-
> > > > >  lib/efi_loader/efi_firmware.c | 253
> ++++++++++++++++++++++++++++++++++
> > > > >  6 files changed, 282 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 lib/efi_loader/efi_firmware.c
> > > > >
> > > > > diff --git a/include/efi_api.h b/include/efi_api.h
> > > > > index b062720e8220..c3fc4edbedc4 100644
> > > > > --- a/include/efi_api.h
> > > > > +++ b/include/efi_api.h
> > > > > @@ -1843,6 +1843,10 @@ struct efi_signature_list {
> > > > >         EFI_GUID(0x86c77a67, 0x0b97, 0x4633, 0xa1, 0x87, \
> > > > >                  0x49, 0x10, 0x4d, 0x06, 0x85, 0xc7)
> > > > >
> > > > > +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \
> > > > > +       EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \
> > > > > +                0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47)
> > > > > +
> > > > >  #define EFI_IMAGE_ATTRIBUTE_IMAGE_UPDATABLE            0x1
> > > > >  #define EFI_IMAGE_ATTRIBUTE_RESET_REQUIRED             0x2
> > > > >  #define EFI_IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED    0x4
> > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > > > index bc58c7e3c1d7..5f574533e732 100644
> > > > > --- a/include/efi_loader.h
> > > > > +++ b/include/efi_loader.h
> > > > > @@ -787,6 +787,8 @@ bool efi_secure_boot_enabled(void);
> > > > >  bool efi_image_parse(void *efi, size_t len, struct
> efi_image_regions
> > > > > **regp,
> > > > >                      WIN_CERTIFICATE **auth, size_t *auth_len);
> > > > >
> > > > > +extern const struct efi_firmware_management_protocol efi_fmp_fit;
> > > > > +
> > > > >  /* Capsule update */
> > > > >  efi_status_t EFIAPI efi_update_capsule(
> > > > >                 struct efi_capsule_header **capsule_header_array,
> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > > index e1413c35e33c..305751f4b15c 100644
> > > > > --- a/lib/efi_loader/Kconfig
> > > > > +++ b/lib/efi_loader/Kconfig
> > > > > @@ -86,6 +86,17 @@ config EFI_CAPSULE_FIRMWARE_MANAGEMENT
> > > > >           Select this option if you want to enable capsule-based
> > > > >           firmware update using Firmware Management Protocol.
> > > > >
> > > > > +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
> > > > > +       default n
> > > > > +       help
> > > > > +         Select this option if you want to enable firmware
> management
> > > > > protocol
> > > > > +         driver for FIT image
> > > > > +
> > > > >  config EFI_DEVICE_PATH_TO_TEXT
> > > > >         bool "Device path to text protocol"
> > > > >         default y
> > > > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > > > > index 67bc5c9c0907..ea1fbc4a9deb 100644
> > > > > --- a/lib/efi_loader/Makefile
> > > > > +++ b/lib/efi_loader/Makefile
> > > > > @@ -24,6 +24,7 @@ obj-$(CONFIG_CMD_BOOTEFI_HELLO) +=
> helloworld_efi.o
> > > > >  obj-y += efi_bootmgr.o
> > > > >  obj-y += efi_boottime.o
> > > > >  obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> > > > > +obj-$(CONFIG_EFI_CAPSULE_FIRMWARE_FIT) += efi_firmware.o
> > > > >
> > > >
> > > > Why can we not build this file based on
> > > > CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT. That way, we do not have to
> > > declare
> > > > the CONFIG_EFI_CAPSULE_FIRMWARE symbol on which this file is being
> built
> > > in
> > > > a subsequent patch.
> > >
> > > That is because, in some cases, efi_firmware.c won't be compiled in
> > > as a specific platform may want to have its own protocol.
> > >
> >
> > I think the whole point of the review comments from Heinrich to my fmp
> > implementation for qemu arm64 platform was to have a common fmp
> > implementation that could be used on all platforms and architectures.
>
> ? I believe that my patch meets Heinrich's requirement.
>

Yes, and that was my point. About the need for having a platform specific
fmp implementation after having your patch.


>
> > Do
> > you see a case for introducing a platform specific implementation now
> that
> > we have support for a generic raw binary image. Any platform specific
> fixup
> > if required, can be handled by adding pre-processing/post-processing
> > functions.
>
> I don't think we should exclude any possibility of adding
> platform-specific protocol as long as it doesn't break
> UEFI specification. Heinrich sometimes makes similar comments
> on different matters.


Ok. My comments were based on what Heinrich had mentioned as part of
reviewing my fmp patches. In any case, i think there is no harm in building
this with the CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT symbol. If any
platform does add a fmp implementation, that should be a different,
platform specific symbol.

-sughosh


More information about the U-Boot mailing list