Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/70870 )
Change subject: soc/intel/xeon_sp: Set IA32_SMRR_PHYSMASK lock bit ......................................................................
soc/intel/xeon_sp: Set IA32_SMRR_PHYSMASK lock bit
smm_relocation_handler is run for each thread but IA32_SMRR_PHYS_BASE and IA32_SMRR_PHYS_MASK are core scope, need to avoid writing the same MSR that has been locked by another thread.
Tested=On OCP Crater Lake, rdmsr -a 0x1f3 can see all cores set the lock bit.
Change-Id: I9cf5a6761c9a9e1578c6132ef83e288540d41176 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/70870 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Jonathan Zhang jonzhang@fb.com --- M src/soc/intel/xeon_sp/smmrelocate.c 1 file changed, 33 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, but someone else must approve Jonathan Zhang: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/smmrelocate.c b/src/soc/intel/xeon_sp/smmrelocate.c index f44fc62..f9d100a 100644 --- a/src/soc/intel/xeon_sp/smmrelocate.c +++ b/src/soc/intel/xeon_sp/smmrelocate.c @@ -113,7 +113,7 @@ void smm_relocation_handler(int cpu, uintptr_t curr_smbase, uintptr_t staggered_smbase) { - msr_t mtrr_cap; + msr_t mtrr_cap, msr; struct smm_relocation_params *relo_params = &smm_reloc_params;
printk(BIOS_DEBUG, "%s : CPU %d\n", __func__, cpu); @@ -123,6 +123,17 @@
/* Write SMRR MSRs based on indicated support. */ mtrr_cap = rdmsr(MTRR_CAP_MSR); + + /* Set Lock bit if supported */ + if (mtrr_cap.lo & SMRR_LOCK_SUPPORTED) { + msr = rdmsr(IA32_SMRR_PHYS_MASK); + /* Don't write the same core scope MSR if another thread has locked it, + otherwise system would hang. */ + if (msr.lo & SMRR_PHYS_MASK_LOCK) + return; + relo_params->smrr_mask.lo |= SMRR_PHYS_MASK_LOCK; + } + if (mtrr_cap.lo & SMRR_SUPPORTED) write_smrr(relo_params); }