Patch set updated for coreboot: mrc: Add support for separate training cache in recovery mode
Furquan Shaikh (furquan@google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17242
-gerrit
commit a870f9dd2674c754980312caf4fb49e35c871ef1 Author: Furquan Shaikh furquan@chromium.org 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. 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 furquan@chromium.org --- src/drivers/intel/fsp2_0/memory_init.c | 60 +++++++++---- src/include/elog.h | 1 + src/soc/intel/common/Kconfig | 4 + src/soc/intel/common/mrc_cache.c | 155 ++++++++++++++++++++++----------- src/soc/intel/common/mrc_cache.h | 9 ++ 5 files changed, 159 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..5dc622e 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -14,6 +14,7 @@ #include <arch/io.h> #include <arch/cpu.h> #include <arch/symbols.h> +#include <bootstate.h> #include <cbfs.h> #include <cbmem.h> #include <console/console.h> @@ -55,13 +56,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 +83,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 +93,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; @@ -322,3 +325,22 @@ void fsp_memory_init(bool s3wake)
do_fsp_memory_init(&hdr, s3wake, &memmap); } + +/* Protect RECOVERY_MRC_CACHE in non-recovery mode. */ +static void protect_recovery_cache(void *unused) +{ + /* + * In recovery mode, mrc cache driver will protect recovery mode cache + * if present. + */ + if (vboot_recovery_mode_enabled()) + return; + + if (!IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE)) + return; + + protect_mrc_cache(RECOVERY_MRC_CACHE); +} + +BOOT_STATE_INIT_ENTRY(BS_WRITE_TABLES, BS_ON_ENTRY, protect_recovery_cache, + NULL); 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..28deacf 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; +} + +int protect_mrc_cache(const char *name) +{ + struct mrc_data_region region; + if (mrc_cache_get_region(name, ®ion) < 0) { + printk(BIOS_ERR, "MRC: Could not find region %s\n", name); + return -1; + } + + return __protect_mrc_cache(®ion, 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(®ion) < 0) + if (!region_name) { + printk(BIOS_ERR, "MRC: Requires memory retraining.\n"); + return -1; + } + + printk(BIOS_ERR, "MRC: Using data from %s\n", region_name); + + if (mrc_cache_get_region(region_name, ®ion) < 0) { + printk(BIOS_ERR, "MRC: Region %s not found. " + "Requires memory retraining.\n", region_name); + return -1; + } + + if (__mrc_cache_get_current(®ion, cache, version) < 0) { + printk(BIOS_ERR, "MRC: Valid slot not found in %s." + "Requires memory retraining.\n", region_name); return -1; + } + + return 0; +}
- return __mrc_cache_get_current(®ion, cache, version); +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,33 @@ 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"); - return; + if (vboot_recovery_mode_enabled() && + IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE)) { + region_name = RECOVERY_MRC_CACHE; + slot = ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY; }
- if (mrc_cache_get_region(®ion)) { - printk(BIOS_ERR, "Could not obtain MRC cache region.\n"); + printk(BIOS_ERR, "MRC: Cache region selected - %s\n", region_name); + + if (mrc_cache_get_region(region_name, ®ion)) { + printk(BIOS_ERR, "MRC: Could not obtain 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(®ion, 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 +379,20 @@ static void update_mrc_cache(void *unused) if (current_saved->size == current_boot->size && !memcmp(¤t_saved->data[0], ¤t_boot->data[0], current_saved->size)) { - printk(BIOS_DEBUG, "MRC cache up to date.\n"); - protect_mrc_cache(®ion); - return; + printk(BIOS_DEBUG, "MRC: Cache up to date.\n"); + goto out; } }
next_slot = mrc_cache_next_slot(®ion, current_saved);
if (!mrc_slot_valid(®ion, 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 +400,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(®ion); +out: + __protect_mrc_cache(®ion, 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..79364e4 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,10 +35,16 @@ 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); int mrc_cache_stash_data_with_version(const void *data, size_t size, uint32_t version);
+/* Protect MRC cache stored in region whose name is provided. */ +int protect_mrc_cache(const char *name); + #endif /* _COMMON_MRC_CACHE_H_ */
-
Furquan Shaikh