Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30709
Change subject: cpu/intel/gen1/smmrelocate: Check for sanity on SMRR ......................................................................
cpu/intel/gen1/smmrelocate: Check for sanity on SMRR
This happens when TSEG is found to be unaligned.
Change-Id: Id0c078a880dddb55857af2bca233cf4dee91250a Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/intel/smm/gen1/smmrelocate.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/30709/1
diff --git a/src/cpu/intel/smm/gen1/smmrelocate.c b/src/cpu/intel/smm/gen1/smmrelocate.c index 426eae5..46f6f93 100644 --- a/src/cpu/intel/smm/gen1/smmrelocate.c +++ b/src/cpu/intel/smm/gen1/smmrelocate.c @@ -60,6 +60,9 @@ { struct cpuinfo_x86 c;
+ if (relo_params->smrr_base.lo == 0) + return; + printk(BIOS_DEBUG, "Writing SMRR. base = 0x%08x, mask=0x%08x\n", relo_params->smrr_base.lo, relo_params->smrr_mask.lo); /* Both model_6fx and model_1067x SMRR function slightly differently
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30709 )
Change subject: cpu/intel/gen1/smmrelocate: Check for sanity on SMRR ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30709 )
Change subject: cpu/intel/gen1/smmrelocate: Check for sanity on SMRR ......................................................................
Patch Set 5:
I'm kinda wondering if we can silently ignore that case, and what breaks then
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30709 )
Change subject: cpu/intel/gen1/smmrelocate: Check for sanity on SMRR ......................................................................
Patch Set 5:
(1 comment)
Patch Set 5:
I'm kinda wondering if we can silently ignore that case, and what breaks then
It's a result from unaligned TSEG, which gets printed earlier. I can rework the code a little to clarify when or when not SMRR will be written to.
https://review.coreboot.org/#/c/30709/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30709/2//COMMIT_MSG@9 PS2, Line 9: This happens when TSEG is found to be unaligned.
Please add that also as a comment to the if statement. […]
A warning already gets printed.
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30709 )
Change subject: cpu/intel/gen1/smmrelocate: Check for sanity on SMRR ......................................................................
cpu/intel/gen1/smmrelocate: Check for sanity on SMRR
This happens when TSEG is found to be unaligned.
Change-Id: Id0c078a880dddb55857af2bca233cf4dee91250a Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/30709 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/intel/smm/gen1/smmrelocate.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/cpu/intel/smm/gen1/smmrelocate.c b/src/cpu/intel/smm/gen1/smmrelocate.c index 426eae5..5667648 100644 --- a/src/cpu/intel/smm/gen1/smmrelocate.c +++ b/src/cpu/intel/smm/gen1/smmrelocate.c @@ -132,7 +132,7 @@
/* Write SMRR MSRs based on indicated support. */ mtrr_cap = rdmsr(MTRR_CAP_MSR); - if (mtrr_cap.lo & SMRR_SUPPORTED) + if (mtrr_cap.lo & SMRR_SUPPORTED && relo_params->smrr_mask.lo != 0) write_smrr(relo_params);
southbridge_clear_smi_status();
-
Arthur Heymans (Code Review) -
Patrick Georgi (Code Review)