x86: adjust MSR_IA32_MISC_ENABLE handling

In the warning message issued on writes, the Xen-modified value should
be printed (and used to determine whether anything needs to be printed
at all), as the guest kernel will usually do a read-modify-write
cycle.

A question is whether Dom0 shouldn't be allowed control over some
bits, or whether some bits shouldn't be fully virtualized. I'm
particularly thinking of MSR_IA32_MISC_ENABLE_FAST_STRING, which
recent Linux kernels want to disable for CONFIG_KMEMCHECK.

While putting this together I also noticed that rdmsr_safe() failed to
initialize its output registers in the failure path, thus leading to
printing of uninitialized data in the guest WRMSR warning message.

Further, the default case value-changed check can be simplified.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 447ce2348d6eb485d1a0f2140b11558d7a060ae1..c1f480fdb58af3443e7b86d791ce690fb23ef0c8 100644 (file)
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1670,6 +1670,16 @@ unsigned long guest_to_host_gpr_switch(unsigned long)
 
 void (*pv_post_outb_hook)(unsigned int port, u8 value);
 
+static inline uint64_t guest_misc_enable(uint64_t val)
+{
+    val &= ~(MSR_IA32_MISC_ENABLE_PERF_AVAIL |
+             MSR_IA32_MISC_ENABLE_MONITOR_ENABLE);
+    val |= MSR_IA32_MISC_ENABLE_BTS_UNAVAIL |
+           MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL |
+           MSR_IA32_MISC_ENABLE_XTPR_DISABLE;
+    return val;
+}
+
 /* Instruction fetch with error handling. */
 #define insn_fetch(type, base, eip, limit)                                  \
 ({  unsigned long _rc, _ptr = (base) + (eip);                               \
@@ -2266,6 +2276,13 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
             if ( wrmsr_safe(MSR_FAM10H_MMIO_CONF_BASE, msr_content) != 0 )
                 goto fail;
             break;
+        case MSR_IA32_MISC_ENABLE:
+            if ( rdmsr_safe(regs->ecx, val) )
+                goto invalid;
+            val = guest_misc_enable(val);
+            if ( msr_content != val )
+                goto invalid;
+            break;
         case MSR_IA32_MPERF:
         case MSR_IA32_APERF:
             if (( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ) &&
@@ -2302,8 +2319,7 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
             if ( rc )
                 break;
 
-            if ( (rdmsr_safe(regs->ecx, val) != 0) ||
-                 (eax != (uint32_t)val) || (edx != (uint32_t)(val >> 32)) )
+            if ( (rdmsr_safe(regs->ecx, val) != 0) || (msr_content != val) )
         invalid:
                 gdprintk(XENLOG_WARNING, "Domain attempted WRMSR %p from "
                         "0x%016"PRIx64" to 0x%016"PRIx64".\n",
@@ -2374,13 +2390,9 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
         case MSR_IA32_MISC_ENABLE:
             if ( rdmsr_safe(regs->ecx, msr_content) )
                 goto fail;
+            msr_content = guest_misc_enable(msr_content);
             regs->eax = (uint32_t)msr_content;
             regs->edx = (uint32_t)(msr_content >> 32);
-            regs->eax &= ~(MSR_IA32_MISC_ENABLE_PERF_AVAIL |
-                           MSR_IA32_MISC_ENABLE_MONITOR_ENABLE);
-            regs->eax |= MSR_IA32_MISC_ENABLE_BTS_UNAVAIL |
-                         MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL |
-                         MSR_IA32_MISC_ENABLE_XTPR_DISABLE;
             break;
         case MSR_EFER:
         case MSR_AMD_PATCHLEVEL:

diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
index d55c81509bf28b7944a1dc06e5f23458919e5cb8..ab3360c6837a3ac75453b1b822d40b900966fac0 100644 (file)
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -41,7 +41,8 @@ static inline void wrmsrl(unsigned int msr, __u64 val)
     __asm__ __volatile__( \
         "1: rdmsr\n2:\n" \
         ".section .fixup,\"ax\"\n" \
-        "3: movl %5,%2\n; jmp 2b\n" \
+        "3: xorl %0,%0\n; xorl %1,%1\n" \
+        "   movl %5,%2\n; jmp 2b\n" \
         ".previous\n" \
         ".section __ex_table,\"a\"\n" \
         "   "__FIXUP_ALIGN"\n" \