[Xen-devel] [PATCH] x86/vvmx: Fix WRMSR interception of VMX MSRs

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

FEATURE_CONTROL is already read with LOCK bit set (so is unmodifiable), and
all VMX MSRs are read-only.  Also, fix the MSR_IA32_VMX_TRUE_ENTRY_CTLS bound
to be MSR_IA32_VMX_VMFUNC, rather than having the intervening MSRs falling
into the default case.

Raise #GP faults if the guest tries to modify any of them.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
CC: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
CC: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>

Haozhong: FYI, I'm rebasing your XTF vvmx series and adding extra tests as
appropriate.  I got as far as patch 2 before finding this.
---
 xen/arch/x86/hvm/vmx/vmx.c         | 8 ++++----
 xen/arch/x86/hvm/vmx/vvmx.c        | 6 ------
 xen/include/asm-x86/hvm/vmx/vvmx.h | 2 --
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c53b249..d637026 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3116,10 +3116,10 @@ static int vmx_msr_write_intercept(unsigned int msr, 
uint64_t msr_content)
         break;
     }
     case MSR_IA32_FEATURE_CONTROL:
-    case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_TRUE_ENTRY_CTLS:
-        if ( !nvmx_msr_write_intercept(msr, msr_content) )
-            goto gp_fault;
-        break;
+    case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
+        /* None of these MSRs are writeable. */
+        goto gp_fault;
+
     case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
     case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(7):
     case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 3560fae..2a47d20 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2123,12 +2123,6 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 
*msr_content)
     return r;
 }
 
-int nvmx_msr_write_intercept(unsigned int msr, u64 msr_content)
-{
-    /* silently ignore for now */
-    return 1;
-}
-
 /* This function uses L2_gpa to walk the P2M page table in L1. If the
  * walk is successful, the translated value is returned in
  * L1_gpa. The result value tells what to do next.
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h 
b/xen/include/asm-x86/hvm/vmx/vvmx.h
index ca2fb25..3285b03 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -224,8 +224,6 @@ int nvmx_handle_invept(struct cpu_user_regs *regs);
 int nvmx_handle_invvpid(struct cpu_user_regs *regs);
 int nvmx_msr_read_intercept(unsigned int msr,
                                 u64 *msr_content);
-int nvmx_msr_write_intercept(unsigned int msr,
-                                 u64 msr_content);
 
 void nvmx_update_exec_control(struct vcpu *v, u32 value);
 void nvmx_update_secondary_exec_control(struct vcpu *v,
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel