Patch set updated for coreboot: mrc: Add support for separate training cache in recovery mode

[Furquan Shaikh]

Furquan Shaikh ([e-mail]) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17242

-gerrit

commit 4385227f2e8869008fa70e32751b09019be1fddb
Author: Furquan Shaikh 
Date:   Sat Nov 5 23:57:02 2016 -0700

mrc: Add support for separate training cache in recovery mode

1. Re-factor MRC cache driver to properly select RW_MRC_CACHE or
    RECOVERY_MRC_CACHE based on the boot mode.
     - If normal mode boot, use RW_MRC_CACHE, if available.
     - If recovery mode boot:
        - Retrain memory if RECOVERY_MRC_CACHE not present, or recovery is
        requested explicity with retrain memory request.
        - Use RECOVERY_MRC_CACHE otherwise.
    2. Protect RECOVERY_MRC_CACHE in non-recovery mode and recovery mode.
    3. Update training data in appropriate cache:
     - Use RW_MRC_CACHE if normal mode.
     - Use RECOVERY_MRC_CACHE if present in recovery mode. Else use
     RW_MRC_CACHE.
    4. Add proper debug logs to indicate which training data cache is used
    at any point.

BUG=chrome-os-partner:59352
    BRANCH=None
    TEST=Verified that correct cache is used in both normal and recovery
    mode on reef.

Change-Id: Ie79737a1450bd1ff71543e44a5a3e16950e70fb3
    Signed-off-by: Furquan Shaikh 
---
 src/drivers/intel/fsp2_0/memory_init.c |  40 +++++----
 src/include/elog.h                     |   1 +
 src/soc/intel/common/Kconfig           |   4 +
 src/soc/intel/common/mrc_cache.c       | 156 ++++++++++++++++++++++-----------
 src/soc/intel/common/mrc_cache.h       |   6 ++
 5 files changed, 137 insertions(+), 70 deletions(-)

diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index d0a22ce..3ea1897 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -55,13 +55,6 @@ static void save_memory_training_data(bool s3wake, uint32_t fsp_version)
    		printk(BIOS_ERR, "Failed to stash MRC data\n");
 }

-/*
- * On every trip to recovery, newly generated MRC data is stored with this
- * version since it is not expected to be a legit version. This ensures that on
- * next normal boot, memory re-training occurs and new MRC data is stored.
- */
-#define MRC_DEAD_VERSION		(0xdeaddead)
-
 static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version)
 {
    struct range_entry fsp_mem;
@@ -89,10 +82,6 @@ static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version)
    	(uintptr_t)cbmem_find(CBMEM_ID_FSP_RESERVED_MEMORY))
    	die("Failed to accommodate FSP reserved memory request!\n");

-	/* Now that CBMEM is up, save the list so ramstage can use it */
-	if (vboot_recovery_mode_enabled())
-		fsp_version = MRC_DEAD_VERSION;
-
    save_memory_training_data(s3wake, fsp_version);

/* Create romstage handof information */
@@ -103,26 +92,39 @@ static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version)
    	printk(BIOS_SPEW, "Romstage handoff structure not added!\n");
 }

+static const char *mrc_cache_get_region_name(void)
+{
+	/* In normal mode, always use DEFAULT_MRC_CACHE */
+	if (!vboot_recovery_mode_enabled())
+		return DEFAULT_MRC_CACHE;
+
+	/*
+	 * In recovery mode, force retraining by returning NULL if:
+	 * 1. Recovery cache is not supported, or
+	 * 2. Memory retrain switch is set.
+	 */
+	if (!IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE) ||
+	    vboot_recovery_mode_memory_retrain())
+		return NULL;
+
+	return RECOVERY_MRC_CACHE;
+}
+
 static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, bool s3wake,
    			uint32_t fsp_version)
 {
    const struct mrc_saved_data *mrc_cache;
+	const char *name;

arch_upd->NvsBufferPtr = NULL;

if (!IS_ENABLED(CONFIG_CACHE_MRC_SETTINGS))
    	return;

-	/* Don't use saved training data when recovery mode is enabled. */
-	if (vboot_recovery_mode_enabled()) {
-		printk(BIOS_SPEW, "Recovery mode. Not using MRC cache.\n");
-		return;
-	}
+	name = mrc_cache_get_region_name();

-	if (mrc_cache_get_current_with_version(&mrc_cache, fsp_version)) {
-		printk(BIOS_SPEW, "MRC cache was not found\n");
+	if (mrc_cache_get_current_from_region(&mrc_cache, fsp_version, name))
    	return;
-	}

/* MRC cache found */
    arch_upd->NvsBufferPtr = (void *)mrc_cache->data;
diff --git a/src/include/elog.h b/src/include/elog.h
index 7fdcda7..fec9d09 100644
--- a/src/include/elog.h
+++ b/src/include/elog.h
@@ -152,6 +152,7 @@ struct elog_event_data_me_extended {
 /* Memory Cache Update */
 #define ELOG_TYPE_MEM_CACHE_UPDATE        0xaa
 #define  ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL    0
+#define  ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY  1
 #define  ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS 0
 #define  ELOG_MEM_CACHE_UPDATE_STATUS_FAIL    1
 struct elog_event_mem_cache_update {
diff --git a/src/soc/intel/common/Kconfig b/src/soc/intel/common/Kconfig
index affec55..5d8bf66 100644
--- a/src/soc/intel/common/Kconfig
+++ b/src/soc/intel/common/Kconfig
@@ -27,6 +27,10 @@ config MRC_SETTINGS_PROTECT
    bool "Enable protection on MRC settings"
    default n

+config HAS_RECOVERY_MRC_CACHE
+	bool
+	default n
+
 endif # CACHE_MRC_SETTINGS

config DISPLAY_MTRRS
diff --git a/src/soc/intel/common/mrc_cache.c b/src/soc/intel/common/mrc_cache.c
index bef3ba2..983cc87 100644
--- a/src/soc/intel/common/mrc_cache.c
+++ b/src/soc/intel/common/mrc_cache.c
@@ -36,12 +36,16 @@ struct mrc_data_region {
 };

/* common code */
-static int mrc_cache_get_region(struct mrc_data_region *region)
+static int mrc_cache_get_region(const char *name,
+				struct mrc_data_region *region)
 {
    bool located_by_fmap = true;
    struct region_device rdev;

-	if (fmap_locate_area_as_rdev("RW_MRC_CACHE", &rdev))
+	region->base = NULL;
+	region->size = 0;
+
+	if (fmap_locate_area_as_rdev(name, &rdev))
    	located_by_fmap =  false;

/* CHROMEOS builds must get their MRC cache from FMAP. */
@@ -62,6 +66,38 @@ static int mrc_cache_get_region(struct mrc_data_region *region)
    return 0;
 }

+/* Protect mrc region with a Protected Range Register */
+static int __protect_mrc_cache(const struct mrc_data_region *region,
+			       const char *name)
+{
+	if (!IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT))
+		return 0;
+
+	if (nvm_is_write_protected() <= 0) {
+		printk(BIOS_INFO, "MRC: NOT enabling PRR for %s.\n", name);
+		return 1;
+	}
+
+	if (nvm_protect(region->base, region->size) < 0) {
+		printk(BIOS_ERR, "MRC: ERROR setting PRR for %s.\n", name);
+		return -1;
+	}
+
+	printk(BIOS_INFO, "MRC: Enabled Protected Range on %s.\n", name);
+	return 0;
+}
+
+static int protect_mrc_cache(const char *name)
+{
+	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;
+	}
+
+	return __protect_mrc_cache(&region, name);
+}
+
 static int mrc_cache_in_region(const struct mrc_data_region *region,
                                const struct mrc_saved_data *cache)
 {
@@ -149,31 +185,57 @@ static int __mrc_cache_get_current(const struct mrc_data_region *region,
    	return -1;

if (verified_cache->version != version) {
-		printk(BIOS_DEBUG, "MRC cache version mismatch: %x vs %x\n",
+		printk(BIOS_DEBUG, "MRC: cache version mismatch: %x vs %x\n",
    		verified_cache->version, version);
    	return -1;
    }

-	printk(BIOS_DEBUG, "MRC cache slot %d @ %p\n", slot-1, verified_cache);
+	printk(BIOS_DEBUG, "MRC: cache slot %d @ %p\n", slot-1, verified_cache);

return 0;
 }

-int mrc_cache_get_current_with_version(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)
 {
    struct mrc_data_region region;

-	if (mrc_cache_get_region(&region) < 0)
+	if (!region_name) {
+		printk(BIOS_ERR, "MRC: Requires memory retraining.\n");
    	return -1;
+	}

-	return __mrc_cache_get_current(&region, cache, version);
+	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;
+	}
+
+	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);
+		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)
 {
    return mrc_cache_get_current_with_version(cache, 0);
 }
+
+
 /* 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)
@@ -197,14 +259,14 @@ int mrc_cache_stash_data_with_version(const void *data, size_t size,
    cache = cbmem_add(CBMEM_ID_MRCDATA, cbmem_size);

if (cache == NULL) {
-		printk(BIOS_ERR, "No space in cbmem for MRC data.\n");
+		printk(BIOS_ERR, "MRC: No space in cbmem for training data.\n");
    	return -1;
    }

/* Clear alignment padding bytes at end of data. */
    memset(&cache->data[size], 0, cbmem_size - size - sizeof(*cache));

-	printk(BIOS_DEBUG, "Relocate MRC DATA from %p to %p (%zu bytes)\n",
+	printk(BIOS_DEBUG, "MRC: Relocate data from %p to %p (%zu bytes)\n",
           data, cache, size);

mrc_cache_fill(cache, data, size, version);
@@ -263,25 +325,6 @@ mrc_cache_next_slot(const struct mrc_data_region *region,
    return next_slot;
 }

-/* Protect RW_MRC_CACHE region with a Protected Range Register */
-static int protect_mrc_cache(const struct mrc_data_region *region)
-{
-#if IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT)
-	if (nvm_is_write_protected() <= 0) {
-		printk(BIOS_INFO, "NOT enabling PRR for RW_MRC_CACHE region\n");
-		return 1;
-	}
-
-	if (nvm_protect(region->base, region->size) < 0) {
-		printk(BIOS_ERR, "ERROR setting PRR for RW_MRC_CACHE region\n");
-		return -1;
-	}
-
-	printk(BIOS_INFO, "Enabled Protected Range on RW_MRC_CACHE region\n");
-#endif
-	return 0;
-}
-
 static void log_event_cache_update(uint8_t slot, uint8_t status)
 {
    const int type = ELOG_TYPE_MEM_CACHE_UPDATE;
@@ -300,23 +343,34 @@ static void update_mrc_cache(void *unused)
    const struct mrc_saved_data *current_saved;
    const struct mrc_saved_data *next_slot;
    struct mrc_data_region region;
+	const char *region_name = DEFAULT_MRC_CACHE;
+	uint8_t slot = ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL;

-	printk(BIOS_DEBUG, "Updating MRC cache data.\n");
+	printk(BIOS_DEBUG, "MRC: Updating cache data.\n");

-	current_boot = cbmem_find(CBMEM_ID_MRCDATA);
-	if (!current_boot) {
-		printk(BIOS_ERR, "No MRC cache in cbmem.\n");
+	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
+		protect_mrc_cache(RECOVERY_MRC_CACHE);
+
+	printk(BIOS_ERR, "MRC: Cache region selected - %s\n", region_name);
+
+	if (mrc_cache_get_region(region_name, &region)) {
+		printk(BIOS_ERR, "MRC: Could not obtain cache region.\n");
    	return;
    }

-	if (mrc_cache_get_region(&region)) {
-		printk(BIOS_ERR, "Could not obtain MRC cache region.\n");
-		return;
+	current_boot = cbmem_find(CBMEM_ID_MRCDATA);
+	if (!current_boot) {
+		printk(BIOS_ERR, "MRC: No cache in cbmem.\n");
+		goto out;
    }

if (!mrc_cache_valid(&region, current_boot)) {
-		printk(BIOS_ERR, "MRC cache data in cbmem invalid.\n");
-		return;
+		printk(BIOS_ERR, "MRC: Cache data in cbmem invalid.\n");
+		goto out;
    }

current_saved = NULL;
@@ -326,20 +380,20 @@ static void update_mrc_cache(void *unused)
    	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");
-			protect_mrc_cache(&region);
-			return;
+			printk(BIOS_DEBUG, "MRC: Cache up to date.\n");
+			goto out;
    	}
    }

next_slot = mrc_cache_next_slot(&region, current_saved);

if (!mrc_slot_valid(&region, next_slot, current_boot)) {
-		printk(BIOS_DEBUG, "Slot @ %p is invalid.\n", next_slot);
+		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, "Failure erasing region.\n");
-				return;
+				printk(BIOS_DEBUG, "MRC: Failure erasing "
+				       "region %s.\n", region_name);
+				goto out;
    		}
    	}
    	next_slot = region.base;
@@ -347,16 +401,16 @@ static void update_mrc_cache(void *unused)

if (nvm_write((void *)next_slot, current_boot,
                   current_boot->size + sizeof(*current_boot))) {
-		printk(BIOS_DEBUG, "Failure writing MRC cache to %p.\n",
-		       next_slot);
-		log_event_cache_update(ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL,
-			ELOG_MEM_CACHE_UPDATE_STATUS_FAIL);
+		printk(BIOS_DEBUG, "MRC: Failure writing MRC cache to %s:%p\n",
+		       region_name, next_slot);
+		log_event_cache_update(slot, ELOG_MEM_CACHE_UPDATE_STATUS_FAIL);
    } else {
-		log_event_cache_update(ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL,
-			ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS);
+		log_event_cache_update(slot,
+				       ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS);
    }

-	protect_mrc_cache(&region);
+out:
+	__protect_mrc_cache(&region, region_name);
 }

BOOT_STATE_INIT_ENTRY(BS_WRITE_TABLES, BS_ON_ENTRY, update_mrc_cache, NULL);
diff --git a/src/soc/intel/common/mrc_cache.h b/src/soc/intel/common/mrc_cache.h
index 9b14c1e..8bbd4ed 100644
--- a/src/soc/intel/common/mrc_cache.h
+++ b/src/soc/intel/common/mrc_cache.h
@@ -19,6 +19,9 @@
 #include <stddef.h>
 #include <stdint.h>

+#define DEFAULT_MRC_CACHE	"RW_MRC_CACHE"
+#define RECOVERY_MRC_CACHE	"RECOVERY_MRC_CACHE"
+
 /* Wrapper object to save MRC information. */
 struct mrc_saved_data {
    uint32_t signature;
@@ -32,6 +35,9 @@ struct mrc_saved_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_current_from_region(const struct mrc_saved_data **cache,
+				      uint32_t version,
+				      const char *region_name);

/* Stash the resulting MRC data to be saved in non-volatile storage later. */
 int mrc_cache_stash_data(const void *data, size_t size);