x86/pv: Introduce and use x86emul_read_dr()

do_get_debugreg() has several bugs:

 * The %cr4.de condition is inverted.  %dr4/5 should be accessible only when
   %cr4.de is disabled.
 * When %cr4.de is disabled, emulation should yield #UD rather than complete
   with zero.
 * Using -EINVAL for errors is a broken ABI, as it overlaps with valid values
   near the top of the address space.

Introduce a common x86emul_read_dr() handler (as we will eventually want to
add HVM support) which separates its success/failure indication from the data
value, and have do_get_debugreg() call into the handler.

The ABI of do_get_debugreg() remains broken, but switches from -EINVAL to
-ENODEV for compatibility with the changes in the following patch.

Take the opportunity to add a missing local variable block to x86_emulate.c

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index b4564080ad59d737ccbc2019ad28dcbe61e5f300..61702d9cd31a3bff7c2c13f01e8f97c7ace17cc6 100644 (file)
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -794,19 +794,6 @@ static int write_cr(unsigned int reg, unsigned long val,
     return X86EMUL_UNHANDLEABLE;
 }
 
-static int read_dr(unsigned int reg, unsigned long *val,
-                   struct x86_emulate_ctxt *ctxt)
-{
-    unsigned long res = do_get_debugreg(reg);
-
-    if ( IS_ERR_VALUE(res) )
-        return X86EMUL_UNHANDLEABLE;
-
-    *val = res;
-
-    return X86EMUL_OKAY;
-}
-
 static int write_dr(unsigned int reg, unsigned long val,
                     struct x86_emulate_ctxt *ctxt)
 {
@@ -1305,7 +1292,7 @@ static const struct x86_emulate_ops priv_op_ops = {
     .read_segment        = read_segment,
     .read_cr             = read_cr,
     .write_cr            = write_cr,
-    .read_dr             = read_dr,
+    .read_dr             = x86emul_read_dr,
     .write_dr            = write_dr,
     .write_xcr           = x86emul_write_xcr,
     .read_msr            = read_msr,

diff --git a/xen/arch/x86/pv/misc-hypercalls.c b/xen/arch/x86/pv/misc-hypercalls.c
index 5862130697c8de3f6534d1e25d5660a758809230..1619be787466939061bb67491406341d77987f8d 100644 (file)
--- a/xen/arch/x86/pv/misc-hypercalls.c
+++ b/xen/arch/x86/pv/misc-hypercalls.c
@@ -30,22 +30,10 @@ long do_set_debugreg(int reg, unsigned long value)
 
 unsigned long do_get_debugreg(int reg)
 {
-    struct vcpu *curr = current;
+    unsigned long val;
+    int res = x86emul_read_dr(reg, &val, NULL);
 
-    switch ( reg )
-    {
-    case 0 ... 3:
-    case 6:
-        return curr->arch.debugreg[reg];
-    case 7:
-        return (curr->arch.debugreg[7] |
-                curr->arch.debugreg[5]);
-    case 4 ... 5:
-        return ((curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ?
-                curr->arch.debugreg[reg + 2] : 0);
-    }
-
-    return -EINVAL;
+    return res == X86EMUL_OKAY ? val : -ENODEV;
 }
 
 long do_fpu_taskswitch(int set)

diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c
index 0729edca7bc84c88660f8730ba6396f6766ce614..d260bdca6df016a7d0405ec4ff2bffa6607ff7c1 100644 (file)
--- a/xen/arch/x86/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate.c
@@ -87,3 +87,52 @@ int x86emul_write_xcr(unsigned int reg, uint64_t val,
 
     return X86EMUL_OKAY;
 }
+
+/* Called with NULL ctxt in hypercall context. */
+int x86emul_read_dr(unsigned int reg, unsigned long *val,
+                    struct x86_emulate_ctxt *ctxt)
+{
+    struct vcpu *curr = current;
+
+    /* HVM support requires a bit more plumbing before it will work. */
+    ASSERT(is_pv_vcpu(curr));
+
+    switch ( reg )
+    {
+    case 0 ... 3:
+    case 6:
+        *val = curr->arch.debugreg[reg];
+        break;
+
+    case 7:
+        *val = (curr->arch.debugreg[7] |
+                curr->arch.debugreg[5]);
+        break;
+
+    case 4 ... 5:
+        if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) )
+        {
+            *val = curr->arch.debugreg[reg + 2];
+            break;
+        }
+
+        /* Fallthrough */
+    default:
+        if ( ctxt )
+            x86_emul_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC, ctxt);
+
+        return X86EMUL_EXCEPTION;
+    }
+
+    return X86EMUL_OKAY;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 13385b0e404ac1bf903df5b9498b34a950b639cf..2ae0518ae15e77984b08b9be9b8bb96b41daa8c9 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -722,6 +722,9 @@ int x86emul_read_xcr(unsigned int reg, uint64_t *val,
 int x86emul_write_xcr(unsigned int reg, uint64_t val,
                       struct x86_emulate_ctxt *ctxt);
 
+int x86emul_read_dr(unsigned int reg, unsigned long *val,
+                    struct x86_emulate_ctxt *ctxt);
+
 #endif
 
 int