[PATCH 4/6] x86/msr: Clean up the MSR_FEATURE_CONTROL constants

[Andrew Cooper <andrew.cooper3@citrix.com>]

The existing bit names are excessively long (45 chars!), and can be trimmed
down substantially.  Drop the IA32 prefix and abbreviate FEATURE_CONTROL to
FEAT_CTL.  Furthermore, all of these are feature enablement bits, so drop
ENABLE/ON parts of the constants.

While altering all the users, take the opportunity to introduce cpu_has_smx
and clean up _vmx_cpu_up() with respect to its MSR_FEATURE_CONTROL handling.

The SENTER constants are unreferenced and dropped.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>

An item on my TODO list for later is to fix the handling of FEAT_CTL_LOCK.  We
now have two places which look for BIOS misconfiguration, and fix it.
Whichever gets called first (LMCE or VMX) will lock the other feature out.
---
 xen/arch/x86/cpu/mcheck/mce_intel.c |  7 +++----
 xen/arch/x86/cpu/mcheck/vmce.c      |  2 +-
 xen/arch/x86/cpu/mwait-idle.c       |  4 ++--
 xen/arch/x86/hvm/vmx/vmcs.c         | 31 ++++++++++++++-----------------
 xen/arch/x86/hvm/vmx/vmx.c          | 13 ++++++-------
 xen/include/asm-x86/cpufeature.h    |  1 +
 xen/include/asm-x86/msr-index.h     | 16 +++++++---------
 7 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c b/xen/arch/x86/cpu/mcheck/mce_intel.c
index e5dd956..2d285d0 100644
--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -727,15 +727,14 @@ static bool intel_enable_lmce(void)
     /*
      * Section "Enabling Local Machine Check" in Intel SDM Vol 3
      * requires software must ensure the LOCK bit and LMCE_ON bit
-     * of MSR_IA32_FEATURE_CONTROL are set before setting
+     * of MSR_FEATURE_CONTROL are set before setting
      * MSR_IA32_MCG_EXT_CTL.LMCE_EN.
      */
 
-    if ( rdmsr_safe(MSR_IA32_FEATURE_CONTROL, msr_content) )
+    if ( rdmsr_safe(MSR_FEATURE_CONTROL, msr_content) )
         return false;
 
-    if ( (msr_content & IA32_FEATURE_CONTROL_LOCK) &&
-         (msr_content & IA32_FEATURE_CONTROL_LMCE_ON) )
+    if ( (msr_content & FEAT_CTL_LOCK) && (msr_content & FEAT_CTL_LMCE) )
     {
         wrmsrl(MSR_IA32_MCG_EXT_CTL, MCG_EXT_CTL_LMCE_EN);
         return true;
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index e07cd2f..87f299b 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -204,7 +204,7 @@ int vmce_rdmsr(uint32_t msr, uint64_t *val)
     case MSR_IA32_MCG_EXT_CTL:
         /*
          * If MCG_LMCE_P is present in guest MSR_IA32_MCG_CAP, the LMCE and LOCK
-         * bits are always set in guest MSR_IA32_FEATURE_CONTROL by Xen, so it
+         * bits are always set in guest MSR_FEATURE_CONTROL by Xen, so it
          * does not need to check them here.
          */
         if ( cur->arch.vmce.mcg_cap & MCG_LMCE_P )
diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index 77fc3dd..bb96b32 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1078,10 +1078,10 @@ static void __init sklh_idle_state_table_update(void)
 
 	/* if SGX is present */
 	if (boot_cpu_has(X86_FEATURE_SGX)) {
-		rdmsrl(MSR_IA32_FEATURE_CONTROL, msr);
+		rdmsrl(MSR_FEATURE_CONTROL, msr);
 
 		/* if SGX is enabled */
-		if (msr & IA32_FEATURE_CONTROL_SGX_ENABLE)
+		if (msr & FEAT_CTL_SGX)
 			return;
 	}
 
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 884c672..95a0e37 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -610,9 +610,9 @@ void vmx_cpu_dead(unsigned int cpu)
 
 int _vmx_cpu_up(bool bsp)
 {
-    u32 eax, edx;
-    int rc, bios_locked, cpu = smp_processor_id();
-    u64 cr0, vmx_cr0_fixed0, vmx_cr0_fixed1;
+    int rc, cpu = smp_processor_id();
+    uint64_t cr0, vmx_cr0_fixed0, vmx_cr0_fixed1, feat_ctl;
+    bool bios_locked;
 
     BUG_ON(!(read_cr4() & X86_CR4_VMXE));
 
@@ -630,14 +630,14 @@ int _vmx_cpu_up(bool bsp)
         return -EINVAL;
     }
 
-    rdmsr(MSR_IA32_FEATURE_CONTROL, eax, edx);
+    rdmsrl(MSR_FEATURE_CONTROL, feat_ctl);
 
-    bios_locked = !!(eax & IA32_FEATURE_CONTROL_LOCK);
+    bios_locked = feat_ctl & FEAT_CTL_LOCK;
     if ( bios_locked )
     {
-        if ( !(eax & (tboot_in_measured_env()
-                      ? IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX
-                      : IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX)) )
+        if ( !(feat_ctl & (tboot_in_measured_env()
+                           ? FEAT_CTL_VMX_INSIDE_SMX
+                           : FEAT_CTL_VMX_OUTSIDE_SMX)) )
         {
             printk("CPU%d: VMX disabled by BIOS.\n", cpu);
             return -EINVAL;
@@ -645,11 +645,9 @@ int _vmx_cpu_up(bool bsp)
     }
     else
     {
-        eax  = IA32_FEATURE_CONTROL_LOCK;
-        eax |= IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX;
-        if ( test_bit(X86_FEATURE_SMX, &boot_cpu_data.x86_capability) )
-            eax |= IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX;
-        wrmsr(MSR_IA32_FEATURE_CONTROL, eax, 0);
+        feat_ctl = (FEAT_CTL_LOCK | FEAT_CTL_VMX_OUTSIDE_SMX |
+                    (cpu_has_smx ? FEAT_CTL_VMX_INSIDE_SMX : 0));
+        wrmsrl(MSR_FEATURE_CONTROL, feat_ctl);
     }
 
     if ( (rc = vmx_init_vmcs_config()) != 0 )
@@ -663,10 +661,9 @@ int _vmx_cpu_up(bool bsp)
     switch ( __vmxon(this_cpu(vmxon_region)) )
     {
     case -2: /* #UD or #GP */
-        if ( bios_locked &&
-             test_bit(X86_FEATURE_SMX, &boot_cpu_data.x86_capability) &&
-             (!(eax & IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX) ||
-              !(eax & IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX)) )
+        if ( bios_locked && cpu_has_smx &&
+             (!(feat_ctl & FEAT_CTL_VMX_OUTSIDE_SMX) ||
+              !(feat_ctl & FEAT_CTL_VMX_INSIDE_SMX)) )
         {
             printk("CPU%d: VMXON failed: perhaps because of TXT settings "
                    "in your BIOS configuration?\n", cpu);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 4318b15..d5334c9 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2861,12 +2861,11 @@ static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
     case MSR_IA32_DEBUGCTLMSR:
         __vmread(GUEST_IA32_DEBUGCTL, msr_content);
         break;
-    case MSR_IA32_FEATURE_CONTROL:
-        *msr_content = IA32_FEATURE_CONTROL_LOCK;
-        if ( vmce_has_lmce(curr) )
-            *msr_content |= IA32_FEATURE_CONTROL_LMCE_ON;
-        if ( nestedhvm_enabled(curr->domain) )
-            *msr_content |= IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX;
+    case MSR_FEATURE_CONTROL:
+        *msr_content = (FEAT_CTL_LOCK |
+                        (nestedhvm_enabled(curr->domain)
+                         ? FEAT_CTL_VMX_OUTSIDE_SMX : 0) |
+                        (vmce_has_lmce(curr) ? FEAT_CTL_LMCE : 0));
         break;
     case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC:
         if ( !nvmx_msr_read_intercept(msr, msr_content) )
@@ -3123,7 +3122,7 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
 
         break;
     }
-    case MSR_IA32_FEATURE_CONTROL:
+    case MSR_FEATURE_CONTROL:
     case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
         /* None of these MSRs are writeable. */
         goto gp_fault;
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 4bc6c91..94f9c9e 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -47,6 +47,7 @@
 #define cpu_has_pclmulqdq       boot_cpu_has(X86_FEATURE_PCLMULQDQ)
 #define cpu_has_monitor         boot_cpu_has(X86_FEATURE_MONITOR)
 #define cpu_has_vmx             boot_cpu_has(X86_FEATURE_VMX)
+#define cpu_has_smx             boot_cpu_has(X86_FEATURE_SMX)
 #define cpu_has_eist            boot_cpu_has(X86_FEATURE_EIST)
 #define cpu_has_ssse3           boot_cpu_has(X86_FEATURE_SSSE3)
 #define cpu_has_fma             boot_cpu_has(X86_FEATURE_FMA)
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 48d80e9..b9072b6 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -15,6 +15,13 @@
  * abbreviated name.
  */
 
+#define MSR_FEATURE_CONTROL             0x0000003a
+#define FEAT_CTL_LOCK                   (_AC(1, ULL) <<  0)
+#define FEAT_CTL_VMX_INSIDE_SMX         (_AC(1, ULL) <<  1)
+#define FEAT_CTL_VMX_OUTSIDE_SMX        (_AC(1, ULL) <<  2)
+#define FEAT_CTL_SGX                    (_AC(1, ULL) << 18)
+#define FEAT_CTL_LMCE                   (_AC(1, ULL) << 20)
+
 /* Speculation Controls. */
 #define MSR_SPEC_CTRL                   0x00000048
 #define SPEC_CTRL_IBRS                  (_AC(1, ULL) <<  0)
@@ -321,15 +328,6 @@
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_IA32_EBC_FREQUENCY_ID	0x0000002c
 
-#define MSR_IA32_FEATURE_CONTROL	0x0000003a
-#define IA32_FEATURE_CONTROL_LOCK                     0x0001
-#define IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX  0x0002
-#define IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX 0x0004
-#define IA32_FEATURE_CONTROL_SENTER_PARAM_CTL         0x7f00
-#define IA32_FEATURE_CONTROL_ENABLE_SENTER            0x8000
-#define IA32_FEATURE_CONTROL_SGX_ENABLE               0x40000
-#define IA32_FEATURE_CONTROL_LMCE_ON                  0x100000
-
 #define MSR_IA32_TSC_ADJUST		0x0000003b
 
 #define MSR_IA32_APICBASE		0x0000001b
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel