[coreboot-gerrit] Change in ...coreboot[master]: cpu/intel/model_1067x: Check for lock bit on IA32_FEATURE_CONTROL

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( 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 
Reviewed-on: https://review.coreboot.org/c/31255
Reviewed-by: Patrick Rudolph 
Reviewed-by: Bill XIE 
Reviewed-by: Paul Menzel 
Tested-by: build bot (Jenkins) 
---
M src/cpu/intel/model_1067x/mp_init.c
1 file changed, 13 insertions(+), 5 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Patrick Rudolph: Looks good to me, approved
  Bill XIE: Looks good to me, but someone else must approve

diff --git a/src/cpu/intel/model_1067x/mp_init.c b/src/cpu/intel/model_1067x/mp_init.c
index ab77859..acd56c8 100644
--- a/src/cpu/intel/model_1067x/mp_init.c
+++ b/src/cpu/intel/model_1067x/mp_init.c
@@ -60,11 +60,19 @@
    	/* 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.lo & (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.lo & (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();
    }