Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44196 )
Change subject: mrc_cache: Update mrc_cache data in romstage ......................................................................
mrc_cache: Update mrc_cache data in romstage
Previously, we were writing to cbmem after memory training and then writing the training data from cbmem to mrc_cache in ramstage. We were doing this because we were unable to read/write to SPI simultaneously on older SIP chips. Now that newer chips allow for simultaneously reads and writes, we can move the mrc_cache update into romstage.
BUG=b:150502246 BRANCH=None TEST=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: I3430bda45484cb8c2b01ab9614508039dfaac9a3 --- M src/drivers/elog/Makefile.inc M src/drivers/mrc_cache/mrc_cache.c M src/include/region_file.h M src/lib/region_file.c 4 files changed, 137 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/44196/1
diff --git a/src/drivers/elog/Makefile.inc b/src/drivers/elog/Makefile.inc index cce1c3d..a1c14ff 100644 --- a/src/drivers/elog/Makefile.inc +++ b/src/drivers/elog/Makefile.inc @@ -1,6 +1,7 @@ bootblock-$(CONFIG_ELOG_PRERAM) += elog.c verstage-$(CONFIG_ELOG_PRERAM) += elog.c romstage-$(CONFIG_ELOG_PRERAM) += elog.c +romstage-$(CONFIG_ELOG) += elog.c postcar-$(CONFIG_ELOG_PRERAM) += elog.c ramstage-$(CONFIG_ELOG) += elog.c
diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index 0e42120..2dc6ee8 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -311,18 +311,26 @@ }
static bool mrc_cache_needs_update(const struct region_device *rdev, - const struct cbmem_entry *to_be_updated) + struct mrc_metadata *new_md, + const void *new_data, size_t new_data_size) { - void *mapping; + void *mapping, *data_mapping; size_t size = region_device_sz(rdev); bool need_update = false;
- if (cbmem_entry_size(to_be_updated) != size) + if (new_data_size != size) return true;
mapping = rdev_mmap_full(rdev); + data_mapping = mapping + sizeof(struct mrc_metadata);
- if (memcmp(cbmem_entry_start(to_be_updated), mapping, size)) + // we need to compare the md and the data separately + // check the mrc_metadata + if (memcmp(new_md, mapping, sizeof(struct mrc_metadata))) + need_update = true; + + // check the data + if (memcmp(new_data, data_mapping, new_data_size)) need_update = true;
rdev_munmap(rdev, mapping); @@ -357,7 +365,10 @@ * read and write. The read assumes a memory-mapped boot device that can be used * to quickly locate and compare the up-to-date data. However, when an update * is required it uses the writeable region access to perform the update. */ -static void update_mrc_cache_by_type(int type) +static void update_mrc_cache_by_type(int type, + struct mrc_metadata *new_md, + const void *new_data, + size_t new_data_size) { const struct cache_region *cr; struct region region; @@ -365,7 +376,6 @@ struct region_device write_rdev; struct region_file cache_file; struct mrc_metadata md; - const struct cbmem_entry *to_be_updated; struct incoherent_rdev backing_irdev; const struct region_device *backing_rdev; struct region_device latest_rdev; @@ -376,13 +386,6 @@ if (cr == NULL) return;
- to_be_updated = cbmem_entry_find(cr->cbmem_id); - if (to_be_updated == NULL) { - printk(BIOS_ERR, "MRC: No data in cbmem for '%s'.\n", - cr->name); - return; - } - printk(BIOS_DEBUG, "MRC: Checking cached data update for '%s'.\n", cr->name);
@@ -411,8 +414,9 @@
return;
- if (!mrc_cache_needs_update(&latest_rdev, to_be_updated)) { - printk(BIOS_DEBUG, "MRC: '%s' does not need update.\n", cr->name); + if (!mrc_cache_needs_update(&latest_rdev, + new_md, + new_data, new_data_size)) { log_event_cache_update(cr->elog_slot, ALREADY_UPTODATE); return; } @@ -420,9 +424,10 @@ printk(BIOS_DEBUG, "MRC: cache data '%s' needs update.\n", cr->name);
if (region_file_update_data(&cache_file, - cbmem_entry_start(to_be_updated), - cbmem_entry_size(to_be_updated)) < 0) { - printk(BIOS_DEBUG, "MRC: failed to update '%s'.\n", cr->name); + new_md, + sizeof(struct mrc_metadata), + new_data, + new_data_size) < 0) { log_event_cache_update(cr->elog_slot, UPDATE_FAILURE); } else { printk(BIOS_DEBUG, "MRC: updated '%s'.\n", cr->name); @@ -544,16 +549,59 @@
/* Push an update that consists of 4 bytes that is smaller than the * MRC metadata as well as an invalid signature. */ - if (region_file_update_data(&cache_file, &invalid, sizeof(invalid)) < 0) + if (region_file_update_data(&cache_file, NULL, 0, + &invalid, sizeof(invalid)) < 0) printk(BIOS_ERR, "MRC: invalidation failed for '%s'.\n", name); }
-static void update_mrc_cache(void *unused) +static void update_mrc_cache(struct mrc_metadata *md, + const void *data, + size_t size) { - update_mrc_cache_by_type(MRC_TRAINING_DATA); + update_mrc_cache_by_type(MRC_TRAINING_DATA, md, data, size);
if (CONFIG(MRC_SETTINGS_VARIABLE_DATA)) - update_mrc_cache_by_type(MRC_VARIABLE_DATA); + update_mrc_cache_by_type(MRC_VARIABLE_DATA, + md, data, size); + + if (CONFIG(MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN)) + invalidate_normal_cache(); + + protect_mrc_region(); +} + +static void update_mrc_cache_from_cbmem(int type) +{ + const struct cache_region *cr; + struct region region; + struct mrc_metadata md; + const struct cbmem_entry *to_be_updated; + + cr = lookup_region(®ion, type); + + if (cr == NULL) + return; + + to_be_updated = cbmem_entry_find(cr->cbmem_id); + + if (to_be_updated == NULL) { + printk(BIOS_ERR, "MRC: No data in cbmem for '%s'.\n", + cr->name); + return; + } + + update_mrc_cache(&md, + cbmem_entry_start(to_be_updated), + cbmem_entry_size(to_be_updated)); +} + +static void finalize_mrc_cache(void *unused) +{ + if (CONFIG(MRC_WRITE_NV_LATE)) + update_mrc_cache_from_cbmem(MRC_TRAINING_DATA); + + if (CONFIG(MRC_SETTINGS_VARIABLE_DATA)) + update_mrc_cache_from_cbmem(MRC_VARIABLE_DATA);
if (CONFIG(MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN)) invalidate_normal_cache(); @@ -562,11 +610,9 @@ }
int mrc_cache_stash_data(int type, uint32_t version, const void *data, - size_t size) + size_t size) { const struct cache_region *cr; - size_t cbmem_size; - struct mrc_metadata *md;
cr = lookup_region_type(type); if (cr == NULL) { @@ -575,24 +621,41 @@ return -1; }
- cbmem_size = sizeof(*md) + size; + if (CONFIG(MRC_WRITE_NV_LATE)) { + /* Store data in cbmem for use in ramstage */ + struct mrc_metadata *md; + size_t cbmem_size; + cbmem_size = sizeof(*md) + size;
- md = cbmem_add(cr->cbmem_id, cbmem_size); + md = cbmem_add(cr->cbmem_id, cbmem_size);
- if (md == NULL) { - printk(BIOS_ERR, "MRC: failed to add '%s' to cbmem.\n", - cr->name); - return -1; + if (md == NULL) { + printk(BIOS_ERR, "MRC: failed to add '%s' to cbmem.\n", + cr->name); + return -1; + } + + memset(md, 0, sizeof(*md)); + md->signature = MRC_DATA_SIGNATURE; + md->data_size = size; + md->version = version; + md->data_checksum = compute_ip_checksum(data, size); + md->header_checksum = compute_ip_checksum(md, sizeof(*md)); + memcpy(&md[1], data, size); + } else { + /* Otherwise store to mrc_cache right away */ + struct mrc_metadata md; + + memset(&md, 0, sizeof(struct mrc_metadata)); + md.signature = MRC_DATA_SIGNATURE; + md.data_size = size; + md.version = version; + md.data_checksum = compute_ip_checksum(data, size); + md.header_checksum = + compute_ip_checksum(&md, sizeof(struct mrc_metadata)); + + update_mrc_cache(&md, data, size); } - - memset(md, 0, sizeof(*md)); - md->signature = MRC_DATA_SIGNATURE; - md->data_size = size; - md->version = version; - md->data_checksum = compute_ip_checksum(data, size); - md->header_checksum = compute_ip_checksum(md, sizeof(*md)); - memcpy(&md[1], data, size); - return 0; }
@@ -602,7 +665,7 @@ */
#if CONFIG(MRC_WRITE_NV_LATE) -BOOT_STATE_INIT_ENTRY(BS_OS_RESUME_CHECK, BS_ON_ENTRY, update_mrc_cache, NULL); +BOOT_STATE_INIT_ENTRY(BS_OS_RESUME_CHECK, BS_ON_ENTRY, finalize_mrc_cache, NULL); #else -BOOT_STATE_INIT_ENTRY(BS_DEV_ENUMERATE, BS_ON_EXIT, update_mrc_cache, NULL); +BOOT_STATE_INIT_ENTRY(BS_DEV_ENUMERATE, BS_ON_EXIT, finalize_mrc_cache, NULL); #endif diff --git a/src/include/region_file.h b/src/include/region_file.h index 063e0e0..068e147 100644 --- a/src/include/region_file.h +++ b/src/include/region_file.h @@ -32,8 +32,11 @@ int region_file_data(const struct region_file *f, struct region_device *rdev);
/* Update region file with latest data. Returns < 0 on error, 0 on success. */ -int region_file_update_data(struct region_file *f, const void *buf, - size_t size); +int region_file_update_data(struct region_file *f, + const void *metadata, + size_t metadata_size, + const void *data, + size_t data_size);
/* Declared here for easy object allocation. */ struct region_file { diff --git a/src/lib/region_file.c b/src/lib/region_file.c index ce2ed30..3f47b28 100644 --- a/src/lib/region_file.c +++ b/src/lib/region_file.c @@ -365,11 +365,17 @@ return 0; }
-static int commit_data(const struct region_file *f, const void *buf, - size_t size) +static int commit_data(const struct region_file *f, + const void *metadata, + size_t metadata_size, + const void *data_buf, + size_t data_size) { size_t offset = block_to_bytes(region_file_data_begin(f)); - if (rdev_writeat(&f->rdev, buf, offset, size) < 0) + if (rdev_writeat(&f->rdev, metadata, offset, metadata_size) < 0) + return -1; + offset += metadata_size; + if (rdev_writeat(&f->rdev, data_buf, offset, data_size) < 0) return -1; return 0; } @@ -399,8 +405,12 @@ return 0; }
-static int handle_update(struct region_file *f, size_t blocks, const void *buf, - size_t size) +static int handle_update(struct region_file *f, + size_t blocks, + const void *metadata, + size_t metadata_size, + const void *data, + size_t data_size) { if (!update_can_fit(f, blocks)) { printk(BIOS_INFO, "REGF update can't fit. Will empty.\n"); @@ -413,7 +423,7 @@ return -1; }
- if (commit_data(f, buf, size)) { + if (commit_data(f, metadata, metadata_size, data, data_size)) { printk(BIOS_ERR, "REGF failed to commit data.\n"); return -1; } @@ -421,12 +431,18 @@ return 0; }
-int region_file_update_data(struct region_file *f, const void *buf, size_t size) +int region_file_update_data(struct region_file *f, + const void *metadata, + size_t metadata_size, + const void *data, + size_t data_size) { int ret; size_t blocks;
- blocks = bytes_to_block(ALIGN_UP(size, REGF_BLOCK_GRANULARITY)); + blocks = bytes_to_block(ALIGN_UP(data_size + + metadata_size, + REGF_BLOCK_GRANULARITY));
while (1) { int prev_slot = f->slot; @@ -442,7 +458,8 @@ ret = -1; break; default: - ret = handle_update(f, blocks, buf, size); + ret = handle_update(f, blocks, metadata, + metadata_size, data, data_size); break; }