[PATCH 4/5] x86: Fix merging of new status bits into %dr6

[Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>]

From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

The current logic used 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 merge_dr6() helper, which also takes care of handing RTM
correctly.

Signed-off-by: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
---
 xen/arch/x86/hvm/svm/svm.c           |  3 ++-
 xen/arch/x86/hvm/vmx/vmx.c           |  3 ++-
 xen/arch/x86/include/asm/debugreg.h  | 30 +++++++++++++++++++++++++++-
 xen/arch/x86/include/asm/x86-defns.h | 10 ----------
 xen/arch/x86/pv/traps.c              |  3 ++-
 5 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index bd3adde0ec..7a5652f3df 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1338,7 +1338,8 @@ static void cf_check svm_inject_event(const struct 
x86_event *event)
          * Item 2 is done by hardware when injecting a #DB exception.
          */
         __restore_debug_registers(vmcb, curr);
-        vmcb_set_dr6(vmcb, vmcb_get_dr6(vmcb) | _event.pending_dbg);
+        vmcb_set_dr6(vmcb, merge_dr6(vmcb_get_dr6(vmcb), _event.pending_dbg,
+                                     curr->domain->arch.cpuid->feat.rtm));
 
         /* fall through */
     case X86_EXC_BP:
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 65166348f1..1fd902ed74 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2031,7 +2031,8 @@ static void cf_check vmx_inject_event(const struct 
x86_event *event)
          * All actions are left up to the hypervisor to perform.
          */
         __restore_debug_registers(curr);
-        write_debugreg(6, read_debugreg(6) | event->pending_dbg);
+        write_debugreg(6, merge_dr6(read_debugreg(6), event->pending_dbg,
+                                    curr->domain->arch.cpuid->feat.rtm));
 
         if ( !nestedhvm_vcpu_in_guestmode(curr) ||
              !nvmx_intercepts_exception(curr, X86_EXC_DB, _event.error_code) )
diff --git a/xen/arch/x86/include/asm/debugreg.h 
b/xen/arch/x86/include/asm/debugreg.h
index a1df74c02e..fd32846397 100644
--- a/xen/arch/x86/include/asm/debugreg.h
+++ b/xen/arch/x86/include/asm/debugreg.h
@@ -23,6 +23,14 @@
 #define X86_DR6_BT              (1u << 15)  /* Task switch             */
 #define X86_DR6_RTM             (1u << 16)  /* #DB/#BP in RTM region   */
 
+#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_BD | X86_DR6_BS | X86_DR6_BT | X86_DR6_RTM)
+
+#define X86_DR6_DEFAULT         0xffff0ff0  /* Default %dr6 value. */
+
 #define DR_TRAP0        (0x1)           /* db0 */
 #define DR_TRAP1        (0x2)           /* db1 */
 #define DR_TRAP2        (0x4)           /* db2 */
@@ -30,7 +38,6 @@
 #define DR_STEP         (0x4000)        /* single-step */
 #define DR_SWITCH       (0x8000)        /* task switch */
 #define DR_NOT_RTM      (0x10000)       /* clear: #BP inside RTM region */
-#define DR_STATUS_RESERVED_ONE  0xffff0ff0UL /* Reserved, read as one */
 
 /* Now define a bunch of things for manipulating the control register.
    The top two bytes of the control register consist of 4 fields of 4
@@ -74,6 +81,8 @@
 #define DR_RTM_ENABLE            (0x00000800UL) /* RTM debugging enable */
 #define DR_GENERAL_DETECT        (0x00002000UL) /* General detect enable */
 
+#define X86_DR7_DEFAULT         0x00000400  /* Default %dr7 value. */
+
 #define write_debugreg(reg, val) do {                       \
     unsigned long __val = val;                              \
     asm volatile ( "mov %0,%%db" #reg : : "r" (__val) );    \
@@ -102,6 +111,25 @@ static inline unsigned long adjust_dr6_rsvd(unsigned long 
dr6, bool rtm)
     return dr6;
 }
 
+static inline unsigned long merge_dr6(unsigned long dr6, unsigned long new,
+                                      bool rtm)
+{
+    /* 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));
+
+    /* Breakpoints 0-3 overridden.  BD, BS, BT and RTM accumulate. */
+    dr6 = (dr6 & ~X86_DR6_BP_MASK) | new;
+
+    /* Flip dr6 back to having default polarity. */
+    dr6 ^= X86_DR6_DEFAULT;
+
+    return adjust_dr6_rsvd(dr6, rtm);
+}
+
 static inline unsigned long adjust_dr7_rsvd(unsigned long dr7, bool rtm)
 {
     /*
diff --git a/xen/arch/x86/include/asm/x86-defns.h 
b/xen/arch/x86/include/asm/x86-defns.h
index e350227e57..7fbc74850a 100644
--- a/xen/arch/x86/include/asm/x86-defns.h
+++ b/xen/arch/x86/include/asm/x86-defns.h
@@ -100,16 +100,6 @@
 #define X86_XCR0_LWP_POS          62
 #define X86_XCR0_LWP              (1ULL << X86_XCR0_LWP_POS)
 
-/*
- * Debug status flags in DR6.
- */
-#define X86_DR6_DEFAULT         0xffff0ff0  /* Default %dr6 value. */
-
-/*
- * Debug control flags in DR7.
- */
-#define X86_DR7_DEFAULT         0x00000400  /* Default %dr7 value. */
-
 /*
  * Invalidation types for the INVPCID instruction.
  */
diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 4f5641a47c..65b41e6115 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -66,7 +66,8 @@ void pv_inject_event(const struct x86_event *event)
         break;
 
     case X86_EXC_DB:
-        curr->arch.dr6 |= event->pending_dbg;
+        curr->arch.dr6 = merge_dr6(curr->arch.dr6, event->pending_dbg,
+                                   curr->domain->arch.cpuid->feat.rtm);
         /* Fallthrough */
 
     default:
-- 
2.41.0