soc/intel/common: remove mrc cache assumptions

Update the mrc cache implementation to use region_file. Instead
of relying on memory-mapped access and pointer arithmetic
use the region_devices and region_file to obtain the latest
data associated with the region. This removes the need for the
nvm wrapper as the region_devices can be used directly. Thus,
the library is more generic and can be extended to work on
different boot mediums.

BUG=chrome-os-partner:56151

Change-Id: Ic14e2d2f7339e50256b4a3a297fc33991861ca44
Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/17717
Tested-by: build bot (Jenkins)
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>

8 files changed, 387 insertions, 478 deletions

diff --git a/src/soc/intel/apollolake/mmap_boot.c b/src/soc/intel/apollolake/mmap_boot.c
index 7159c24f52..3213b09b23 100644
--- a/src/soc/intel/apollolake/mmap_boot.c
+++ b/src/soc/intel/apollolake/mmap_boot.c
@@ -23,7 +23,6 @@
 #include <console/console.h>
 #include <fmap.h>
 #include <soc/flash_ctrlr.h>
-#include <soc/intel/common/nvm.h>
 #include <soc/mmap_boot.h>
 
 /*
@@ -148,22 +147,6 @@ const struct cbfs_locator cbfs_master_header_locator = {
 	.locate = iafw_boot_region_properties,
 };
 
-uint32_t nvm_mmio_to_flash_offset(void *p)
-{
-	bios_mmap_init();
-
-	size_t start, size;
-	start = car_get_var(bios_start);
-	size = car_get_var(bios_size);
-
-	/*
-	 * Returns :
-	 * addr - base of mmaped region in addr space + offset of mmaped region
-	 * start on flash
-	 */
-	return (uintptr_t)p - (4ULL * GiB - size) + start;
-}
-
 size_t get_bios_size(void)
 {
 	bios_mmap_init();
diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c
index 77562c4699..8c2a1a330a 100644
--- a/src/soc/intel/apollolake/romstage.c
+++ b/src/soc/intel/apollolake/romstage.c
@@ -202,8 +202,9 @@ asmlinkage void car_stage_entry(void)
 	new_var_data = fsp_find_extension_hob_by_guid(hob_variable_guid,
 							&var_size);
 	if (new_var_data)
-		mrc_cache_stash_vardata(new_var_data, var_size,
-					car_get_var(fsp_version));
+		mrc_cache_stash_data(MRC_VARIABLE_DATA,
+				car_get_var(fsp_version), new_var_data,
+				var_size);
 	else
 		printk(BIOS_ERR, "Failed to determine variable data\n");
 
@@ -258,7 +259,7 @@ static void fill_console_params(FSPM_UPD *mupd)
 
 void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)
 {
-	const struct mrc_saved_data *msd;
+	struct region_device rdev;
 
 	fill_console_params(mupd);
 	mainboard_memory_init_params(mupd);
@@ -293,10 +294,10 @@ void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)
 	 * wrong/missing key renders DRAM contents useless.
 	 */
 
-	if (mrc_cache_get_vardata(&msd, version) < 0) {
-		printk(BIOS_DEBUG, "MRC variable data missing/invalid\n");
-	} else {
-		mupd->FspmConfig.VariableNvsBufferPtr = (void*) msd->data;
+	if (mrc_cache_get_current(MRC_VARIABLE_DATA, version, &rdev) == 0) {
+		/* Assume leaking is ok. */
+		assert(IS_ENABLED(CONFIG_BOOT_DEVICE_MEMORY_MAPPED));
+		mupd->FspmConfig.VariableNvsBufferPtr = rdev_mmap_full(&rdev);
 	}
 
 	car_set_var(fsp_version, version);
diff --git a/src/soc/intel/baytrail/romstage/raminit.c b/src/soc/intel/baytrail/romstage/raminit.c
index a79fe9533b..190231d7c3 100644
--- a/src/soc/intel/baytrail/romstage/raminit.c
+++ b/src/soc/intel/baytrail/romstage/raminit.c
@@ -15,7 +15,7 @@
 
 #include <stddef.h>
 #include <arch/acpi.h>
-#include <arch/io.h>
+#include <assert.h>
 #include <cbfs.h>
 #include <cbmem.h>
 #include <console/console.h>
@@ -111,7 +111,7 @@ void raminit(struct mrc_params *mp, int prev_sleep_state)
 {
 	int ret;
 	mrc_wrapper_entry_t mrc_entry;
-	const struct mrc_saved_data *cache;
+	struct region_device rdev;
 
 	/* Fill in default entries. */
 	mp->version = MRC_PARAMS_VER;
@@ -125,9 +125,11 @@ void raminit(struct mrc_params *mp, int prev_sleep_state)
 
 	if (vboot_recovery_mode_enabled()) {
 		printk(BIOS_DEBUG, "Recovery mode: not using MRC cache.\n");
-	} else if (!mrc_cache_get_current(&cache)) {
-		mp->saved_data_size = cache->size;
-		mp->saved_data = &cache->data[0];
+	} else if (!mrc_cache_get_current(MRC_TRAINING_DATA, 0, &rdev)) {
+		mp->saved_data_size = region_device_sz(&rdev);
+		mp->saved_data = rdev_mmap_full(&rdev);
+		/* Assume boot device is memory mapped. */
+		assert(IS_ENABLED(CONFIG_BOOT_DEVICE_MEMORY_MAPPED));
 	} else if (prev_sleep_state == ACPI_S3) {
 		/* If waking from S3 and no cache then. */
 		printk(BIOS_DEBUG, "No MRC cache found in S3 resume path.\n");
@@ -178,5 +180,6 @@ void raminit(struct mrc_params *mp, int prev_sleep_state)
 	       mp->data_to_save_size);
 
 	if (mp->data_to_save != NULL && mp->data_to_save_size > 0)
-		mrc_cache_stash_data(mp->data_to_save, mp->data_to_save_size);
+		mrc_cache_stash_data(MRC_TRAINING_DATA, 0, mp->data_to_save,
+					mp->data_to_save_size);
 }
diff --git a/src/soc/intel/broadwell/romstage/raminit.c b/src/soc/intel/broadwell/romstage/raminit.c
index 61b1bc9cb9..1185788ea7 100644
--- a/src/soc/intel/broadwell/romstage/raminit.c
+++ b/src/soc/intel/broadwell/romstage/raminit.c
@@ -15,6 +15,7 @@
 
 #include <arch/cbfs.h>
 #include <arch/io.h>
+#include <assert.h>
 #include <cbfs.h>
 #include <cbmem.h>
 #include <console/console.h>
@@ -41,7 +42,7 @@
  */
 void raminit(struct pei_data *pei_data)
 {
-	const struct mrc_saved_data *cache;
+	struct region_device rdev;
 	struct memory_info* mem_info;
 	pei_wrapper_entry_t entry;
 	int ret;
@@ -51,10 +52,12 @@ void raminit(struct pei_data *pei_data)
 	if (vboot_recovery_mode_enabled()) {
 		/* Recovery mode does not use MRC cache */
 		printk(BIOS_DEBUG, "Recovery mode: not using MRC cache.\n");
-	} else if (!mrc_cache_get_current(&cache)) {
+	} else if (!mrc_cache_get_current(MRC_TRAINING_DATA, 0, &rdev)) {
 		/* MRC cache found */
-		pei_data->saved_data_size = cache->size;
-		pei_data->saved_data = &cache->data[0];
+		pei_data->saved_data_size = region_device_sz(&rdev);
+		pei_data->saved_data = rdev_mmap_full(&rdev);
+		/* Assume boot device is memory mapped. */
+		assert(IS_ENABLED(CONFIG_BOOT_DEVICE_MEMORY_MAPPED));
 	} else if (pei_data->boot_mode == ACPI_S3) {
 		/* Waking from S3 and no cache. */
 		printk(BIOS_DEBUG, "No MRC cache found in S3 resume path.\n");
@@ -118,8 +121,9 @@ void raminit(struct pei_data *pei_data)
 	       pei_data->data_to_save_size);
 
 	if (pei_data->data_to_save != NULL && pei_data->data_to_save_size > 0)
-		mrc_cache_stash_data(pei_data->data_to_save,
-				     pei_data->data_to_save_size);
+		mrc_cache_stash_data(MRC_TRAINING_DATA, 0,
+					pei_data->data_to_save,
+					pei_data->data_to_save_size);
 
 	printk(BIOS_DEBUG, "create cbmem for dimm information\n");
 	mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(struct memory_info));
diff --git a/src/soc/intel/common/mrc_cache.c b/src/soc/intel/common/mrc_cache.c
index e9d670ec40..69e5b00de1 100644
--- a/src/soc/intel/common/mrc_cache.c
+++ b/src/soc/intel/common/mrc_cache.c
@@ -14,344 +14,343 @@
  */
 
 #include <string.h>
+#include <boot_device.h>
 #include <bootstate.h>
 #include <console/console.h>
 #include <cbmem.h>
 #include <elog.h>
 #include <fmap.h>
 #include <ip_checksum.h>
+#include <region_file.h>
 #include <vboot/vboot_common.h>
 
 #include "mrc_cache.h"
 #include "nvm.h"
 
-#define MRC_DATA_ALIGN           0x1000
+#define DEFAULT_MRC_CACHE	"RW_MRC_CACHE"
+#define VARIABLE_MRC_CACHE	"RW_VAR_MRC_CACHE"
+#define RECOVERY_MRC_CACHE	"RECOVERY_MRC_CACHE"
+#define UNIFIED_MRC_CACHE	"UNIFIED_MRC_CACHE"
+
 #define MRC_DATA_SIGNATURE       (('M'<<0)|('R'<<8)|('C'<<16)|('D'<<24))
 
-/* The mrc_data_region describes the larger non-volatile area to store
- * mrc_saved_data objects.*/
-struct mrc_data_region {
-	void *base;
-	uint32_t size;
-};
+struct mrc_metadata {
+	uint32_t signature;
+	uint32_t data_size;
+	uint16_t data_checksum;
+	uint16_t header_checksum;
+	uint32_t version;
+} __attribute__((packed));
 
 enum result {
-	WRITE_FAILURE		= -1,
-	ERASE_FAILURE		= -2,
-	OTHER_FAILURE		= -3,
+	UPDATE_FAILURE		= -1,
 	UPDATE_SUCCESS		= 0,
 	ALREADY_UPTODATE	= 1
 };
 
-/* common code */
-static int mrc_cache_get_region(const char *name,
-				struct mrc_data_region *region)
-{
-	bool located_by_fmap = true;
-	struct region_device rdev;
+#define NORMAL_FLAG (1 << 0)
+#define RECOVERY_FLAG (1 << 1)
 
-	region->base = NULL;
-	region->size = 0;
-
-	if (fmap_locate_area_as_rdev(name, &rdev))
-		located_by_fmap =  false;
+struct cache_region {
+	const char *name;
+	uint32_t cbmem_id;
+	int type;
+	int elog_slot;
+	int flags;
+};
 
-	/* CHROMEOS builds must get their MRC cache from FMAP. */
-	if (IS_ENABLED(CONFIG_CHROMEOS) && !located_by_fmap)
-		return -1;
+static const struct cache_region recovery_training = {
+	.name = RECOVERY_MRC_CACHE,
+	.cbmem_id = CBMEM_ID_MRCDATA,
+	.type = MRC_TRAINING_DATA,
+	.elog_slot = ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY,
+#if IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE)
+	.flags = RECOVERY_FLAG,
+#else
+	.flags = 0,
+#endif
+};
 
-	if (located_by_fmap) {
-		region->size = region_device_sz(&rdev);
-		region->base = rdev_mmap_full(&rdev);
+static const struct cache_region normal_training = {
+	.name = DEFAULT_MRC_CACHE,
+	.cbmem_id = CBMEM_ID_MRCDATA,
+	.type = MRC_TRAINING_DATA,
+	.elog_slot = ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL,
+	.flags = NORMAL_FLAG | RECOVERY_FLAG,
+};
 
-		if (region->base == NULL)
-			return -1;
-	} else {
-		region->base = (void *)CONFIG_MRC_SETTINGS_CACHE_BASE;
-		region->size = CONFIG_MRC_SETTINGS_CACHE_SIZE;
-	}
+static const struct cache_region variable_data = {
+	.name = VARIABLE_MRC_CACHE,
+	.cbmem_id = CBMEM_ID_VAR_MRCDATA,
+	.type = MRC_VARIABLE_DATA,
+	.elog_slot = ELOG_MEM_CACHE_UPDATE_SLOT_VARIABLE,
+	.flags = NORMAL_FLAG | RECOVERY_FLAG,
+};
 
-	return 0;
-}
+/* Order matters here for priority in matching. */
+static const struct cache_region *cache_regions[] = {
+	&recovery_training,
+	&normal_training,
+	&variable_data,
+};
 
-/* Protect mrc region with a Protected Range Register */
-static int __protect_mrc_cache(const struct mrc_data_region *region,
-			       const char *name)
+static int lookup_region_by_name(const char *name, struct region *r)
 {
-	if (!IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT))
+	/* This assumes memory mapped boot media just under 4GiB. */
+	const uint32_t pointer_base_32bit = -CONFIG_ROM_SIZE;
+
+	if (fmap_locate_area(name, r) == 0)
 		return 0;
 
-	if (nvm_is_write_protected() <= 0) {
-		printk(BIOS_INFO, "MRC: NOT enabling PRR for %s.\n", name);
-		return 1;
+	/* CHROMEOS builds must get their MRC cache from FMAP. */
+	if (IS_ENABLED(CONFIG_CHROMEOS)) {
+		printk(BIOS_ERR, "MRC: Chrome OS lookup failure.\n");
+		return -1;
 	}
 
-	if (nvm_protect(region->base, region->size) < 0) {
-		printk(BIOS_ERR, "MRC: ERROR setting PRR for %s.\n", name);
+	if (!IS_ENABLED(CONFIG_BOOT_DEVICE_MEMORY_MAPPED))
 		return -1;
-	}
 
-	printk(BIOS_INFO, "MRC: Enabled Protected Range on %s.\n", name);
+	/* Base is in the form of a pointer. Make it an offset. */
+	r->offset = CONFIG_MRC_SETTINGS_CACHE_BASE - pointer_base_32bit;
+	r->size = CONFIG_MRC_SETTINGS_CACHE_SIZE;
+
 	return 0;
 }
 
-static int protect_mrc_cache(const char *name)
+static const struct cache_region *lookup_region_type(int type)
 {
-	struct mrc_data_region region;
-	if (mrc_cache_get_region(name, &region) < 0) {
-		printk(BIOS_ERR, "MRC: Could not find region %s\n", name);
-		return -1;
+	int i;
+	int flags;
+
+	if (vboot_recovery_mode_enabled())
+		flags = RECOVERY_FLAG;
+	else
+		flags = NORMAL_FLAG;
+
+	for (i = 0; i < ARRAY_SIZE(cache_regions); i++) {
+		if (cache_regions[i]->type != type)
+			continue;
+		if ((cache_regions[i]->flags & flags) == flags)
+			return cache_regions[i];
 	}
 
-	return __protect_mrc_cache(&region, name);
+	return NULL;
 }
 
-static int mrc_cache_in_region(const struct mrc_data_region *region,
-                               const struct mrc_saved_data *cache)
+int mrc_cache_stash_data(int type, uint32_t version, const void *data,
+			size_t size)
 {
-	uintptr_t region_end;
-	uintptr_t cache_end;
-
-	if ((uintptr_t)cache < (uintptr_t)region->base)
-		return 0;
+	const struct cache_region *cr;
+	size_t cbmem_size;
+	struct mrc_metadata *md;
+
+	cr = lookup_region_type(type);
+	if (cr == NULL) {
+		printk(BIOS_ERR, "MRC: failed to add to cbmem for type %d.\n",
+			type);
+		return -1;
+	}
 
-	region_end = (uintptr_t)region->base;
-	region_end += region->size;
+	cbmem_size = sizeof(*md) + size;
 
-	if ((uintptr_t)cache >= region_end)
-		return 0;
+	md = cbmem_add(cr->cbmem_id, cbmem_size);
 
-	if ((sizeof(*cache) + (uintptr_t)cache) >= region_end)
-		return 0;
+	if (md == NULL) {
+		printk(BIOS_ERR, "MRC: failed to add '%s' to cbmem.\n",
+			cr->name);
+		return -1;
+	}
 
-	cache_end = (uintptr_t)cache;
-	cache_end += cache->size + sizeof(*cache);
+	memset(md, 0, sizeof(*md));
+	md->signature = MRC_DATA_SIGNATURE;
+	md->data_size = size;
+	md->version = version;
+	md->data_checksum = compute_ip_checksum(data, size);
+	md->header_checksum = compute_ip_checksum(md, sizeof(*md));
+	memcpy(&md[1], data, size);
 
-	if (cache_end > region_end)
-		return 0;
-
-	return 1;
+	return 0;
 }
 
-static int mrc_cache_valid(const struct mrc_data_region *region,
-                           const struct mrc_saved_data *cache)
+static const struct cache_region *lookup_region(struct region *r, int type)
 {
-	uint32_t checksum;
-
-	if (cache->signature != MRC_DATA_SIGNATURE)
-		return 0;
-
-	if (cache->size > region->size)
-		return 0;
-
-	checksum = compute_ip_checksum((void *)&cache->data[0], cache->size);
-
-	if (cache->checksum != checksum)
-		return 0;
+	const struct cache_region *cr;
 
-	return 1;
-}
+	cr = lookup_region_type(type);
 
-static const struct mrc_saved_data *
-next_cache_block(const struct mrc_saved_data *cache)
-{
-	uintptr_t next = (uintptr_t)cache;
+	if (cr == NULL) {
+		printk(BIOS_ERR, "MRC: failed to locate region type %d.\n",
+			type);
+		return NULL;
+	}
 
-	next += ALIGN(cache->size + sizeof(*cache), MRC_DATA_ALIGN);
+	if (lookup_region_by_name(cr->name, r) < 0)
+		return NULL;
 
-	return (const struct mrc_saved_data *)next;
+	return cr;
 }
 
-/* Locate the most recently saved MRC data. */
-static int __mrc_cache_get_current(const struct mrc_data_region *region,
-                                   const struct mrc_saved_data **cache,
-                                   uint32_t version)
+static int mrc_header_valid(struct region_device *rdev, struct mrc_metadata *md)
 {
-	const struct mrc_saved_data *msd;
-	const struct mrc_saved_data *verified_cache;
-	int slot = 0;
+	uint16_t checksum;
+	uint16_t checksum_result;
+	size_t size;
 
-	msd = region->base;
-
-	verified_cache = NULL;
+	if (rdev_readat(rdev, md, 0, sizeof(*md)) < 0) {
+		printk(BIOS_ERR, "MRC: couldn't read metadata\n");
+		return -1;
+	}
 
-	while (mrc_cache_in_region(region, msd) &&
-	       mrc_cache_valid(region, msd)) {
-		verified_cache = msd;
-		msd = next_cache_block(msd);
-		slot++;
+	if (md->signature != MRC_DATA_SIGNATURE) {
+		printk(BIOS_ERR, "MRC: invalid header signature\n");
+		return -1;
 	}
 
-	/*
-	 * Update pointer to the most recently saved MRC data before returning
-	 * any error. This ensures that the caller can use next available slot
-	 * if required.
-	 */
-	*cache = verified_cache;
+	/* Compute checksum over header with 0 as the value. */
+	checksum = md->header_checksum;
+	md->header_checksum = 0;
+	checksum_result = compute_ip_checksum(md, sizeof(*md));
 
-	if (verified_cache == NULL)
+	if (checksum != checksum_result) {
+		printk(BIOS_ERR, "MRC: header checksum mismatch: %x vs %x\n",
+			checksum, checksum_result);
 		return -1;
+	}
+
+	/* Put back original. */
+	md->header_checksum = checksum;
 
-	if (verified_cache->version != version) {
-		printk(BIOS_DEBUG, "MRC: cache version mismatch: %x vs %x\n",
-			verified_cache->version, version);
+	/* Re-size the region device according to the metadata as a region_file
+	 * does block allocation. */
+	size = sizeof(*md) + md->data_size;
+	if (rdev_chain(rdev, rdev, 0, size) < 0) {
+		printk(BIOS_ERR, "MRC: size exceeds rdev size: %zx vs %zx\n",
+			size, region_device_sz(rdev));
 		return -1;
 	}
 
-	printk(BIOS_DEBUG, "MRC: cache slot %d @ %p\n", slot-1, verified_cache);
-
 	return 0;
 }
 
-int mrc_cache_get_current_from_region(const struct mrc_saved_data **cache,
-				      uint32_t version,
-				      const char *region_name)
+static int mrc_data_valid(const struct region_device *rdev,
+				const struct mrc_metadata *md)
 {
-	struct mrc_data_region region;
-
-	if (!region_name) {
-		printk(BIOS_ERR, "MRC: Requires memory retraining.\n");
+	void *data;
+	uint16_t checksum;
+	const size_t md_size = sizeof(*md);
+	const size_t data_size = md->data_size;
+
+	data = rdev_mmap(rdev, md_size, data_size);
+	if (data == NULL) {
+		printk(BIOS_ERR, "MRC: mmap failure on data verification.\n");
 		return -1;
 	}
 
-	printk(BIOS_ERR, "MRC: Using data from %s\n", region_name);
-
-	if (mrc_cache_get_region(region_name, &region) < 0) {
-		printk(BIOS_ERR, "MRC: Region %s not found. "
-		       "Requires memory retraining.\n", region_name);
-		return -1;
-	}
+	checksum = compute_ip_checksum(data, data_size);
 
-	if (__mrc_cache_get_current(&region, cache, version) < 0) {
-		printk(BIOS_ERR, "MRC: Valid slot not found in %s."
-		       "Requires memory retraining.\n", region_name);
+	rdev_munmap(rdev, data);
+	if (md->data_checksum != checksum) {
+		printk(BIOS_ERR, "MRC: data checksum mismatch: %x vs %x\n",
+			md->data_checksum, checksum);
 		return -1;
 	}
 
 	return 0;
 }
 
-int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache,
-				       uint32_t version)
-{
-	return mrc_cache_get_current_from_region(cache, version,
-						 DEFAULT_MRC_CACHE);
-}
-
-int mrc_cache_get_current(const struct mrc_saved_data **cache)
+static int mrc_cache_latest(const char *name,
+				const struct region_device *backing_rdev,
+				struct mrc_metadata *md,
+				struct region_file *cache_file,
+				struct region_device *rdev,
+				bool fail_bad_data)
 {
-	return mrc_cache_get_current_with_version(cache, 0);
-}
-
-int mrc_cache_get_vardata(const struct mrc_saved_data **cache, uint32_t version)
-{
-	struct mrc_data_region region;
-
-	if (mrc_cache_get_region(VARIABLE_MRC_CACHE, &region) < 0)
+	/* Init and obtain a handle to the file data. */
+	if (region_file_init(cache_file, backing_rdev) < 0) {
+		printk(BIOS_ERR, "MRC: region file invalid in '%s'\n", name);
 		return -1;
+	}
 
-	return __mrc_cache_get_current(&region, cache, version);
-}
-
-/* Fill in mrc_saved_data structure with payload. */
-static void mrc_cache_fill(struct mrc_saved_data *cache, const void *data,
-                           size_t size, uint32_t version)
-{
-	cache->signature = MRC_DATA_SIGNATURE;
-	cache->size = size;
-	cache->version = version;
-	memcpy(&cache->data[0], data, size);
-	cache->checksum = compute_ip_checksum((void *)&cache->data[0],
-	                                      cache->size);
-}
-
-static int _mrc_stash_data(const void *data, size_t size, uint32_t version,
-			    uint32_t cbmem_id)
-{
-	int cbmem_size;
-	struct mrc_saved_data *cache;
-
-	cbmem_size = sizeof(*cache) + ALIGN(size, 16);
-
-	cache = cbmem_add(cbmem_id, cbmem_size);
+	/* Provide a 0 sized region_device from here on out so the caller
+	 * has a valid yet unusable region_device. */
+	rdev_chain(rdev, backing_rdev, 0, 0);
 
-	if (cache == NULL) {
-		printk(BIOS_ERR, "MRC: No space in cbmem for training data.\n");
-		return -1;
+	/* No data to return. */
+	if (region_file_data(cache_file, rdev) < 0) {
+		printk(BIOS_ERR, "MRC: no data in '%s'\n", name);
+		return fail_bad_data ? -1 : 0;
 	}
 
-	/* Clear alignment padding bytes at end of data. */
-	memset(&cache->data[size], 0, cbmem_size - size - sizeof(*cache));
-
-	printk(BIOS_DEBUG, "MRC: Relocate data from %p to %p (%zu bytes)\n",
-	       data, cache, size);
+	/* 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);
+		return fail_bad_data ? -1 : 0;
+	}
 
-	mrc_cache_fill(cache, data, size, version);
+	/* Validate Data */
+	if (mrc_data_valid(rdev, md) < 0) {
+		printk(BIOS_ERR, "MRC: invalid data in '%s'\n", name);
+		return fail_bad_data ? -1 : 0;
+	}
 
 	return 0;
 }
 
-int mrc_cache_stash_data_with_version(const void *data, size_t size,
-					uint32_t version)
+int mrc_cache_get_current(int type, uint32_t version,
+				struct region_device *rdev)
 {
-	return _mrc_stash_data(data, size, version, CBMEM_ID_MRCDATA);
-}
+	const struct cache_region *cr;
+	struct region region;
+	struct region_device read_rdev;
+	struct region_file cache_file;
+	struct mrc_metadata md;
+	size_t data_size;
+	const size_t md_size = sizeof(md);
+	const bool fail_bad_data = true;
+
+	cr = lookup_region(&region, type);
+
+	if (cr == NULL)
+		return -1;
 
-int mrc_cache_stash_vardata(const void *data, size_t size, uint32_t version)
-{
-	return _mrc_stash_data(data, size, version, CBMEM_ID_VAR_MRCDATA);
-}
+	if (boot_device_ro_subregion(&region, &read_rdev) < 0)
+		return -1;
 
+	if (mrc_cache_latest(cr->name, &read_rdev, &md, &cache_file, rdev,
+		fail_bad_data) < 0)
+		return -1;
 
-int mrc_cache_stash_data(const void *data, size_t size)
-{
-	return mrc_cache_stash_data_with_version(data, size, 0);
+	if (version != md.version) {
+		printk(BIOS_INFO, "MRC: version mismatch: %x vs %x\n",
+			md.version, version);
+		return -1;
+	}
+
+	/* Re-size rdev to only contain the data. i.e. remove metadata. */
+	data_size = md.data_size;
+	return rdev_chain(rdev, rdev, md_size, data_size);
 }
 
-static int mrc_slot_valid(const struct mrc_data_region *region,
-                          const struct mrc_saved_data *slot,
-                          const struct mrc_saved_data *to_save)
+static bool mrc_cache_needs_update(const struct region_device *rdev,
+				const struct cbmem_entry *to_be_updated)
 {
-	uintptr_t region_begin;
-	uintptr_t region_end;
-	uintptr_t slot_end;
-	uintptr_t slot_begin;
-	uint32_t size;
-
-	region_begin = (uintptr_t)region->base;
-	region_end = region_begin + region->size;
-	slot_begin = (uintptr_t)slot;
-	size = to_save->size + sizeof(*to_save);
-	slot_end = slot_begin + size;
-
-	if (slot_begin < region_begin || slot_begin >= region_end)
-		return 0;
+	void *mapping;
+	size_t size = region_device_sz(rdev);
+	bool need_update = false;
 
-	if (size > region->size)
-		return 0;
-
-	if (slot_end > region_end || slot_end < region_begin)
-		return 0;
-
-	if (!nvm_is_erased(slot, size))
-		return 0;
+	if (cbmem_entry_size(to_be_updated) != size)
+		return true;
 
-	return 1;
-}
+	mapping = rdev_mmap_full(rdev);
 
-static const struct mrc_saved_data *
-mrc_cache_next_slot(const struct mrc_data_region *region,
-                    const struct mrc_saved_data *current_slot)
-{
-	const struct mrc_saved_data *next_slot;
+	if (memcmp(cbmem_entry_start(to_be_updated), mapping, size))
+		need_update = true;
 
-	if (current_slot == NULL) {
-		next_slot = region->base;
-	} else {
-		next_slot = next_cache_block(current_slot);
-	}
+	rdev_munmap(rdev, mapping);
 
-	return next_slot;
+	return need_update;
 }
 
 static void log_event_cache_update(uint8_t slot, enum result res)
@@ -363,9 +362,7 @@ static void log_event_cache_update(uint8_t slot, enum result res)
 
 	/* Filter through interesting events only */
 	switch (res) {
-	case WRITE_FAILURE:				/* fall-through */
-	case ERASE_FAILURE:				/* fall-through */
-	case OTHER_FAILURE:				/* fall-through */
+	case UPDATE_FAILURE:
 		event.status = ELOG_MEM_CACHE_UPDATE_STATUS_FAIL;
 		break;
 	case UPDATE_SUCCESS:
@@ -379,63 +376,95 @@ static void log_event_cache_update(uint8_t slot, enum result res)
 		printk(BIOS_ERR, "Failed to log mem cache update event.\n");
 }
 
-static int update_mrc_cache_type(uint32_t cbmem_id, const char *region_name)
+/* During ramstage this code purposefully uses incoherent transactions between
+ * read and write. The read assumes a memory-mapped boot device that can be used
+ * to quickly locate and compare the up-to-date data. However, when an update
+ * is required it uses the writeable region access to perform the update. */
+static void update_mrc_cache_by_type(int type)
 {
-	const struct mrc_saved_data *current_boot;
-	const struct mrc_saved_data *current_saved;
-	const struct mrc_saved_data *next_slot;
-	struct mrc_data_region region;
-	int res;
+	const struct cache_region *cr;
+	struct region region;
+	struct region_device read_rdev;
+	struct region_device write_rdev;
+	struct region_file cache_file;
+	struct mrc_metadata md;
+	const struct cbmem_entry *to_be_updated;
+	struct incoherent_rdev backing_irdev;
+	const struct region_device *backing_rdev;
+	struct region_device latest_rdev;
+	const bool fail_bad_data = false;
+
+	cr = lookup_region(&region, type);
+
+	if (cr == NULL)
+		return;
 
-	printk(BIOS_DEBUG, "MRC: Updating cache data.\n");
+	to_be_updated = cbmem_entry_find(cr->cbmem_id);
+	if (to_be_updated == NULL) {
+		printk(BIOS_ERR, "MRC: No data in cbmem for '%s'.\n",
+			cr->name);
+		return;
+	}
 
-	printk(BIOS_ERR, "MRC: Cache region selected - %s\n", region_name);
+	printk(BIOS_DEBUG, "MRC: Checking cached data update for '%s'.\n",
+		cr->name);
 
-	if (mrc_cache_get_region(region_name, &region)) {
-		printk(BIOS_ERR, "MRC: Could not obtain cache region.\n");
-		return OTHER_FAILURE;
-	}
+	if (boot_device_ro_subregion(&region, &read_rdev) < 0)
+		return;
 
-	current_boot = cbmem_find(cbmem_id);
-	if (!current_boot) {
-		printk(BIOS_ERR, "MRC: No cache in cbmem.\n");
-		return OTHER_FAILURE;
-	}
+	if (boot_device_rw_subregion(&region, &write_rdev) < 0)
+		return;
+
+	backing_rdev = incoherent_rdev_init(&backing_irdev, &region, &read_rdev,
+						&write_rdev);
 
-	if (!mrc_cache_valid(&region, current_boot)) {
-		printk(BIOS_ERR, "MRC: Cache data in cbmem invalid.\n");
-		return OTHER_FAILURE;
+	if (backing_rdev == NULL)
+		return;
+
+	if (mrc_cache_latest(cr->name, backing_rdev, &md, &cache_file,
+		&latest_rdev, fail_bad_data) < 0)
+		return;
+
+	if (!mrc_cache_needs_update(&latest_rdev, to_be_updated)) {
+		log_event_cache_update(cr->elog_slot, ALREADY_UPTODATE);
+		return;
 	}
 
-	current_saved = NULL;
+	printk(BIOS_DEBUG, "MRC: cache data '%s' needs update.\n", cr->name);
+
+	if (region_file_update_data(&cache_file,
+		cbmem_entry_start(to_be_updated),
+		cbmem_entry_size(to_be_updated)) < 0)
+		log_event_cache_update(cr->elog_slot, UPDATE_FAILURE);
+	else
+		log_event_cache_update(cr->elog_slot, UPDATE_SUCCESS);
+}
+
+/* Protect mrc region with a Protected Range Register */
+static int protect_mrc_cache(const char *name)
+{
+	struct region region;
+
+	if (!IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT))
+		return 0;
+
+	if (lookup_region_by_name(name, &region) < 0) {
+		printk(BIOS_ERR, "MRC: Could not find region '%s'\n", name);
+		return -1;
+	}
 
-	if (!__mrc_cache_get_current(&region, &current_saved,
-					current_boot->version)) {
-		if (current_saved->size == current_boot->size &&
-		    !memcmp(&current_saved->data[0], &current_boot->data[0],
-		            current_saved->size)) {
-			printk(BIOS_DEBUG, "MRC: Cache up to date.\n");
-			return ALREADY_UPTODATE;
-		}
+	if (nvm_is_write_protected() <= 0) {
+		printk(BIOS_INFO, "MRC: NOT enabling PRR for '%s'.\n", name);
+		return 0;
 	}
 
-	next_slot = mrc_cache_next_slot(&region, current_saved);
-
-	if (!mrc_slot_valid(&region, next_slot, current_boot)) {
-		printk(BIOS_DEBUG, "MRC: Slot @ %p is invalid.\n", next_slot);
-		if (!nvm_is_erased(region.base, region.size)) {
-			if (nvm_erase(region.base, region.size) < 0) {
-				printk(BIOS_DEBUG, "MRC: Failure erasing "
-				       "region %s.\n", region_name);
-				return ERASE_FAILURE;
-			}
-		}
-		next_slot = region.base;
+	if (nvm_protect(&region) < 0) {
+		printk(BIOS_ERR, "MRC: ERROR setting PRR for '%s'.\n", name);
+		return -1;
 	}
 
-	res = nvm_write((void *)next_slot, current_boot, current_boot->size
-			 + sizeof(*current_boot));
-	return res < 0 ? WRITE_FAILURE : UPDATE_SUCCESS;
+	printk(BIOS_INFO, "MRC: Enabled Protected Range on '%s'.\n", name);
+	return 0;
 }
 
 static void protect_mrc_region(void)
@@ -459,30 +488,10 @@ static void protect_mrc_region(void)
 
 static void update_mrc_cache(void *unused)
 {
-	uint8_t slot;
-	const char *region_name;
-	enum result res;
-
-	/* First update either recovery or default cache */
-	if (vboot_recovery_mode_enabled() &&
-	    IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE)) {
-		region_name = RECOVERY_MRC_CACHE;
-		slot = ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY;
-	} else {
-		region_name = DEFAULT_MRC_CACHE;
-		slot = ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL;
-	}
+	update_mrc_cache_by_type(MRC_TRAINING_DATA);
 
-	res = update_mrc_cache_type(CBMEM_ID_MRCDATA, region_name);
-	log_event_cache_update(slot, res);
-
-	/* Next update variable cache if in use */
-	if (IS_ENABLED(CONFIG_MRC_SETTINGS_VARIABLE_DATA)) {
-		res = update_mrc_cache_type(CBMEM_ID_VAR_MRCDATA,
-					    VARIABLE_MRC_CACHE);
-		log_event_cache_update(ELOG_MEM_CACHE_UPDATE_SLOT_VARIABLE,
-					res);
-	}
+	if (IS_ENABLED(CONFIG_MRC_SETTINGS_VARIABLE_DATA))
+		update_mrc_cache_by_type(MRC_VARIABLE_DATA);
 
 	protect_mrc_region();
 }
diff --git a/src/soc/intel/common/mrc_cache.h b/src/soc/intel/common/mrc_cache.h
index 6414accfcf..4511fc3016 100644
--- a/src/soc/intel/common/mrc_cache.h
+++ b/src/soc/intel/common/mrc_cache.h
@@ -19,34 +19,25 @@
 #include <stddef.h>
 #include <stdint.h>
 
-#define DEFAULT_MRC_CACHE	"RW_MRC_CACHE"
-#define VARIABLE_MRC_CACHE	"RW_VAR_MRC_CACHE"
-#define RECOVERY_MRC_CACHE	"RECOVERY_MRC_CACHE"
-#define UNIFIED_MRC_CACHE	"UNIFIED_MRC_CACHE"
+enum {
+	MRC_TRAINING_DATA,
+	MRC_VARIABLE_DATA,
+};
 
-/* Wrapper object to save MRC information. */
-struct mrc_saved_data {
-	uint32_t signature;
-	uint32_t size;
-	uint32_t checksum;
-	uint32_t version;
-	uint8_t  data[0];
-} __attribute__((packed));
-
-/* Locate the most recently saved MRC data. */
-int mrc_cache_get_current(const struct mrc_saved_data **cache);
-int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache,
-					uint32_t version);
-int mrc_cache_get_vardata(const struct mrc_saved_data **cache,
-			    uint32_t version);
-int mrc_cache_get_current_from_region(const struct mrc_saved_data **cache,
-				      uint32_t version,
-				      const char *region_name);
+/*
+ * It's up to the caller to decide when to retrieve and stash data. There is
+ * differentiation on recovery mode CONFIG_HAS_RECOVERY_MRC_CACHE, but that's
+ * only for locating where to retrieve and save the data. If a platform doesn't
+ * want to update the data then it shouldn't stash the data for saving.
+ * Similarly, if the platform doesn't need the data for booting because of a
+ * policy don't request the data.
+ */
 
-/* Stash the resulting MRC data to be saved in non-volatile storage later. */
-int mrc_cache_stash_data(const void *data, size_t size);
-int mrc_cache_stash_data_with_version(const void *data, size_t size,
-					uint32_t version);
-int mrc_cache_stash_vardata(const void *data, size_t size, uint32_t version);
+/* Get and stash data for saving provided the type passed in. The functions
+ * return < 0 on error, 0 on success. */
+int mrc_cache_get_current(int type, uint32_t version,
+				struct region_device *rdev);
+int mrc_cache_stash_data(int type, uint32_t version, const void *data,
+			size_t size);
 
 #endif /* _COMMON_MRC_CACHE_H_ */
diff --git a/src/soc/intel/common/nvm.c b/src/soc/intel/common/nvm.c
index 99a1fbfb15..b7e0f4e7ac 100644
--- a/src/soc/intel/common/nvm.c
+++ b/src/soc/intel/common/nvm.c
@@ -24,113 +24,43 @@
 #include "nvm.h"
 #include "spi_flash.h"
 
-/* This module assumes the flash is memory mapped just below 4GiB in the
- * address space for reading. Also this module assumes an area it erased
- * when all bytes read as all 0xff's. */
-
-static struct spi_flash *flash;
-
-static int nvm_init(void)
+/* Read flash status register to determine if write protect is active */
+int nvm_is_write_protected(void)
 {
-	if (flash != NULL)
-		return 0;
+	u8 sr1;
+	u8 wp_gpio;
+	u8 wp_spi;
 
-	spi_init();
-	flash = spi_flash_probe(0, 0);
-	if (!flash) {
-		printk(BIOS_DEBUG, "Could not find SPI device\n");
-		return -1;
-	}
-
-	return 0;
-}
+	if (!IS_ENABLED(CONFIG_CHROMEOS))
+		return 0;
 
-/*
- * Convert memory mapped pointer to flash offset.
- *
- * This is weak because not every platforms memory-maps the NVM media in the
- * same manner. This is a stop-gap solution.
- *
- * The root of the problem is that users of this API work in memory space for
- * both reads and writes, but erase and write must be done in flash space. This
- * also only works when the media is memory-mapped, which is no longer
- * universally true. The long-term approach should be to rewrite this and its
- * users to work in flash space, while using rdev_read() instead of rdev_mmap().
- */
-__attribute__((weak))
-uint32_t nvm_mmio_to_flash_offset(void *p)
-{
-	return CONFIG_ROM_SIZE + (uintptr_t)p;
-}
+	if (!IS_ENABLED(CONFIG_BOOT_DEVICE_SPI_FLASH))
+		return 0;
 
-int nvm_is_erased(const void *start, size_t size)
-{
-	const uint8_t *cur = start;
-	const uint8_t erased_value = 0xff;
+	/* Read Write Protect GPIO if available */
+	wp_gpio = get_write_protect_state();
 
-	while (size > 0) {
-		if (*cur != erased_value)
-			return 0;
-		cur++;
-		size--;
+	/* Read Status Register 1 */
+	if (spi_flash_status(boot_device_spi_flash(), &sr1) < 0) {
+		printk(BIOS_ERR, "Failed to read SPI status register 1\n");
+		return -1;
 	}
-	return 1;
-}
+	wp_spi = !!(sr1 & 0x80);
 
-int nvm_erase(void *start, size_t size)
-{
-	if (nvm_init() < 0)
-		return -1;
-	return spi_flash_erase(flash, nvm_mmio_to_flash_offset(start), size);
-}
+	printk(BIOS_DEBUG, "SPI flash protection: WPSW=%d SRP0=%d\n",
+		wp_gpio, wp_spi);
 
-/* Write data to NVM. Returns 0 on success < 0 on error.  */
-int nvm_write(void *start, const void *data, size_t size)
-{
-	if (nvm_init() < 0)
-		return -1;
-	return spi_flash_write(flash, nvm_mmio_to_flash_offset(start), size,
-				data);
+	return wp_gpio && wp_spi;
 }
 
-/* Read flash status register to determine if write protect is active */
-int nvm_is_write_protected(void)
+/* Apply protection to a range of flash */
+int nvm_protect(const struct region *r)
 {
-	if (nvm_init() < 0)
-		return -1;
-
-	if (IS_ENABLED(CONFIG_CHROMEOS)) {
-		u8 sr1;
-		u8 wp_gpio;
-		u8 wp_spi;
-
-		/* Read Write Protect GPIO if available */
-		wp_gpio = get_write_protect_state();
-
-		/* Read Status Register 1 */
-		if (spi_flash_status(flash, &sr1) < 0) {
-			printk(BIOS_ERR,
-				"Failed to read SPI status register 1\n");
-			return -1;
-		}
-		wp_spi = !!(sr1 & 0x80);
-
-		printk(BIOS_DEBUG, "SPI flash protection: WPSW=%d SRP0=%d\n",
-		       wp_gpio, wp_spi);
+	if (!IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT))
+		return 0;
 
-		return wp_gpio && wp_spi;
-	}
-	return 0;
-}
+	if (!IS_ENABLED(CONFIG_BOOT_DEVICE_SPI_FLASH))
+		return 0;
 
-/* Apply protection to a range of flash */
-int nvm_protect(void *start, size_t size)
-{
-#if IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT)
-	if (nvm_init() < 0)
-		return -1;
-	return spi_flash_protect(nvm_mmio_to_flash_offset(start), size);
-#else
-	return -1;
-#endif
+	return spi_flash_protect(region_offset(r), region_sz(r));
 }
diff --git a/src/soc/intel/common/nvm.h b/src/soc/intel/common/nvm.h
index 074a2ce406..1e32ed8637 100644
--- a/src/soc/intel/common/nvm.h
+++ b/src/soc/intel/common/nvm.h
@@ -16,24 +16,12 @@
 #ifndef _COMMON_NVM_H_
 #define _COMMON_NVM_H_
 
-#include <stddef.h>
-
-/* Determine if area is erased. returns 1 if erased. 0 otherwise. */
-int nvm_is_erased(const void *start, size_t size);
-
-/* Erase region according to start and size. Returns < 0 on error else 0. */
-int nvm_erase(void *start, size_t size);
-
-/* Write data to NVM. Returns 0 on success < 0 on error.  */
-int nvm_write(void *start, const void *data, size_t size);
+#include <commonlib/region.h>
 
 /* Determine if flash device is write protected */
 int nvm_is_write_protected(void);
 
 /* Apply protection to a range of flash */
-int nvm_protect(void *start, size_t size);
-
-/* Map MMIO address to actual address in flash */
-uint32_t nvm_mmio_to_flash_offset(void *p);
+int nvm_protect(const struct region *region);
 
 #endif /* _COMMON_NVM_H_ */