Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31255
Change subject: cpu/intel/model_1067x: Check for lock bit on IA32_FEATURE_CONTROL ......................................................................
cpu/intel/model_1067x: Check for lock bit on IA32_FEATURE_CONTROL
df7aecd " cpu/intel: Configure IA32_FEATURE_CONTROL for alternative SMRR" introduced a regression because it unconditionally writes to IA32_FEATURE_CONTROL, which if it is already locked results in an unhandled exception. The lock bit is already set on a system reboot.
Change-Id: I7d2df9e1b9d767809da7a61ccd877c6c40f132eb Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/intel/model_1067x/mp_init.c 1 file changed, 14 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/31255/1
diff --git a/src/cpu/intel/model_1067x/mp_init.c b/src/cpu/intel/model_1067x/mp_init.c index ab77859..f3890fd 100644 --- a/src/cpu/intel/model_1067x/mp_init.c +++ b/src/cpu/intel/model_1067x/mp_init.c @@ -60,11 +60,20 @@ /* We don't care if the lock is already setting as our smm relocation handler is able to handle setups where SMRR is not enabled here. */ - if (!IS_ENABLED(CONFIG_SET_IA32_FC_LOCK_BIT)) - printk(BIOS_INFO, - "Overriding CONFIG_SET_IA32_FC_LOCK_BIT to enable SMRR\n"); - ia32_ft_ctrl.lo |= (1 << 3) | (1 << 0); - wrmsr(IA32_FEATURE_CONTROL, ia32_ft_ctrl); + if (ia32_ft_ctrl & (1 << 0)) { + /* IA32_FEATURE_CONTROL locked. If we set it again we get an + * illegal instruction + */ + printk(BIOS_DEBUG, "IA32_FEATURE_CONTROL already locked\n"); + printk(BIOS_DEBUG, "SMRR status: %senabled\n", + ia32_ft_ctrl & (1 << 3) ? "" : "not "); + } else { + if (!IS_ENABLED(CONFIG_SET_IA32_FC_LOCK_BIT)) + printk(BIOS_INFO, + "Overriding CONFIG_SET_IA32_FC_LOCK_BIT to enable SMRR\n"); + ia32_ft_ctrl.lo |= (1 << 3) | (1 << 0); + wrmsr(IA32_FEATURE_CONTROL, ia32_ft_ctrl); + } } else { set_vmx_and_lock(); }