|
|
Re: [PATCH] i386 Hardware watchpoints using kgdb-2-2.6.14.tar.gz: msg#00156linux.kernel.debugging.kgdb.bugs
Attached is my hardware break/watchpoint patch. Review and comments requested. This patch is meant to be applied to a kernel.org 2.6.14.3 kernel which has first had the patches from kgdb-2-2.6.14.tar.gz, and then linux-2_6_14_3-dbgregs-smp.patch from my previous post. I've tried to use the debug register allocation interface in a way which keeps kgdb from conflicting with other in-kernel or user-space users of debug registers. Also, I've made an attempt to keep this SMP safe, but others should check me on this as I have no SMP machine to test this on. My approach was to save dr6, dr7, and the type and address of any debug exception when the exception handler calls kgdb_disable_hw_debug() early on. That information is used on the way out in kgdb_arch_handle_exception(), which is also the only place dr6 is modified. kgdb_correct_hw_break() is the only place where dr7 and the address registers are actually modified, and they are synchronized across cpus when that happens. When gdb is clearing and resetting breakpoints, only the shadowed value of dr7 is actually changed. One thing I'm uncertain about is if this scheme holds up if more than one cpu happens to hit a hardware breakpoint nearly at the same time, regardless of whether they hit the same or different breakpoints. --Dennis Dennis W. Tokarski wrote: Index: Makefile =================================================================== RCS file: /usr/src/kernel-devel/../cvs-repository/linux-2.6.14.3-vanilla/Makefile,v retrieving revision 1.1.1.1.2.1 retrieving revision 1.1.1.1.2.2 diff -u -r1.1.1.1.2.1 -r1.1.1.1.2.2 --- Makefile 9 Dec 2005 22:34:20 -0000 1.1.1.1.2.1 +++ Makefile 30 Jan 2006 03:01:20 -0000 1.1.1.1.2.2 @@ -1,7 +1,7 @@ VERSION = 2 PATCHLEVEL = 6 SUBLEVEL = 14 -EXTRAVERSION = .3 +EXTRAVERSION = .3-kgdb NAME=Affluent Albatross # *DOCUMENTATION* Index: arch/i386/kernel/kgdb.c =================================================================== RCS file: /usr/src/kernel-devel/../cvs-repository/linux-2.6.14.3-vanilla/arch/i386/kernel/Attic/kgdb.c,v retrieving revision 1.1.2.1 retrieving revision 1.1.2.2 diff -u -r1.1.2.1 -r1.1.2.2 --- arch/i386/kernel/kgdb.c 9 Dec 2005 22:36:18 -0000 1.1.2.1 +++ arch/i386/kernel/kgdb.c 30 Jan 2006 03:01:23 -0000 1.1.2.2 @@ -34,6 +34,7 @@ #include <linux/delay.h> #include <asm/vm86.h> #include <asm/system.h> +#include <asm/debugreg.h> #include <asm/ptrace.h> /* for linux pt_regs struct */ #include <linux/kgdb.h> #include <linux/init.h> @@ -114,128 +115,135 @@ regs->eip = gdb_regs[_PC]; } +static unsigned trap_dr6, trap_dr7, // See kgdb_disable_hw_debug(), below + trap_type, trap_addr; + static struct hw_breakpoint { - unsigned enabled; - unsigned type; - unsigned len; + unsigned drnum; // != DR_MAX iff we own the designated dr + unsigned type; // DR_RW_{EXECUTE,WRITE,READ}, READ is really RW access + unsigned len; // 1, 2, 3, or 4 unsigned addr; -} breakinfo[4] = { - { .enabled = 0 }, - { .enabled = 0 }, - { .enabled = 0 }, - { .enabled = 0 }, -}; +} breakinfo[DR_MAX] = { // DR_MAX is 4 in i386 + { .drnum = DR_MAX }, + { .drnum = DR_MAX }, + { .drnum = DR_MAX }, + { .drnum = DR_MAX }}; + +#define breakinfoend ((struct hw_breakpoint *) ((char *) breakinfo + sizeof (breakinfo))) + void kgdb_correct_hw_break(void) { - int breakno; - int correctit; - int breakbit; - unsigned dr7; + struct hw_breakpoint *breakpt = breakinfo; + - asm volatile ("movl %%db7, %0\n":"=r" (dr7) - :); do { - unsigned addr0, addr1, addr2, addr3; - asm volatile ("movl %%db0, %0\n" - "movl %%db1, %1\n" - "movl %%db2, %2\n" - "movl %%db3, %3\n":"=r" (addr0), "=r"(addr1), - "=r"(addr2), "=r"(addr3):); - } while (0); - correctit = 0; - for (breakno = 0; breakno < 3; breakno++) { - breakbit = 2 << (breakno << 1); - if (!(dr7 & breakbit) && breakinfo[breakno].enabled) { - correctit = 1; - dr7 |= breakbit; - dr7 &= ~(0xf0000 << (breakno << 2)); - dr7 |= (((breakinfo[breakno].len << 2) | - breakinfo[breakno].type) << 16) << - (breakno << 2); - switch (breakno) { - case 0: - asm volatile ("movl %0, %%dr0\n"::"r" - (breakinfo[breakno].addr)); - break; - - case 1: - asm volatile ("movl %0, %%dr1\n"::"r" - (breakinfo[breakno].addr)); - break; - - case 2: - asm volatile ("movl %0, %%dr2\n"::"r" - (breakinfo[breakno].addr)); - break; - - case 3: - asm volatile ("movl %0, %%dr3\n"::"r" - (breakinfo[breakno].addr)); - break; - } - } else if ((dr7 & breakbit) && !breakinfo[breakno].enabled) { - correctit = 1; - dr7 &= ~breakbit; - dr7 &= ~(0xf0000 << (breakno << 2)); - } - } - if (correctit) - asm volatile ("movl %0, %%db7\n"::"r" (dr7)); + if (breakpt->drnum >= DR_MAX) + continue; + + SET_DR7(trap_dr7, breakpt->drnum, breakpt->type, breakpt->len); + write_dr(breakpt->drnum, breakpt->addr); + sync_dr(breakpt->drnum, breakpt->addr, breakpt->type); + + } while (++breakpt < breakinfoend); + + write_dr(DR_CONTROL, trap_dr7); + sync_dr(DR_CONTROL, trap_dr7, 0); } int kgdb_remove_hw_break(unsigned long addr) { - int i, idx = -1; - for (i = 0; i < 4; i++) { - if (breakinfo[i].addr == addr && breakinfo[i].enabled) { - idx = i; - break; - } - } - if (idx == -1) - return -1; + struct hw_breakpoint *breakpt = breakinfo; - breakinfo[idx].enabled = 0; - return 0; + do { + unsigned drnum = breakpt->drnum; + + if (drnum >= DR_MAX || breakpt->addr != addr) + continue; + + dr_free(drnum); + trap_dr7 &= dr7_global_mask; + breakpt->drnum = DR_MAX; + break; + + } while (++breakpt < breakinfoend); + + return (breakpt < breakinfoend) ? 0 : -1; } void kgdb_remove_all_hw_break(void) { - int i; + struct hw_breakpoint *breakpt = breakinfo; - for (i = 0; i < 4; i++) { - if (breakinfo[i].enabled) { - /* Do what? */ - ; - } - memset(&breakinfo[i], 0, sizeof(struct hw_breakpoint)); - } + do { + unsigned drnum = breakpt->drnum; + + if (drnum >= DR_MAX) + continue; + + dr_free(drnum); + trap_dr7 &= dr7_global_mask; + breakpt->drnum = DR_MAX; + + } while (++breakpt < breakinfoend); } -int kgdb_set_hw_break(unsigned long addr) +int kgdb_set_hw_watch(unsigned long addr, unsigned int len, char bpt_type) { - int i, idx = -1; - for (i = 0; i < 4; i++) { - if (!breakinfo[i].enabled) { - idx = i; - break; - } - } - if (idx == -1) + unsigned drnum, type; + struct hw_breakpoint *breakpt = breakinfo; + + + /* We "know" bpt_type is '1', '2', '3', or '4', + * and we don't support '3'. Require len be 1, 2, or 4. + */ + if (len < 1 || len > 4 || len == 3 || bpt_type == '3') + return -EINVAL; + + while (breakpt->drnum < DR_MAX && ++breakpt < breakinfoend); + + if (breakpt >= breakinfoend) + return -1; + + if ((drnum = dr_alloc(DR_ANY, DR_ALLOC_GLOBAL)) < 0) return -1; - breakinfo[idx].enabled = 1; - breakinfo[idx].type = 1; - breakinfo[idx].len = 1; - breakinfo[idx].addr = addr; + type = (bpt_type == '1') ? DR_RW_EXECUTE + :((bpt_type == '2') ? DR_RW_WRITE : DR_RW_READ); + + breakpt->drnum = drnum; + breakpt->type = type; + breakpt->len = len; + breakpt->addr = addr; + return 0; } +int kgdb_set_hw_break(unsigned long addr) +{ + return kgdb_set_hw_watch(addr, 1, '1'); +} + +int kgdb_remove_hw_watch(unsigned long addr) +{ + return kgdb_remove_hw_break(addr); +} + void kgdb_disable_hw_debug(struct pt_regs *regs) { + /* Leave a trail of breadcrumbs showing which + * if any debug exception got us into the handler; + * kgdb_arch_handle_exception() needs to know + * on the way out. These need to be based on the + * current value of dr7, which we're about to clobber. + */ + trap_dr6 = read_dr(DR_STATUS); + trap_dr7 = read_dr(DR_CONTROL); + trap_type = dr_trap_type(trap_dr6); + trap_addr = dr_trap_addr(trap_dr6); + /* Disable hardware debugging while we are in kgdb */ - asm volatile ("movl %0,%%db7": /* no output */ :"r" (0)); + write_dr(DR_CONTROL, 0); } void kgdb_post_master_code(struct pt_regs *regs, int e_vector, int err_code) @@ -257,7 +265,7 @@ { long addr; char *ptr; - int newPC, dr6; + int newPC; switch (remcom_in_buffer[0]) { case 'c': @@ -281,20 +289,22 @@ smp_processor_id()); } - asm volatile ("movl %%db6, %0\n":"=r" (dr6)); - if (!(dr6 & 0x4000)) { - long breakno; - for (breakno = 0; breakno < 4; ++breakno) { - if (dr6 & (1 << breakno) && - breakinfo[breakno].type == 0) { + if (!(trap_dr6 & DR_STEP) && trap_type == DR_RW_EXECUTE) { + struct hw_breakpoint *breakpt = breakinfo; + + do { + if (breakpt->type != DR_RW_EXECUTE) + continue; + + if (breakpt->addr == trap_addr) { /* Set restore flag */ linux_regs->eflags |= X86_EFLAGS_RF; break; } - } + } while (++breakpt < breakinfoend); } kgdb_correct_hw_break(); - asm volatile ("movl %0, %%db6\n"::"r" (0)); + write_dr(DR_STATUS, 0); return (0); } /* switch */ Index: arch/ia64/kernel/kgdb.c =================================================================== RCS file: /usr/src/kernel-devel/../cvs-repository/linux-2.6.14.3-vanilla/arch/ia64/kernel/Attic/kgdb.c,v retrieving revision 1.1.2.1 retrieving revision 1.1.2.2 diff -u -r1.1.2.1 -r1.1.2.2 --- arch/ia64/kernel/kgdb.c 9 Dec 2005 22:36:41 -0000 1.1.2.1 +++ arch/ia64/kernel/kgdb.c 30 Jan 2006 03:01:23 -0000 1.1.2.2 @@ -479,7 +479,7 @@ {u, u, u}, /* 1F */ }; -static int kgdb_arch_set_breakpoint(unsigned long addr, char *saved_instr) +int kgdb_arch_set_breakpoint(unsigned long addr, char *saved_instr) { extern unsigned long _start[]; unsigned long slot = addr & 0xf, bundle_addr; @@ -533,7 +533,7 @@ BREAK_INSTR_SIZE); } -static int kgdb_arch_remove_breakpoint(unsigned long addr, char *bundle) +int kgdb_arch_remove_breakpoint(unsigned long addr, char *bundle) { extern unsigned long _start[]; @@ -1013,8 +1013,6 @@ } struct kgdb_arch arch_kgdb_ops = { - .set_breakpoint = kgdb_arch_set_breakpoint, - .remove_breakpoint = kgdb_arch_remove_breakpoint, .set_hw_breakpoint = kgdb_arch_set_hw_breakpoint, .remove_hw_breakpoint = kgdb_arch_remove_hw_breakpoint, .gdb_bpt_instr = {0xcc}, Index: include/linux/kgdb.h =================================================================== RCS file: /usr/src/kernel-devel/../cvs-repository/linux-2.6.14.3-vanilla/include/linux/Attic/kgdb.h,v retrieving revision 1.1.2.1 retrieving revision 1.1.2.2 diff -u -r1.1.2.1 -r1.1.2.2 --- include/linux/kgdb.h 9 Dec 2005 22:43:18 -0000 1.1.2.1 +++ include/linux/kgdb.h 30 Jan 2006 03:01:34 -0000 1.1.2.2 @@ -50,7 +50,8 @@ bp_hardware_breakpoint, bp_write_watchpoint, bp_read_watchpoint, - bp_access_watchpoint + bp_access_watchpoint, + bp_type_max }; enum kgdb_bpstate { @@ -178,14 +179,6 @@ * @gdb_bpt_instr: The instruction to trigger a breakpoint. * @flags: Flags for the breakpoint, currently just %KGDB_HW_BREAKPOINT. * @shadowth: A value of %1 indicates we shadow information on processes. - * @set_breakpoint: Allow an architecture to specify how to set a software - * breakpoint. - * @remove_breakpoint: Allow an architecture to specify how to remove a - * software breakpoint. - * @set_hw_breakpoint: Allow an architecture to specify how to set a hardware - * breakpoint. - * @remove_hw_breakpoint: Allow an architecture to specify how to remove a - * hardware breakpoint. * * The @shadowth flag is an option to shadow information not retrievable by * gdb otherwise. This is deprecated in favor of a binutils which supports @@ -195,10 +188,6 @@ unsigned char gdb_bpt_instr[BREAK_INSTR_SIZE]; unsigned long flags; unsigned shadowth; - int (*set_breakpoint) (unsigned long, char *); - int (*remove_breakpoint)(unsigned long, char *); - int (*set_hw_breakpoint)(unsigned long, int, enum kgdb_bptype); - int (*remove_hw_breakpoint)(unsigned long, int, enum kgdb_bptype); }; /* Thread reference */ Index: kernel/kgdb.c =================================================================== RCS file: /usr/src/kernel-devel/../cvs-repository/linux-2.6.14.3-vanilla/kernel/Attic/kgdb.c,v retrieving revision 1.1.2.1 retrieving revision 1.1.2.2 diff -u -r1.1.2.1 -r1.1.2.2 --- kernel/kgdb.c 9 Dec 2005 22:43:36 -0000 1.1.2.1 +++ kernel/kgdb.c 30 Jan 2006 03:01:35 -0000 1.1.2.2 @@ -157,7 +157,7 @@ int __attribute__ ((weak)) kgdb_set_hw_break(unsigned long addr) { - return 0; + return -EINVAL; } /** @@ -167,11 +167,11 @@ int __attribute__ ((weak)) kgdb_remove_hw_break(unsigned long addr) { - return 0; + return -EINVAL; } /** - * kgdb_remove_all_hw_break - Clear all hardware breakpoints. + * kgdb_remove_all_hw_break - Clear all hardware break/watchpoints. */ void __attribute__ ((weak)) kgdb_remove_all_hw_break(void) @@ -179,6 +179,28 @@ } /** + * kgdb_set_hw_watch - Set a hardware watchpoint at @addr. + * @addr: The address of the memory region to be watched. + * @len: The length in bytes (1 to 4) of the memory region to be watched. + * @bpt_type: The watchpoint type (access/read/write) + */ +int __attribute__ ((weak)) + kgdb_set_hw_watch(unsigned long addr, unsigned int len, char bpt_type) +{ + return -EINVAL; +} + +/** + * kgdb_remove_hw_watch - Remove a hardware watchpoint at @addr. + * @addr: The address of the watchpoint to be removed. + */ +int __attribute__ ((weak)) + kgdb_remove_hw_watch(unsigned long addr) +{ + return -EINVAL; +} + +/** * kgdb_correct_hw_break - Correct hardware breakpoints. * * A hook to allow for changes to the hardware breakpoint, called @@ -272,6 +294,47 @@ return NULL; } +/** + * kgdb_arch_set_breakpoint - Set a software breakpoint at @addr + * @addr: The address at which to set the breakpoint. + * @saved_instr: Pointer to a buffer used to save the original + * code bytes replaced by the breakpoint. + * + * RETURN: 0 if the breakpoint is successfully set, < 0 otherwise + */ +int __attribute__ ((weak)) + kgdb_arch_set_breakpoint(unsigned long addr, char *saved_instr) +{ + int error = 0; + if ((error = kgdb_get_mem((char *)addr, + saved_instr, BREAK_INSTR_SIZE)) < 0) + return error; + + if ((error = kgdb_set_mem((char *)addr, kgdb_ops->gdb_bpt_instr, + BREAK_INSTR_SIZE)) < 0) + return error; + return 0; +} + +/** + * kgdb_arch_remove_breakpoint - Remove the software breakpoint at @addr + * @addr: The address of the breakpoint to be removed. + * @saved_instr: Pointer to a buffer containing the original code bytes + * to be restored at the breakpoint locations. + * + * RETURN: 0 if the breakpoint is succesfully removed, < 0 otherwise + */ +int __attribute__ ((weak)) + kgdb_arch_remove_breakpoint(unsigned long addr, char *saved_instr) +{ + + int error = 0; + if ((error =kgdb_set_mem((char *)addr, (char *)saved_instr, + BREAK_INSTR_SIZE)) < 0) + return error; + return 0; +} + static int hex(char ch) { if ((ch >= 'a') && (ch <= 'f')) @@ -758,20 +821,10 @@ if (breakno == -1) return -E2BIG; - if (kgdb_ops->set_breakpoint) { - if ((error = kgdb_ops->set_breakpoint(addr, - kgdb_break[breakno].saved_instr)) < 0) - return error; - } else { - if ((error = kgdb_get_mem((char *)addr, - kgdb_break[breakno].saved_instr, - BREAK_INSTR_SIZE)) < 0) - return error; + if ((error = kgdb_arch_set_breakpoint(addr, + kgdb_break[breakno].saved_instr))) + return error; - if ((error = kgdb_set_mem((char *)addr, kgdb_ops->gdb_bpt_instr, - BREAK_INSTR_SIZE)) < 0) - return error; - } if (CACHE_FLUSH_IS_SAFE) { if (current->mm && addr < TASK_SIZE) flush_cache_range(current->mm->mmap_cache, addr, @@ -792,27 +845,24 @@ int error; for (i = 0; i < MAX_BREAKPOINTS; i++) { - if ((kgdb_break[i].state == bp_enabled) && - (kgdb_break[i].bpt_addr == addr)) { - if (kgdb_ops->remove_breakpoint) { - if ((error = kgdb_ops->remove_breakpoint(addr, - kgdb_break[i].saved_instr)) < 0) - return error; - } else if ((error = - kgdb_set_mem((char *)addr, - kgdb_break[i].saved_instr, - BREAK_INSTR_SIZE)) < 0) - return error; - if (CACHE_FLUSH_IS_SAFE && current->mm && - addr < TASK_SIZE) - flush_cache_range(current->mm->mmap_cache, - addr, addr + BREAK_INSTR_SIZE); - else if (CACHE_FLUSH_IS_SAFE) + if ((kgdb_break[i].state != bp_enabled) || + (kgdb_break[i].bpt_addr != addr)) + continue; + + if ((error = kgdb_arch_remove_breakpoint(addr, + kgdb_break[i].saved_instr))) + return error; + + if (CACHE_FLUSH_IS_SAFE) { + if (current->mm && addr < TASK_SIZE) + flush_cache_range(current->mm->mmap_cache, addr, + addr + BREAK_INSTR_SIZE); + else flush_icache_range(addr, - addr + BREAK_INSTR_SIZE); - kgdb_break[i].state = bp_disabled; - return 0; + addr + BREAK_INSTR_SIZE); } + kgdb_break[i].state = bp_disabled; + return 0; } return -ENOENT; } @@ -1322,16 +1372,12 @@ bpt_type = &remcom_in_buffer[1]; ptr = &remcom_in_buffer[2]; - if (kgdb_ops->set_hw_breakpoint && *bpt_type >= '1') { - /* Unsupported */ - if (*bpt_type > '4') - break; - } else if (*bpt_type != '0' && *bpt_type != '1') - /* Unsupported. */ + if (*bpt_type < '0' || *bpt_type > '4') break; + /* Test if this is a hardware breakpoint, and * if we support it. */ - if (*bpt_type == '1' && + if (*bpt_type >= '1' && !kgdb_ops->flags & KGDB_HW_BREAKPOINT) /* Unsupported. */ break; @@ -1360,14 +1406,9 @@ else if (remcom_in_buffer[0] == 'z' && *bpt_type == '1') error = kgdb_remove_hw_break(addr); else if (remcom_in_buffer[0] == 'Z') - error = kgdb_ops->set_hw_breakpoint(addr, - (int)length, - *bpt_type); +error = kgdb_set_hw_watch(addr, length, *bpt_type); else if (remcom_in_buffer[0] == 'z') - error = kgdb_ops->remove_hw_breakpoint(addr, - (int) - length, - *bpt_type); + error = kgdb_remove_hw_watch(addr); if (error == 0) strcpy(remcom_out_buffer, "OK");
|
|
| <Prev in Thread] | Current Thread | [Next in Thread> |
|---|---|---|
| Previous by Date: | Re: [PATCH] i386 Hardware watchpoints using kgdb-2-2.6.14.tar.gz: 00156, Dennis W. Tokarski |
|---|---|
| Next by Date: | KGDB tracepoints Trac instance up: 00156, Jim Blandy |
| Previous by Thread: | Re: [PATCH] i386 Hardware watchpoints using kgdb-2-2.6.14.tar.gzi: 00156, Dennis W. Tokarski |
| Next by Thread: | EXTRAVERSION -kgdb [Re: [PATCH] i386 Hardware watchpoints using kgdb-2-2.6.14.tar.gz]: 00156, Amit Kale |
| Indexes: | [Date] [Thread] [Top] [All Lists] |
| News | FAQ | advertise |
