Jamie Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39138 )
Change subject: drivers/mrc_cache: Add SPD cache to MRC region ......................................................................
drivers/mrc_cache: Add SPD cache to MRC region
Add a new region for store memory SPD specific data
BUG=b:146457985 BRANCH=None TEST=Build puff pass.
Change-Id: If5d315cc9021161472567c18e6f36aed7f3e4e96 Signed-off-by: Jamie Chen jamie.chen@intel.com --- M src/commonlib/include/commonlib/cbmem_id.h M src/drivers/mrc_cache/Kconfig M src/drivers/mrc_cache/mrc_cache.c M src/include/elog.h M src/include/mrc_cache.h 5 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/39138/1
diff --git a/src/commonlib/include/commonlib/cbmem_id.h b/src/commonlib/include/commonlib/cbmem_id.h index 93d1464..5d31573 100644 --- a/src/commonlib/include/commonlib/cbmem_id.h +++ b/src/commonlib/include/commonlib/cbmem_id.h @@ -41,6 +41,7 @@ #define CBMEM_ID_MPTABLE 0x534d5054 #define CBMEM_ID_MRCDATA 0x4d524344 #define CBMEM_ID_VAR_MRCDATA 0x4d524345 +#define CBMEM_ID_SPD_MRCDATA 0x4d524346 #define CBMEM_ID_MTC 0xcb31d31c #define CBMEM_ID_NONE 0x00000000 #define CBMEM_ID_PIRQ 0x49525154 @@ -108,6 +109,7 @@ { CBMEM_ID_MPTABLE, "SMP TABLE " }, \ { CBMEM_ID_MRCDATA, "MRC DATA " }, \ { CBMEM_ID_VAR_MRCDATA, "VARMRC DATA" }, \ + { CBMEM_ID_SPD_MRCDATA, "SPDMRC_DATA" }, \ { CBMEM_ID_MTC, "MTC " }, \ { CBMEM_ID_PIRQ, "IRQ TABLE " }, \ { CBMEM_ID_POWER_STATE, "POWER STATE" }, \ diff --git a/src/drivers/mrc_cache/Kconfig b/src/drivers/mrc_cache/Kconfig index 543f310..ad95680 100644 --- a/src/drivers/mrc_cache/Kconfig +++ b/src/drivers/mrc_cache/Kconfig @@ -25,6 +25,10 @@ bool default n
+config MRC_SETTINGS_SPD_DATA + bool + default n + config MRC_WRITE_NV_LATE bool default n diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index d4a4aab..ce7379c 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -28,6 +28,7 @@
#define DEFAULT_MRC_CACHE "RW_MRC_CACHE" #define VARIABLE_MRC_CACHE "RW_VAR_MRC_CACHE" +#define SPD_MRC_CACHE "RW_SPD_MRC_CACHE" #define RECOVERY_MRC_CACHE "RECOVERY_MRC_CACHE" #define UNIFIED_MRC_CACHE "UNIFIED_MRC_CACHE"
@@ -86,10 +87,19 @@ .flags = NORMAL_FLAG | RECOVERY_FLAG, };
+static const struct cache_region spd_data = { + .name = SPD_MRC_CACHE, + .cbmem_id = CBMEM_ID_SPD_MRCDATA, + .type = MRC_SPD_DATA, + .elog_slot = ELOG_MEM_CACHE_UPDATE_SLOT_SPD, + .flags = NORMAL_FLAG | RECOVERY_FLAG, +}; + /* Order matters here for priority in matching. */ static const struct cache_region *cache_regions[] = { &recovery_training, &normal_training, + &spd_data, &variable_data, };
@@ -549,6 +559,9 @@ if (CONFIG(MRC_SETTINGS_VARIABLE_DATA)) update_mrc_cache_by_type(MRC_VARIABLE_DATA);
+ if (CONFIG(MRC_SETTINGS_SPD_DATA)) + update_mrc_cache_by_type(MRC_SPD_DATA); + if (CONFIG(MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN)) invalidate_normal_cache();
diff --git a/src/include/elog.h b/src/include/elog.h index 8d1b3ba..72e000a 100644 --- a/src/include/elog.h +++ b/src/include/elog.h @@ -173,6 +173,7 @@ #define ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL 0 #define ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY 1 #define ELOG_MEM_CACHE_UPDATE_SLOT_VARIABLE 2 +#define ELOG_MEM_CACHE_UPDATE_SLOT_SPD 3 #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/include/mrc_cache.h b/src/include/mrc_cache.h index 498ecbf..534ea10 100644 --- a/src/include/mrc_cache.h +++ b/src/include/mrc_cache.h @@ -23,6 +23,7 @@ enum { MRC_TRAINING_DATA, MRC_VARIABLE_DATA, + MRC_SPD_DATA, };
/*
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39138 )
Change subject: drivers/mrc_cache: Add SPD cache to MRC region ......................................................................
Patch Set 1:
Please elaborate why it's useful. Usually only the SPD serial number is stored in MRC cache. It can be used to detect if DIMMs have been changed. Why do you need to store the whole SPD?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39138 )
Change subject: drivers/mrc_cache: Add SPD cache to MRC region ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39138/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39138/1//COMMIT_MSG@9 PS1, Line 9: for store for storing/to store
https://review.coreboot.org/c/coreboot/+/39138/1//COMMIT_MSG@9 PS1, Line 9: Add a new region for store memory SPD specific data Please add a dot/period at the end.
https://review.coreboot.org/c/coreboot/+/39138/1//COMMIT_MSG@10 PS1, Line 10: Please describe, why this feature is needed, and how it can be decided, if the cached SPD is not valid anymore.
Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39138 )
Change subject: drivers/mrc_cache: Add SPD cache to MRC region ......................................................................
Patch Set 1:
Patch Set 1:
Please elaborate why it's useful. Usually only the SPD serial number is stored in MRC cache. It can be used to detect if DIMMs have been changed. Why do you need to store the whole SPD?
This is a good information. I will look into the MRC cache.
Thank you.
Jamie Chen has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39138 )
Change subject: drivers/mrc_cache: Add SPD cache to MRC region ......................................................................
Abandoned
-
Jamie Chen (Code Review) -
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)