[Xen-devel] [PATCH 4/5] x86/pv: Remove deferred RDTSC{, P} handling in pv_emulate_privileged_op()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The handling of RDTSCP for PV guests has been broken (AFAICT forever).

To start with, RDTSCP is hidden from PV guests so the MSR_TSC_AUX path should
be unreachable.  However, this appears to be a "feature" of TSC_MODE_PVRDTSCP,
and the emulator doesn't perform appropriate feature checking.  (Conversely,
we unilaterally advertise RDPID which uses the same path, but it should never
trap on #GP to arrive here in the first place).

A PV guest typically can see RDTSCP in native CPUID, so userspace will
probably end up using it.  On a capable pipeline (without TSD, see below), it
will execute normally and return non-virtualised data.

When a virtual TSC mode is not specified for the domain, CR4.TSD is left
clear, so executing RDTSCP will execute without trapping.  However, a guest
kernel may set TSD itself, at which point the emulator should not suddenly
switch to virtualised TSC mode and start handing out differently-scaled
values.

Drop all the deferral logic, and return scaled or raw TSC values depending
only on currd->arch.vtsc.  This changes the exact moment at which the
timestamp is taken, but that doesn't matter from the guests point of view, and
is consistent with the HVM side of things.  It also means that RDTSC and
RDTSCP are now consistent WRT handing out native or virtualised timestamps.

The MSR_TSC_AUX case unconditionally returns the migration incarnation or
zero, depending on TSC_MODE_PVRDTSCP, which is faster than re-reading it out
of hardware.

This is a behavioural change for guests, but the semantics are rather more
sane.  It lays groundwork for further fixes.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 xen/arch/x86/pv/emul-priv-op.c | 35 +++++------------------------------
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index d4d64f2..4e3641d 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -60,9 +60,6 @@ struct priv_op_ctxt {
     } cs;
     char *io_emul_stub;
     unsigned int bpmatch;
-    unsigned int tsc;
-#define TSC_BASE 1
-#define TSC_AUX 2
 };
 
 /* I/O emulation support. Helper routines for, and type of, the stack stub. */
@@ -843,8 +840,7 @@ static inline bool is_cpufreq_controller(const struct 
domain *d)
 static int read_msr(unsigned int reg, uint64_t *val,
                     struct x86_emulate_ctxt *ctxt)
 {
-    struct priv_op_ctxt *poc = container_of(ctxt, struct priv_op_ctxt, ctxt);
-    const struct vcpu *curr = current;
+    struct vcpu *curr = current;
     const struct domain *currd = curr->domain;
     bool vpmu_msr = false;
     int ret;
@@ -880,20 +876,13 @@ static int read_msr(unsigned int reg, uint64_t *val,
         *val = curr->arch.pv_vcpu.gs_base_user;
         return X86EMUL_OKAY;
 
-    /*
-     * In order to fully retain original behavior, defer calling
-     * pv_soft_rdtsc() until after emulation. This may want/need to be
-     * reconsidered.
-     */
     case MSR_IA32_TSC:
-        poc->tsc |= TSC_BASE;
-        goto normal;
+        *val = currd->arch.vtsc ? pv_soft_rdtsc(curr, ctxt->regs) : rdtsc();
+        return X86EMUL_OKAY;
 
     case MSR_TSC_AUX:
-        poc->tsc |= TSC_AUX;
-        if ( cpu_has_rdtscp )
-            goto normal;
-        *val = 0;
+        *val = (uint32_t)((currd->arch.tsc_mode == TSC_MODE_PVRDTSCP)
+                          ? currd->arch.incarnation : 0);
         return X86EMUL_OKAY;
 
     case MSR_EFER:
@@ -1372,20 +1361,6 @@ int pv_emulate_privileged_op(struct cpu_user_regs *regs)
     switch ( rc )
     {
     case X86EMUL_OKAY:
-        if ( ctxt.tsc & TSC_BASE )
-        {
-            if ( currd->arch.vtsc || (ctxt.tsc & TSC_AUX) )
-            {
-                msr_split(regs, pv_soft_rdtsc(curr, regs));
-
-                if ( ctxt.tsc & TSC_AUX )
-                    regs->rcx = ((currd->arch.tsc_mode == TSC_MODE_PVRDTSCP)
-                                 ? currd->arch.incarnation : 0);
-            }
-            else
-                msr_split(regs, rdtsc());
-        }
-
         if ( ctxt.ctxt.retire.singlestep )
             ctxt.bpmatch |= DR_STEP;
         if ( ctxt.bpmatch )
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel