[xen master] x86/PV: conditionally avoid raising #GP for early guest MSR reads

[patchbot@xxxxxxx]

commit 6eef0a99262cc903495879448cc4ffefe305ef2a
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Fri Mar 12 12:02:42 2021 +0100
Commit:     Ian Jackson <iwj@xxxxxxxxxxxxxx>
CommitDate: Fri Mar 12 17:00:08 2021 +0000

    x86/PV: conditionally avoid raising #GP for early guest MSR reads
    
    Prior to 4.15 Linux, when running in PV mode, did not install a #GP
    handler early enough to cover for example the rdmsrl_safe() of
    MSR_K8_TSEG_ADDR in bsp_init_amd() (not to speak of the unguarded read
    of MSR_K7_HWCR later in the same function). The respective change
    (42b3a4cb5609 "x86/xen: Support early interrupts in xen pv guests") was
    backported to 4.14, but no further - presumably since it wasn't really
    easy because of other dependencies.
    
    Therefore, to prevent our change in the handling of guest MSR accesses
    to render PV Linux 4.13 and older unusable on at least AMD systems, make
    the raising of #GP on this paths conditional upon the guest having
    installed a handler, provided of course the MSR can be read in the first
    place (we would have raised #GP in that case even before). Producing
    zero for reads isn't necessarily correct and may trip code trying to
    detect presence of MSRs early, but since such detection logic won't work
    without a #GP handler anyway, this ought to be a fair workaround.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Release-Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>
    Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>
---
 xen/arch/x86/pv/emul-priv-op.c    | 20 ++++++++++++++++----
 xen/include/public/arch-x86/xen.h |  6 ++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 74e71403ff..8889509d2a 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -874,7 +874,7 @@ static int read_msr(unsigned int reg, uint64_t *val,
     struct vcpu *curr = current;
     const struct domain *currd = curr->domain;
     const struct cpuid_policy *cp = currd->arch.cpuid;
-    bool vpmu_msr = false;
+    bool vpmu_msr = false, warn = false;
     uint64_t tmp;
     int ret;
 
@@ -883,7 +883,7 @@ static int read_msr(unsigned int reg, uint64_t *val,
         if ( ret == X86EMUL_EXCEPTION )
             x86_emul_hw_exception(TRAP_gp_fault, 0, ctxt);
 
-        return ret;
+        goto done;
     }
 
     switch ( reg )
@@ -993,7 +993,7 @@ static int read_msr(unsigned int reg, uint64_t *val,
             return X86EMUL_OKAY;
         }
 
-        gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", reg);
+        warn = true;
         break;
 
     normal:
@@ -1002,7 +1002,19 @@ static int read_msr(unsigned int reg, uint64_t *val,
         return X86EMUL_OKAY;
     }
 
-    return X86EMUL_UNHANDLEABLE;
+ done:
+    if ( ret != X86EMUL_OKAY && !curr->arch.pv.trap_ctxt[X86_EXC_GP].address &&
+         (reg >> 16) != 0x4000 && !rdmsr_safe(reg, tmp) )
+    {
+        gprintk(XENLOG_WARNING, "faking RDMSR 0x%08x\n", reg);
+        *val = 0;
+        x86_emul_reset_event(ctxt);
+        ret = X86EMUL_OKAY;
+    }
+    else if ( warn )
+        gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", reg);
+
+    return ret;
 }
 
 static int write_msr(unsigned int reg, uint64_t val,
diff --git a/xen/include/public/arch-x86/xen.h 
b/xen/include/public/arch-x86/xen.h
index 6bf1e8cccb..7acd94c8eb 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -143,6 +143,12 @@ typedef unsigned long xen_ulong_t;
  *  Level == 1: Kernel may enter
  *  Level == 2: Kernel may enter
  *  Level == 3: Everyone may enter
+ *
+ * Note: For compatibility with kernels not setting up exception handlers
+ *       early enough, Xen will avoid trying to inject #GP (and hence crash
+ *       the domain) when an RDMSR would require this, but no handler was
+ *       set yet. The precise conditions are implementation specific, and
+ *       new code may not rely on such behavior anyway.
  */
 #define TI_GET_DPL(_ti)      ((_ti)->flags & 3)
 #define TI_GET_IF(_ti)       ((_ti)->flags & 4)
--
generated by git-patchbot for /home/xen/git/xen.git#master