[U-Boot] [PATCH] Add KGDB support for ARM platforms

These patches add kgdb support for ARM platforms.
The add and invocation of the bad_restore_user_regs macro in cpu/arm720t/start.S should be made to other cpu/arm*/start.S files as well.
Signed-off-by: Tonny Tzeng tonny.tzeng@gmail.com --- common/kgdb.c | 30 ++++++ cpu/arm720t/start.S | 7 ++ include/asm-arm/kgdb.h | 67 ++++++++++++ include/kgdb.h | 33 ++++++ lib_arm/Makefile | 1 + lib_arm/board.c | 6 + lib_arm/interrupts.c | 10 ++ lib_arm/kgdb.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 416 insertions(+), 0 deletions(-) create mode 100644 include/asm-arm/kgdb.h create mode 100644 lib_arm/kgdb.c
diff --git a/common/kgdb.c b/common/kgdb.c index 0531452..15ff6e8 100644 --- a/common/kgdb.c +++ b/common/kgdb.c @@ -220,6 +220,29 @@ hexToInt(char **ptr, int *intValue) return (numChars); }
+/* Handle the 'z' or 'Z' breakpoint remove or set packets */ +static void gdb_cmd_break(kgdb_data *kdp) +{ + /* + * Since GDB-5.3, it's been drafted that '0' is a software + * breakpoint, '1' is a hardware breakpoint, so let's do that. + */ + char *bpt_type = &remcomInBuffer[1]; + char *ptr = &remcomInBuffer[2]; + int addr, length; + + if (*bpt_type != '0') + return; /* Unsupported. */ + if (*ptr++ != ',' || !hexToInt(&ptr, &addr) || + *ptr++ != ',' || !hexToInt(&ptr, &length)) { + kgdb_error(KGDBERR_BADPARAMS); + } + + if (kdp->hook_break && + !(*kdp->hook_break)(remcomInBuffer[0] == 'Z', addr)) + strcpy(remcomOutBuffer, "OK"); +} + /* scan for the sequence $<data>#<checksum> */ static void getpacket(char *buffer) @@ -341,11 +364,14 @@ handle_exception (struct pt_regs *regs)
kgdb_interruptible(0);
+#ifdef KGDB_DEBUG printf("kgdb: handle_exception; trap [0x%x]\n", kgdb_trap(regs)); +#endif
if (kgdb_setjmp(error_jmp_buf) != 0) panic("kgdb: error or fault in entry init!\n");
+ memset(&kd, 0, sizeof(kgdb_data)); kgdb_enter(regs, &kd);
if (first_entry) { @@ -516,6 +542,10 @@ handle_exception (struct pt_regs *regs) kgdb_error(KGDBERR_BADPARAMS); } break; + case 'Z': /* [Z|z]N,AA..AA,LLLL Set/Remove breakpoint type N LLLL bytes at address AA.AA return OK */ + case 'z': + gdb_cmd_break(&kd); + break; } /* switch */
if (errnum != 0) diff --git a/cpu/arm720t/start.S b/cpu/arm720t/start.S index 022b873..a59e186 100644 --- a/cpu/arm720t/start.S +++ b/cpu/arm720t/start.S @@ -455,6 +455,12 @@ lock_loop: mov r0, sp .endm
+ .macro bad_restore_user_regs + ldr lr, [sp, #S_PSR] @ Get SVC cpsr + msr spsr_cxsf, lr + ldmia sp, {r0 - pc}^ @ Restore SVC registers + .endm + .macro irq_save_user_regs sub sp, sp, #S_FRAME_SIZE stmia sp, {r0 - r12} @ Calling r0-r12 @@ -506,6 +512,7 @@ undefined_instruction: get_bad_stack bad_save_user_regs bl do_undefined_instruction + bad_restore_user_regs
.align 5 software_interrupt: diff --git a/include/asm-arm/kgdb.h b/include/asm-arm/kgdb.h new file mode 100644 index 0000000..c33254b --- /dev/null +++ b/include/asm-arm/kgdb.h @@ -0,0 +1,67 @@ +/* + * ARM KGDB support + * + * Author: Deepak Saxena dsaxena@mvista.com + * + * Copyright (C) 2002 MontaVista Software Inc. + * + */ + +#ifndef __ARM_KGDB_H__ +#define __ARM_KGDB_H__ + +#define BREAK_INSTR_SIZE 4 +#define KGDB_COMPILED_BREAK 0xe7ffdeff + +#ifndef __ASSEMBLY__ + +static inline void arch_kgdb_breakpoint(void) +{ + asm(".word 0xe7ffdeff"); +} + +#endif /* !__ASSEMBLY__ */ + +/* + * From Kevin Hilman: + * + * gdb is expecting the following registers layout. + * + * r0-r15: 1 long word each + * f0-f7: unused, 3 long words each !! + * fps: unused, 1 long word + * cpsr: 1 long word + * + * Even though f0-f7 and fps are not used, they need to be + * present in the registers sent for correct processing in + * the host-side gdb. + * + * In particular, it is crucial that CPSR is in the right place, + * otherwise gdb will not be able to correctly interpret stepping over + * conditional branches. + */ +#define _GP_REGS 16 +#define _FP_REGS 8 +#define _EXTRA_REGS 2 +#define GDB_MAX_REGS (_GP_REGS + (_FP_REGS * 3) + _EXTRA_REGS) + +#define _R0 0 +#define _R1 1 +#define _R2 2 +#define _R3 3 +#define _R4 4 +#define _R5 5 +#define _R6 6 +#define _R7 7 +#define _R8 8 +#define _R9 9 +#define _R10 10 +#define _FP 11 +#define _IP 12 +#define _SPT 13 +#define _LR 14 +#define _PC 15 +#define _CPSR (GDB_MAX_REGS - 1) + +#endif /* __ASM_KGDB_H__ */ + diff --git a/include/kgdb.h b/include/kgdb.h index f543cd6..b4c5d81 100644 --- a/include/kgdb.h +++ b/include/kgdb.h @@ -8,6 +8,38 @@ #define KGDBERR_MEMFAULT 3 #define KGDBERR_NOSPACE 4 #define KGDBERR_ALIGNFAULT 5 +#define KGDBERR_BPEXIST 6 +#define KGDBERR_BPNOENT 7 + +#ifndef BREAK_INSTR_SIZE +#define BREAK_INSTR_SIZE 4 +#endif + +#ifndef KGDB_MAX_BREAKPOINTS +#define KGDB_MAX_BREAKPOINTS 1000 +#endif + +enum kgdb_bptype { + BP_BREAKPOINT = 0, + BP_HARDWARE_BREAKPOINT, + BP_WRITE_WATCHPOINT, + BP_READ_WATCHPOINT, + BP_ACCESS_WATCHPOINT +}; + +enum kgdb_bpstate { + BP_UNDEFINED = 0, + BP_REMOVED, + BP_SET, + BP_ACTIVE +}; + +struct kgdb_bkpt { + unsigned long bpt_addr; + unsigned char saved_instr[BREAK_INSTR_SIZE]; + enum kgdb_bptype type; + enum kgdb_bpstate state; +};
#define KGDBDATA_MAXREGS 8 #define KGDBDATA_MAXPRIV 8 @@ -35,6 +67,7 @@ typedef int nregs; kgdb_reg regs[KGDBDATA_MAXREGS]; unsigned long private[KGDBDATA_MAXPRIV]; + int (*hook_break)(int set, int addr); } kgdb_data;
diff --git a/lib_arm/Makefile b/lib_arm/Makefile index 0293348..36fe528 100644 --- a/lib_arm/Makefile +++ b/lib_arm/Makefile @@ -44,6 +44,7 @@ COBJS-y += cache-cp15.o endif COBJS-y += interrupts.o COBJS-y += reset.o +COBJS-$(CONFIG_CMD_KGDB) += kgdb.o
SRCS := $(GLSOBJS:.o=.S) $(GLCOBJS:.o=.c) \ $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) diff --git a/lib_arm/board.c b/lib_arm/board.c index f5660a9..edc2120 100644 --- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -39,6 +39,7 @@ */
#include <common.h> +#include <watchdog.h> #include <command.h> #include <malloc.h> #include <stdio_dev.h> @@ -381,6 +382,11 @@ void start_armboot (void) /* miscellaneous platform dependent initialisations */ misc_init_r (); #endif +#if defined(CONFIG_CMD_KGDB) + WATCHDOG_RESET(); + puts("KGDB: "); + kgdb_init(); +#endif
/* enable exceptions */ enable_interrupts (); diff --git a/lib_arm/interrupts.c b/lib_arm/interrupts.c index 1f2b815..d62b355 100644 --- a/lib_arm/interrupts.c +++ b/lib_arm/interrupts.c @@ -37,6 +37,10 @@
#include <common.h> #include <asm/proc-armv/ptrace.h> +#include <kgdb.h> +#ifdef CONFIG_CMD_KGDB +#include <asm/kgdb.h> +#endif
#ifdef CONFIG_USE_IRQ DECLARE_GLOBAL_DATA_PTR; @@ -137,6 +141,12 @@ void show_regs (struct pt_regs *regs)
void do_undefined_instruction (struct pt_regs *pt_regs) { +#ifdef CONFIG_CMD_KGDB + if (*(unsigned long *)(instruction_pointer(pt_regs) - 4) == KGDB_COMPILED_BREAK) { + (*debugger_exception_handler)(pt_regs); + return; + } +#endif printf ("undefined instruction\n"); show_regs (pt_regs); bad_mode (); diff --git a/lib_arm/kgdb.c b/lib_arm/kgdb.c new file mode 100644 index 0000000..b76b8ed --- /dev/null +++ b/lib_arm/kgdb.c @@ -0,0 +1,262 @@ +/* + * Functions in this file are excerpt from the Linux KGDB + * supports in the following sources + * + * (1) linux/kernel/kgdb.c, contributed by: + * David Grothe dave@gcom.com, + * Tigran Aivazian tigran@sco.com + * Jason Wessel ( jason.wessel@windriver.com ) + * George Anzinger george@mvista.com + * Anurekh Saxena (anurekh.saxena@timesys.com) + * Lake Stevens Instrument Division (Glenn Engel) + * Jim Kingdon, Cygnus Support. + * + * (2) linux/arch/arm/kernel/kgdb.c + * Copyright (c) 2002-2004 MontaVista Software, Inc + * Copyright (c) 2008 Wind River Systems, Inc. + * Authors: George Davis davis_g@mvista.com + * Deepak Saxena dsaxena@plexity.net + */ +#include <common.h> +#include <asm-generic/signal.h> +#include <asm/kgdb.h> +#include <kgdb.h> + +/* + * kgdb_breakpoint - generate breakpoint exception + * + * This function will generate a breakpoint exception. It is used at the + * beginning of a program to sync up with a debugger and can be used + * otherwise as a quick means to stop program execution and "break" into + * the debugger. + */ +void kgdb_breakpoint (int argc, char *argv[]) +{ + arch_kgdb_breakpoint(); +} + +/* + * Holds information about breakpoints in a kernel. These breakpoints are + * added and removed by gdb. + */ +static struct kgdb_bkpt kgdb_break[KGDB_MAX_BREAKPOINTS]; + +static int kgdb_set_sw_break(int addr) +{ + int i, breakno = -1; + struct kgdb_bkpt *bkpt; + + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) { + if ((kgdb_break[i].state == BP_SET) && + (kgdb_break[i].bpt_addr == addr)) + return -KGDBERR_BPEXIST; + if ((kgdb_break[i].state == BP_REMOVED) && + (kgdb_break[i].bpt_addr == addr)) { + breakno = i; + break; + } + } + if (breakno == -1) + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) { + if (kgdb_break[i].state == BP_UNDEFINED) { + breakno = i; + break; + } + } + if (breakno == -1) + return -KGDBERR_BPNOENT; + + bkpt = kgdb_break + breakno; + bkpt->state = BP_SET; + bkpt->type = BP_BREAKPOINT; + bkpt->bpt_addr = addr; + memcpy(bkpt->saved_instr, addr, BREAK_INSTR_SIZE); + *(unsigned long *)addr = KGDB_COMPILED_BREAK; + + return 0; +} + +static int kgdb_remove_sw_break(int addr) +{ + int i; + struct kgdb_bkpt *bkpt; + + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) { + bkpt = kgdb_break + i; + if ((bkpt->state == BP_SET) && (bkpt->bpt_addr == addr)) { + bkpt->state = BP_REMOVED; + memcpy(addr, bkpt->saved_instr, BREAK_INSTR_SIZE); + return 0; + } + } + return -KGDBERR_BPNOENT; +} + +static int kgdb_sw_break(int set, int addr) +{ + return (set) ? kgdb_set_sw_break(addr) : kgdb_remove_sw_break(addr); +} + +int kgdb_setjmp(long *buf) +{ + asm(" stmia r0, {r0-r14}\n \ + str lr,[r0, #60]\n \ + mrs r1,cpsr\n \ + str r1,[r0,#64]\n \ + ldr r1,[r0,#4]\n \ + "); + return 0; +} + +void kgdb_longjmp(long *buf, int val) +{ + asm(" str r1,[r0]\n \ + ldr r1,[r0, #64]\n \ + msr spsr,r1\n \ + ldmia r0,{r0-pc}^\n \ + "); +} + +int kgdb_trap(struct pt_regs *regs) +{ + return SIGTRAP; +} + +void kgdb_enter(struct pt_regs *regs, kgdb_data *kdp) +{ + kdp->sigval = kgdb_trap(regs); + kdp->nregs = 0; + kdp->hook_break = kgdb_sw_break; +} + +void kgdb_exit(struct pt_regs *regs, kgdb_data *kdp) +{ +} + +int kgdb_getregs(struct pt_regs *regs, char *buf, int max) +{ + int i; + unsigned long *gdb_regs = (unsigned long *)buf; + + if (max < (GDB_MAX_REGS * sizeof(unsigned long))) + kgdb_error(KGDBERR_NOSPACE); + + /* Initialize all to zero. */ + for (i = 0; i < GDB_MAX_REGS; i++) + gdb_regs[i] = 0; + + gdb_regs[_R0] = regs->ARM_r0; + gdb_regs[_R1] = regs->ARM_r1; + gdb_regs[_R2] = regs->ARM_r2; + gdb_regs[_R3] = regs->ARM_r3; + gdb_regs[_R4] = regs->ARM_r4; + gdb_regs[_R5] = regs->ARM_r5; + gdb_regs[_R6] = regs->ARM_r6; + gdb_regs[_R7] = regs->ARM_r7; + gdb_regs[_R8] = regs->ARM_r8; + gdb_regs[_R9] = regs->ARM_r9; + gdb_regs[_R10] = regs->ARM_r10; + gdb_regs[_FP] = regs->ARM_fp; + gdb_regs[_IP] = regs->ARM_ip; + gdb_regs[_SPT] = regs->ARM_sp; + gdb_regs[_LR] = regs->ARM_lr; + gdb_regs[_PC] = regs->ARM_pc; + gdb_regs[_CPSR] = regs->ARM_cpsr; + + return GDB_MAX_REGS *sizeof(unsigned long); +} + +void kgdb_putreg(struct pt_regs *regs, int regno, char *buf, int length) +{ + unsigned long val, *ptr = (unsigned long *)buf; + + if (regno < 0 || GDB_MAX_REGS <= regno) + kgdb_error(KGDBERR_BADPARAMS); + else { + if (length < 4) + kgdb_error(KGDBERR_NOSPACE); + } + if ((unsigned long)ptr & 3) + kgdb_error(KGDBERR_ALIGNFAULT); + else + val = *ptr; + + switch (regno) { + case _R0: + regs->ARM_r0 = val; break; + case _R1: + regs->ARM_r1 = val; break; + case _R2: + regs->ARM_r2 = val; break; + case _R3: + regs->ARM_r3 = val; break; + case _R4: + regs->ARM_r4 = val; break; + case _R5: + regs->ARM_r5 = val; break; + case _R6: + regs->ARM_r6 = val; break; + case _R7: + regs->ARM_r7 = val; break; + case _R8: + regs->ARM_r8 = val; break; + case _R9: + regs->ARM_r9 = val; break; + case _R10: + regs->ARM_r10 = val; break; + case _FP: + regs->ARM_fp = val; break; + case _IP: + regs->ARM_ip = val; break; + case _SPT: + regs->ARM_sp = val; break; + case _LR: + regs->ARM_lr = val; break; + case _PC: + regs->ARM_pc = val; break; + case 0x19: + regs->ARM_cpsr = val; break; + default: + kgdb_error(KGDBERR_BADPARAMS); + } +} + +void kgdb_putregs(struct pt_regs *regs, char *buf, int length) +{ + unsigned long *gdb_regs = (unsigned long *)buf; + + if (length < (GDB_MAX_REGS *sizeof(unsigned long))) + kgdb_error(KGDBERR_NOSPACE); + + if ((unsigned long)gdb_regs & 3) + kgdb_error(KGDBERR_ALIGNFAULT); + + regs->ARM_r0 = gdb_regs[_R0]; + regs->ARM_r1 = gdb_regs[_R1]; + regs->ARM_r2 = gdb_regs[_R2]; + regs->ARM_r3 = gdb_regs[_R3]; + regs->ARM_r4 = gdb_regs[_R4]; + regs->ARM_r5 = gdb_regs[_R5]; + regs->ARM_r6 = gdb_regs[_R6]; + regs->ARM_r7 = gdb_regs[_R7]; + regs->ARM_r8 = gdb_regs[_R8]; + regs->ARM_r9 = gdb_regs[_R9]; + regs->ARM_r10 = gdb_regs[_R10]; + regs->ARM_fp = gdb_regs[_FP]; + regs->ARM_ip = gdb_regs[_IP]; + regs->ARM_sp = gdb_regs[_SPT]; + regs->ARM_lr = gdb_regs[_LR]; + regs->ARM_pc = gdb_regs[_PC]; + regs->ARM_cpsr = gdb_regs[_CPSR]; +} + +void kgdb_interruptible(int yes) +{ +#ifdef CONFIG_USE_IRQ + if (yes) + enable_interrupts(); + else + disable_interrupts(); +#endif +} +

These patches add kgdb support for ARM platforms.
The add and invocation of the bad_restore_user_regs macro in cpu/arm720t/start.S should be made to other cpu/arm*/start.S files as well.
Signed-off-by: Tonny Tzeng tonny.tzeng@gmail.com --- common/kgdb.c | 30 ++++++ cpu/arm720t/start.S | 7 ++ include/asm-arm/kgdb.h | 67 ++++++++++++ include/kgdb.h | 33 ++++++ lib_arm/Makefile | 1 + lib_arm/board.c | 6 + lib_arm/interrupts.c | 10 ++ lib_arm/kgdb.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 416 insertions(+), 0 deletions(-) create mode 100644 include/asm-arm/kgdb.h create mode 100644 lib_arm/kgdb.c
diff --git a/common/kgdb.c b/common/kgdb.c index 0531452..15ff6e8 100644 --- a/common/kgdb.c +++ b/common/kgdb.c @@ -220,6 +220,29 @@ hexToInt(char **ptr, int *intValue) return (numChars); }
+/* Handle the 'z' or 'Z' breakpoint remove or set packets */ +static void gdb_cmd_break(kgdb_data *kdp) +{ + /* + * Since GDB-5.3, it's been drafted that '0' is a software + * breakpoint, '1' is a hardware breakpoint, so let's do that. + */ + char *bpt_type = &remcomInBuffer[1]; + char *ptr = &remcomInBuffer[2]; + int addr, length; + + if (*bpt_type != '0') + return; /* Unsupported. */ + if (*ptr++ != ',' || !hexToInt(&ptr, &addr) || + *ptr++ != ',' || !hexToInt(&ptr, &length)) { + kgdb_error(KGDBERR_BADPARAMS); + } + + if (kdp->hook_break && + !(*kdp->hook_break)(remcomInBuffer[0] == 'Z', addr)) + strcpy(remcomOutBuffer, "OK"); +} + /* scan for the sequence $<data>#<checksum> */ static void getpacket(char *buffer) @@ -341,11 +364,14 @@ handle_exception (struct pt_regs *regs)
kgdb_interruptible(0);
+#ifdef KGDB_DEBUG printf("kgdb: handle_exception; trap [0x%x]\n", kgdb_trap(regs)); +#endif
if (kgdb_setjmp(error_jmp_buf) != 0) panic("kgdb: error or fault in entry init!\n");
+ memset(&kd, 0, sizeof(kgdb_data)); kgdb_enter(regs, &kd);
if (first_entry) { @@ -516,6 +542,10 @@ handle_exception (struct pt_regs *regs) kgdb_error(KGDBERR_BADPARAMS); } break; + case 'Z': /* [Z|z]N,AA..AA,LLLL Set/Remove breakpoint type N LLLL bytes at address AA.AA return OK */ + case 'z': + gdb_cmd_break(&kd); + break; } /* switch */
if (errnum != 0) diff --git a/cpu/arm720t/start.S b/cpu/arm720t/start.S index 022b873..a59e186 100644 --- a/cpu/arm720t/start.S +++ b/cpu/arm720t/start.S @@ -455,6 +455,12 @@ lock_loop: mov r0, sp .endm
+ .macro bad_restore_user_regs + ldr lr, [sp, #S_PSR] @ Get SVC cpsr + msr spsr_cxsf, lr + ldmia sp, {r0 - pc}^ @ Restore SVC registers + .endm + .macro irq_save_user_regs sub sp, sp, #S_FRAME_SIZE stmia sp, {r0 - r12} @ Calling r0-r12 @@ -506,6 +512,7 @@ undefined_instruction: get_bad_stack bad_save_user_regs bl do_undefined_instruction + bad_restore_user_regs
.align 5 software_interrupt: diff --git a/include/asm-arm/kgdb.h b/include/asm-arm/kgdb.h new file mode 100644 index 0000000..c33254b --- /dev/null +++ b/include/asm-arm/kgdb.h @@ -0,0 +1,67 @@ +/* + * ARM KGDB support + * + * Author: Deepak Saxena dsaxena@mvista.com + * + * Copyright (C) 2002 MontaVista Software Inc. + * + */ + +#ifndef __ARM_KGDB_H__ +#define __ARM_KGDB_H__ + +#define BREAK_INSTR_SIZE 4 +#define KGDB_COMPILED_BREAK 0xe7ffdeff + +#ifndef __ASSEMBLY__ + +static inline void arch_kgdb_breakpoint(void) +{ + asm(".word 0xe7ffdeff"); +} + +#endif /* !__ASSEMBLY__ */ + +/* + * From Kevin Hilman: + * + * gdb is expecting the following registers layout. + * + * r0-r15: 1 long word each + * f0-f7: unused, 3 long words each !! + * fps: unused, 1 long word + * cpsr: 1 long word + * + * Even though f0-f7 and fps are not used, they need to be + * present in the registers sent for correct processing in + * the host-side gdb. + * + * In particular, it is crucial that CPSR is in the right place, + * otherwise gdb will not be able to correctly interpret stepping over + * conditional branches. + */ +#define _GP_REGS 16 +#define _FP_REGS 8 +#define _EXTRA_REGS 2 +#define GDB_MAX_REGS (_GP_REGS + (_FP_REGS * 3) + _EXTRA_REGS) + +#define _R0 0 +#define _R1 1 +#define _R2 2 +#define _R3 3 +#define _R4 4 +#define _R5 5 +#define _R6 6 +#define _R7 7 +#define _R8 8 +#define _R9 9 +#define _R10 10 +#define _FP 11 +#define _IP 12 +#define _SPT 13 +#define _LR 14 +#define _PC 15 +#define _CPSR (GDB_MAX_REGS - 1) + +#endif /* __ASM_KGDB_H__ */ + diff --git a/include/kgdb.h b/include/kgdb.h index f543cd6..b4c5d81 100644 --- a/include/kgdb.h +++ b/include/kgdb.h @@ -8,6 +8,38 @@ #define KGDBERR_MEMFAULT 3 #define KGDBERR_NOSPACE 4 #define KGDBERR_ALIGNFAULT 5 +#define KGDBERR_BPEXIST 6 +#define KGDBERR_BPNOENT 7 + +#ifndef BREAK_INSTR_SIZE +#define BREAK_INSTR_SIZE 4 +#endif + +#ifndef KGDB_MAX_BREAKPOINTS +#define KGDB_MAX_BREAKPOINTS 1000 +#endif + +enum kgdb_bptype { + BP_BREAKPOINT = 0, + BP_HARDWARE_BREAKPOINT, + BP_WRITE_WATCHPOINT, + BP_READ_WATCHPOINT, + BP_ACCESS_WATCHPOINT +}; + +enum kgdb_bpstate { + BP_UNDEFINED = 0, + BP_REMOVED, + BP_SET, + BP_ACTIVE +}; + +struct kgdb_bkpt { + unsigned long bpt_addr; + unsigned char saved_instr[BREAK_INSTR_SIZE]; + enum kgdb_bptype type; + enum kgdb_bpstate state; +};
#define KGDBDATA_MAXREGS 8 #define KGDBDATA_MAXPRIV 8 @@ -35,6 +67,7 @@ typedef int nregs; kgdb_reg regs[KGDBDATA_MAXREGS]; unsigned long private[KGDBDATA_MAXPRIV]; + int (*hook_break)(int set, int addr); } kgdb_data;
diff --git a/lib_arm/Makefile b/lib_arm/Makefile index 0293348..36fe528 100644 --- a/lib_arm/Makefile +++ b/lib_arm/Makefile @@ -44,6 +44,7 @@ COBJS-y += cache-cp15.o endif COBJS-y += interrupts.o COBJS-y += reset.o +COBJS-$(CONFIG_CMD_KGDB) += kgdb.o
SRCS := $(GLSOBJS:.o=.S) $(GLCOBJS:.o=.c) \ $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) diff --git a/lib_arm/board.c b/lib_arm/board.c index f5660a9..edc2120 100644 --- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -39,6 +39,7 @@ */
#include <common.h> +#include <watchdog.h> #include <command.h> #include <malloc.h> #include <stdio_dev.h> @@ -381,6 +382,11 @@ void start_armboot (void) /* miscellaneous platform dependent initialisations */ misc_init_r (); #endif +#if defined(CONFIG_CMD_KGDB) + WATCHDOG_RESET(); + puts("KGDB: "); + kgdb_init(); +#endif
/* enable exceptions */ enable_interrupts (); diff --git a/lib_arm/interrupts.c b/lib_arm/interrupts.c index 1f2b815..d62b355 100644 --- a/lib_arm/interrupts.c +++ b/lib_arm/interrupts.c @@ -37,6 +37,10 @@
#include <common.h> #include <asm/proc-armv/ptrace.h> +#include <kgdb.h> +#ifdef CONFIG_CMD_KGDB +#include <asm/kgdb.h> +#endif
#ifdef CONFIG_USE_IRQ DECLARE_GLOBAL_DATA_PTR; @@ -137,6 +141,12 @@ void show_regs (struct pt_regs *regs)
void do_undefined_instruction (struct pt_regs *pt_regs) { +#ifdef CONFIG_CMD_KGDB + if (*(unsigned long *)(instruction_pointer(pt_regs) - 4) == KGDB_COMPILED_BREAK) { + (*debugger_exception_handler)(pt_regs); + return; + } +#endif printf ("undefined instruction\n"); show_regs (pt_regs); bad_mode (); diff --git a/lib_arm/kgdb.c b/lib_arm/kgdb.c new file mode 100644 index 0000000..b76b8ed --- /dev/null +++ b/lib_arm/kgdb.c @@ -0,0 +1,262 @@ +/* + * Functions in this file are excerpt from the Linux KGDB + * supports in the following sources + * + * (1) linux/kernel/kgdb.c, contributed by: + * David Grothe dave@gcom.com, + * Tigran Aivazian tigran@sco.com + * Jason Wessel ( jason.wessel@windriver.com ) + * George Anzinger george@mvista.com + * Anurekh Saxena (anurekh.saxena@timesys.com) + * Lake Stevens Instrument Division (Glenn Engel) + * Jim Kingdon, Cygnus Support. + * + * (2) linux/arch/arm/kernel/kgdb.c + * Copyright (c) 2002-2004 MontaVista Software, Inc + * Copyright (c) 2008 Wind River Systems, Inc. + * Authors: George Davis davis_g@mvista.com + * Deepak Saxena dsaxena@plexity.net + */ +#include <common.h> +#include <asm-generic/signal.h> +#include <asm/kgdb.h> +#include <kgdb.h> + +/* + * kgdb_breakpoint - generate breakpoint exception + * + * This function will generate a breakpoint exception. It is used at the + * beginning of a program to sync up with a debugger and can be used + * otherwise as a quick means to stop program execution and "break" into + * the debugger. + */ +void kgdb_breakpoint (int argc, char *argv[]) +{ + arch_kgdb_breakpoint(); +} + +/* + * Holds information about breakpoints in a kernel. These breakpoints are + * added and removed by gdb. + */ +static struct kgdb_bkpt kgdb_break[KGDB_MAX_BREAKPOINTS]; + +static int kgdb_set_sw_break(int addr) +{ + int i, breakno = -1; + struct kgdb_bkpt *bkpt; + + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) { + if ((kgdb_break[i].state == BP_SET) && + (kgdb_break[i].bpt_addr == addr)) + return -KGDBERR_BPEXIST; + if ((kgdb_break[i].state == BP_REMOVED) && + (kgdb_break[i].bpt_addr == addr)) { + breakno = i; + break; + } + } + if (breakno == -1) + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) { + if (kgdb_break[i].state == BP_UNDEFINED) { + breakno = i; + break; + } + } + if (breakno == -1) + return -KGDBERR_BPNOENT; + + bkpt = kgdb_break + breakno; + bkpt->state = BP_SET; + bkpt->type = BP_BREAKPOINT; + bkpt->bpt_addr = addr; + memcpy(bkpt->saved_instr, addr, BREAK_INSTR_SIZE); + *(unsigned long *)addr = KGDB_COMPILED_BREAK; + + return 0; +} + +static int kgdb_remove_sw_break(int addr) +{ + int i; + struct kgdb_bkpt *bkpt; + + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) { + bkpt = kgdb_break + i; + if ((bkpt->state == BP_SET) && (bkpt->bpt_addr == addr)) { + bkpt->state = BP_REMOVED; + memcpy(addr, bkpt->saved_instr, BREAK_INSTR_SIZE); + return 0; + } + } + return -KGDBERR_BPNOENT; +} + +static int kgdb_sw_break(int set, int addr) +{ + return (set) ? kgdb_set_sw_break(addr) : kgdb_remove_sw_break(addr); +} + +int kgdb_setjmp(long *buf) +{ + asm(" stmia r0, {r0-r14}\n \ + str lr,[r0, #60]\n \ + mrs r1,cpsr\n \ + str r1,[r0,#64]\n \ + ldr r1,[r0,#4]\n \ + "); + return 0; +} + +void kgdb_longjmp(long *buf, int val) +{ + asm(" str r1,[r0]\n \ + ldr r1,[r0, #64]\n \ + msr spsr,r1\n \ + ldmia r0,{r0-pc}^\n \ + "); +} + +int kgdb_trap(struct pt_regs *regs) +{ + return SIGTRAP; +} + +void kgdb_enter(struct pt_regs *regs, kgdb_data *kdp) +{ + kdp->sigval = kgdb_trap(regs); + kdp->nregs = 0; + kdp->hook_break = kgdb_sw_break; +} + +void kgdb_exit(struct pt_regs *regs, kgdb_data *kdp) +{ +} + +int kgdb_getregs(struct pt_regs *regs, char *buf, int max) +{ + int i; + unsigned long *gdb_regs = (unsigned long *)buf; + + if (max < (GDB_MAX_REGS * sizeof(unsigned long))) + kgdb_error(KGDBERR_NOSPACE); + + /* Initialize all to zero. */ + for (i = 0; i < GDB_MAX_REGS; i++) + gdb_regs[i] = 0; + + gdb_regs[_R0] = regs->ARM_r0; + gdb_regs[_R1] = regs->ARM_r1; + gdb_regs[_R2] = regs->ARM_r2; + gdb_regs[_R3] = regs->ARM_r3; + gdb_regs[_R4] = regs->ARM_r4; + gdb_regs[_R5] = regs->ARM_r5; + gdb_regs[_R6] = regs->ARM_r6; + gdb_regs[_R7] = regs->ARM_r7; + gdb_regs[_R8] = regs->ARM_r8; + gdb_regs[_R9] = regs->ARM_r9; + gdb_regs[_R10] = regs->ARM_r10; + gdb_regs[_FP] = regs->ARM_fp; + gdb_regs[_IP] = regs->ARM_ip; + gdb_regs[_SPT] = regs->ARM_sp; + gdb_regs[_LR] = regs->ARM_lr; + gdb_regs[_PC] = regs->ARM_pc; + gdb_regs[_CPSR] = regs->ARM_cpsr; + + return GDB_MAX_REGS *sizeof(unsigned long); +} + +void kgdb_putreg(struct pt_regs *regs, int regno, char *buf, int length) +{ + unsigned long val, *ptr = (unsigned long *)buf; + + if (regno < 0 || GDB_MAX_REGS <= regno) + kgdb_error(KGDBERR_BADPARAMS); + else { + if (length < 4) + kgdb_error(KGDBERR_NOSPACE); + } + if ((unsigned long)ptr & 3) + kgdb_error(KGDBERR_ALIGNFAULT); + else + val = *ptr; + + switch (regno) { + case _R0: + regs->ARM_r0 = val; break; + case _R1: + regs->ARM_r1 = val; break; + case _R2: + regs->ARM_r2 = val; break; + case _R3: + regs->ARM_r3 = val; break; + case _R4: + regs->ARM_r4 = val; break; + case _R5: + regs->ARM_r5 = val; break; + case _R6: + regs->ARM_r6 = val; break; + case _R7: + regs->ARM_r7 = val; break; + case _R8: + regs->ARM_r8 = val; break; + case _R9: + regs->ARM_r9 = val; break; + case _R10: + regs->ARM_r10 = val; break; + case _FP: + regs->ARM_fp = val; break; + case _IP: + regs->ARM_ip = val; break; + case _SPT: + regs->ARM_sp = val; break; + case _LR: + regs->ARM_lr = val; break; + case _PC: + regs->ARM_pc = val; break; + case 0x19: + regs->ARM_cpsr = val; break; + default: + kgdb_error(KGDBERR_BADPARAMS); + } +} + +void kgdb_putregs(struct pt_regs *regs, char *buf, int length) +{ + unsigned long *gdb_regs = (unsigned long *)buf; + + if (length < (GDB_MAX_REGS *sizeof(unsigned long))) + kgdb_error(KGDBERR_NOSPACE); + + if ((unsigned long)gdb_regs & 3) + kgdb_error(KGDBERR_ALIGNFAULT); + + regs->ARM_r0 = gdb_regs[_R0]; + regs->ARM_r1 = gdb_regs[_R1]; + regs->ARM_r2 = gdb_regs[_R2]; + regs->ARM_r3 = gdb_regs[_R3]; + regs->ARM_r4 = gdb_regs[_R4]; + regs->ARM_r5 = gdb_regs[_R5]; + regs->ARM_r6 = gdb_regs[_R6]; + regs->ARM_r7 = gdb_regs[_R7]; + regs->ARM_r8 = gdb_regs[_R8]; + regs->ARM_r9 = gdb_regs[_R9]; + regs->ARM_r10 = gdb_regs[_R10]; + regs->ARM_fp = gdb_regs[_FP]; + regs->ARM_ip = gdb_regs[_IP]; + regs->ARM_sp = gdb_regs[_SPT]; + regs->ARM_lr = gdb_regs[_LR]; + regs->ARM_pc = gdb_regs[_PC]; + regs->ARM_cpsr = gdb_regs[_CPSR]; +} + +void kgdb_interruptible(int yes) +{ +#ifdef CONFIG_USE_IRQ + if (yes) + enable_interrupts(); + else + disable_interrupts(); +#endif +} + -- 1.6.0.6

On Thursday 08 April 2010 05:40:02 Tonny Tzeng wrote:
--- a/common/kgdb.c +++ b/common/kgdb.c @@ -220,6 +220,29 @@ hexToInt(char **ptr, int *intValue) return (numChars); }
+/* Handle the 'z' or 'Z' breakpoint remove or set packets */ +static void gdb_cmd_break(kgdb_data *kdp)
this breakpoint support isnt related to ARM KGDB at all. it should be split into a separate changeset.
if (kgdb_setjmp(error_jmp_buf) != 0) panic("kgdb: error or fault in entry init!\n");
memset(&kd, 0, sizeof(kgdb_data));
there is no logic as to why this is needed. should probably be a different changeset too. personally, i'd also use sizeof(kd).
--- a/include/kgdb.h +++ b/include/kgdb.h +#ifndef BREAK_INSTR_SIZE +#define BREAK_INSTR_SIZE 4 +#endif
i'm not sure this makes sense. every arch should define it themselves.
--- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -381,6 +382,11 @@ void start_armboot (void) /* miscellaneous platform dependent initialisations */ misc_init_r (); #endif +#if defined(CONFIG_CMD_KGDB)
WATCHDOG_RESET();
puts("KGDB: ");
kgdb_init();
+#endif
the serial funcs (like puts()) already call the watchdog funcs. this call is spurious. -mike

Mike, thanks for your review. Here need your further clarifications, thanks in advance.
On Fri, Apr 9, 2010 at 5:36 AM, Mike Frysinger vapier@gentoo.org wrote:
On Thursday 08 April 2010 05:40:02 Tonny Tzeng wrote:
--- a/common/kgdb.c +++ b/common/kgdb.c @@ -220,6 +220,29 @@ hexToInt(char **ptr, int *intValue) return (numChars); }
+/* Handle the 'z' or 'Z' breakpoint remove or set packets */ +static void gdb_cmd_break(kgdb_data *kdp)
this breakpoint support isnt related to ARM KGDB at all. it should be split into a separate changeset.
Yes, these 2 breakpoint commands are not specific to ARM, but I think they are mandatory required for debugging ARM kernel space, otherwise gdb will use M command to write a SWI instruction code to the trap location.
Anyway, agree with you that it might be more clear to separate these code to other patch, as this is going to modify to common/kgdb.c which is architecture / platform independent.
if (kgdb_setjmp(error_jmp_buf) != 0) panic("kgdb: error or fault in entry init!\n");
- memset(&kd, 0, sizeof(kgdb_data));
there is no logic as to why this is needed. should probably be a different changeset too. personally, i'd also use sizeof(kd).
Since I added a (*hook_break)() to the kgdb_data structure, and in order to make it transparent to other architectures (e.g. ppc, blackfin), I believe we need to clear the entire kd here. But will separate this into other patchset as your suggestion.
--- a/include/kgdb.h +++ b/include/kgdb.h +#ifndef BREAK_INSTR_SIZE +#define BREAK_INSTR_SIZE 4 +#endif
i'm not sure this makes sense. every arch should define it themselves.
Since this include/kgdb.h file has been referenced by other architectures, the existing code may not define BREAK_INSTR_SIZE, so this is to prevent the new added code break the KGDB support for other architectures.
--- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -381,6 +382,11 @@ void start_armboot (void) /* miscellaneous platform dependent initialisations */ misc_init_r (); #endif +#if defined(CONFIG_CMD_KGDB)
- WATCHDOG_RESET();
- puts("KGDB: ");
- kgdb_init();
+#endif
the serial funcs (like puts()) already call the watchdog funcs. this call is spurious. -mike
These code are identical as ppc, m68k, and i386 architectures, and I check the serial driver of my board, it looks like some serial drivers may not call watchdog funcs, so I'd keep these changes. Any comment on this? Thanks.
Best Regards, Tonny

On Friday 09 April 2010 11:32:19 Tonny Tzeng wrote:
On Fri, Apr 9, 2010 at 5:36 AM, Mike Frysinger wrote:
On Thursday 08 April 2010 05:40:02 Tonny Tzeng wrote:
if (kgdb_setjmp(error_jmp_buf) != 0) panic("kgdb: error or fault in entry init!\n");
memset(&kd, 0, sizeof(kgdb_data));
there is no logic as to why this is needed. should probably be a different changeset too. personally, i'd also use sizeof(kd).
Since I added a (*hook_break)() to the kgdb_data structure, and in order to make it transparent to other architectures (e.g. ppc, blackfin), I believe we need to clear the entire kd here. But will separate this into other patchset as your suggestion.
hmm, kgdb_enter() is more for initializing state, not new hooks. my understanding is that either an arch always implements hook_break or it doesnt, it doesnt just do it some of the time.
that means the better way to add breakpoint support is via kgdb_stubs.c. add a new weak func to that where it always returns 0. then the remaining kgdb code can always assume the func exists and merely checks the return value.
although, now that i look at the breakpoint logic added to lib_arm/kgdb.c, it looks like most of that should be common code. the interface between common bp code and arch bp code should be the breakinst value (perhaps an unsigned char array encoding the opcode) and the size of the breakpoint array.
also, unless i missed something, your code that sets the bp is missing an icache flush.
--- a/include/kgdb.h +++ b/include/kgdb.h +#ifndef BREAK_INSTR_SIZE +#define BREAK_INSTR_SIZE 4 +#endif
i'm not sure this makes sense. every arch should define it themselves.
Since this include/kgdb.h file has been referenced by other architectures, the existing code may not define BREAK_INSTR_SIZE, so this is to prevent the new added code break the KGDB support for other architectures.
i think an undefined size should be used as a tip that the arch doesnt yet support break points
--- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -381,6 +382,11 @@ void start_armboot (void) /* miscellaneous platform dependent initialisations */ misc_init_r (); #endif +#if defined(CONFIG_CMD_KGDB)
WATCHDOG_RESET();
puts("KGDB: ");
kgdb_init();
+#endif
the serial funcs (like puts()) already call the watchdog funcs. this call is spurious.
These code are identical as ppc, m68k, and i386 architectures, and I check the serial driver of my board, it looks like some serial drivers may not call watchdog funcs, so I'd keep these changes. Any comment on this?
historical warts should be fixed, not extended. in other words, any serial driver not calling WATCHDOG_RESET() in putc()/tstc() is broken. i know this isnt documented properly, but it's still true. look at the cpu/blackfin/serial.c driver for a simple example. -mike
participants (2)
-
Mike Frysinger
-
Tonny Tzeng