[PATCH 3/3] x86: Fix calculation of %dr6/dr7 reserved bits

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

RTM debugging and BusLock Detect have both introduced conditional behaviour
into the %dr6/7 calculations which Xen's existing logic doesn't account for.

Introduce the CPUID bit for BusLock Detect, so we can get the %dr6 behaviour
correct from the outset.

Implement x86_adj_dr{6,7}_rsvd() fully, and use them in place of the plain
bitmasks.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Signed-off-by: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>

Note for reviewers: The dr7 calculation lacking BLD is correct.  BLD is is
activated by MSR_DBG_CTRL.BLD.  RTM is activated by %dr7.RTM && DBG_CTRL.RTM,
for reasons best answered by the designers...
---
 xen/arch/x86/debug.c                        | 27 +++++++++++++++++++++
 xen/arch/x86/domain.c                       |  5 ++--
 xen/arch/x86/hvm/hvm.c                      |  6 +++--
 xen/arch/x86/include/asm/debugreg.h         |  4 +--
 xen/arch/x86/include/asm/x86-defns.h        | 21 ++++++++++++++--
 xen/arch/x86/pv/misc-hypercalls.c           | 16 +++---------
 xen/include/public/arch-x86/cpufeatureset.h |  1 +
 7 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index 9900b555d6d3..127fe83021cd 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -10,10 +10,37 @@
 
 unsigned int x86_adj_dr6_rsvd(const struct cpu_policy *p, unsigned int dr6)
 {
+    unsigned int ones = X86_DR6_DEFAULT;
+
+    /*
+     * The i586 and later processors had most but not all reserved bits read
+     * as 1s.  New features allocated in this space have inverted polarity,
+     * and don't force their respective bit to 1.
+     */
+    if ( p->feat.rtm )
+        ones &= ~X86_DR6_RTM;
+    if ( p->feat.bld )
+        ones &= ~X86_DR6_BLD;
+
+    dr6 |= ones;
+    dr6 &= ~X86_DR6_ZEROS;
+
     return dr6;
 }
 
 unsigned int x86_adj_dr7_rsvd(const struct cpu_policy *p, unsigned int dr7)
 {
+    unsigned int zeros = X86_DR7_ZEROS;
+
+    /*
+     * Most but not all reserved bits force to zero.  Hardware lacking
+     * optional features force more bits to zero.
+     */
+    if ( !p->feat.rtm )
+        zeros |= X86_DR7_RTM;
+
+    dr7 &= ~zeros;
+    dr7 |= X86_DR7_DEFAULT;
+
     return dr7;
 }
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 0698e6d486fe..2d77b83c0bf8 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1053,6 +1053,7 @@ int arch_set_info_guest(
     struct vcpu *v, vcpu_guest_context_u c)
 {
     struct domain *d = v->domain;
+    const struct cpu_policy *p = d->arch.cpu_policy;
     unsigned int i;
     unsigned long flags;
     bool compat;
@@ -1186,8 +1187,8 @@ int arch_set_info_guest(
     {
         for ( i = 0; i < ARRAY_SIZE(v->arch.dr); ++i )
             v->arch.dr[i] = c(debugreg[i]);
-        v->arch.dr6 = c(debugreg[6]);
-        v->arch.dr7 = c(debugreg[7]);
+        v->arch.dr6 = x86_adj_dr6_rsvd(p, c(debugreg[6]));
+        v->arch.dr7 = x86_adj_dr7_rsvd(p, c(debugreg[7]));
 
         if ( v->vcpu_id == 0 )
             d->vm_assist = c.nat->vm_assist;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3dc2019eca67..482eebbabf7f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -33,6 +33,7 @@
 #include <asm/shadow.h>
 #include <asm/hap.h>
 #include <asm/current.h>
+#include <asm/debugreg.h>
 #include <asm/e820.h>
 #include <asm/io.h>
 #include <asm/regs.h>
@@ -985,6 +986,7 @@ unsigned long hvm_cr4_guest_valid_bits(const struct domain 
*d)
 
 static int cf_check hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t 
*h)
 {
+    const struct cpu_policy *p = d->arch.cpu_policy;
     unsigned int vcpuid = hvm_load_instance(h);
     struct vcpu *v;
     struct hvm_hw_cpu ctxt;
@@ -1174,8 +1176,8 @@ static int cf_check hvm_load_cpu_ctxt(struct domain *d, 
hvm_domain_context_t *h)
     v->arch.dr[1] = ctxt.dr1;
     v->arch.dr[2] = ctxt.dr2;
     v->arch.dr[3] = ctxt.dr3;
-    v->arch.dr6   = ctxt.dr6;
-    v->arch.dr7   = ctxt.dr7;
+    v->arch.dr6   = x86_adj_dr6_rsvd(p, ctxt.dr6);
+    v->arch.dr7   = x86_adj_dr7_rsvd(p, ctxt.dr7);
 
     hvmemul_cancel(v);
 
diff --git a/xen/arch/x86/include/asm/debugreg.h 
b/xen/arch/x86/include/asm/debugreg.h
index 673b81ec5eda..bdeedc4c4c99 100644
--- a/xen/arch/x86/include/asm/debugreg.h
+++ b/xen/arch/x86/include/asm/debugreg.h
@@ -1,6 +1,7 @@
 #ifndef _X86_DEBUGREG_H
 #define _X86_DEBUGREG_H
 
+#include <asm/x86-defns.h>
 
 /* Indicate the register numbers for a number of the specific
    debug registers.  Registers 0-3 contain the addresses we wish to trap on */
@@ -21,7 +22,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_ZERO (~0xffffefffUL) /* Reserved, read as zero */
 #define DR_STATUS_RESERVED_ONE  0xffff0ff0UL /* Reserved, read as one */
 
 /* Now define a bunch of things for manipulating the control register.
@@ -61,8 +61,6 @@
    We can slow the instruction pipeline for instructions coming via the
    gdt or the ldt if we want to.  I am not sure why this is an advantage */
 
-#define DR_CONTROL_RESERVED_ZERO (~0xffff27ffUL) /* Reserved, read as zero */
-#define DR_CONTROL_RESERVED_ONE  (0x00000400UL) /* Reserved, read as one */
 #define DR_LOCAL_EXACT_ENABLE    (0x00000100UL) /* Local exact enable */
 #define DR_GLOBAL_EXACT_ENABLE   (0x00000200UL) /* Global exact enable */
 #define DR_RTM_ENABLE            (0x00000800UL) /* RTM debugging enable */
diff --git a/xen/arch/x86/include/asm/x86-defns.h 
b/xen/arch/x86/include/asm/x86-defns.h
index e350227e57eb..74fb0322cb84 100644
--- a/xen/arch/x86/include/asm/x86-defns.h
+++ b/xen/arch/x86/include/asm/x86-defns.h
@@ -102,13 +102,30 @@
 
 /*
  * Debug status flags in DR6.
+ *
+ * For backwards compatibility, status flags which overlap with
+ * X86_DR6_DEFAULT have inverted polarity.
  */
-#define X86_DR6_DEFAULT         0xffff0ff0  /* Default %dr6 value. */
+#define X86_DR6_B0              (_AC(1, UL) <<  0)   /* Breakpoint 0           
     */
+#define X86_DR6_B1              (_AC(1, UL) <<  1)   /* Breakpoint 1           
     */
+#define X86_DR6_B2              (_AC(1, UL) <<  2)   /* Breakpoint 2           
     */
+#define X86_DR6_B3              (_AC(1, UL) <<  3)   /* Breakpoint 3           
     */
+#define X86_DR6_BLD             (_AC(1, UL) << 11)   /* BusLock detect (INV)   
     */
+#define X86_DR6_BD              (_AC(1, UL) << 13)   /* %dr access             
     */
+#define X86_DR6_BS              (_AC(1, UL) << 14)   /* Single step            
     */
+#define X86_DR6_BT              (_AC(1, UL) << 15)   /* Task switch            
     */
+#define X86_DR6_RTM             (_AC(1, UL) << 16)   /* #DB/#BP in RTM region 
(INV) */
+
+#define X86_DR6_ZEROS           _AC(0x00010000, UL)  /* %dr6 bits forced to 0  
     */
+#define X86_DR6_DEFAULT         _AC(0xffff0ff0, UL)  /* Default %dr6 value     
     */
 
 /*
  * Debug control flags in DR7.
  */
-#define X86_DR7_DEFAULT         0x00000400  /* Default %dr7 value. */
+#define X86_DR7_RTM             (_AC(1, UL) << 11)   /* RTM debugging enable   
     */
+
+#define X86_DR7_ZEROS           _AC(0x0000d000, UL)  /* %dr7 bits forced to 0  
     */
+#define X86_DR7_DEFAULT         _AC(0x00000400, UL)  /* Default %dr7 value     
     */
 
 /*
  * Invalidation types for the INVPCID instruction.
diff --git a/xen/arch/x86/pv/misc-hypercalls.c 
b/xen/arch/x86/pv/misc-hypercalls.c
index b11bd718b7de..99f502812868 100644
--- a/xen/arch/x86/pv/misc-hypercalls.c
+++ b/xen/arch/x86/pv/misc-hypercalls.c
@@ -56,6 +56,7 @@ long do_fpu_taskswitch(int set)
 long set_debugreg(struct vcpu *v, unsigned int reg, unsigned long value)
 {
     struct vcpu *curr = current;
+    const struct cpu_policy *p = curr->domain->arch.cpu_policy;
 
     switch ( reg )
     {
@@ -86,12 +87,7 @@ long set_debugreg(struct vcpu *v, unsigned int reg, unsigned 
long value)
         if ( value != (uint32_t)value )
             return -EINVAL;
 
-        /*
-         * DR6: Bits 4-11,16-31 reserved (set to 1).
-         *      Bit 12 reserved (set to 0).
-         */
-        value &= ~DR_STATUS_RESERVED_ZERO; /* reserved bits => 0 */
-        value |=  DR_STATUS_RESERVED_ONE;  /* reserved bits => 1 */
+        value = x86_adj_dr6_rsvd(p, value);
 
         v->arch.dr6 = value;
         if ( v == curr )
@@ -108,12 +104,8 @@ long set_debugreg(struct vcpu *v, unsigned int reg, 
unsigned long value)
         if ( value != (uint32_t)value )
             return -EINVAL;
 
-        /*
-         * DR7: Bit 10 reserved (set to 1).
-         *      Bits 11-12,14-15 reserved (set to 0).
-         */
-        value &= ~DR_CONTROL_RESERVED_ZERO; /* reserved bits => 0 */
-        value |=  DR_CONTROL_RESERVED_ONE;  /* reserved bits => 1 */
+        value = x86_adj_dr7_rsvd(p, value);
+
         /*
          * Privileged bits:
          *      GD (bit 13): must be 0.
diff --git a/xen/include/public/arch-x86/cpufeatureset.h 
b/xen/include/public/arch-x86/cpufeatureset.h
index 50fda581f2df..6b6ce2745cfe 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -223,6 +223,7 @@ XEN_CPUFEATURE(AVX512_VNNI,   6*32+11) /*A  Vector Neural 
Network Instrs */
 XEN_CPUFEATURE(AVX512_BITALG, 6*32+12) /*A  Support for VPOPCNT[B,W] and 
VPSHUFBITQMB */
 XEN_CPUFEATURE(AVX512_VPOPCNTDQ, 6*32+14) /*A  POPCNT for vectors of DW/QW */
 XEN_CPUFEATURE(RDPID,         6*32+22) /*A  RDPID instruction */
+XEN_CPUFEATURE(BLD,           6*32+24) /*   BusLock Detect (#DB trap) support 
*/
 XEN_CPUFEATURE(CLDEMOTE,      6*32+25) /*A  CLDEMOTE instruction */
 XEN_CPUFEATURE(MOVDIRI,       6*32+27) /*a  MOVDIRI instruction */
 XEN_CPUFEATURE(MOVDIR64B,     6*32+28) /*a  MOVDIR64B instruction */
-- 
2.30.2