[coreboot-gerrit] Change in ...coreboot[master]: cpu/intel/common: separate IA32_FEATURE_CONTROL lock from set_vmx()

Matt DeVillier (Code Review) gerrit at coreboot.org
Sat Dec 15 00:31:27 CET 2018 

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30229


Change subject: cpu/intel/common: separate IA32_FEATURE_CONTROL lock from set_vmx()
......................................................................

cpu/intel/common: separate IA32_FEATURE_CONTROL lock from set_vmx()

Newer CPUs/SoCs need to configure other features via the
IA32_FEATURE_CONTROL msr, such as SGX, which cannot be done if the
msr is already locked.  Separate out the lock function so that the
msr can be locked after all features are configured.

This will allow Skylake/Kabylake (and others?) to use the common
VMX code without breaking SGX.

Test: build/boot each affected platform, ensure no change in functionality

Change-Id: Iee772fe87306b4729ca012cef8640d3858e2cb06
Signed-off-by: Matt DeVillier <matt.devillier at gmail.com>
---
M src/cpu/intel/common/Kconfig
M src/cpu/intel/common/common.h
M src/cpu/intel/common/common_init.c
M src/cpu/intel/fsp_model_406dx/model_406dx_init.c
M src/cpu/intel/haswell/haswell_init.c
M src/cpu/intel/model_1067x/model_1067x_init.c
M src/cpu/intel/model_106cx/model_106cx_init.c
M src/cpu/intel/model_2065x/model_2065x_init.c
M src/cpu/intel/model_206ax/model_206ax_init.c
M src/cpu/intel/model_6ex/model_6ex_init.c
M src/cpu/intel/model_6fx/model_6fx_init.c
M src/soc/intel/baytrail/cpu.c
M src/soc/intel/braswell/cpu.c
M src/soc/intel/broadwell/cpu.c
14 files changed, 36 insertions(+), 6 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/30229/1

diff --git a/src/cpu/intel/common/Kconfig b/src/cpu/intel/common/Kconfig
index 739333e..b1f5613 100644
--- a/src/cpu/intel/common/Kconfig
+++ b/src/cpu/intel/common/Kconfig
@@ -7,8 +7,8 @@
 	bool "Enable VMX for virtualization"
 	default y
 
-config SET_VMX_LOCK_BIT
-	bool "Set lock bit after configuring VMX"
+config SET_IA32_FC_LOCK_BIT
+	bool "Set IA32_FEATURE_CONTROL lock bit after configuring VMX"
 	depends on ENABLE_VMX
 	default y
 	help
diff --git a/src/cpu/intel/common/common.h b/src/cpu/intel/common/common.h
index 81c9f16..a317fce 100644
--- a/src/cpu/intel/common/common.h
+++ b/src/cpu/intel/common/common.h
@@ -16,6 +16,7 @@
 #define _CPU_INTEL_COMMON_H
 
 void set_vmx(void);
+void set_feature_ctrl_lock(void);
 
 /*
  * Init CPPC block with MSRs for Intel Enhanced Speed Step Technology.
diff --git a/src/cpu/intel/common/common_init.c b/src/cpu/intel/common/common_init.c
index 7dbbfda..1d4cd9e 100644
--- a/src/cpu/intel/common/common_init.c
+++ b/src/cpu/intel/common/common_init.c
@@ -26,7 +26,6 @@
 	msr_t msr;
 	uint32_t feature_flag;
 	int enable = IS_ENABLED(CONFIG_ENABLE_VMX);
-	int lock = IS_ENABLED(CONFIG_SET_VMX_LOCK_BIT);
 
 	feature_flag = cpu_get_feature_flags_ecx();
 	/* Check that the VMX is supported before reading or writing the MSR. */
@@ -38,7 +37,7 @@
 	msr = rdmsr(IA32_FEATURE_CONTROL);
 
 	if (msr.lo & (1 << 0)) {
-		printk(BIOS_ERR, "VMX is locked, so %s will do nothing\n",
+		printk(BIOS_ERR, "IA32_FEATURE_CONTROL is locked, so %s will do nothing\n",
 			__func__);
 		/* VMX locked. If we set it again we get an illegal
 		 * instruction
@@ -59,14 +58,33 @@
 
 	wrmsr(IA32_FEATURE_CONTROL, msr);
 
+	printk(BIOS_DEBUG, "VMX status: %s\n",
+		enable ? "enabled" : "disabled");
+}
+
+void set_feature_ctrl_lock(void)
+{
+	msr_t msr;
+	int lock = IS_ENABLED(CONFIG_SET_IA32_FC_LOCK_BIT);
+
+	msr = rdmsr(IA32_FEATURE_CONTROL);
+
+	if (msr.lo & (1 << 0)) {
+		printk(BIOS_ERR, "IA32_FEATURE_CONTROL is locked, so %s will do nothing\n",
+			__func__);
+		/* VMX locked. If we set it again we get an illegal
+		 * instruction
+		 */
+		return;
+	}
+
 	if (lock) {
 		/* Set lock bit */
 		msr.lo |= (1 << 0);
 		wrmsr(IA32_FEATURE_CONTROL, msr);
 	}
 
-	printk(BIOS_DEBUG, "VMX status: %s, %s\n",
-		enable ? "enabled" : "disabled",
+	printk(BIOS_DEBUG, "IA32_FEATURE_CONTROL status: %s\n",
 		lock ? "locked" : "unlocked");
 }
 
diff --git a/src/cpu/intel/fsp_model_406dx/model_406dx_init.c b/src/cpu/intel/fsp_model_406dx/model_406dx_init.c
index 289305f..34ed651 100644
--- a/src/cpu/intel/fsp_model_406dx/model_406dx_init.c
+++ b/src/cpu/intel/fsp_model_406dx/model_406dx_init.c
@@ -149,6 +149,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure Enhanced SpeedStep and Thermal Sensors */
 	configure_misc();
diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c
index 20c1fac..ac1dea4 100644
--- a/src/cpu/intel/haswell/haswell_init.c
+++ b/src/cpu/intel/haswell/haswell_init.c
@@ -688,6 +688,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure C States */
 	configure_c_states();
diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c
index 7eb121e..1503e05 100644
--- a/src/cpu/intel/model_1067x/model_1067x_init.c
+++ b/src/cpu/intel/model_1067x/model_1067x_init.c
@@ -297,6 +297,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure C States */
 	configure_c_states(quad);
diff --git a/src/cpu/intel/model_106cx/model_106cx_init.c b/src/cpu/intel/model_106cx/model_106cx_init.c
index 56598fa..33fe858 100644
--- a/src/cpu/intel/model_106cx/model_106cx_init.c
+++ b/src/cpu/intel/model_106cx/model_106cx_init.c
@@ -100,6 +100,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure C States */
 	configure_c_states();
diff --git a/src/cpu/intel/model_2065x/model_2065x_init.c b/src/cpu/intel/model_2065x/model_2065x_init.c
index 222c2ed..7690d02 100644
--- a/src/cpu/intel/model_2065x/model_2065x_init.c
+++ b/src/cpu/intel/model_2065x/model_2065x_init.c
@@ -335,6 +335,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure Enhanced SpeedStep and Thermal Sensors */
 	configure_misc();
diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c
index 27f56be..97c2775 100644
--- a/src/cpu/intel/model_206ax/model_206ax_init.c
+++ b/src/cpu/intel/model_206ax/model_206ax_init.c
@@ -557,6 +557,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure C States */
 	configure_c_states();
diff --git a/src/cpu/intel/model_6ex/model_6ex_init.c b/src/cpu/intel/model_6ex/model_6ex_init.c
index 5041dcd..a32c413 100644
--- a/src/cpu/intel/model_6ex/model_6ex_init.c
+++ b/src/cpu/intel/model_6ex/model_6ex_init.c
@@ -137,6 +137,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure C States */
 	configure_c_states();
diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c
index c5659f3..58666c5 100644
--- a/src/cpu/intel/model_6fx/model_6fx_init.c
+++ b/src/cpu/intel/model_6fx/model_6fx_init.c
@@ -151,6 +151,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure C States */
 	configure_c_states();
diff --git a/src/soc/intel/baytrail/cpu.c b/src/soc/intel/baytrail/cpu.c
index 6c37aa86..bcd54ed 100644
--- a/src/soc/intel/baytrail/cpu.c
+++ b/src/soc/intel/baytrail/cpu.c
@@ -58,6 +58,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Set core MSRs */
 	reg_script_run(core_msr_script);
diff --git a/src/soc/intel/braswell/cpu.c b/src/soc/intel/braswell/cpu.c
index 9063c2a..ca7cebc 100644
--- a/src/soc/intel/braswell/cpu.c
+++ b/src/soc/intel/braswell/cpu.c
@@ -63,6 +63,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Set core MSRs */
 	reg_script_run(core_msr_script);
diff --git a/src/soc/intel/broadwell/cpu.c b/src/soc/intel/broadwell/cpu.c
index ec8f7f3..6d06bae 100644
--- a/src/soc/intel/broadwell/cpu.c
+++ b/src/soc/intel/broadwell/cpu.c
@@ -583,6 +583,7 @@
 
 	/* Set virtualization based on Kconfig option */
 	set_vmx();
+	set_feature_ctrl_lock();
 
 	/* Configure C States */
 	configure_c_states();

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/30229
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iee772fe87306b4729ca012cef8640d3858e2cb06
Gerrit-Change-Number: 30229
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier at gmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181214/b2945266/attachment.html>

More information about the coreboot-gerrit mailing list