On Thu, 24 Mar 2022 at 20:21, Michal Simek <michal.si...@xilinx.com> wrote: > > > > On 3/24/22 15:44, Sughosh Ganu wrote: > > On Thu, 24 Mar 2022 at 19:55, Michal Simek <michal.si...@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.g...@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
Okay, I will move the array definition to the suggested file in the next version. > > 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. Okay, will do as you suggest. -sughosh
