<p>Matt DeVillier has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/c/coreboot/+/30229">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">cpu/intel/common: separate IA32_FEATURE_CONTROL lock from set_vmx()<br><br>Newer CPUs/SoCs need to configure other features via the<br>IA32_FEATURE_CONTROL msr, such as SGX, which cannot be done if the<br>msr is already locked. Separate out the lock function so that the<br>msr can be locked after all features are configured.<br><br>This will allow Skylake/Kabylake (and others?) to use the common<br>VMX code without breaking SGX.<br><br>Test: build/boot each affected platform, ensure no change in functionality<br><br>Change-Id: Iee772fe87306b4729ca012cef8640d3858e2cb06<br>Signed-off-by: Matt DeVillier <matt.devillier@gmail.com><br>---<br>M src/cpu/intel/common/Kconfig<br>M src/cpu/intel/common/common.h<br>M src/cpu/intel/common/common_init.c<br>M src/cpu/intel/fsp_model_406dx/model_406dx_init.c<br>M src/cpu/intel/haswell/haswell_init.c<br>M src/cpu/intel/model_1067x/model_1067x_init.c<br>M src/cpu/intel/model_106cx/model_106cx_init.c<br>M src/cpu/intel/model_2065x/model_2065x_init.c<br>M src/cpu/intel/model_206ax/model_206ax_init.c<br>M src/cpu/intel/model_6ex/model_6ex_init.c<br>M src/cpu/intel/model_6fx/model_6fx_init.c<br>M src/soc/intel/baytrail/cpu.c<br>M src/soc/intel/braswell/cpu.c<br>M src/soc/intel/broadwell/cpu.c<br>14 files changed, 36 insertions(+), 6 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/30229/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/cpu/intel/common/Kconfig b/src/cpu/intel/common/Kconfig</span><br><span>index 739333e..b1f5613 100644</span><br><span>--- a/src/cpu/intel/common/Kconfig</span><br><span>+++ b/src/cpu/intel/common/Kconfig</span><br><span>@@ -7,8 +7,8 @@</span><br><span> bool "Enable VMX for virtualization"</span><br><span> default y</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-config SET_VMX_LOCK_BIT</span><br><span style="color: hsl(0, 100%, 40%);">- bool "Set lock bit after configuring VMX"</span><br><span style="color: hsl(120, 100%, 40%);">+config SET_IA32_FC_LOCK_BIT</span><br><span style="color: hsl(120, 100%, 40%);">+ bool "Set IA32_FEATURE_CONTROL lock bit after configuring VMX"</span><br><span> depends on ENABLE_VMX</span><br><span> default y</span><br><span> help</span><br><span>diff --git a/src/cpu/intel/common/common.h b/src/cpu/intel/common/common.h</span><br><span>index 81c9f16..a317fce 100644</span><br><span>--- a/src/cpu/intel/common/common.h</span><br><span>+++ b/src/cpu/intel/common/common.h</span><br><span>@@ -16,6 +16,7 @@</span><br><span> #define _CPU_INTEL_COMMON_H</span><br><span> </span><br><span> void set_vmx(void);</span><br><span style="color: hsl(120, 100%, 40%);">+void set_feature_ctrl_lock(void);</span><br><span> </span><br><span> /*</span><br><span> * Init CPPC block with MSRs for Intel Enhanced Speed Step Technology.</span><br><span>diff --git a/src/cpu/intel/common/common_init.c b/src/cpu/intel/common/common_init.c</span><br><span>index 7dbbfda..1d4cd9e 100644</span><br><span>--- a/src/cpu/intel/common/common_init.c</span><br><span>+++ b/src/cpu/intel/common/common_init.c</span><br><span>@@ -26,7 +26,6 @@</span><br><span> msr_t msr;</span><br><span> uint32_t feature_flag;</span><br><span> int enable = IS_ENABLED(CONFIG_ENABLE_VMX);</span><br><span style="color: hsl(0, 100%, 40%);">- int lock = IS_ENABLED(CONFIG_SET_VMX_LOCK_BIT);</span><br><span> </span><br><span> feature_flag = cpu_get_feature_flags_ecx();</span><br><span> /* Check that the VMX is supported before reading or writing the MSR. */</span><br><span>@@ -38,7 +37,7 @@</span><br><span> msr = rdmsr(IA32_FEATURE_CONTROL);</span><br><span> </span><br><span> if (msr.lo & (1 << 0)) {</span><br><span style="color: hsl(0, 100%, 40%);">- printk(BIOS_ERR, "VMX is locked, so %s will do nothing\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ printk(BIOS_ERR, "IA32_FEATURE_CONTROL is locked, so %s will do nothing\n",</span><br><span> __func__);</span><br><span> /* VMX locked. If we set it again we get an illegal</span><br><span> * instruction</span><br><span>@@ -59,14 +58,33 @@</span><br><span> </span><br><span> wrmsr(IA32_FEATURE_CONTROL, msr);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ printk(BIOS_DEBUG, "VMX status: %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ enable ? "enabled" : "disabled");</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+void set_feature_ctrl_lock(void)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ msr_t msr;</span><br><span style="color: hsl(120, 100%, 40%);">+ int lock = IS_ENABLED(CONFIG_SET_IA32_FC_LOCK_BIT);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ msr = rdmsr(IA32_FEATURE_CONTROL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msr.lo & (1 << 0)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ printk(BIOS_ERR, "IA32_FEATURE_CONTROL is locked, so %s will do nothing\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ __func__);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* VMX locked. If we set it again we get an illegal</span><br><span style="color: hsl(120, 100%, 40%);">+ * instruction</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (lock) {</span><br><span> /* Set lock bit */</span><br><span> msr.lo |= (1 << 0);</span><br><span> wrmsr(IA32_FEATURE_CONTROL, msr);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- printk(BIOS_DEBUG, "VMX status: %s, %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- enable ? "enabled" : "disabled",</span><br><span style="color: hsl(120, 100%, 40%);">+ printk(BIOS_DEBUG, "IA32_FEATURE_CONTROL status: %s\n",</span><br><span> lock ? "locked" : "unlocked");</span><br><span> }</span><br><span> </span><br><span>diff --git a/src/cpu/intel/fsp_model_406dx/model_406dx_init.c b/src/cpu/intel/fsp_model_406dx/model_406dx_init.c</span><br><span>index 289305f..34ed651 100644</span><br><span>--- a/src/cpu/intel/fsp_model_406dx/model_406dx_init.c</span><br><span>+++ b/src/cpu/intel/fsp_model_406dx/model_406dx_init.c</span><br><span>@@ -149,6 +149,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure Enhanced SpeedStep and Thermal Sensors */</span><br><span> configure_misc();</span><br><span>diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c</span><br><span>index 20c1fac..ac1dea4 100644</span><br><span>--- a/src/cpu/intel/haswell/haswell_init.c</span><br><span>+++ b/src/cpu/intel/haswell/haswell_init.c</span><br><span>@@ -688,6 +688,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure C States */</span><br><span> configure_c_states();</span><br><span>diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c</span><br><span>index 7eb121e..1503e05 100644</span><br><span>--- a/src/cpu/intel/model_1067x/model_1067x_init.c</span><br><span>+++ b/src/cpu/intel/model_1067x/model_1067x_init.c</span><br><span>@@ -297,6 +297,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure C States */</span><br><span> configure_c_states(quad);</span><br><span>diff --git a/src/cpu/intel/model_106cx/model_106cx_init.c b/src/cpu/intel/model_106cx/model_106cx_init.c</span><br><span>index 56598fa..33fe858 100644</span><br><span>--- a/src/cpu/intel/model_106cx/model_106cx_init.c</span><br><span>+++ b/src/cpu/intel/model_106cx/model_106cx_init.c</span><br><span>@@ -100,6 +100,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure C States */</span><br><span> configure_c_states();</span><br><span>diff --git a/src/cpu/intel/model_2065x/model_2065x_init.c b/src/cpu/intel/model_2065x/model_2065x_init.c</span><br><span>index 222c2ed..7690d02 100644</span><br><span>--- a/src/cpu/intel/model_2065x/model_2065x_init.c</span><br><span>+++ b/src/cpu/intel/model_2065x/model_2065x_init.c</span><br><span>@@ -335,6 +335,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure Enhanced SpeedStep and Thermal Sensors */</span><br><span> configure_misc();</span><br><span>diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c</span><br><span>index 27f56be..97c2775 100644</span><br><span>--- a/src/cpu/intel/model_206ax/model_206ax_init.c</span><br><span>+++ b/src/cpu/intel/model_206ax/model_206ax_init.c</span><br><span>@@ -557,6 +557,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure C States */</span><br><span> configure_c_states();</span><br><span>diff --git a/src/cpu/intel/model_6ex/model_6ex_init.c b/src/cpu/intel/model_6ex/model_6ex_init.c</span><br><span>index 5041dcd..a32c413 100644</span><br><span>--- a/src/cpu/intel/model_6ex/model_6ex_init.c</span><br><span>+++ b/src/cpu/intel/model_6ex/model_6ex_init.c</span><br><span>@@ -137,6 +137,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure C States */</span><br><span> configure_c_states();</span><br><span>diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c</span><br><span>index c5659f3..58666c5 100644</span><br><span>--- a/src/cpu/intel/model_6fx/model_6fx_init.c</span><br><span>+++ b/src/cpu/intel/model_6fx/model_6fx_init.c</span><br><span>@@ -151,6 +151,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure C States */</span><br><span> configure_c_states();</span><br><span>diff --git a/src/soc/intel/baytrail/cpu.c b/src/soc/intel/baytrail/cpu.c</span><br><span>index 6c37aa86..bcd54ed 100644</span><br><span>--- a/src/soc/intel/baytrail/cpu.c</span><br><span>+++ b/src/soc/intel/baytrail/cpu.c</span><br><span>@@ -58,6 +58,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Set core MSRs */</span><br><span> reg_script_run(core_msr_script);</span><br><span>diff --git a/src/soc/intel/braswell/cpu.c b/src/soc/intel/braswell/cpu.c</span><br><span>index 9063c2a..ca7cebc 100644</span><br><span>--- a/src/soc/intel/braswell/cpu.c</span><br><span>+++ b/src/soc/intel/braswell/cpu.c</span><br><span>@@ -63,6 +63,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Set core MSRs */</span><br><span> reg_script_run(core_msr_script);</span><br><span>diff --git a/src/soc/intel/broadwell/cpu.c b/src/soc/intel/broadwell/cpu.c</span><br><span>index ec8f7f3..6d06bae 100644</span><br><span>--- a/src/soc/intel/broadwell/cpu.c</span><br><span>+++ b/src/soc/intel/broadwell/cpu.c</span><br><span>@@ -583,6 +583,7 @@</span><br><span> </span><br><span> /* Set virtualization based on Kconfig option */</span><br><span> set_vmx();</span><br><span style="color: hsl(120, 100%, 40%);">+ set_feature_ctrl_lock();</span><br><span> </span><br><span> /* Configure C States */</span><br><span> configure_c_states();</span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/c/coreboot/+/30229">change 30229</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/c/coreboot/+/30229"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Iee772fe87306b4729ca012cef8640d3858e2cb06 </div>
<div style="display:none"> Gerrit-Change-Number: 30229 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Matt DeVillier <matt.devillier@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>