[RFC PATCH 1/6] capsule: Add Image GUIDs for platforms using capsule updates

[Sughosh Ganu]

On Thu, 24 Mar 2022 at 19:55, Michal Simek <michal.simek at xilinx.com> wrote:
>
>
>
> On 3/24/22 13:38, Sughosh Ganu wrote:
> > Currently, all platforms that enable capsule updates do so using
> > either EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID or
> > EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID. This is based on the Firmware
> > Management Protocol(FMP) instance used on the platform. However, this
> > means that all platforms that enable a particular FMP instance have
> > the same GUID value for all the updatable images, either the FIT image
> > GUID or the raw image GUID, and that an image for some platform can be
> > updated on any other platform which uses the same FMP instance. Another
> > issue with this implementation is that the ESRT table shows the same
> > GUID value for all images on the platform and also across platforms,
> > which is not in compliance with the UEFI specification.
> >
> > Fix this by defining image GUID values and firmware names for
> > individual images per platform. The GetImageInfo FMP hook would then
> > populate these values in the image descriptor array.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> >   .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c       | 19 +++++++++++++++
> >   .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c   | 18 +++++++++++++++
> >   board/emulation/qemu-arm/qemu-arm.c           | 20 ++++++++++++++++
> >   board/kontron/pitx_imx8m/pitx_imx8m.c         | 15 +++++++++++-
> >   board/kontron/sl-mx8mm/sl-mx8mm.c             | 14 +++++++++++
> >   board/kontron/sl28/sl28.c                     | 14 +++++++++++
> >   board/sandbox/sandbox.c                       | 17 ++++++++++++++
> >   board/socionext/developerbox/developerbox.c   | 23 +++++++++++++++++++
> >   board/xilinx/common/board.h                   | 18 +++++++++++++++
> >   board/xilinx/zynq/board.c                     | 18 +++++++++++++++
> >   board/xilinx/zynqmp/zynqmp.c                  | 18 +++++++++++++++
> >   include/configs/imx8mm-cl-iot-gate.h          | 10 ++++++++
> >   include/configs/imx8mp_rsb3720.h              | 10 ++++++++
> >   include/configs/kontron-sl-mx8mm.h            |  6 +++++
> >   include/configs/kontron_pitx_imx8m.h          |  6 +++++
> >   include/configs/kontron_sl28.h                |  6 +++++
> >   include/configs/qemu-arm.h                    | 10 ++++++++
> >   include/configs/sandbox.h                     | 10 ++++++++
> >   include/configs/synquacer.h                   | 14 +++++++++++
> >   include/efi_loader.h                          | 15 ++++++++++++
> >   20 files changed, 280 insertions(+), 1 deletion(-)

<snip>

> > diff --git a/board/xilinx/common/board.h b/board/xilinx/common/board.h
> > index 69e642429b..9bcac14946 100644
> > --- a/board/xilinx/common/board.h
> > +++ b/board/xilinx/common/board.h
> > @@ -7,6 +7,24 @@
> >   #ifndef _BOARD_XILINX_COMMON_BOARD_H
> >   #define _BOARD_XILINX_COMMON_BOARD_H
> >
> > +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> > +#define ZYNQ_BOOT_IMAGE_GUID \
> > +     EFI_GUID(0x1ba29a15, 0x9969, 0x40aa, 0xb4, 0x24, \
> > +              0xe8, 0x61, 0x21, 0x61, 0x86, 0x64)
> > +
> > +#define ZYNQ_UBOOT_IMAGE_GUID \
> > +     EFI_GUID(0x1a5178f0, 0x87d3, 0x4f36, 0xac, 0x63, \
> > +              0x3b, 0x31, 0xa2, 0x3b, 0xe3, 0x05)
> > +
> > +#define ZYNQMP_BOOT_IMAGE_GUID \
> > +     EFI_GUID(0xde6066e8, 0x0256, 0x4fad, 0x82, 0x38, \
> > +              0xe4, 0x06, 0xe2, 0x74, 0xc4, 0xcf)
> > +
> > +#define ZYNQMP_UBOOT_IMAGE_GUID \
> > +     EFI_GUID(0xcf9ecfd4, 0x938b, 0x41c5, 0x85, 0x51, \
> > +              0x1f, 0x88, 0x3a, 0xb7, 0xdc, 0x18)
> > +#endif /* EFI_HAVE_CAPSULE_SUPPORT */
>
> I can't see any benefit to have it defined here for all.
> Directly in board or in include/configs/* seems to be better option.

I had initially put these definitions in
include/configs/zynq-common.h, but that breaks the build for both
zynq_virt and zynqmp_virt, since this file does not get included in
the corresponding board file. Which is why I put these under
board/xilinx/common/board.h, as this gets included in both the board
files. I don't see any other zynq* file under include/configs/. Can
you suggest one for both the platforms.

-sughosh