From cefafca6dfb4b1610369ce62961a4a65b015eb83 Mon Sep 17 00:00:00 2001 From: Sergii Dmytruk Date: Sun, 5 Apr 2026 17:29:23 +0300 Subject: [PATCH] util/cbfstool: don't invalidate MH cache unnecessarily Commit 7c7feca25882 ("CBFS verification: support Top Swap redundancy") forced reset of metadata cache before processing every region to make it possible to process slots A and B in a single invocation of cbfstool when Top Swap (TS) redundancy is enabled. The slots use separate bootblock copies each with its own metadata anchor, so cache cannot be blindly reused. This change invalidates the cache only when a transition from a TS to a non-TS (or vice versa) region is detected, which is achieved by tracking what is currently in the cache. Additionally, make the function fail if TOPSWAP region is not found. Upstream-Status: Backport [CB:92025] Change-Id: I37da5585ceffdaa243c6b77471637d2457134768 Signed-off-by: Sergii Dmytruk Reviewed-on: https://review.coreboot.org/c/coreboot/+/92025 Tested-by: build bot (Jenkins) Reviewed-by: Julius Werner --- util/cbfstool/cbfstool.c | 45 ++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 96ebc6c7ac..56f0a72eb1 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -118,6 +118,15 @@ static struct param { .u64val = -1, }; +/* Indicates which CBFS is described by an mh_cache. Multiple firmware slots imply multiple + hash anchors, necessitating differentiating between them when caching. */ +enum mhc_kind { + MHC_NONE, /* The cache is uninitialized. */ + MHC_PRIMARY, /* Normal and always-present "COREBOOT" CBFS (separate or embedded + bootblock). */ + MHC_TOPSWAP /* Top Swap CBFS called "COREBOOT_TS" with bootblock in "TOPSWAP". */ +}; + /* * This "metadata_hash cache" caches the value and location of the CBFS metadata * hash embedded in the bootblock when CBFS verification is enabled. The first @@ -132,7 +141,7 @@ struct mh_cache { size_t offset; struct vb2_hash cbfs_hash; platform_fixup_func fixup; - bool initialized; + enum mhc_kind kind; }; static bool is_main_cbfs_region(const char *region_name) @@ -141,27 +150,39 @@ static bool is_main_cbfs_region(const char *region_name) strcmp(region_name, SECTION_NAME_TOPSWAP_CBFS) == 0; } -static bool is_topswap_region(const char *region_name) +static enum mhc_kind derive_mhc_kind(const char *region_name) { - return strcmp(region_name, SECTION_NAME_TOPSWAP_CBFS) == 0 || - strcmp(region_name, SECTION_NAME_TOPSWAP) == 0; + /* Only these two regions are specific to Top Swap. */ + if (strcmp(region_name, SECTION_NAME_TOPSWAP_CBFS) == 0 || + strcmp(region_name, SECTION_NAME_TOPSWAP) == 0) + return MHC_TOPSWAP; + + return MHC_PRIMARY; } static struct mh_cache *get_mh_cache(void) { static struct mh_cache mhc; - if (mhc.initialized) + /* + * A single invocation of cbfstool can process regions that use different CBFS metadata. + * Right now, the only such case is when Top Swap redundancy is in use. Check for Top + * Swap region to decide if the cache can be reused. + * + * This implicitly checks whether the cache is initialized. + */ + const enum mhc_kind kind = derive_mhc_kind(param.region_name); + if (mhc.kind == kind) return &mhc; - mhc.initialized = true; + mhc.kind = kind; const struct fmap *fmap = partitioned_file_get_fmap(param.image_file); if (!fmap) goto no_metadata_hash; const char *bootblock_region = SECTION_NAME_BOOTBLOCK; - if (is_topswap_region(param.region_name)) + if (kind == MHC_TOPSWAP) bootblock_region = SECTION_NAME_TOPSWAP; /* Find the metadata_hash container. If there is a "BOOTBLOCK" FMAP section, it's @@ -175,6 +196,12 @@ static struct mh_cache *get_mh_cache(void) offset = 0; size = buffer.size; } else { + if (kind != MHC_PRIMARY) { + /* Top Swap requires TOPSWAP region. */ + ERROR("Malformed image: '%s' region is missing\n", bootblock_region); + goto no_metadata_hash; + } + struct cbfs_image cbfs; struct cbfs_file *mh_container; if (!partitioned_file_read_region(&buffer, param.image_file, SECTION_NAME_PRIMARY_CBFS)) @@ -2381,10 +2408,6 @@ int main(int argc, char **argv) strcpy(region_name_scratch, param.region_name); param.region_name = strtok(region_name_scratch, ","); for (unsigned region = 0; region < num_regions; ++region) { - // Reset metadata cache in case region uses anchor from a different - // location. - get_mh_cache()->initialized = false; - if (!param.region_name) { ERROR("Encountered illegal degenerate region name in -r list\n"); ERROR("The image will be left unmodified.\n");