Date: Fri, 17 Mar 2017 13:49:06 +0000 (UTC) From: Bruce Evans <bde@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r315454 - in head/sys: amd64/amd64 amd64/include i386/i386 i386/include Message-ID: <201703171349.v2HDn6u0054727@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bde Date: Fri Mar 17 13:49:05 2017 New Revision: 315454 URL: https://svnweb.freebsd.org/changeset/base/315454 Log: Don't access the reserved registers %dr4 and %dr5 on i386. On the original i386, %dr[4-5] were unimplemented but not very clearly reserved, so debuggers read them to print them. i386 was still doing this. On the original athlon64, %dr[4-5] are documented as reserved but are aliased to %dr[6-7] unless CR4_DE is set, when accessing them traps. On 2 of my systems, accessing %dr[4-5] trapped sometimes. On my Haswell system, the apparent randomness was because the boot CPU starts with CR4_DE set while all other CPUs start with CR4_DE clear. FreeBSD doesn't support the data breakpoints enabled by CR4_DE and it never changes this flag, so the flag remains different across CPUs and the behaviour seemed inconsistent except while booting when the CPU doesn't change. The invalid accesses broke: - read access for printing the registers in ddb "show watches" on CPUs with CR4_DE set - read accesses in fill_dbregs() on CPUs with CR4_DE set. This didn't implement panic(3) since the user case always skipped %dr[4-5]. - write accesses in set_dbregs(). This also didn't affect userland. When it didn't trap, the aliasing made it fragile. Don't print the dummy (zero) values of %dr[4-5] in "show watches" for i386 or amd64. Fix style bugs near this printing. amd64 also has space in the dbregs struct for the reserved %dr[8-15] and already didn't print the dummy values for these, and never accessed any of the 10 reserved debug registers. Remove cpufuncs for making the invalid accesses. Even amd64 had these. Modified: head/sys/amd64/amd64/db_trace.c head/sys/amd64/include/cpufunc.h head/sys/i386/i386/db_trace.c head/sys/i386/i386/machdep.c head/sys/i386/include/cpufunc.h Modified: head/sys/amd64/amd64/db_trace.c ============================================================================== --- head/sys/amd64/amd64/db_trace.c Fri Mar 17 13:37:37 2017 (r315453) +++ head/sys/amd64/amd64/db_trace.c Fri Mar 17 13:49:05 2017 (r315454) @@ -575,7 +575,7 @@ watchtype_str(type) void -db_md_list_watchpoints() +db_md_list_watchpoints(void) { struct dbreg d; int i, len, type; @@ -595,7 +595,7 @@ db_md_list_watchpoints() len++; db_printf(" %-5d %-8s %10s %3d ", i, "enabled", watchtype_str(type), len); - db_printsym((db_addr_t)DBREG_DRX((&d), i), DB_STGY_ANY); + db_printsym((db_addr_t)DBREG_DRX(&d, i), DB_STGY_ANY); db_printf("\n"); } else { db_printf(" %-5d disabled\n", i); @@ -603,9 +603,9 @@ db_md_list_watchpoints() } db_printf("\ndebug register values:\n"); - for (i = 0; i < 8; i++) { - db_printf(" dr%d 0x%016lx\n", i, DBREG_DRX((&d), i)); - } + for (i = 0; i < 8; i++) + if (i != 4 && i != 5) + db_printf(" dr%d 0x%016lx\n", i, DBREG_DRX(&d, i)); db_printf("\n"); } Modified: head/sys/amd64/include/cpufunc.h ============================================================================== --- head/sys/amd64/include/cpufunc.h Fri Mar 17 13:37:37 2017 (r315453) +++ head/sys/amd64/include/cpufunc.h Fri Mar 17 13:49:05 2017 (r315454) @@ -759,34 +759,6 @@ load_dr3(uint64_t dr3) } static __inline uint64_t -rdr4(void) -{ - uint64_t data; - __asm __volatile("movq %%dr4,%0" : "=r" (data)); - return (data); -} - -static __inline void -load_dr4(uint64_t dr4) -{ - __asm __volatile("movq %0,%%dr4" : : "r" (dr4)); -} - -static __inline uint64_t -rdr5(void) -{ - uint64_t data; - __asm __volatile("movq %%dr5,%0" : "=r" (data)); - return (data); -} - -static __inline void -load_dr5(uint64_t dr5) -{ - __asm __volatile("movq %0,%%dr5" : : "r" (dr5)); -} - -static __inline uint64_t rdr6(void) { uint64_t data; @@ -863,8 +835,6 @@ void load_dr0(uint64_t dr0); void load_dr1(uint64_t dr1); void load_dr2(uint64_t dr2); void load_dr3(uint64_t dr3); -void load_dr4(uint64_t dr4); -void load_dr5(uint64_t dr5); void load_dr6(uint64_t dr6); void load_dr7(uint64_t dr7); void load_fs(u_short sel); @@ -887,8 +857,6 @@ uint64_t rdr0(void); uint64_t rdr1(void); uint64_t rdr2(void); uint64_t rdr3(void); -uint64_t rdr4(void); -uint64_t rdr5(void); uint64_t rdr6(void); uint64_t rdr7(void); uint64_t rdtsc(void); Modified: head/sys/i386/i386/db_trace.c ============================================================================== --- head/sys/i386/i386/db_trace.c Fri Mar 17 13:37:37 2017 (r315453) +++ head/sys/i386/i386/db_trace.c Fri Mar 17 13:49:05 2017 (r315454) @@ -735,7 +735,7 @@ watchtype_str(type) void -db_md_list_watchpoints() +db_md_list_watchpoints(void) { struct dbreg d; int i, len, type; @@ -751,7 +751,7 @@ db_md_list_watchpoints() len = DBREG_DR7_LEN(d.dr[7], i); db_printf(" %-5d %-8s %10s %3d ", i, "enabled", watchtype_str(type), len + 1); - db_printsym((db_addr_t)DBREG_DRX((&d), i), DB_STGY_ANY); + db_printsym((db_addr_t)DBREG_DRX(&d, i), DB_STGY_ANY); db_printf("\n"); } else { db_printf(" %-5d disabled\n", i); @@ -759,10 +759,8 @@ db_md_list_watchpoints() } db_printf("\ndebug register values:\n"); - for (i = 0; i < 8; i++) { - db_printf(" dr%d 0x%08x\n", i, DBREG_DRX((&d), i)); - } + for (i = 0; i < 8; i++) + if (i != 4 && i != 5) + db_printf(" dr%d 0x%08x\n", i, DBREG_DRX(&d, i)); db_printf("\n"); } - - Modified: head/sys/i386/i386/machdep.c ============================================================================== --- head/sys/i386/i386/machdep.c Fri Mar 17 13:37:37 2017 (r315453) +++ head/sys/i386/i386/machdep.c Fri Mar 17 13:49:05 2017 (r315454) @@ -2901,8 +2901,6 @@ fill_dbregs(struct thread *td, struct db dbregs->dr[1] = rdr1(); dbregs->dr[2] = rdr2(); dbregs->dr[3] = rdr3(); - dbregs->dr[4] = rdr4(); - dbregs->dr[5] = rdr5(); dbregs->dr[6] = rdr6(); dbregs->dr[7] = rdr7(); } else { @@ -2911,11 +2909,11 @@ fill_dbregs(struct thread *td, struct db dbregs->dr[1] = pcb->pcb_dr1; dbregs->dr[2] = pcb->pcb_dr2; dbregs->dr[3] = pcb->pcb_dr3; - dbregs->dr[4] = 0; - dbregs->dr[5] = 0; dbregs->dr[6] = pcb->pcb_dr6; dbregs->dr[7] = pcb->pcb_dr7; } + dbregs->dr[4] = 0; + dbregs->dr[5] = 0; return (0); } @@ -2930,8 +2928,6 @@ set_dbregs(struct thread *td, struct dbr load_dr1(dbregs->dr[1]); load_dr2(dbregs->dr[2]); load_dr3(dbregs->dr[3]); - load_dr4(dbregs->dr[4]); - load_dr5(dbregs->dr[5]); load_dr6(dbregs->dr[6]); load_dr7(dbregs->dr[7]); } else { Modified: head/sys/i386/include/cpufunc.h ============================================================================== --- head/sys/i386/include/cpufunc.h Fri Mar 17 13:37:37 2017 (r315453) +++ head/sys/i386/include/cpufunc.h Fri Mar 17 13:49:05 2017 (r315454) @@ -632,34 +632,6 @@ load_dr3(u_int dr3) } static __inline u_int -rdr4(void) -{ - u_int data; - __asm __volatile("movl %%dr4,%0" : "=r" (data)); - return (data); -} - -static __inline void -load_dr4(u_int dr4) -{ - __asm __volatile("movl %0,%%dr4" : : "r" (dr4)); -} - -static __inline u_int -rdr5(void) -{ - u_int data; - __asm __volatile("movl %%dr5,%0" : "=r" (data)); - return (data); -} - -static __inline void -load_dr5(u_int dr5) -{ - __asm __volatile("movl %0,%%dr5" : : "r" (dr5)); -} - -static __inline u_int rdr6(void) { u_int data; @@ -750,8 +722,6 @@ void load_dr0(u_int dr0); void load_dr1(u_int dr1); void load_dr2(u_int dr2); void load_dr3(u_int dr3); -void load_dr4(u_int dr4); -void load_dr5(u_int dr5); void load_dr6(u_int dr6); void load_dr7(u_int dr7); void load_fs(u_short sel); @@ -773,8 +743,6 @@ u_int rdr0(void); u_int rdr1(void); u_int rdr2(void); u_int rdr3(void); -u_int rdr4(void); -u_int rdr5(void); u_int rdr6(void); u_int rdr7(void); uint64_t rdtsc(void);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703171349.v2HDn6u0054727>