[coreboot-gerrit] Change in ...coreboot[master]: cpu/intel/common: decouple IA32_FEATURE_CONTROL lock from set_vmx()
Matt DeVillier (Code Review)
gerrit at coreboot.org
Sat Dec 15 23:02:59 CET 2018
Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30243
Change subject: cpu/intel/common: decouple IA32_FEATURE_CONTROL lock from set_vmx()
......................................................................
cpu/intel/common: decouple 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. Create separate functions for setting the
vmx flag and lock bit, and rename existing function to indiate that
the lock bit will be set in addition to vmx flag (per Kconfig).
This will allow Skylake/Kabylake (and others?) to use the common
VMX code without breaking SGX, while ensuring no change in functionality
to existing platforms which current set both together.
Test: build/boot each affected platform, ensure no change in functionality
Change-Id: Ib662fd0c62f3ae3d2344aee12149e4a3f0de69fb
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, 47 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/30243/1
diff --git a/src/cpu/intel/common/Kconfig b/src/cpu/intel/common/Kconfig
index 739333e..499c5b7 100644
--- a/src/cpu/intel/common/Kconfig
+++ b/src/cpu/intel/common/Kconfig
@@ -7,10 +7,9 @@
bool "Enable VMX for virtualization"
default y
-config SET_VMX_LOCK_BIT
- bool "Set lock bit after configuring VMX"
- depends on ENABLE_VMX
- default y
+config SET_IA32_FC_LOCK_BIT
+ bool "Set IA32_FEATURE_CONTROL lock bit"
+ default y if ENABLE_VMX
help
Although the Intel manual says you must set the lock bit in addition
to the VMX bit in order for VMX to work, this isn't strictly true, so
diff --git a/src/cpu/intel/common/common.h b/src/cpu/intel/common/common.h
index 81c9f16..b9ac056 100644
--- a/src/cpu/intel/common/common.h
+++ b/src/cpu/intel/common/common.h
@@ -15,7 +15,9 @@
#ifndef _CPU_INTEL_COMMON_H
#define _CPU_INTEL_COMMON_H
-void set_vmx(void);
+void set_vmx_and_lock(void);
+void set_feature_ctrl_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..9c0fcbb 100644
--- a/src/cpu/intel/common/common_init.c
+++ b/src/cpu/intel/common/common_init.c
@@ -21,12 +21,17 @@
#include <cpu/x86/msr.h>
#include "common.h"
-void set_vmx(void)
+void set_vmx_and_lock(void)
+{
+ set_feature_ctrl_vmx();
+ set_feature_ctrl_lock();
+}
+
+void set_feature_ctrl_vmx(void)
{
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,10 +43,10 @@
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
+ /* IA32_FEATURE_CONTROL locked. If we set it again we get an
+ * illegal instruction
*/
return;
}
@@ -59,14 +64,32 @@
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__);
+ /* IA32_FEATURE_CONTROL 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..7994f0b 100644
--- a/src/cpu/intel/fsp_model_406dx/model_406dx_init.c
+++ b/src/cpu/intel/fsp_model_406dx/model_406dx_init.c
@@ -148,7 +148,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..1029817 100644
--- a/src/cpu/intel/haswell/haswell_init.c
+++ b/src/cpu/intel/haswell/haswell_init.c
@@ -687,7 +687,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..dbb9631 100644
--- a/src/cpu/intel/model_1067x/model_1067x_init.c
+++ b/src/cpu/intel/model_1067x/model_1067x_init.c
@@ -296,7 +296,7 @@
init_timer();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..a609aed 100644
--- a/src/cpu/intel/model_106cx/model_106cx_init.c
+++ b/src/cpu/intel/model_106cx/model_106cx_init.c
@@ -99,7 +99,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..178b753 100644
--- a/src/cpu/intel/model_2065x/model_2065x_init.c
+++ b/src/cpu/intel/model_2065x/model_2065x_init.c
@@ -334,7 +334,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..58aabdb 100644
--- a/src/cpu/intel/model_206ax/model_206ax_init.c
+++ b/src/cpu/intel/model_206ax/model_206ax_init.c
@@ -556,7 +556,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..78ece74 100644
--- a/src/cpu/intel/model_6ex/model_6ex_init.c
+++ b/src/cpu/intel/model_6ex/model_6ex_init.c
@@ -136,7 +136,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..9d11478 100644
--- a/src/cpu/intel/model_6fx/model_6fx_init.c
+++ b/src/cpu/intel/model_6fx/model_6fx_init.c
@@ -150,7 +150,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..9526932 100644
--- a/src/soc/intel/baytrail/cpu.c
+++ b/src/soc/intel/baytrail/cpu.c
@@ -57,7 +57,7 @@
enable_turbo();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..12f34fb 100644
--- a/src/soc/intel/braswell/cpu.c
+++ b/src/soc/intel/braswell/cpu.c
@@ -62,7 +62,7 @@
enable_turbo();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_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..dc97469 100644
--- a/src/soc/intel/broadwell/cpu.c
+++ b/src/soc/intel/broadwell/cpu.c
@@ -582,7 +582,7 @@
setup_lapic();
/* Set virtualization based on Kconfig option */
- set_vmx();
+ set_vmx_and_lock();
/* Configure C States */
configure_c_states();
--
To view, visit https://review.coreboot.org/c/coreboot/+/30243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib662fd0c62f3ae3d2344aee12149e4a3f0de69fb
Gerrit-Change-Number: 30243
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/20181215/2347feb1/attachment.html>
More information about the coreboot-gerrit
mailing list