[PATCH 04/11] x86: Fix calculation of %dr6/7 reserved bits

[Andrew Cooper <andrew.cooper3@citrix.com>]

The reserved bit calculations for %dr6 and %dr7 depend on whether the VM has
the Restricted Transnational Memory feature available.

Introduce adjust_dr{6,7}_rsvd() and replace the opencoded logic and constants
(except for DR_STATUS_RESERVED_ONE which is (mis)used elsewhere and will be
removed after future bugfixes).  The use of these helpers in set_debugreg()
covers toolstack values for PV guests, but HVM guests need similar treatment.

The use of the guests cpuid policy is less than optimal in the create/restore
paths.  However in such cases, the policy will be the guest maximum policy,
which will be more permissive with respect to the RTM feature.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/domain.c          |  5 ++++-
 xen/arch/x86/hvm/hvm.c         |  5 +++--
 xen/arch/x86/traps.c           | 17 +++++-----------
 xen/include/asm-x86/debugreg.h | 44 ++++++++++++++++++++++++++++++++++++------
 4 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 7ae9789..cee57a8 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -783,6 +783,7 @@ int arch_set_info_guest(
     struct vcpu *v, vcpu_guest_context_u c)
 {
     struct domain *d = v->domain;
+    const struct cpuid_policy *cp = d->arch.cpuid;
     unsigned long cr3_gfn;
     struct page_info *cr3_page;
     unsigned long flags, cr4;
@@ -894,8 +895,10 @@ int arch_set_info_guest(
 
     if ( is_hvm_domain(d) )
     {
-        for ( i = 0; i < ARRAY_SIZE(v->arch.debugreg); ++i )
+        for ( i = 0; i < ARRAY_SIZE(v->arch.debugreg) - 2; ++i )
             v->arch.debugreg[i] = c(debugreg[i]);
+        v->arch.debugreg[6] = adjust_dr6_rsvd(c(debugreg[6]), cp->feat.rtm);
+        v->arch.debugreg[7] = adjust_dr7_rsvd(c(debugreg[7]), cp->feat.rtm);
 
         hvm_set_info_guest(v);
         goto out;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 10415e6..7fddae1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -977,6 +977,7 @@ unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore)
 
 static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 {
+    const struct cpuid_policy *cp = d->arch.cpuid;
     int vcpuid;
     struct vcpu *v;
     struct hvm_hw_cpu ctxt;
@@ -1154,8 +1155,8 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
     v->arch.debugreg[1] = ctxt.dr1;
     v->arch.debugreg[2] = ctxt.dr2;
     v->arch.debugreg[3] = ctxt.dr3;
-    v->arch.debugreg[6] = ctxt.dr6;
-    v->arch.debugreg[7] = ctxt.dr7;
+    v->arch.debugreg[6] = adjust_dr6_rsvd(ctxt.dr6, cp->feat.rtm);
+    v->arch.debugreg[7] = adjust_dr7_rsvd(ctxt.dr7, cp->feat.rtm);
 
     v->arch.vgc_flags = VGCF_online;
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 6990c67..e9bfbc7 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2057,6 +2057,7 @@ void activate_debugregs(const struct vcpu *curr)
 long set_debugreg(struct vcpu *v, unsigned int reg, unsigned long value)
 {
     struct vcpu *curr = current;
+    const struct cpuid_policy *cp = curr->domain->arch.cpuid;
 
     switch ( reg )
     {
@@ -2086,12 +2087,8 @@ 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 = adjust_dr6_rsvd(value, cp->feat.rtm);
+
         if ( v == curr )
             write_debugreg(6, value);
         break;
@@ -2106,12 +2103,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 = adjust_dr7_rsvd(value, cp->feat.rtm);
+
         /*
          * Privileged bits:
          *      GD (bit 13): must be 0.
diff --git a/xen/include/asm-x86/debugreg.h b/xen/include/asm-x86/debugreg.h
index 8e6a656..8df566b 100644
--- a/xen/include/asm-x86/debugreg.h
+++ b/xen/include/asm-x86/debugreg.h
@@ -10,9 +10,18 @@
 #define DR_STATUS    6
 #define DR_CONTROL   7
 
-/* Define a few things for the status register.  We can use this to determine
-   which debugging register was responsible for the trap.  The other bits
-   are either reserved or not of interest to us. */
+/*
+ * DR6 status bits.
+ *   N.B. For backwards compatibility, X86_DR6_RTM has inverted polarity.
+ */
+#define X86_DR6_B0              (1u <<  0)  /* Breakpoint 0 triggered  */
+#define X86_DR6_B1              (1u <<  1)  /* Breakpoint 1 triggered  */
+#define X86_DR6_B2              (1u <<  2)  /* Breakpoint 2 triggered  */
+#define X86_DR6_B3              (1u <<  3)  /* Breakpoint 3 triggered  */
+#define X86_DR6_BD              (1u << 13)  /* Debug register accessed */
+#define X86_DR6_BS              (1u << 14)  /* Single step             */
+#define X86_DR6_BT              (1u << 15)  /* Task switch             */
+#define X86_DR6_RTM             (1u << 16)  /* #DB/#BP in RTM region   */
 
 #define DR_TRAP0        (0x1)           /* db0 */
 #define DR_TRAP1        (0x2)           /* db1 */
@@ -21,7 +30,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 */
 
 #define X86_DR6_DEFAULT 0xffff0ff0ul    /* Default %dr6 value. */
@@ -63,8 +71,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 */
@@ -84,4 +90,30 @@
 long set_debugreg(struct vcpu *, unsigned int reg, unsigned long value);
 void activate_debugregs(const struct vcpu *);
 
+static inline unsigned long adjust_dr6_rsvd(unsigned long dr6, bool rtm)
+{
+    /*
+     * DR6: Bits 4-11,17-31 reserved (set to 1).
+     *      Bit  16 reserved (set to 1) if RTM unavailable.
+     *      Bit  12 reserved (set to 0).
+     */
+    dr6 |= 0xfffe0ff0 | (rtm ? 0 : X86_DR6_RTM);
+    dr6 &= 0xffffefff;
+
+    return dr6;
+}
+
+static inline unsigned long adjust_dr7_rsvd(unsigned long dr7, bool rtm)
+{
+    /*
+     * DR7: Bit  10 reserved (set to 1).
+     *      Bit  11 reserved (set to 0) if RTM unavailable.
+     *      Bits 12,14-15 reserved (set to 0).
+     */
+    dr7 |= 0x00000400;
+    dr7 &= 0xffff23ff & (rtm ? 0 : ~DR_RTM_ENABLE);
+
+    return dr7;
+}
+
 #endif /* _X86_DEBUGREG_H */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel