Author: jhb
Date: Tue May 22 00:45:00 2018
New Revision: 334009
URL: https://svnweb.freebsd.org/changeset/base/334009

Log:
  Cleanups related to debug exceptions on x86.
  
  - Add constants for fields in DR6 and the reserved fields in DR7.  Use
    these constants instead of magic numbers in most places that use DR6
    and DR7.
  - Refer to T_TRCTRAP as "debug exception" rather than a "trace trap"
    as it is not just for trace exceptions.
  - Always read DR6 for debug exceptions and only clear TF in the flags
    register for user exceptions where DR6.BS is set.
  - Clear DR6 before returning from a debug exception handler as
    recommended by the SDM dating all the way back to the 386.  This
    allows debuggers to determine the cause of each exception.  For
    kernel traps, clear DR6 in the T_TRCTRAP case and pass DR6 by value
    to other parts of the handler (namely, user_dbreg_trap()).  For user
    traps, wait until after trapsignal to clear DR6 so that userland
    debuggers can read DR6 via PT_GETDBREGS while the thread is stopped
    in trapsignal().
  
  Reviewed by:  kib, rgrimes
  MFC after:    1 month
  Differential Revision:        https://reviews.freebsd.org/D15189

Modified:
  head/sys/amd64/amd64/machdep.c
  head/sys/amd64/amd64/trap.c
  head/sys/amd64/include/db_machdep.h
  head/sys/amd64/vmm/amd/svm.c
  head/sys/amd64/vmm/intel/vmx.c
  head/sys/i386/i386/machdep.c
  head/sys/i386/i386/trap.c
  head/sys/i386/include/db_machdep.h
  head/sys/x86/include/reg.h
  head/sys/x86/include/x86_var.h

Modified: head/sys/amd64/amd64/machdep.c
==============================================================================
--- head/sys/amd64/amd64/machdep.c      Mon May 21 21:52:48 2018        
(r334008)
+++ head/sys/amd64/amd64/machdep.c      Tue May 22 00:45:00 2018        
(r334009)
@@ -2486,14 +2486,23 @@ reset_dbregs(void)
  * breakpoint was in user space.  Return 0, otherwise.
  */
 int
-user_dbreg_trap(void)
+user_dbreg_trap(register_t dr6)
 {
-        u_int64_t dr7, dr6; /* debug registers dr6 and dr7 */
+        u_int64_t dr7;
         u_int64_t bp;       /* breakpoint bits extracted from dr6 */
         int nbp;            /* number of breakpoints that triggered */
         caddr_t addr[4];    /* breakpoint addresses */
         int i;
-        
+
+        bp = dr6 & DBREG_DR6_BMASK;
+        if (bp == 0) {
+                /*
+                 * None of the breakpoint bits are set meaning this
+                 * trap was not caused by any of the debug registers
+                 */
+                return 0;
+        }
+
         dr7 = rdr7();
         if ((dr7 & 0x000000ff) == 0) {
                 /*
@@ -2505,16 +2514,6 @@ user_dbreg_trap(void)
         }
 
         nbp = 0;
-        dr6 = rdr6();
-        bp = dr6 & 0x0000000f;
-
-        if (!bp) {
-                /*
-                 * None of the breakpoint bits are set meaning this
-                 * trap was not caused by any of the debug registers
-                 */
-                return 0;
-        }
 
         /*
          * at least one of the breakpoints were hit, check to see

Modified: head/sys/amd64/amd64/trap.c
==============================================================================
--- head/sys/amd64/amd64/trap.c Mon May 21 21:52:48 2018        (r334008)
+++ head/sys/amd64/amd64/trap.c Tue May 22 00:45:00 2018        (r334009)
@@ -126,7 +126,7 @@ static char *trap_msg[] = {
        "",                                     /*  7 unused */
        "",                                     /*  8 unused */
        "general protection fault",             /*  9 T_PROTFLT */
-       "trace trap",                           /* 10 T_TRCTRAP */
+       "debug exception",                      /* 10 T_TRCTRAP */
        "",                                     /* 11 unused */
        "page fault",                           /* 12 T_PAGEFLT */
        "",                                     /* 13 unused */
@@ -173,10 +173,7 @@ trap(struct trapframe *frame)
        ksiginfo_t ksi;
        struct thread *td;
        struct proc *p;
-       register_t addr;
-#ifdef KDB
-       register_t dr6;
-#endif
+       register_t addr, dr6;
        int signo, ucode;
        u_int type;
 
@@ -185,6 +182,7 @@ trap(struct trapframe *frame)
        signo = 0;
        ucode = 0;
        addr = 0;
+       dr6 = 0;
 
        VM_CNT_INC(v_trap);
        type = frame->tf_trapno;
@@ -272,20 +270,25 @@ trap(struct trapframe *frame)
                        break;
 
                case T_BPTFLT:          /* bpt instruction fault */
-               case T_TRCTRAP:         /* trace trap */
                        enable_intr();
 #ifdef KDTRACE_HOOKS
-                       if (type == T_BPTFLT) {
-                               if (dtrace_pid_probe_ptr != NULL &&
-                                   dtrace_pid_probe_ptr(frame) == 0)
-                                       return;
-                       }
+                       if (dtrace_pid_probe_ptr != NULL &&
+                           dtrace_pid_probe_ptr(frame) == 0)
+                               return;
 #endif
-                       frame->tf_rflags &= ~PSL_T;
                        signo = SIGTRAP;
-                       ucode = (type == T_TRCTRAP ? TRAP_TRACE : TRAP_BRKPT);
+                       ucode = TRAP_BRKPT;
                        break;
 
+               case T_TRCTRAP:         /* debug exception */
+                       enable_intr();
+                       signo = SIGTRAP;
+                       ucode = TRAP_TRACE;
+                       dr6 = rdr6();
+                       if (dr6 & DBREG_DR6_BS)
+                               frame->tf_rflags &= ~PSL_T;
+                       break;
+
                case T_ARITHTRAP:       /* arithmetic trap */
                        ucode = fputrap_x87();
                        if (ucode == -1)
@@ -521,9 +524,13 @@ trap(struct trapframe *frame)
                        }
                        break;
 
-               case T_TRCTRAP:  /* trace trap */
+               case T_TRCTRAP:  /* debug exception */
+                       /* Clear any pending debug events. */
+                       dr6 = rdr6();
+                       load_dr6(0);
+
                        /*
-                        * Ignore debug register trace traps due to
+                        * Ignore debug register exceptions due to
                         * accesses in the user's address space, which
                         * can happen under several conditions such as
                         * if a user sets a watchpoint on a buffer and
@@ -532,14 +539,8 @@ trap(struct trapframe *frame)
                         * in kernel space because that is useful when
                         * debugging the kernel.
                         */
-                       if (user_dbreg_trap()) {
-                               /*
-                                * Reset breakpoint bits because the
-                                * processor doesn't
-                                */
-                               load_dr6(rdr6() & ~0xf);
+                       if (user_dbreg_trap(dr6))
                                return;
-                       }
 
                        /*
                         * Malicious user code can configure a debug
@@ -595,9 +596,6 @@ trap(struct trapframe *frame)
                         * Otherwise, debugger traps "can't happen".
                         */
 #ifdef KDB
-                       /* XXX %dr6 is not quite reentrant. */
-                       dr6 = rdr6();
-                       load_dr6(dr6 & ~0x4000);
                        if (kdb_trap(type, dr6, frame))
                                return;
 #endif
@@ -640,6 +638,13 @@ trap(struct trapframe *frame)
        }
        KASSERT((read_rflags() & PSL_I) != 0, ("interrupts disabled"));
        trapsignal(td, &ksi);
+
+       /*
+        * Clear any pending debug exceptions after allowing a
+        * debugger to read DR6 while stopped in trapsignal().
+        */
+       if (type == T_TRCTRAP)
+               load_dr6(0);
 userret:
        userret(td, frame);
        KASSERT(PCB_USER_FPU(td->td_pcb),

Modified: head/sys/amd64/include/db_machdep.h
==============================================================================
--- head/sys/amd64/include/db_machdep.h Mon May 21 21:52:48 2018        
(r334008)
+++ head/sys/amd64/include/db_machdep.h Tue May 22 00:45:00 2018        
(r334009)
@@ -30,6 +30,7 @@
 #define        _MACHINE_DB_MACHDEP_H_
 
 #include <machine/frame.h>
+#include <machine/reg.h>
 #include <machine/trap.h>
 
 typedef        vm_offset_t     db_addr_t;      /* address - unsigned */
@@ -64,7 +65,8 @@ do {                                          \
  * unknown addresses and doesn't turn them off while it is running.
  */
 #define        IS_BREAKPOINT_TRAP(type, code)  ((type) == T_BPTFLT)
-#define        IS_SSTEP_TRAP(type, code)       ((type) == T_TRCTRAP && (code) 
& 0x4000)
+#define        IS_SSTEP_TRAP(type, code)                                       
\
+       ((type) == T_TRCTRAP && (code) & DBREG_DR6_BS)
 #define        IS_WATCHPOINT_TRAP(type, code)  0
 
 #define        I_CALL          0xe8

Modified: head/sys/amd64/vmm/amd/svm.c
==============================================================================
--- head/sys/amd64/vmm/amd/svm.c        Mon May 21 21:52:48 2018        
(r334008)
+++ head/sys/amd64/vmm/amd/svm.c        Tue May 22 00:45:00 2018        
(r334009)
@@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$");
 #include <machine/cpufunc.h>
 #include <machine/psl.h>
 #include <machine/md_var.h>
+#include <machine/reg.h>
 #include <machine/specialreg.h>
 #include <machine/smp.h>
 #include <machine/vmm.h>
@@ -507,8 +508,8 @@ vmcb_init(struct svm_softc *sc, int vcpu, uint64_t iop
            PAT_VALUE(7, PAT_UNCACHEABLE);
 
        /* Set up DR6/7 to power-on state */
-       state->dr6 = 0xffff0ff0;
-       state->dr7 = 0x400;
+       state->dr6 = DBREG_DR6_RESERVED1;
+       state->dr7 = DBREG_DR7_RESERVED1;
 }
 
 /*

Modified: head/sys/amd64/vmm/intel/vmx.c
==============================================================================
--- head/sys/amd64/vmm/intel/vmx.c      Mon May 21 21:52:48 2018        
(r334008)
+++ head/sys/amd64/vmm/intel/vmx.c      Tue May 22 00:45:00 2018        
(r334009)
@@ -46,6 +46,7 @@ __FBSDID("$FreeBSD$");
 #include <machine/psl.h>
 #include <machine/cpufunc.h>
 #include <machine/md_var.h>
+#include <machine/reg.h>
 #include <machine/segments.h>
 #include <machine/smp.h>
 #include <machine/specialreg.h>
@@ -994,8 +995,8 @@ vmx_vminit(struct vm *vm, pmap_t pmap)
                        exc_bitmap = 1 << IDT_MC;
                error += vmwrite(VMCS_EXCEPTION_BITMAP, exc_bitmap);
 
-               vmx->ctx[i].guest_dr6 = 0xffff0ff0;
-               error += vmwrite(VMCS_GUEST_DR7, 0x400);
+               vmx->ctx[i].guest_dr6 = DBREG_DR6_RESERVED1;
+               error += vmwrite(VMCS_GUEST_DR7, DBREG_DR7_RESERVED1);
 
                if (virtual_interrupt_delivery) {
                        error += vmwrite(VMCS_APIC_ACCESS, APIC_ACCESS_ADDRESS);

Modified: head/sys/i386/i386/machdep.c
==============================================================================
--- head/sys/i386/i386/machdep.c        Mon May 21 21:52:48 2018        
(r334008)
+++ head/sys/i386/i386/machdep.c        Tue May 22 00:45:00 2018        
(r334009)
@@ -3151,14 +3151,23 @@ set_dbregs(struct thread *td, struct dbreg *dbregs)
  * breakpoint was in user space.  Return 0, otherwise.
  */
 int
-user_dbreg_trap(void)
+user_dbreg_trap(register_t dr6)
 {
-        u_int32_t dr7, dr6; /* debug registers dr6 and dr7 */
+        u_int32_t dr7;
         u_int32_t bp;       /* breakpoint bits extracted from dr6 */
         int nbp;            /* number of breakpoints that triggered */
         caddr_t addr[4];    /* breakpoint addresses */
         int i;
-        
+
+        bp = dr6 & DBREG_DR6_BMASK;
+        if (bp == 0) {
+                /*
+                 * None of the breakpoint bits are set meaning this
+                 * trap was not caused by any of the debug registers
+                 */
+                return 0;
+        }
+
         dr7 = rdr7();
         if ((dr7 & 0x000000ff) == 0) {
                 /*
@@ -3170,16 +3179,6 @@ user_dbreg_trap(void)
         }
 
         nbp = 0;
-        dr6 = rdr6();
-        bp = dr6 & 0x0000000f;
-
-        if (!bp) {
-                /*
-                 * None of the breakpoint bits are set meaning this
-                 * trap was not caused by any of the debug registers
-                 */
-                return 0;
-        }
 
         /*
          * at least one of the breakpoints were hit, check to see

Modified: head/sys/i386/i386/trap.c
==============================================================================
--- head/sys/i386/i386/trap.c   Mon May 21 21:52:48 2018        (r334008)
+++ head/sys/i386/i386/trap.c   Tue May 22 00:45:00 2018        (r334009)
@@ -132,7 +132,7 @@ static const struct trap_data trap_data[] = {
        [T_BPTFLT] =    { .ei = false,  .msg = "breakpoint instruction fault" },
        [T_ARITHTRAP] = { .ei = true,   .msg = "arithmetic trap" },
        [T_PROTFLT] =   { .ei = true,   .msg = "general protection fault" },
-       [T_TRCTRAP] =   { .ei = false,  .msg = "trace trap" },
+       [T_TRCTRAP] =   { .ei = false,  .msg = "debug exception" },
        [T_PAGEFLT] =   { .ei = true,   .msg = "page fault" },
        [T_ALIGNFLT] =  { .ei = true,   .msg = "alignment fault" },
        [T_DIVIDE] =    { .ei = true,   .msg = "integer divide fault" },
@@ -199,12 +199,9 @@ trap(struct trapframe *frame)
        ksiginfo_t ksi;
        struct thread *td;
        struct proc *p;
-#ifdef KDB
-       register_t dr6;
-#endif
        int signo, ucode;
        u_int type;
-       register_t addr;
+       register_t addr, dr6;
        vm_offset_t eva;
 #ifdef POWERFAIL_NMI
        static int lastalert = 0;
@@ -215,6 +212,7 @@ trap(struct trapframe *frame)
        signo = 0;
        ucode = 0;
        addr = 0;
+       dr6 = 0;
 
        VM_CNT_INC(v_trap);
        type = frame->tf_trapno;
@@ -323,19 +321,24 @@ trap(struct trapframe *frame)
                        break;
 
                case T_BPTFLT:          /* bpt instruction fault */
-               case T_TRCTRAP:         /* trace trap */
                        enable_intr();
 #ifdef KDTRACE_HOOKS
-                       if (type == T_BPTFLT) {
-                               if (dtrace_pid_probe_ptr != NULL &&
-                                   dtrace_pid_probe_ptr(frame) == 0)
-                                       return;
-                       }
+                       if (dtrace_pid_probe_ptr != NULL &&
+                           dtrace_pid_probe_ptr(frame) == 0)
+                               return;
 #endif
+                       signo = SIGTRAP;
+                       ucode = TRAP_BRKPT;
+                       break;
+
+               case T_TRCTRAP:         /* debug exception */
+                       enable_intr();
 user_trctrap_out:
-                       frame->tf_eflags &= ~PSL_T;
                        signo = SIGTRAP;
-                       ucode = (type == T_TRCTRAP ? TRAP_TRACE : TRAP_BRKPT);
+                       ucode = TRAP_TRACE;
+                       dr6 = rdr6();
+                       if (dr6 & DBREG_DR6_BS)
+                               frame->tf_rflags &= ~PSL_T;
                        break;
 
                case T_ARITHTRAP:       /* arithmetic trap */
@@ -643,10 +646,14 @@ user_trctrap_out:
                        }
                        break;
 
-               case T_TRCTRAP:  /* trace trap */
+               case T_TRCTRAP:  /* debug exception */
 kernel_trctrap:
+                       /* Clear any pending debug events. */
+                       dr6 = rdr6();
+                       load_dr6(0);
+
                        /*
-                        * Ignore debug register trace traps due to
+                        * Ignore debug register exceptions due to
                         * accesses in the user's address space, which
                         * can happen under several conditions such as
                         * if a user sets a watchpoint on a buffer and
@@ -655,15 +662,9 @@ kernel_trctrap:
                         * in kernel space because that is useful when
                         * debugging the kernel.
                         */
-                       if (user_dbreg_trap() && 
-                          !(curpcb->pcb_flags & PCB_VM86CALL)) {
-                               /*
-                                * Reset breakpoint bits because the
-                                * processor doesn't
-                                */
-                               load_dr6(rdr6() & ~0xf);
+                       if (user_dbreg_trap(dr6) &&
+                          !(curpcb->pcb_flags & PCB_VM86CALL))
                                return;
-                       }
 
                        /*
                         * Malicious user code can configure a debug
@@ -703,9 +704,6 @@ kernel_trctrap:
                         * Otherwise, debugger traps "can't happen".
                         */
 #ifdef KDB
-                       /* XXX %dr6 is not quite reentrant. */
-                       dr6 = rdr6();
-                       load_dr6(dr6 & ~0x4000);
                        if (kdb_trap(type, dr6, frame))
                                return;
 #endif
@@ -759,6 +757,12 @@ kernel_trctrap:
        KASSERT((read_eflags() & PSL_I) != 0, ("interrupts disabled"));
        trapsignal(td, &ksi);
 
+       /*
+        * Clear any pending debug exceptions after allowing a
+        * debugger to read DR6 while stopped in trapsignal().
+        */
+       if (type == T_TRCTRAP)
+               load_dr6(0);
 user:
        userret(td, frame);
        KASSERT(PCB_USER_FPU(td->td_pcb),

Modified: head/sys/i386/include/db_machdep.h
==============================================================================
--- head/sys/i386/include/db_machdep.h  Mon May 21 21:52:48 2018        
(r334008)
+++ head/sys/i386/include/db_machdep.h  Tue May 22 00:45:00 2018        
(r334009)
@@ -30,6 +30,7 @@
 #define        _MACHINE_DB_MACHDEP_H_
 
 #include <machine/frame.h>
+#include <machine/reg.h>
 #include <machine/trap.h>
 
 typedef        vm_offset_t     db_addr_t;      /* address - unsigned */
@@ -67,7 +68,8 @@ do {                                          \
  * unknown addresses and doesn't turn them off while it is running.
  */
 #define        IS_BREAKPOINT_TRAP(type, code)  ((type) == T_BPTFLT)
-#define        IS_SSTEP_TRAP(type, code)       ((type) == T_TRCTRAP && (code) 
& 0x4000)
+#define        IS_SSTEP_TRAP(type, code)                                       
\
+       ((type) == T_TRCTRAP && (code) & DBREG_DR6_BS)
 #define        IS_WATCHPOINT_TRAP(type, code)  0
 
 #define        I_CALL          0xe8

Modified: head/sys/x86/include/reg.h
==============================================================================
--- head/sys/x86/include/reg.h  Mon May 21 21:52:48 2018        (r334008)
+++ head/sys/x86/include/reg.h  Tue May 22 00:45:00 2018        (r334009)
@@ -206,6 +206,14 @@ struct __dbreg64 {
                                /* Index 8-15: reserved */
 };
 
+#define        DBREG_DR6_RESERVED1     0xffff0ff0
+#define        DBREG_DR6_BMASK         0x000f
+#define        DBREG_DR6_B(i)          (1 << (i))
+#define        DBREG_DR6_BD            0x2000
+#define        DBREG_DR6_BS            0x4000
+#define        DBREG_DR6_BT            0x8000
+
+#define        DBREG_DR7_RESERVED1     0x0400
 #define        DBREG_DR7_LOCAL_ENABLE  0x01
 #define        DBREG_DR7_GLOBAL_ENABLE 0x02
 #define        DBREG_DR7_LEN_1         0x00    /* 1 byte length          */
@@ -236,6 +244,8 @@ struct __dbreg64 {
 #undef __dbreg64
 
 #ifdef _KERNEL
+struct thread;
+
 /*
  * XXX these interfaces are MI, so they should be declared in a MI place.
  */

Modified: head/sys/x86/include/x86_var.h
==============================================================================
--- head/sys/x86/include/x86_var.h      Mon May 21 21:52:48 2018        
(r334008)
+++ head/sys/x86/include/x86_var.h      Tue May 22 00:45:00 2018        
(r334009)
@@ -145,7 +145,7 @@ void        nmi_handle_intr(u_int type, struct trapframe 
*fra
 void   pagecopy(void *from, void *to);
 void   printcpuinfo(void);
 int    pti_get_default(void);
-int    user_dbreg_trap(void);
+int    user_dbreg_trap(register_t dr6);
 int    minidumpsys(struct dumperinfo *);
 struct pcb *get_pcb_td(struct thread *td);
 
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to