[Xen-devel] [PATCH] x86/vmx: fix dr6 handling issue

[Yang Zhang <yang.z.zhang@xxxxxxxxx>]

From: Yang Zhang <yang.z.zhang@xxxxxxxxx>

Guest's dr6 may lost in some corner case. For example:
1. Guest set TF bit to enable single step.
2. Debug trap occurs and delivered to guest direclty.
3. VCPU is scheduled out and dr6 is not saved due to flag_debug_dirty
is not set and db7 has no debug event enabled.
4. After VCPU is scheduled in agagin, db6 may lost.

With this patch, hypervisor will intercept the first debug trap after
vcpu is scheduled in and restore dr6 in debug_trap handler.

Reported-by: Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>
Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
---

I don't have the environment to test this patch. So Juergen can you help to
verify it? Thanks.

 xen/arch/x86/hvm/vmx/vmx.c     |   37 +++++++++++++++++++++++++------------
 xen/arch/x86/traps.c           |    4 ++--
 xen/include/asm-x86/debugreg.h |    3 +++
 xen/include/asm-x86/hvm/hvm.h  |    3 ++-
 4 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b6c022b..520098f 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -376,6 +376,10 @@ static void vmx_save_dr(struct vcpu *v)
     v->arch.hvm_vmx.exec_control |= CPU_BASED_MOV_DR_EXITING;
     vmx_update_cpu_exec_control(v);
 
+    /* Trap debug for signle stepping. */
+    v->arch.hvm_vmx.exception_bitmap |= 1 << TRAP_debug;
+    vmx_update_exception_bitmap(v);
+
     v->arch.debugreg[0] = read_debugreg(0);
     v->arch.debugreg[1] = read_debugreg(1);
     v->arch.debugreg[2] = read_debugreg(2);
@@ -392,6 +396,13 @@ static void __restore_debug_registers(struct vcpu *v)
 
     v->arch.hvm_vcpu.flag_dr_dirty = 1;
 
+    /* 
+     * After restore, hardware has the right context.
+     * So no need to trap debug anymore.
+     */
+    v->arch.hvm_vmx.exception_bitmap &= ~(1 << TRAP_debug);
+    vmx_update_exception_bitmap(v);
+
     write_debugreg(0, v->arch.debugreg[0]);
     write_debugreg(1, v->arch.debugreg[1]);
     write_debugreg(2, v->arch.debugreg[2]);
@@ -1178,16 +1189,10 @@ static void vmx_update_host_cr3(struct vcpu *v)
 
 void vmx_update_debug_state(struct vcpu *v)
 {
-    unsigned long mask;
-
-    mask = 1u << TRAP_int3;
-    if ( !cpu_has_monitor_trap_flag )
-        mask |= 1u << TRAP_debug;
-
     if ( v->arch.hvm_vcpu.debug_state_latch )
-        v->arch.hvm_vmx.exception_bitmap |= mask;
+        v->arch.hvm_vmx.exception_bitmap |= 1u << TRAP_int3;
     else
-        v->arch.hvm_vmx.exception_bitmap &= ~mask;
+        v->arch.hvm_vmx.exception_bitmap &= ~(1u << TRAP_int3);
 
     vmx_vmcs_enter(v);
     vmx_update_exception_bitmap(v);
@@ -2717,10 +2722,18 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
              */
             __vmread(EXIT_QUALIFICATION, &exit_qualification);
             HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
-            write_debugreg(6, exit_qualification | 0xffff0ff0);
-            if ( !v->domain->debugger_attached || cpu_has_monitor_trap_flag )
-                goto exit_and_crash;
-            domain_pause_for_debugger();
+            exit_qualification |= DR_STATUS_RESERVED_ONE;
+            if ( v->domain->debugger_attached )
+            {
+                write_debugreg(6, exit_qualification);
+                domain_pause_for_debugger();
+            }
+            else 
+            {
+                v->arch.debugreg[6] |= exit_qualification;
+                __restore_debug_registers(v);
+                hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE);
+            }
             break;
         case TRAP_int3: 
         {
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 40366f1..e86ca2c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3763,8 +3763,8 @@ long set_debugreg(struct vcpu *v, unsigned int reg, 
unsigned long value)
          * DR6: Bits 4-11,16-31 reserved (set to 1).
          *      Bit 12 reserved (set to 0).
          */
-        value &= 0xffffefff; /* reserved bits => 0 */
-        value |= 0xffff0ff0; /* reserved bits => 1 */
+        value &= ~DR_STATUS_RESERVED_ZERO;  /* reserved bits => 0 */
+        value |= DR_STATUS_RESERVED_ONE;    /* reserved bits => 1 */
         if ( v == curr ) 
             write_debugreg(6, value);
         break;
diff --git a/xen/include/asm-x86/debugreg.h b/xen/include/asm-x86/debugreg.h
index 62007bb..4a9a7d6 100644
--- a/xen/include/asm-x86/debugreg.h
+++ b/xen/include/asm-x86/debugreg.h
@@ -76,4 +76,7 @@
 long set_debugreg(struct vcpu *, unsigned int reg, unsigned long value);
 void activate_debugregs(const struct vcpu *);
 
+#define DR_STATUS_RESERVED_ZERO  (0x00001000ul) /* Reserved, read as zero */
+#define DR_STATUS_RESERVED_ONE   (0xffff0ff0ul) /* Reserved, read as one */
+
 #endif /* _X86_DEBUGREG_H */
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index b5a56e0..eb73da7 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -420,7 +420,8 @@ static inline bool_t hvm_vcpu_has_smap(void)
         (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))))
 
 /* These exceptions must always be intercepted. */
-#define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U << TRAP_invalid_op))
+#define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U << TRAP_invalid_op) |\
+                       (1U << TRAP_debug))
 
 /*
  * x86 event types. This enumeration is valid for:
-- 
1.7.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel