Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44006 )
Change subject: mrc_cache: Add mrc_cache fetch functions to support non-x86 platforms ......................................................................
mrc_cache: Add mrc_cache fetch functions to support non-x86 platforms
Create two new functions to fetch mrc_cache data (replacing mrc_cache_get_current):
- mrc_cache_load_current: fetches the mrc_cache data and drops it into the given buffer. This is useful for ARM platforms where the mmap operation is very expensive.
- mrc_cache_mmap_leak: fetch the mrc_cache data and puts it into a given buffer. This is useful for platforms where the mmap operation is a no-op (like x86 platforms). As the name mentions, we are not freeing the memory that we allocated with the mmap, so it is the caller's responsibility to do so.
Additionally, we are replacing mrc_cache_latest with mrc_cache_get_latest_slot_info, which does not check the validity of the data when retrieving the current mrc_cache slot. This allows the caller some flexibility in deciding where they want the mrc_cache data stored (either in an mmaped region or at a given address).
BUG=b:150502246 BRANCH=None TEST=Testing on a nami (x86) device: reboot from ec console. Make sure memory training happens. reboot from ec console. Make sure that we don't do training again.
Signed-off-by: Shelley Chen shchen@google.com Change-Id: I259dd4f550719d821bbafa2d445cbae6ea22e988 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44006 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/fsp1_1/romstage.c M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/include/mrc_cache.h M src/northbridge/intel/haswell/raminit.c M src/northbridge/intel/ironlake/raminit.c M src/northbridge/intel/sandybridge/raminit.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/northbridge/intel/x4x/raminit.c M src/soc/amd/common/block/s3/s3_resume.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/baytrail/romstage/raminit.c M src/soc/intel/broadwell/romstage/raminit.c 13 files changed, 198 insertions(+), 127 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/drivers/intel/fsp1_1/romstage.c b/src/drivers/intel/fsp1_1/romstage.c index 46df1c8..5a59c50 100644 --- a/src/drivers/intel/fsp1_1/romstage.c +++ b/src/drivers/intel/fsp1_1/romstage.c @@ -23,7 +23,7 @@ static void raminit_common(struct romstage_params *params) { bool s3wake; - struct region_device rdev; + size_t mrc_size;
post_code(0x32);
@@ -45,24 +45,31 @@ /* Recovery mode does not use MRC cache */ printk(BIOS_DEBUG, "Recovery mode: not using MRC cache.\n"); - } else if (CONFIG(CACHE_MRC_SETTINGS) - && (!mrc_cache_get_current(MRC_TRAINING_DATA, - params->fsp_version, - &rdev))) { - /* MRC cache found */ - params->saved_data_size = region_device_sz(&rdev); - params->saved_data = rdev_mmap_full(&rdev); + } else { /* Assume boot device is memory mapped. */ assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED)); - } else if (s3wake) { - /* Waking from S3 and no cache. */ - printk(BIOS_DEBUG, - "No MRC cache found in S3 resume path.\n"); - post_code(POST_RESUME_FAILURE); - /* FIXME: A "system" reset is likely enough: */ - full_reset(); - } else { - printk(BIOS_DEBUG, "No MRC cache found.\n"); + + params->saved_data = NULL; + if (CONFIG(CACHE_MRC_SETTINGS)) + params->saved_data = + mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + params->fsp_version, + &mrc_size); + if (params->saved_data) { + /* MRC cache found */ + params->saved_data_size = mrc_size; + + } else if (s3wake) { + /* Waking from S3 and no cache. */ + printk(BIOS_DEBUG, + "No MRC cache " + "found in S3 resume path.\n"); + post_code(POST_RESUME_FAILURE); + /* FIXME: A "system" reset is likely enough: */ + full_reset(); + } else { + printk(BIOS_DEBUG, "No MRC cache found.\n"); + } } }
@@ -283,13 +290,6 @@ { }
-/* Get the memory configuration data */ -__weak int mrc_cache_get_current(int type, uint32_t version, - struct region_device *rdev) -{ - return -1; -} - /* Save the memory configuration data */ __weak int mrc_cache_stash_data(int type, uint32_t version, const void *data, size_t size) diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 57a0520..07c4463 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -93,8 +93,8 @@
static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, uint32_t fsp_version) { - struct region_device rdev; void *data; + size_t mrc_size;
arch_upd->NvsBufferPtr = NULL;
@@ -113,25 +113,22 @@ return; }
- if (mrc_cache_get_current(MRC_TRAINING_DATA, fsp_version, &rdev) < 0) - return; - /* Assume boot device is memory mapped. */ assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED)); - data = rdev_mmap_full(&rdev);
+ data = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, fsp_version, + &mrc_size); if (data == NULL) return;
if (CONFIG(FSP2_0_USES_TPM_MRC_HASH) && - !mrc_cache_verify_hash(data, region_device_sz(&rdev))) + !mrc_cache_verify_hash(data, mrc_size)) return;
/* MRC cache found */ arch_upd->NvsBufferPtr = data;
- printk(BIOS_SPEW, "MRC cache found, size %zx\n", - region_device_sz(&rdev)); + printk(BIOS_SPEW, "MRC cache found, size %zx\n", mrc_size); }
static enum cb_err check_region_overlap(const struct memranges *ranges, diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index d567a20..1b1ad63 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -204,23 +204,16 @@ return 0; }
-static int mrc_data_valid(const struct region_device *rdev, - const struct mrc_metadata *md) +static int mrc_data_valid(const struct mrc_metadata *md, + void *data, size_t data_size) { - 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"); + if (md->data_size != data_size) return -1; - }
checksum = compute_ip_checksum(data, data_size);
- rdev_munmap(rdev, data); if (md->data_checksum != checksum) { printk(BIOS_ERR, "MRC: data checksum mismatch: %x vs %x\n", md->data_checksum, checksum); @@ -230,7 +223,7 @@ return 0; }
-static int mrc_cache_latest(const char *name, +static int mrc_cache_get_latest_slot_info(const char *name, const struct region_device *backing_rdev, struct mrc_metadata *md, struct region_file *cache_file, @@ -260,25 +253,19 @@ return fail_bad_data ? -1 : 0; }
- /* 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_get_current(int type, uint32_t version, - struct region_device *rdev) +static int mrc_cache_find_current(int type, uint32_t version, + struct region_device *rdev, + struct mrc_metadata *md) { 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 size_t md_size = sizeof(*md); const bool fail_bad_data = true;
cr = lookup_region(®ion, type); @@ -289,21 +276,75 @@ if (boot_device_ro_subregion(®ion, &read_rdev) < 0) return -1;
- if (mrc_cache_latest(cr->name, &read_rdev, &md, &cache_file, rdev, - fail_bad_data) < 0) + if (mrc_cache_get_latest_slot_info(cr->name, + &read_rdev, + md, + &cache_file, + rdev, + fail_bad_data) < 0) return -1;
- if (version != md.version) { + if (version != md->version) { printk(BIOS_INFO, "MRC: version mismatch: %x vs %x\n", - md.version, version); + md->version, version); return -1; }
/* Re-size rdev to only contain the data. i.e. remove metadata. */ - data_size = md.data_size; + data_size = md->data_size; return rdev_chain(rdev, rdev, md_size, data_size); }
+int mrc_cache_load_current(int type, uint32_t version, void *buffer, + size_t buffer_size) +{ + struct region_device rdev; + struct mrc_metadata md; + size_t data_size; + + if (mrc_cache_find_current(type, version, &rdev, &md) < 0) + return -1; + + data_size = region_device_sz(&rdev); + if (buffer_size < data_size) + return -1; + + if (rdev_readat(&rdev, buffer, 0, data_size) != data_size) + return -1; + + if (mrc_data_valid(&md, buffer, data_size) < 0) + return -1; + + return 0; +} + +void *mrc_cache_current_mmap_leak(int type, uint32_t version, + size_t *data_size) +{ + struct region_device rdev; + void *data; + size_t region_device_size; + struct mrc_metadata md; + + if (mrc_cache_find_current(type, version, &rdev, &md) < 0) + return NULL; + + region_device_size = region_device_sz(&rdev); + if (data_size) + *data_size = region_device_size; + data = rdev_mmap_full(&rdev); + + if (data == NULL) { + printk(BIOS_INFO, "MRC: mmap failure.\n"); + return NULL; + } + + if (mrc_data_valid(&md, data, region_device_size) < 0) + return NULL; + + return data; +} + static bool mrc_cache_needs_update(const struct region_device *rdev, const struct cbmem_entry *to_be_updated) { @@ -392,8 +433,17 @@ if (backing_rdev == NULL) return;
- if (mrc_cache_latest(cr->name, backing_rdev, &md, &cache_file, - &latest_rdev, fail_bad_data) < 0) + /* Note that mrc_cache_get_latest_slot_info doesn't check the + * validity of the current slot. If the slot is invalid, + * we'll overwrite it anyway when we update the mrc_cache. + */ + if (mrc_cache_get_latest_slot_info(cr->name, + backing_rdev, + &md, + &cache_file, + &latest_rdev, + fail_bad_data) < 0) + return;
if (!mrc_cache_needs_update(&latest_rdev, to_be_updated)) { diff --git a/src/include/mrc_cache.h b/src/include/mrc_cache.h index 1cefba9..da2bf79 100644 --- a/src/include/mrc_cache.h +++ b/src/include/mrc_cache.h @@ -21,11 +21,30 @@ * policy don't request the data. */
-/* 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); +/* Get and stash data for saving provided the type passed in. */ + +/** + * mrc_cache_load_current + * + * Fill in the buffer with the latest slot data. This will be a + * common entry point for ARM platforms. Returns < 0 on error, 0 on + * success. + */ +int mrc_cache_load_current(int type, uint32_t version, void *buffer, + size_t buffer_size); +/** + * mrc_cache_mmap_leak + * + * Return a pointer to a buffer with the latest slot data. An mmap + * will be executed (without a matching unmap). This will be a common + * entry point for platforms where mmap is considered a noop, like x86 + */ +void *mrc_cache_current_mmap_leak(int type, uint32_t version, + size_t *data_size); +/** + * Returns < 0 on error, 0 on success. + */ int mrc_cache_stash_data(int type, uint32_t version, const void *data, - size_t size); + size_t size);
#endif /* _COMMON_MRC_CACHE_H_ */ diff --git a/src/northbridge/intel/haswell/raminit.c b/src/northbridge/intel/haswell/raminit.c index 0b59692..9c6c00f 100644 --- a/src/northbridge/intel/haswell/raminit.c +++ b/src/northbridge/intel/haswell/raminit.c @@ -31,21 +31,24 @@
static void prepare_mrc_cache(struct pei_data *pei_data) { - struct region_device rdev; + size_t mrc_size;
/* Preset just in case there is an error */ pei_data->mrc_input = NULL; pei_data->mrc_input_len = 0;
- if (mrc_cache_get_current(MRC_TRAINING_DATA, MRC_CACHE_VERSION, &rdev)) + pei_data->mrc_input = + mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + MRC_CACHE_VERSION, + &mrc_size); + if (!pei_data->mrc_input) /* Error message printed in find_current_mrc_cache */ return;
- pei_data->mrc_input = rdev_mmap_full(&rdev); - pei_data->mrc_input_len = region_device_sz(&rdev); + pei_data->mrc_input_len = mrc_size;
- printk(BIOS_DEBUG, "%s: at %p, size %x\n", __func__, pei_data->mrc_input, - pei_data->mrc_input_len); + printk(BIOS_DEBUG, "%s: at %p, size %zx\n", __func__, + pei_data->mrc_input, mrc_size); }
static const char *ecc_decoder[] = { diff --git a/src/northbridge/intel/ironlake/raminit.c b/src/northbridge/intel/ironlake/raminit.c index dd1dbd0..81ba450 100644 --- a/src/northbridge/intel/ironlake/raminit.c +++ b/src/northbridge/intel/ironlake/raminit.c @@ -1618,11 +1618,9 @@
static const struct ram_training *get_cached_training(void) { - struct region_device rdev; - if (mrc_cache_get_current(MRC_TRAINING_DATA, MRC_CACHE_VERSION, - &rdev)) - return 0; - return (void *)rdev_mmap_full(&rdev); + return mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + MRC_CACHE_VERSION, + NULL); }
/* FIXME: add timeout. */ diff --git a/src/northbridge/intel/sandybridge/raminit.c b/src/northbridge/intel/sandybridge/raminit.c index 06b4d1e..6d0e845 100644 --- a/src/northbridge/intel/sandybridge/raminit.c +++ b/src/northbridge/intel/sandybridge/raminit.c @@ -297,7 +297,7 @@ int me_uma_size, cbmem_was_inited, fast_boot, err; ramctr_timing ctrl; spd_raw_data spds[4]; - struct region_device rdev; + size_t mrc_size; ramctr_timing *ctrl_cached = NULL;
MCHBAR32(SAPMCTL) |= 1; @@ -324,10 +324,11 @@ early_thermal_init();
/* Try to find timings in MRC cache */ - err = mrc_cache_get_current(MRC_TRAINING_DATA, MRC_CACHE_VERSION, &rdev); - - if (!err && !(region_device_sz(&rdev) < sizeof(ctrl))) - ctrl_cached = rdev_mmap_full(&rdev); + ctrl_cached = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + MRC_CACHE_VERSION, + &mrc_size); + if (mrc_size < sizeof(ctrl)) + ctrl_cached = NULL;
/* Before reusing training data, assert that the CPU has not been replaced */ if (ctrl_cached && cpuid != ctrl_cached->cpu) { diff --git a/src/northbridge/intel/sandybridge/raminit_mrc.c b/src/northbridge/intel/sandybridge/raminit_mrc.c index b6b3989..5e5cc63 100644 --- a/src/northbridge/intel/sandybridge/raminit_mrc.c +++ b/src/northbridge/intel/sandybridge/raminit_mrc.c @@ -72,8 +72,8 @@
static void prepare_mrc_cache(struct pei_data *pei_data) { - struct region_device rdev; u16 c1, c2, checksum, seed_checksum; + size_t mrc_size;
/* Preset just in case there is an error */ pei_data->mrc_input = NULL; @@ -103,16 +103,18 @@ return; }
- if (mrc_cache_get_current(MRC_TRAINING_DATA, MRC_CACHE_VERSION, &rdev)) { + pei_data->mrc_input = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + MRC_CACHE_VERSION, + &mrc_size); + if (pei_data->mrc_input == NULL) { /* Error message printed in find_current_mrc_cache */ return; }
- pei_data->mrc_input = rdev_mmap_full(&rdev); - pei_data->mrc_input_len = region_device_sz(&rdev); + pei_data->mrc_input_len = mrc_size;
- printk(BIOS_DEBUG, "%s: at %p, size %x\n", __func__, pei_data->mrc_input, - pei_data->mrc_input_len); + printk(BIOS_DEBUG, "%s: at %p, size %zx\n", __func__, + pei_data->mrc_input, mrc_size); }
/** diff --git a/src/northbridge/intel/x4x/raminit.c b/src/northbridge/intel/x4x/raminit.c index 9f361b6..a62771d 100644 --- a/src/northbridge/intel/x4x/raminit.c +++ b/src/northbridge/intel/x4x/raminit.c @@ -610,8 +610,8 @@ { struct sysinfo s, *ctrl_cached; u8 reg8; - int fast_boot, cbmem_was_inited, cache_not_found; - struct region_device rdev; + int fast_boot, cbmem_was_inited; + size_t mrc_size;
timestamp_add_now(TS_BEFORE_INITRAM); printk(BIOS_DEBUG, "Setting up RAM controller.\n"); @@ -620,10 +620,11 @@
memset(&s, 0, sizeof(struct sysinfo));
- cache_not_found = mrc_cache_get_current(MRC_TRAINING_DATA, - MRC_CACHE_VERSION, &rdev); + ctrl_cached = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + MRC_CACHE_VERSION, + &mrc_size);
- if (cache_not_found || (region_device_sz(&rdev) < sizeof(s))) { + if (!ctrl_cached || mrc_size < sizeof(s)) { if (boot_path == BOOT_PATH_RESUME) { /* Failed S3 resume, reset to come up cleanly */ system_reset(); @@ -632,9 +633,6 @@ and therefore requiring valid cached settings */ full_reset(); } - ctrl_cached = NULL; - } else { - ctrl_cached = rdev_mmap_full(&rdev); }
/* verify MRC cache for fast boot */ diff --git a/src/soc/amd/common/block/s3/s3_resume.c b/src/soc/amd/common/block/s3/s3_resume.c index 3752384..2094931 100644 --- a/src/soc/amd/common/block/s3/s3_resume.c +++ b/src/soc/amd/common/block/s3/s3_resume.c @@ -25,14 +25,10 @@ size_t size; int i; uint32_t erased = 0xffffffff; - struct region_device rdev;
- if (mrc_cache_get_current(MRC_TRAINING_DATA, DEFAULT_MRC_VERSION, - &rdev)) - reboot_from_resume("mrc_cache_get_current error, rebooting.\n"); - - base = rdev_mmap_full(&rdev); - size = region_device_sz(&rdev); + base = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + DEFAULT_MRC_VERSION, + &size); if (!base || !size) reboot_from_resume("Error: S3 NV data not found, rebooting.\n");
diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c index 54bdd2e..48ae9a7 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -267,8 +267,6 @@
void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) { - struct region_device rdev; - check_full_retrain(mupd);
fill_console_params(mupd); @@ -310,11 +308,11 @@ * wrong/missing key renders DRAM contents useless. */
- if (mrc_cache_get_current(MRC_VARIABLE_DATA, version, &rdev) == 0) { - /* Assume leaking is ok. */ - assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED)); - mupd->FspmConfig.VariableNvsBufferPtr = rdev_mmap_full(&rdev); - } + mupd->FspmConfig.VariableNvsBufferPtr = + mrc_cache_current_mmap_leak(MRC_VARIABLE_DATA, version, + NULL); + + assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED));
fsp_version = version;
diff --git a/src/soc/intel/baytrail/romstage/raminit.c b/src/soc/intel/baytrail/romstage/raminit.c index 6ff6c03..9a67c22 100644 --- a/src/soc/intel/baytrail/romstage/raminit.c +++ b/src/soc/intel/baytrail/romstage/raminit.c @@ -122,8 +122,8 @@ { int ret; mrc_wrapper_entry_t mrc_entry; - struct region_device rdev; size_t i; + size_t mrc_size;
/* Fill in default entries. */ mp->version = MRC_PARAMS_VER; @@ -135,11 +135,14 @@ if (!mp->io_hole_mb) mp->io_hole_mb = 2048;
- 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(CONFIG(BOOT_DEVICE_MEMORY_MAPPED)); + /* Assume boot device is memory mapped. */ + assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED)); + + mp->saved_data = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, + 0, + &mrc_size); + if (mp->saved_data) { + mp->saved_data_size = mrc_size; } 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"); diff --git a/src/soc/intel/broadwell/romstage/raminit.c b/src/soc/intel/broadwell/romstage/raminit.c index 4165c67..0580ca6 100644 --- a/src/soc/intel/broadwell/romstage/raminit.c +++ b/src/soc/intel/broadwell/romstage/raminit.c @@ -26,7 +26,7 @@ */ void raminit(struct pei_data *pei_data) { - struct region_device rdev; + size_t mrc_size; struct memory_info *mem_info; pei_wrapper_entry_t entry; int ret; @@ -39,19 +39,25 @@ 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(MRC_TRAINING_DATA, 0, &rdev)) { - /* MRC cache found */ - pei_data->saved_data_size = region_device_sz(&rdev); - pei_data->saved_data = rdev_mmap_full(&rdev); + } else { /* Assume boot device is memory mapped. */ assert(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"); - post_code(POST_RESUME_FAILURE); - system_reset(); - } else { - printk(BIOS_DEBUG, "No MRC cache found.\n"); + + pei_data->saved_data = + mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, 0, + &mrc_size); + if (pei_data->saved_data) { + /* MRC cache found */ + pei_data->saved_data_size = mrc_size; + } 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"); + post_code(POST_RESUME_FAILURE); + system_reset(); + } else { + printk(BIOS_DEBUG, "No MRC cache found.\n"); + } }
/*