Hi all,
I'd like some early feedback on an approach before writing it up as patches for Gerrit review. It would relax the generic FWU metadata sanity checks in fwu.c, and let us remove some STM32MP-specific code that would become dead as a result.
Motivation
On STM32MP platforms we need to support boards that can be flashed either with a GPT/block-device layout (SD/eMMC) or with a raw/MTD layout (NOR/NAND), while reusing the same U-Boot binary/configuration across both. The two layouts don't need to describe the same firmware image set in their FWU metadata: the block-device variant uses an A/B redundant layout covering FIP + BootFS + RootFS (3 images per bank), while the raw-storage variant only ever needs a single FIP per bank (no GPT, no room for a generic BootFS/RootFS A/B scheme).
This was highlighted by a 4-patch series by Dario already in TF-A tree ("feat(st): add extended bootloader GUID" / "feat(st): add Linux data partition GUID" / "feat(st): add STM32MP_PSA_FWU_AB_SUPPORT build flag" / "feat(st): enable A/B redundancy support"), which introduces exactly that 3-image A/B layout for GPT-based STM32MP boards by raising NR_OF_IMAGES_IN_FW_BANK to 3.
The problem: fwu_metadata_sanity_check() currently requires an exact match between the metadata-declared num_images/size and the platform's compiled configuration, and computes the CRC over sizeof(struct fwu_metadata) instead of the metadata-declared metadata_size. This forces every board variant sharing a single TF-A build to describe exactly the same image set/size in its FWU metadata, which isn't what the spec intends.
What the spec says (DEN0118 FWU-1.0 A EAC1 see https://developer.arm.com/documentation/den0118/latest/)
Table A3.2 explicitly introduces metadata_size in v2 as "The size in bytes of the complete metadata structure", and defines the CRC as computed over the declared size, not a fixed maximum. Table A3.3 defines num_images as "The number of entries in the img_entry array" (img_entry[num_images]), again variable by design. The spec also explicitly allows #banks < 4, but in the current C struct layout num_banks also determines the byte stride of each img_entry (bank_info_entry_size * num_banks), so relaxing it would require decoding img_entry[] with a runtime-computed stride instead of the fixed compile-time layout so I'd rather keep that out of scope for now.
What I'm proposing
In the common FWU driver (fwu.c), replace the equality checks with bounds that only reject metadata a platform genuinely cannot handle:
- Compute the CRC over metadata.metadata_size instead of sizeof(struct fwu_metadata), matching the spec. - Accept num_images <= NR_OF_IMAGES_IN_FW_BANK instead of requiring strict equality. - Bound metadata_size itself between FWU_FW_STORE_DESC_OFFSET (0x20, the minimum valid v2 size) and sizeof(struct fwu_metadata), to reject malformed metadata and avoid any out-of-bounds read. - Keep num_banks as an exact match against NR_OF_FW_BANKS, for the layout reason above.
On the STM32MP side, plat_fwu_set_images_source() currently iterates every image entry described by the metadata and tries to resolve a storage offset for each of them. On raw storage devices (NOR/NAND) only two hardcoded bank GUIDs are known, so any entry other than the FIP (e.g. BootFS/RootFS added by STM32MP_PSA_FWU_AB_SUPPORT) causes a panic, even though BL2 never actually loads BootFS/RootFS itself (only U-Boot/Linux/the update agent do). I'd change it to search explicitly for the FIP image type GUID and resolve only that entry, using metadata.fw_desc.num_images (now safely bounded by point 1) as the search bound instead of the compiled NR_OF_IMAGES_IN_FW_BANK. The function then becomes agnostic of how many, or which, other image types a given metadata instance may describe.
As a direct consequence of point 2, the per-image IO policies, image IDs and GUID definitions (XBOOTLDR_GUID, LINUX_FILE_SYSTEM_DATA_GUID) added for BootFS/RootFS become dead code from BL2's point of view (they were only needed so plat_fwu_set_images_source() could resolve those entries). I'd remove them, while keeping the STM32_EXTRA_PARTS accounting fix (load_partition_table() truncates the parsed GPT to a fixed-size list, so the extra bootfs-a/b, rootfs-a/b, vendorfs and u-boot-env partitions still physically present on an A/B board must still be accounted for, even though BL2 no longer resolves them individually).
Validation
Tested on hardware:
- A TF-A build configured for the 3-image A/B layout (NR_OF_IMAGES_IN_FW_BANK = 3, STM32MP_PSA_FWU_AB_SUPPORT=1) booting successfully off FWU metadata that only declares a single image entry (num_images = 1), confirming a platform built for the larger capacity can consume metadata that uses a smaller subset. - The existing single-image configuration (NR_OF_IMAGES_IN_FW_BANK = 1) still boots correctly with unchanged, exact-match metadata, confirming no regression on the base FWU flow.
Questions for the list
- Is the community comfortable relaxing this check in common code (fwu.c), given it affects every platform enabling PSA_FWU_METADATA_FW_STORE_DESC? - Is anyone aware of a downstream user relying on the current exact-match behaviour that this change would affect?
If this sounds reasonable I'll post the actual patches to Gerrit for review.
Thanks, Maxime