[xen master] x86/pv: Introduce x86_merge_dr6() and fix do_debug()

[patchbot@xxxxxxx]

commit 54ef601a66e8d812a6a6a308f02524e81201825e
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Mon May 28 15:20:18 2018 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Wed Aug 21 23:59:19 2024 +0100

    x86/pv: Introduce x86_merge_dr6() and fix do_debug()
    
    Pretty much everywhere in Xen the logic to update %dr6 when injecting #DB is
    buggy.  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>
---
 xen/arch/x86/debug.c                 | 40 ++++++++++++++++++++++++++++++++++++
 xen/arch/x86/include/asm/debugreg.h  |  7 +++++++
 xen/arch/x86/include/asm/x86-defns.h |  7 +++++++
 xen/arch/x86/traps.c                 | 11 +++++++---
 4 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index 127fe83021..b10f1f12b6 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -2,12 +2,52 @@
 /*
  * Copyright (C) 2023 XenServer.
  */
+#include <xen/bug.h>
 #include <xen/kernel.h>
 
 #include <xen/lib/x86/cpu-policy.h>
 
 #include <asm/debugreg.h>
 
+/*
+ * Merge new bits into dr6.  'new' is always given in positive polarity,
+ * matching the Intel VMCS PENDING_DBG semantics.
+ *
+ * At the time of writing (August 2024), on the subject of %dr6 updates the
+ * manuals are either vague (Intel "certain exceptions may clear bits 0-3"),
+ * or disputed (AMD makes statements which don't match observed behaviour).
+ *
+ * The only debug exception I can find which doesn't clear the breakpoint bits
+ * is ICEBP(/INT1) on AMD systems.  This is also the one source of #DB that
+ * doesn't have an explicit status bit, meaning we can't easily identify this
+ * case either (AMD systems don't virtualise PENDING_DBG and only provide a
+ * post-merge %dr6 value).
+ *
+ * Treat %dr6 merging as unconditionally writing the breakpoint bits.
+ *
+ * We can't really manage any better, and guest kernels handling #DB as
+ * instructed by the SDM/APM (i.e. reading %dr6 then resetting it back to
+ * default) wont notice.
+ */
+unsigned int x86_merge_dr6(const struct cpu_policy *p, unsigned int dr6,
+                           unsigned int new)
+{
+    /* 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(!(new & ~X86_DR6_KNOWN_MASK));
+
+    /* Breakpoint bits overridden.  All others accumulate. */
+    dr6 = (dr6 & ~X86_DR6_BP_MASK) | new;
+
+    /* 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 96c406ad53..6baa725441 100644
--- a/xen/arch/x86/include/asm/debugreg.h
+++ b/xen/arch/x86/include/asm/debugreg.h
@@ -108,4 +108,11 @@ 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);
 
+/*
+ * Merge new bits into dr6.  'new' is always given in positive polarity,
+ * matching the Intel VMCS PENDING_DBG semantics.
+ */
+unsigned int x86_merge_dr6(const struct cpu_policy *p, unsigned int dr6,
+                           unsigned int new);
+
 #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 3bcdbaccd3..caa92829ea 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 552a07e6aa..feef6faa50 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2016,9 +2016,14 @@ 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-band, so guest state is not modified
+     * 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 )
     {
--
generated by git-patchbot for /home/xen/git/xen.git#master