VMX/vPMU: fix DebugCtl MSR handling - writes with one of the vPMU-supported flags and some unsupported one set got accepted, leading to a VM entry failure - writes with one of the vPMU-supported flags set but the Debug Store feature unavailable produced a #GP (other than other writes to this MSR with unsupported bits set) Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1789,7 +1789,7 @@ static int svm_msr_write_intercept(unsig case MSR_AMD_FAM15H_EVNTSEL3: case MSR_AMD_FAM15H_EVNTSEL4: case MSR_AMD_FAM15H_EVNTSEL5: - vpmu_do_wrmsr(msr, msr_content); + vpmu_do_wrmsr(msr, msr_content, 0); break; case MSR_IA32_MCx_MISC(4): /* Threshold register */ --- a/xen/arch/x86/hvm/svm/vpmu.c +++ b/xen/arch/x86/hvm/svm/vpmu.c @@ -278,11 +278,14 @@ static void context_update(unsigned int } } -static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content) +static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, + uint64_t supported) { struct vcpu *v = current; struct vpmu_struct *vpmu = vcpu_vpmu(v); + ASSERT(!supported); + /* For all counters, enable guest only mode for HVM guest */ if ( (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) && !(is_guest_mode(msr_content)) ) --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2249,7 +2249,7 @@ static int vmx_msr_write_intercept(unsig if ( msr_content & ~supported ) { /* Perhaps some other bits are supported in vpmu. */ - if ( !vpmu_do_wrmsr(msr, msr_content) ) + if ( !vpmu_do_wrmsr(msr, msr_content, supported) ) break; } if ( msr_content & IA32_DEBUGCTLMSR_LBR ) @@ -2278,7 +2278,7 @@ static int vmx_msr_write_intercept(unsig goto gp_fault; break; default: - if ( vpmu_do_wrmsr(msr, msr_content) ) + if ( vpmu_do_wrmsr(msr, msr_content, 0) ) return X86EMUL_OKAY; if ( passive_domain_do_wrmsr(msr, msr_content) ) return X86EMUL_OKAY; --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c @@ -454,7 +454,8 @@ static int core2_vpmu_msr_common_check(u return 1; } -static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content) +static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, + uint64_t supported) { u64 global_ctrl, non_global_ctrl; char pmu_enable = 0; @@ -469,24 +470,26 @@ static int core2_vpmu_do_wrmsr(unsigned /* Special handling for BTS */ if ( msr == MSR_IA32_DEBUGCTLMSR ) { - uint64_t supported = IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS | - IA32_DEBUGCTLMSR_BTINT; + supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS | + IA32_DEBUGCTLMSR_BTINT; if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) ) supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS | IA32_DEBUGCTLMSR_BTS_OFF_USR; - if ( msr_content & supported ) - { - if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) ) - return 1; - gdprintk(XENLOG_WARNING, "Debug Store is not supported on this cpu\n"); - hvm_inject_hw_exception(TRAP_gp_fault, 0); - return 0; - } + if ( !(msr_content & ~supported) && + vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) ) + return 1; + if ( (msr_content & supported) && + !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) ) + printk(XENLOG_G_WARNING + "%pv: Debug Store unsupported on this CPU\n", + current); } return 0; } + ASSERT(!supported); + core2_vpmu_cxt = vpmu->context; switch ( msr ) { --- a/xen/arch/x86/hvm/vpmu.c +++ b/xen/arch/x86/hvm/vpmu.c @@ -64,12 +64,12 @@ static void __init parse_vpmu_param(char } } -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content) +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported) { struct vpmu_struct *vpmu = vcpu_vpmu(current); if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr ) - return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content); + return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported); return 0; } --- a/xen/include/asm-x86/hvm/vpmu.h +++ b/xen/include/asm-x86/hvm/vpmu.h @@ -45,7 +45,8 @@ /* Arch specific operations shared by all vpmus */ struct arch_vpmu_ops { - int (*do_wrmsr)(unsigned int msr, uint64_t msr_content); + int (*do_wrmsr)(unsigned int msr, uint64_t msr_content, + uint64_t supported); int (*do_rdmsr)(unsigned int msr, uint64_t *msr_content); int (*do_interrupt)(struct cpu_user_regs *regs); void (*do_cpuid)(unsigned int input, @@ -86,7 +87,7 @@ struct vpmu_struct { #define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x)) #define vpmu_clear(_vpmu) ((_vpmu)->flags = 0) -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content); +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported); int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content); int vpmu_do_interrupt(struct cpu_user_regs *regs); void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,