[coreboot-gerrit] Patch set updated for coreboot: mrc: Add support for separate training cache in recovery mode
Furquan Shaikh (furquan@google.com)
gerrit at coreboot.org
Sun Nov 6 17:59:28 CET 2016
Furquan Shaikh (furquan at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17242
-gerrit
commit 0ec826f8812f676b26c6e45be175e4cd2e74bafd
Author: Furquan Shaikh <furquan at 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 before jumping from RO to RW in 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 at chromium.org>
---
src/drivers/intel/fsp2_0/memory_init.c | 21 +----
src/soc/intel/common/Kconfig | 4 +
src/soc/intel/common/mrc_cache.c | 159 +++++++++++++++++++++++----------
src/soc/intel/common/mrc_cache.h | 3 +
4 files changed, 122 insertions(+), 65 deletions(-)
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index d0a22ce..a561d76 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 */
@@ -113,16 +102,8 @@ static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, bool s3wake,
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");
+ if (mrc_cache_get_current_with_version(&mrc_cache, fsp_version))
return;
- }
-
- if (mrc_cache_get_current_with_version(&mrc_cache, fsp_version)) {
- printk(BIOS_SPEW, "MRC cache was not found\n");
- return;
- }
/* MRC cache found */
arch_upd->NvsBufferPtr = (void *)mrc_cache->data;
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 c123da9..186c556 100644
--- a/src/soc/intel/common/mrc_cache.c
+++ b/src/soc/intel/common/mrc_cache.c
@@ -34,13 +34,37 @@ struct mrc_data_region {
uint32_t size;
};
+/* Protect RW_MRC_CACHE 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)
+ 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);
+#endif
+ return 0;
+}
+
/* 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. */
@@ -61,6 +85,19 @@ static int mrc_cache_get_region(struct mrc_data_region *region)
return 0;
}
+/*
+ * Check if RECOVERY_MRC_CACHE exists on this device.
+ * Returns 1 if region exists, 0 otherwise.
+ */
+static int mrc_recovery_cache_exists(void)
+{
+ if (!IS_ENABLED(CONFIG_CHROMEOS) ||
+ !IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE))
+ return 0;
+
+ return fmap_region_exists(RECOVERY_MRC_CACHE);
+}
+
static int mrc_cache_in_region(const struct mrc_data_region *region,
const struct mrc_saved_data *cache)
{
@@ -148,22 +185,65 @@ 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;
}
+static const char *mrc_cache_get_region_name(void)
+{
+ struct mrc_data_region region;
+
+ /*
+ * In recovery mode:
+ * 1. If recovery cache does not exist, retrain memory.
+ * 2. Else if memory retrain switch is set, retrain memory.
+ * 3. Else use RECOVERY_MRC_CACHE.
+ */
+ if (vboot_recovery_mode_enabled()) {
+ if (!mrc_recovery_cache_exists())
+ return NULL;
+
+ if (vboot_recovery_mode_memory_retrain())
+ return NULL;
+
+ return RECOVERY_MRC_CACHE;
+ }
+
+ /*
+ * In normal mode:
+ * 1. If recovery mrc cache exists, protect it.
+ * 2. Use DEFAULT_MRC_CACHE.
+ */
+ if (mrc_recovery_cache_exists() &&
+ !mrc_cache_get_region(RECOVERY_MRC_CACHE, ®ion)) {
+ printk(BIOS_ERR, "MRC: Protecting recovery cache.\n");
+ protect_mrc_cache(®ion, RECOVERY_MRC_CACHE);
+ }
+
+ return DEFAULT_MRC_CACHE;
+}
+
int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache,
- uint32_t version)
+ uint32_t version)
{
struct mrc_data_region region;
+ const char *region_name = NULL;
+
+ region_name = mrc_cache_get_region_name();
+ 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(®ion) < 0)
+ if (mrc_cache_get_region(region_name, ®ion) < 0)
return -1;
return __mrc_cache_get_current(®ion, cache, version);
@@ -196,14 +276,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);
@@ -262,48 +342,35 @@ 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 update_mrc_cache(void *unused)
{
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;
+ const char *region_name = DEFAULT_MRC_CACHE;
- 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() && mrc_recovery_cache_exists())
+ region_name = RECOVERY_MRC_CACHE;
+
+ 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;
}
- if (mrc_cache_get_region(®ion)) {
- 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(®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;
@@ -313,20 +380,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;
@@ -334,10 +401,12 @@ 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);
+ printk(BIOS_DEBUG, "MRC: Failure writing cache to %s(%p).\n",
+ region_name, next_slot);
}
- 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..09ab306 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;
More information about the coreboot-gerrit
mailing list