[coreboot-gerrit] [M] Change in coreboot[master]: drivers/mrc_cache: Prevent printing errors in expected use cases

[Yu-Ping Wu (Code Review)]

Yu-Ping Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69542 )

Change subject: drivers/mrc_cache: Prevent printing errors in expected use cases
......................................................................

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 warning 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 a warning.

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 
---
M src/drivers/mrc_cache/mrc_cache.c
1 file changed, 45 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/69542/1

diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c
index fd3853f..1216463 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,7 @@
    cr = lookup_region_type(type);

if (cr == NULL) {
-		printk(BIOS_ERR, "MRC: failed to locate region type %d.\n",
-		       type);
+		printk(BIOS_WARNING, "MRC: failed to locate region type %d\n", type);
    	return NULL;
    }

@@ -172,6 +173,14 @@
    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_WARNING, "MRC: metadata was invalidated\n");
+			return -1;
+		}
+
    	printk(BIOS_ERR, "MRC: couldn't read metadata\n");
    	return -1;
    }
@@ -263,7 +272,7 @@
    /* 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);
+		printk(BIOS_WARNING, "MRC: invalid header in '%s'\n", name);
    	return fail_bad_data ? -1 : 0;
    }

@@ -595,7 +604,6 @@
    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 @@

/* 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);
 }