From 4f30539b476852b5f0da3174b84bf180e76e3ec7 Mon Sep 17 00:00:00 2001 From: Yu-Ping Wu Date: Mon, 14 Nov 2022 16:08:03 +0800 Subject: [PATCH] drivers/mrc_cache: Prevent printing errors in expected use cases The following are considered "expected" situations, where we shouldn't print error messages as in other unexpected errors: 1. When the previous boot is in recovery mode, under certain config combination the normal MRC cache would have been invalidated. Therefore the "couldn't read metadata" error is expected to show in the current normal boot. Special-case this situation by printing a different message. 2. If the platform doesn't have recovery cache (!HAS_RECOVERY_MRC_CACHE) and vboot starts before romstage (!VBOOT_STARTS_IN_ROMSTAGE), then there should be no region for recovery cache. In this case, "failed to locate region type 0" will be shown. Since it's pretty clear from the code that this is the only case for the error to happen, simply change it to BIOS_DEBUG. Also remove a duplicate message when mrc_header_valid() fails. BUG=b:257401937 TEST=emerge-corsola coreboot TEST=Ran `cbmem -1 | grep ERROR` in recovery boot TEST=Ran `cbmem -1 | grep ERROR` in normal boot following recovery boot BRANCH=corsola Change-Id: Ia942eeecaca3f6b2b90bac725279d2dc6174e0fd Signed-off-by: Yu-Ping Wu Reviewed-on: https://review.coreboot.org/c/coreboot/+/69542 Reviewed-by: Julius Werner Tested-by: build bot (Jenkins) --- src/drivers/mrc_cache/mrc_cache.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index fd3853fc0c..484b414dfa 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -25,6 +25,8 @@ /* Signature "MRCD" was used for older header format before CB:67670. */ #define MRC_DATA_SIGNATURE (('M'<<0)|('R'<<8)|('C'<<16)|('d'<<24)) +const static uint32_t mrc_invalid_sig = ~MRC_DATA_SIGNATURE; + struct mrc_metadata { uint32_t signature; uint32_t data_size; @@ -154,8 +156,9 @@ static const struct cache_region *lookup_region(struct region *r, int type) cr = lookup_region_type(type); if (cr == NULL) { - printk(BIOS_ERR, "MRC: failed to locate region type %d.\n", - type); + /* There will be no recovery MRC cache region if (!HAS_RECOVERY_MRC_CACHE && + !VBOOT_STARTS_IN_ROMSTAGE). */ + printk(BIOS_DEBUG, "MRC: failed to locate region type %d\n", type); return NULL; } @@ -172,6 +175,14 @@ static int mrc_header_valid(struct region_device *rdev, struct mrc_metadata *md) size_t size; if (rdev_readat(rdev, md, 0, sizeof(*md)) < 0) { + /* When the metadata was invalidated intentionally (for example from the + previous recovery boot), print a warning instead of an error. */ + if (rdev_readat(rdev, md, 0, sizeof(mrc_invalid_sig)) >= 0 && + md->signature == mrc_invalid_sig) { + printk(BIOS_INFO, "MRC: metadata was invalidated\n"); + return -1; + } + printk(BIOS_ERR, "MRC: couldn't read metadata\n"); return -1; } @@ -262,10 +273,8 @@ static int mrc_cache_get_latest_slot_info(const char *name, /* Validate header and resize region to reflect actual usage on the * saved medium (including metadata and data). */ - if (mrc_header_valid(rdev, md) < 0) { - printk(BIOS_ERR, "MRC: invalid header in '%s'\n", name); + if (mrc_header_valid(rdev, md) < 0) return fail_bad_data ? -1 : 0; - } return 0; } @@ -595,7 +604,6 @@ static void invalidate_normal_cache(void) struct region_file cache_file; struct region_device rdev; const char *name = DEFAULT_MRC_CACHE; - const uint32_t invalid = ~MRC_DATA_SIGNATURE; /* * If !HAS_RECOVERY_MRC_CACHE and VBOOT_STARTS_IN_ROMSTAGE is @@ -631,7 +639,8 @@ static void invalidate_normal_cache(void) /* Push an update that consists of 4 bytes that is smaller than the * MRC metadata as well as an invalid signature. */ - if (region_file_update_data(&cache_file, &invalid, sizeof(invalid)) < 0) + if (region_file_update_data(&cache_file, &mrc_invalid_sig, + sizeof(mrc_invalid_sig)) < 0) printk(BIOS_ERR, "MRC: invalidation failed for '%s'.\n", name); }