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 --- 2010-09-20.orig/xen/arch/x86/traps.c 2010-09-20 10:14:37.000000000 +0200 +++ 2010-09-20/xen/arch/x86/traps.c 2010-09-24 13:24:39.000000000 +0200 @@ -1671,6 +1671,16 @@ unsigned long guest_to_host_gpr_switch(u 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); \ @@ -2267,6 +2277,13 @@ static int emulate_privileged_op(struct 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 ) && @@ -2303,8 +2320,7 @@ static int emulate_privileged_op(struct 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", @@ -2375,13 +2391,9 @@ static int emulate_privileged_op(struct 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: --- 2010-09-20.orig/xen/include/asm-x86/msr.h 2010-06-28 09:38:51.000000000 +0200 +++ 2010-09-20/xen/include/asm-x86/msr.h 2010-09-24 12:22:38.000000000 +0200 @@ -41,7 +41,8 @@ static inline void wrmsrl(unsigned int m __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" \