[xen staging-4.10] x86/msr: fix handling of MSR_IA32_PERF_{STATUS/CTL}

[patchbot@xxxxxxx]

commit 398f91c505b06e6614ddabde97e71a29affd8015
Author:     Roger Pau Monné <roger.pau@xxxxxxxxxx>
AuthorDate: Tue Oct 6 18:23:27 2020 +0200
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Nov 10 17:51:25 2020 +0000

    x86/msr: fix handling of MSR_IA32_PERF_{STATUS/CTL}
    
    Currently a PV hardware domain can also be given control over the CPU
    frequency, and such guest is allowed to write to MSR_IA32_PERF_CTL.
    However since commit 322ec7c89f6 the default behavior has been changed
    to reject accesses to not explicitly handled MSRs, preventing PV
    guests that manage CPU frequency from reading
    MSR_IA32_PERF_{STATUS/CTL}.
    
    Additionally some HVM guests (Windows at least) will attempt to read
    MSR_IA32_PERF_CTL and will panic if given back a #GP fault:
    
      vmx.c:3035:d8v0 RDMSR 0x00000199 unimplemented
      d8v0 VIRIDIAN CRASH: 3b c0000096 fffff806871c1651 ffffda0253683720 0
    
    Move the handling of MSR_IA32_PERF_{STATUS/CTL} to the common MSR
    handling shared between HVM and PV guests, and add an explicit case
    for reads to MSR_IA32_PERF_{STATUS/CTL}.
    
    Restore previous behavior and allow PV guests with the required
    permissions to read the contents of the mentioned MSRs. Non privileged
    guests will get 0 when trying to read those registers, as writes to
    MSR_IA32_PERF_CTL by such guest will already be silently dropped.
    
    Fixes: 322ec7c89f6 ('x86/pv: disallow access to unknown MSRs')
    Fixes: 84e848fd7a1 ('x86/hvm: disallow access to unknown MSRs')
    Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    (cherry picked from commit 3059178798a23ba870ff86ff54d442a07e6651fc)
---
 xen/arch/x86/msr.c             | 36 ++++++++++++++++++++++++++++++++++++
 xen/arch/x86/pv/emul-priv-op.c | 14 --------------
 xen/include/xen/sched.h        | 16 ++++++++++++++++
 3 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 90561a5392..a2a0634b4b 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -122,6 +122,7 @@ int init_vcpu_msr_policy(struct vcpu *v)
 
 int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
 {
+    const struct domain *d = v->domain;
     const struct cpuid_policy *cp = v->domain->arch.cpuid;
     const struct msr_domain_policy *dp = v->domain->arch.msr;
     const struct msr_vcpu_policy *vp = v->arch.msr;
@@ -163,6 +164,25 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
uint64_t *val)
                _MSR_MISC_FEATURES_CPUID_FAULTING;
         break;
 
+        /*
+         * These MSRs are not enumerated in CPUID.  They have been around
+         * since the Pentium 4, and implemented by other vendors.
+         *
+         * Some versions of Windows try reading these before setting up a #GP
+         * handler, and Linux has several unguarded reads as well.  Provide
+         * RAZ semantics, in general, but permit a cpufreq controller dom0 to
+         * have full access.
+         */
+    case MSR_IA32_PERF_STATUS:
+    case MSR_IA32_PERF_CTL:
+        if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
+            goto gp_fault;
+
+        *val = 0;
+        if ( likely(!is_cpufreq_controller(d)) || rdmsr_safe(msr, *val) == 0 )
+            break;
+        goto gp_fault;
+
         /*
          * TODO: Implement when we have better topology representation.
     case MSR_INTEL_CORE_THREAD_COUNT:
@@ -192,6 +212,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
     case MSR_INTEL_CORE_THREAD_COUNT:
     case MSR_INTEL_PLATFORM_INFO:
     case MSR_ARCH_CAPABILITIES:
+    case MSR_IA32_PERF_STATUS:
         /* Read-only */
     case MSR_TSX_FORCE_ABORT:
     case MSR_TSX_CTRL:
@@ -283,6 +304,21 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
         break;
     }
 
+        /*
+         * This MSR is not enumerated in CPUID.  It has been around since the
+         * Pentium 4, and implemented by other vendors.
+         *
+         * To match the RAZ semantics, implement as write-discard, except for
+         * a cpufreq controller dom0 which has full access.
+         */
+    case MSR_IA32_PERF_CTL:
+        if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
+            goto gp_fault;
+
+        if ( likely(!is_cpufreq_controller(d)) || wrmsr_safe(msr, val) == 0 )
+            break;
+        goto gp_fault;
+
     default:
         return X86EMUL_UNHANDLEABLE;
     }
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 0344c987e1..9857a786dd 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -835,12 +835,6 @@ static inline uint64_t guest_misc_enable(uint64_t val)
     return val;
 }
 
-static inline bool is_cpufreq_controller(const struct domain *d)
-{
-    return ((cpufreq_controller == FREQCTL_dom0_kernel) &&
-            is_hardware_domain(d));
-}
-
 static int read_msr(unsigned int reg, uint64_t *val,
                     struct x86_emulate_ctxt *ctxt)
 {
@@ -1129,14 +1123,6 @@ static int write_msr(unsigned int reg, uint64_t val,
             return X86EMUL_OKAY;
         break;
 
-    case MSR_IA32_PERF_CTL:
-        if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
-            break;
-        if ( likely(!is_cpufreq_controller(currd)) ||
-             wrmsr_safe(reg, val) == 0 )
-            return X86EMUL_OKAY;
-        break;
-
     case MSR_IA32_THERM_CONTROL:
     case MSR_IA32_ENERGY_PERF_BIAS:
         if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 0a13b81c21..831927fbc4 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -917,6 +917,22 @@ extern enum cpufreq_controller {
     FREQCTL_none, FREQCTL_dom0_kernel, FREQCTL_xen
 } cpufreq_controller;
 
+static always_inline bool is_cpufreq_controller(const struct domain *d)
+{
+    /*
+     * A PV dom0 can be nominated as the cpufreq controller, instead of using
+     * Xen's cpufreq driver, at which point dom0 gets direct access to certain
+     * MSRs.
+     *
+     * This interface only works when dom0 is identity pinned and has the same
+     * number of vCPUs as pCPUs on the system.
+     *
+     * It would be far better to paravirtualise the interface.
+     */
+    return (is_pv_domain(d) && is_hardware_domain(d) &&
+            cpufreq_controller == FREQCTL_dom0_kernel);
+}
+
 #define CPUPOOLID_NONE    -1
 
 struct cpupool *cpupool_get_by_id(int poolid);
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.10