[PATCH v2 1/3] x86/pv: Introduce x86_merge_dr6() and fix do_debug()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Pretty much everywhere in Xen the logic to update %dr6 when injecting #DB is
buggy.  The architectural behaviour is to overwrite B{0..3} (rather than
accumulate) and accumulate all other bits.

Introduce a new x86_merge_dr6() helper, and start fixing the mess by adjusting
the dr6 merge in do_debug().  Also correct the comment.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

v2:
 * Rebase (~6y worth)
 * Split PV changes out of joint HVM patch.

In some theoretical future where deubgging is implemented in terms of
introspection even for PV guests, the TODO will complete itself.
---
 xen/arch/x86/debug.c                 | 20 ++++++++++++++++++++
 xen/arch/x86/include/asm/debugreg.h  |  8 ++++++++
 xen/arch/x86/include/asm/x86-defns.h |  7 +++++++
 xen/arch/x86/traps.c                 | 10 +++++++---
 4 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index 127fe83021cd..429752b8cc83 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -2,12 +2,32 @@
 /*
  * Copyright (C) 2023 XenServer.
  */
+#include <xen/bug.h>
 #include <xen/kernel.h>
 
 #include <xen/lib/x86/cpu-policy.h>
 
 #include <asm/debugreg.h>
 
+unsigned int x86_merge_dr6(const struct cpu_policy *p, unsigned int dr6,
+                           unsigned int pending_dbg)
+{
+    /* Flip dr6 to have positive polarity. */
+    dr6 ^= X86_DR6_DEFAULT;
+
+    /* Sanity check that only known values are passed in. */
+    ASSERT(!(dr6 & ~X86_DR6_KNOWN_MASK));
+    ASSERT(!(pending_dbg & ~X86_DR6_KNOWN_MASK));
+
+    /* Breakpoints 0-3 overridden.  Others accumulate. */
+    dr6 = (dr6 & ~X86_DR6_BP_MASK) | pending_dbg;
+
+    /* Flip dr6 back to having default polarity. */
+    dr6 ^= X86_DR6_DEFAULT;
+
+    return x86_adj_dr6_rsvd(p, dr6);
+}
+
 unsigned int x86_adj_dr6_rsvd(const struct cpu_policy *p, unsigned int dr6)
 {
     unsigned int ones = X86_DR6_DEFAULT;
diff --git a/xen/arch/x86/include/asm/debugreg.h 
b/xen/arch/x86/include/asm/debugreg.h
index 96c406ad53c8..969f2697aee1 100644
--- a/xen/arch/x86/include/asm/debugreg.h
+++ b/xen/arch/x86/include/asm/debugreg.h
@@ -108,4 +108,12 @@ struct cpu_policy;
 unsigned int x86_adj_dr6_rsvd(const struct cpu_policy *p, unsigned int dr6);
 unsigned int x86_adj_dr7_rsvd(const struct cpu_policy *p, unsigned int dr7);
 
+/*
+ * Merging new status bits into dr6 is far from simple.  Breakpoints override,
+ * while others accumulate.  New bits to be merged are taken with positive
+ * polarity.
+ */
+unsigned int x86_merge_dr6(const struct cpu_policy *p, unsigned int dr6,
+                           unsigned int pending_dbg);
+
 #endif /* _X86_DEBUGREG_H */
diff --git a/xen/arch/x86/include/asm/x86-defns.h 
b/xen/arch/x86/include/asm/x86-defns.h
index 3bcdbaccd3aa..caa92829eaa9 100644
--- a/xen/arch/x86/include/asm/x86-defns.h
+++ b/xen/arch/x86/include/asm/x86-defns.h
@@ -132,6 +132,13 @@
 #define X86_DR6_ZEROS           _AC(0x00001000, UL)  /* %dr6 bits forced to 0  
     */
 #define X86_DR6_DEFAULT         _AC(0xffff0ff0, UL)  /* Default %dr6 value     
     */
 
+#define X86_DR6_BP_MASK                                 \
+    (X86_DR6_B0 | X86_DR6_B1 | X86_DR6_B2 | X86_DR6_B3)
+
+#define X86_DR6_KNOWN_MASK                                              \
+    (X86_DR6_BP_MASK | X86_DR6_BLD | X86_DR6_BD | X86_DR6_BS |          \
+     X86_DR6_BT | X86_DR6_RTM)
+
 /*
  * Debug control flags in DR7.
  */
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 552a07e6aa56..521ed4dd816d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2016,9 +2016,13 @@ void asmlinkage do_debug(struct cpu_user_regs *regs)
         return;
     }
 
-    /* Save debug status register where guest OS can peek at it */
-    v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
-    v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
+    /*
+     * Update the guest's dr6 so the debugger can peek at it.
+     * TODO: This should be passed out-of-bad to the debugger, so guest state
+     * is not corrupted by debugging actions completed behind it's back.
+     */
+    v->arch.dr6 = x86_merge_dr6(v->domain->arch.cpu_policy,
+                                v->arch.dr6, dr6 ^ X86_DR6_DEFAULT);
 
     if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
     {
-- 
2.39.2