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

[Michal Simek]

On 3/24/22 15:44, Sughosh Ganu wrote:
> 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.

This is for zynq
include/configs/zynq-common.h

This is for zynqmp
include/configs/xilinx_zynqmp.h

there is also one for versal which doesn't have capsule enabled yet
include/configs/xilinx_versal.h

but maybe make sense to add fw_images structure to
board/xilinx/common/board.c

and define that macros in headers above with generic macro name 
XILINX_BOOT_IMAGE_GUID and XILINX_UBOOT_IMAGE_GUID
and fw_name and XILINX-BOOT and XILINX-UBOOT to have it ready for versal.

Thanks,
Michal