Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44006 )
Change subject: mrc_cache: mrc_cache fetch functions to support non-x86 platforms ......................................................................
mrc_cache: 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.
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 --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/include/mrc_cache.h 3 files changed, 99 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/44006/1
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 7f5d389..21bbf67 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,24 @@ return; }
- if (mrc_cache_get_current(MRC_TRAINING_DATA, fsp_version, &rdev) < 0) + data = mrc_cache_current_mmap_leak(MRC_TRAINING_DATA, fsp_version, + &mrc_size); + if (data == NULL) return;
/* Assume boot device is memory mapped. */ assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED)); - data = rdev_mmap_full(&rdev);
- 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)); + 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..8191a53 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, buffer_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(&rdev, 0, region_device_size); + + if (data == NULL) { + printk(BIOS_ERR, "MRC: mmap failure on data verification.\n"); + return NULL; + } + + if (mrc_data_valid(&md, data, *data_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,13 @@ if (backing_rdev == NULL) return;
- if (mrc_cache_latest(cr->name, backing_rdev, &md, &cache_file, - &latest_rdev, fail_bad_data) < 0) + 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..c34e0cc 100644 --- a/src/include/mrc_cache.h +++ b/src/include/mrc_cache.h @@ -23,9 +23,25 @@
/* 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); +/** + * mrc_cache_load_current + * + * Fill in the buffer with the latest slot data. This will be a + * common entry point for ARM platforms. + */ +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 unmmap), so it is up to the caller + * to unmap if necessary. 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); int mrc_cache_stash_data(int type, uint32_t version, const void *data, - size_t size); + size_t size);
#endif /* _COMMON_MRC_CACHE_H_ */