* [U-Boot] [RFC PATCH v1 0/2] kgdb:add breakpoint and arm support
@ 2014-11-01 5:12 Peng Fan
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 1/2] kgdb: add breakpoint support Peng Fan
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 2/2] arm:kgdb add KGDB support for arm platform Peng Fan
0 siblings, 2 replies; 7+ messages in thread
From: Peng Fan @ 2014-11-01 5:12 UTC (permalink / raw)
To: u-boot
There were patches about this part before, such as
http://lists.denx.de/pipermail/u-boot/2010-April/069592.html
https://www.mail-archive.com/u-boot at lists.denx.de/msg13464.html
And I reference that piece of code.
I have verified this patch set on freescale mx6sxsabresd revb board.
Because the board related kgdb serial code is not fine for patch review,
I just sent out the common code for review.
I am not sure whether this patch set will break other platform's kgdb support
or not, so request for comment.
Peng Fan (2):
kgdb: add breakpoint support
arm:kgdb add KGDB support for arm platform
arch/arm/include/asm/proc-armv/ptrace.h | 2 +-
arch/arm/include/asm/signal.h | 1 +
arch/arm/lib/Makefile | 3 +
arch/arm/lib/crt0.S | 4 +
arch/arm/lib/interrupts.c | 24 +++
arch/arm/lib/kgdb/kgdb.c | 231 +++++++++++++++++++++++++++
arch/arm/lib/kgdb/setjmp.S | 20 +++
arch/arm/lib/vectors.S | 28 ++++
common/kgdb.c | 273 ++++++++++++++++++++++++++++++++
include/kgdb.h | 35 ++++
10 files changed, 620 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/include/asm/signal.h
create mode 100644 arch/arm/lib/kgdb/kgdb.c
create mode 100644 arch/arm/lib/kgdb/setjmp.S
--
1.8.4.5
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [RFC PATCH v1 1/2] kgdb: add breakpoint support
2014-11-01 5:12 [U-Boot] [RFC PATCH v1 0/2] kgdb:add breakpoint and arm support Peng Fan
@ 2014-11-01 5:12 ` Peng Fan
2014-11-04 6:46 ` Simon Glass
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 2/2] arm:kgdb add KGDB support for arm platform Peng Fan
1 sibling, 1 reply; 7+ messages in thread
From: Peng Fan @ 2014-11-01 5:12 UTC (permalink / raw)
To: u-boot
Add Z/z protocal support for breakpoint set/remove.
Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
---
common/kgdb.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/kgdb.h | 35 ++++++++
2 files changed, 308 insertions(+)
diff --git a/common/kgdb.c b/common/kgdb.c
index d357463..fd83ccd 100644
--- a/common/kgdb.c
+++ b/common/kgdb.c
@@ -92,6 +92,8 @@
#include <kgdb.h>
#include <command.h>
+#include <asm-generic/errno.h>
+
#undef KGDB_DEBUG
/*
@@ -111,6 +113,17 @@ static int longjmp_on_fault = 0;
static int kdebug = 1;
#endif
+struct kgdb_bkpt kgdb_break[KGDB_MAX_BREAKPOINTS] = {
+ [0 ... KGDB_MAX_BREAKPOINTS-1] = { .state = BP_UNDEFINED }
+};
+
+#ifdef CONFIG_ARM
+unsigned char gdb_bpt_instr[4] = {0xfe, 0xde, 0xff, 0xe7};
+#else
+#error "Please implement gdb_bpt_instr!"
+#endif
+
+
static const char hexchars[]="0123456789abcdef";
/* Convert ch from a hex digit to an int */
@@ -309,6 +322,200 @@ putpacket(unsigned char *buffer)
} while ((recv & 0x7f) != '+');
}
+int kgdb_validate_break_address(unsigned addr)
+{
+ /* Need More */
+ return 0;
+}
+
+static int probe_kernel_read(unsigned char *dst, void *src, size_t size)
+{
+ int i;
+ unsigned char *dst_ptr = dst;
+ unsigned char *src_ptr = src;
+
+ for (i = 0; i < size; i++)
+ *dst_ptr++ = *src_ptr++;
+
+ return 0;
+}
+
+static int probe_kernel_write(char *dst, void *src, size_t size)
+{
+ int i;
+ char *dst_ptr = dst;
+ char *src_ptr = src;
+
+ for (i = 0; i < size; i++)
+ *dst_ptr++ = *src_ptr++;
+
+ return 0;
+}
+
+__weak int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
+{
+ int err;
+
+ err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
+ BREAK_INSTR_SIZE);
+ if (err)
+ return err;
+
+ err = probe_kernel_write((char *)bpt->bpt_addr, gdb_bpt_instr,
+ BREAK_INSTR_SIZE);
+
+ return err;
+}
+
+/*
+ * Set the breakpoints whose state is BP_SET to BP_ACTIVE
+ */
+int kgdb_active_sw_breakpoints(void)
+{
+ int err;
+ int ret = 0;
+ int i;
+
+ for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
+ if (kgdb_break[i].state != BP_SET)
+ continue;
+
+ err = kgdb_arch_set_breakpoint(&kgdb_break[i]);
+ if (err) {
+ ret = err;
+ printf("KGDB: BP install failed: %lx\n",
+ kgdb_break[i].bpt_addr);
+ continue;
+ }
+
+ kgdb_break[i].state = BP_ACTIVE;
+
+ /*
+ * kgdb_arch_set_breakpoint touched dcache and memory.
+ * cache should be flushed to let icache can see the updated
+ * inst.
+ */
+ /* flush work is done in do_exit */
+ kgdb_flush_cache_all();
+ }
+
+ return ret;
+}
+
+/*
+ * Set state from BP_SET to BP_REMOVED
+ */
+int kgdb_remove_sw_breakpoint(unsigned int addr)
+{
+ int i;
+
+ for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
+ if ((kgdb_break[i].state == BP_SET) &&
+ (kgdb_break[i].bpt_addr == addr)) {
+ kgdb_break[i].state = BP_REMOVED;
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+int kgdb_set_sw_breakpoint(unsigned int addr)
+{
+ int err = kgdb_validate_break_address(addr);
+ int breakno = -1;
+ int i;
+
+ if (err)
+ return err;
+
+ for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
+ if ((kgdb_break[i].state == BP_SET) &&
+ (kgdb_break[i].bpt_addr == addr))
+ return -EEXIST;
+ }
+
+ for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
+ /* removed or unused, use it */
+ if ((kgdb_break[i].state == BP_REMOVED) ||
+ (kgdb_break[i].state == BP_UNDEFINED)) {
+ breakno = i;
+ break;
+ }
+ }
+
+ if (breakno == -1)
+ return -E2BIG;
+
+ kgdb_break[breakno].state = BP_SET;
+ kgdb_break[breakno].type = BP_BREAKPOINT;
+ kgdb_break[breakno].bpt_addr = addr;
+
+ return 0;
+}
+
+__weak int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
+{
+ return probe_kernel_write((char *)bpt->bpt_addr,
+ (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
+}
+
+/*
+ * set breakpoints whose state is BP_ACTIVE to BP_SET
+ */
+int kgdb_deactivate_sw_breakpoints(void)
+{
+ int err;
+ int ret = 0;
+ int i;
+
+ for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
+ if (kgdb_break[i].state != BP_ACTIVE)
+ continue;
+
+ err = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
+ if (err) {
+ printf("KGDB: BP remove failed: %lx\n",
+ kgdb_break[i].bpt_addr);
+ ret = err;
+ }
+
+ kgdb_break[i].state = BP_SET;
+
+ /*
+ * kgdb_arch_remove_breakpoint touched dcache and memory.
+ * cache should be flushed to let icache can see the updated
+ * inst.
+ */
+ kgdb_flush_cache_all();
+ }
+
+ return ret;
+}
+
+static int kgdb_remove_all_break(void)
+{
+ int err;
+ int i;
+
+ /* Clear memory breakpoints. */
+ for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
+ if (kgdb_break[i].state != BP_ACTIVE)
+ goto setundefined;
+ err = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
+ if (err)
+ printf("KGDB: breakpoint remove failed: %lx\n",
+ kgdb_break[i].bpt_addr);
+setundefined:
+ kgdb_break[i].state = BP_UNDEFINED;
+ }
+
+ /* clear hardware breakpoints. */
+ /* ToDo in future. */
+
+ return 0;
+}
+
/*
* This function does all command processing for interfacing to gdb.
*/
@@ -318,6 +525,7 @@ handle_exception (struct pt_regs *regs)
int addr;
int length;
char *ptr;
+ char *bpt_type;
kgdb_data kd;
int i;
@@ -376,6 +584,12 @@ handle_exception (struct pt_regs *regs)
putpacket((unsigned char *)&remcomOutBuffer);
+ /*
+ * Each time trigger a kgdb break, first deactive all active
+ * breakpoints.
+ */
+ kgdb_deactivate_sw_breakpoints();
+
while (1) {
volatile int errnum;
@@ -394,6 +608,8 @@ handle_exception (struct pt_regs *regs)
if (errnum == 0) switch (remcomInBuffer[0]) {
case '?': /* report most recent signal */
+ kgdb_remove_all_break();
+
remcomOutBuffer[0] = 'S';
remcomOutBuffer[1] = hexchars[kd.sigval >> 4];
remcomOutBuffer[2] = hexchars[kd.sigval & 0xf];
@@ -465,6 +681,8 @@ handle_exception (struct pt_regs *regs)
kd.extype |= KGDBEXIT_WITHADDR;
}
+ kgdb_active_sw_breakpoints();
+
goto doexit;
case 'S': /* SSS single step with signal SS */
@@ -479,6 +697,8 @@ handle_exception (struct pt_regs *regs)
kd.extype |= KGDBEXIT_WITHADDR;
}
+ kgdb_active_sw_breakpoints();
+
doexit:
/* Need to flush the instruction cache here, as we may have deposited a
* breakpoint, and the icache probably has no way of knowing that a data ref to
@@ -506,6 +726,59 @@ handle_exception (struct pt_regs *regs)
kgdb_error(KGDBERR_BADPARAMS);
}
break;
+ case 'z':
+ /*
+ * Break point remove
+ * packet: zt,addr,length
+ */
+ case 'Z':
+ /*
+ * Break point set
+ * packet: Zt,addr,length
+ *
+ * t is type: `0' - software breakpoint,
+ * `1' - hardware breakpoint, `2' - write watchpoint,
+ * `3' - read watchpoint, `4' - access watchpoint;
+ * addr is address; length is in bytes. For a software
+ * breakpoint, length specifies the size of the
+ * instruction to be patched. For hardware breakpoints
+ * and watchpoints length specifies the memory region
+ * to be monitored. To avoid potential problems with
+ * duplicate packets, the operations should be
+ * implemented in an idempotent way.
+ */
+ bpt_type = &remcomInBuffer[1];
+ ptr = &remcomInBuffer[2];
+
+ if (*(ptr++) != ',') {
+ errnum = -EINVAL;
+ break;
+ }
+
+ if (!hexToInt(&ptr, &addr)) {
+ errnum = -EINVAL;
+ break;
+ }
+
+ if (*ptr++ != ',') {
+ errnum = -EINVAL;
+ break;
+ }
+
+ /* only software breakpoint is implemented */
+ if ((remcomInBuffer[0] == 'Z') && (*bpt_type == '0')) {
+ errnum = kgdb_set_sw_breakpoint(addr);
+ } else if ((remcomInBuffer[0] == 'z') &&
+ (*bpt_type == '0')) {
+ errnum = kgdb_remove_sw_breakpoint(addr);
+ } else {
+ /* Unsupported */
+ errnum = -EINVAL;
+ }
+
+ if (errnum == 0)
+ strcpy(remcomOutBuffer, "OK");
+ break;
} /* switch */
if (errnum != 0)
diff --git a/include/kgdb.h b/include/kgdb.h
index b6ba742..f9152b5 100644
--- a/include/kgdb.h
+++ b/include/kgdb.h
@@ -20,6 +20,12 @@
#define KGDBEXIT_WITHADDR 0x100
+#if defined(CONFIG_ARM)
+#define BREAK_INSTR_SIZE 4
+#else
+#error "BREAK_INSTR_SIZE not set"
+#endif
+
typedef
struct {
int num;
@@ -38,6 +44,29 @@ typedef
}
kgdb_data;
+enum kgdb_bptype {
+ BP_BREAKPOINT = 0,
+ BP_HARDWARE_BREAKPOINT,
+ BP_WRITE_WATCHPOINT,
+ BP_READ_WATCHPOINT,
+ BP_ACCESS_WATCHPOINT,
+ BP_POKE_BREAKPOINT,
+};
+
+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;
+};
+
/* these functions are provided by the generic kgdb support */
extern void kgdb_init(void);
extern void kgdb_error(int);
@@ -67,4 +96,10 @@ extern void kgdb_interruptible(int);
/* this is referenced in the trap handler for the platform */
extern int (*debugger_exception_handler)(struct pt_regs *);
+int kgdb_set_sw_break(unsigned int addr);
+int kgdb_remove_sw_break(unsigned int addr);
+int kgdb_validate_break_address(unsigned int addr);
+
+#define KGDB_MAX_BREAKPOINTS 32
+
#endif /* __KGDB_H__ */
--
1.8.4.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [RFC PATCH v1 2/2] arm:kgdb add KGDB support for arm platform
2014-11-01 5:12 [U-Boot] [RFC PATCH v1 0/2] kgdb:add breakpoint and arm support Peng Fan
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 1/2] kgdb: add breakpoint support Peng Fan
@ 2014-11-01 5:12 ` Peng Fan
2014-11-04 6:58 ` Simon Glass
1 sibling, 1 reply; 7+ messages in thread
From: Peng Fan @ 2014-11-01 5:12 UTC (permalink / raw)
To: u-boot
The register save/restore:
Use get_bad_stack and bad_save_user_regs to save regs.
Introduce und_restore_regs to restore the previous regs before
trigger a breakpoint.
Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
---
arch/arm/include/asm/proc-armv/ptrace.h | 2 +-
arch/arm/include/asm/signal.h | 1 +
arch/arm/lib/Makefile | 3 +
arch/arm/lib/crt0.S | 4 +
arch/arm/lib/interrupts.c | 24 ++++
arch/arm/lib/kgdb/kgdb.c | 231 ++++++++++++++++++++++++++++++++
arch/arm/lib/kgdb/setjmp.S | 20 +++
arch/arm/lib/vectors.S | 28 ++++
8 files changed, 312 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/proc-armv/ptrace.h b/arch/arm/include/asm/proc-armv/ptrace.h
index 71df5a9..33fe587 100644
--- a/arch/arm/include/asm/proc-armv/ptrace.h
+++ b/arch/arm/include/asm/proc-armv/ptrace.h
@@ -58,7 +58,7 @@ struct pt_regs {
stack during a system call. */
struct pt_regs {
- long uregs[18];
+ unsigned long uregs[18];
};
#define ARM_cpsr uregs[16]
diff --git a/arch/arm/include/asm/signal.h b/arch/arm/include/asm/signal.h
new file mode 100644
index 0000000..7b1573c
--- /dev/null
+++ b/arch/arm/include/asm/signal.h
@@ -0,0 +1 @@
+#include <asm-generic/signal.h>
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 1ef2400..c543563 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -52,3 +52,6 @@ endif
ifneq (,$(findstring -mabi=aapcs-linux,$(PLATFORM_CPPFLAGS)))
extra-y += eabi_compat.o
endif
+
+obj-$(CONFIG_CMD_KGDB) += kgdb/setjmp.o
+obj-$(CONFIG_CMD_KGDB) += kgdb/kgdb.o
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 29cdad0..d96e70b 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -99,9 +99,13 @@ clr_gd:
sub r9, r9, #GD_SIZE /* new GD is below bd */
adr lr, here
+#ifndef CONFIG_CMD_KGDB
ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */
add lr, lr, r0
ldr r0, [r9, #GD_RELOCADDR] /* r0 = gd->relocaddr */
+#else
+ ldr r0, =__image_copy_start
+#endif
b relocate_code
here:
diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c
index 4dacfd9..d16bd58 100644
--- a/arch/arm/lib/interrupts.c
+++ b/arch/arm/lib/interrupts.c
@@ -22,6 +22,7 @@
#include <common.h>
#include <asm/proc-armv/ptrace.h>
#include <asm/u-boot-arm.h>
+#include <kgdb.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -160,9 +161,32 @@ void show_regs (struct pt_regs *regs)
void do_undefined_instruction (struct pt_regs *pt_regs)
{
+#if defined(CONFIG_CMD_KGDB)
+ unsigned long *tmp_pc = NULL;
+
+ pt_regs->ARM_pc -= 4;
+ tmp_pc = (unsigned long *)pt_regs->ARM_pc;
+
+ if (*tmp_pc == 0xe7ffdeff) {
+ pt_regs->ARM_pc += 4;
+ if (debugger_exception_handler &&
+ debugger_exception_handler(pt_regs)) {
+ return;
+ }
+ } else if (*tmp_pc == 0xe7ffdefe) {
+ if (debugger_exception_handler &&
+ debugger_exception_handler(pt_regs)) {
+ return;
+ }
+ } else {
+ printf("DCache/ICACHE May need flush\n");
+ return;
+ }
+#else
printf ("undefined instruction\n");
show_regs (pt_regs);
bad_mode ();
+#endif
}
void do_software_interrupt (struct pt_regs *pt_regs)
diff --git a/arch/arm/lib/kgdb/kgdb.c b/arch/arm/lib/kgdb/kgdb.c
new file mode 100644
index 0000000..6b2e542
--- /dev/null
+++ b/arch/arm/lib/kgdb/kgdb.c
@@ -0,0 +1,231 @@
+#include <common.h>
+#include <command.h>
+#include <kgdb.h>
+#include <asm/signal.h>
+#include <asm/processor.h>
+
+#define KGDB_ARM_GP_REGS 16
+#define KGDB_ARM_FP_REGS 8
+#define KGDB_ARM_EXTRA_REGS 2
+#define KGDB_ARM_MAX_REGS (KGDB_ARM_GP_REGS +\
+ (KGDB_ARM_FP_REGS * 3) +\
+ KGDB_ARM_EXTRA_REGS)
+#define KGDB_NUMREGBYTES (KGDB_ARM_MAX_REGS << 2)
+
+enum arm_regs {
+ KGDB_ARM_R0,
+ KGDB_ARM_R1,
+ KGDB_ARM_R2,
+ KGDB_ARM_R3,
+ KGDB_ARM_R4,
+ KGDB_ARM_R5,
+ KGDB_ARM_R6,
+ KGDB_ARM_R7,
+ KGDB_ARM_R8,
+ KGDB_ARM_R9,
+ KGDB_ARM_R10,
+ KGDB_ARM_FP,
+ KGDB_ARM_IP,
+ KGDB_ARM_SP,
+ KGDB_ARM_LR,
+ KGDB_ARM_PC,
+ KGDB_ARM_CPSR = KGDB_ARM_MAX_REGS - 1
+};
+
+void kgdb_enter(struct pt_regs *regs, kgdb_data *kdp)
+{
+ disable_interrupts();
+
+ kdp->sigval = kgdb_trap(regs);
+ kdp->nregs = 2;
+ kdp->regs[0].num = KGDB_ARM_PC;
+ kdp->regs[0].val = regs->ARM_pc;
+
+ kdp->regs[1].num = KGDB_ARM_SP;
+ kdp->regs[1].val = regs->ARM_sp;
+
+ return;
+}
+
+void kgdb_exit(struct pt_regs *regs, kgdb_data *kdp)
+{
+ /* Mark Not sure ??? */
+
+ if (kdp->extype & KGDBEXIT_WITHADDR) {
+ regs->ARM_pc = kdp->exaddr;
+ printf("KGDBEXIT_WITHADDR 0x%lx\n", regs->ARM_pc);
+ }
+
+ switch (kdp->extype & KGDBEXIT_TYPEMASK) {
+ case KGDBEXIT_KILL:
+ printf("KGDBEXIT_KILL:\n");
+ break;
+ case KGDBEXIT_CONTINUE:
+ printf("KGDBEXIT_CONTINUE\n");
+ break;
+ case KGDBEXIT_SINGLE:
+ printf("KGDBEXIT_SINGLE\n");
+ break;
+ default:
+ printf("KGDBEXIT : %d\n", kdp->extype);
+ }
+
+ enable_interrupts();
+}
+
+int kgdb_trap(struct pt_regs *regs)
+{
+ /* Mark Not sure ??? */
+ return SIGTRAP;
+}
+
+int kgdb_getregs(struct pt_regs *regs, char *buf, int max)
+{
+ unsigned long *gdb_regs = (unsigned long *)buf;
+
+ if (max < KGDB_NUMREGBYTES)
+ kgdb_error(KGDBERR_NOSPACE);
+
+ if ((unsigned long)gdb_regs & 3)
+ kgdb_error(KGDBERR_ALIGNFAULT);
+
+ gdb_regs[KGDB_ARM_R0] = regs->ARM_r0;
+ gdb_regs[KGDB_ARM_R1] = regs->ARM_r1;
+ gdb_regs[KGDB_ARM_R2] = regs->ARM_r2;
+ gdb_regs[KGDB_ARM_R3] = regs->ARM_r3;
+ gdb_regs[KGDB_ARM_R4] = regs->ARM_r4;
+ gdb_regs[KGDB_ARM_R5] = regs->ARM_r5;
+ gdb_regs[KGDB_ARM_R6] = regs->ARM_r6;
+ gdb_regs[KGDB_ARM_R7] = regs->ARM_r7;
+ gdb_regs[KGDB_ARM_R8] = regs->ARM_r8;
+ gdb_regs[KGDB_ARM_R9] = regs->ARM_r9;
+ gdb_regs[KGDB_ARM_R10] = regs->ARM_r10;
+ gdb_regs[KGDB_ARM_IP] = regs->ARM_ip;
+ gdb_regs[KGDB_ARM_FP] = regs->ARM_fp;
+ /* set sp */
+ gdb_regs[KGDB_ARM_SP] = (unsigned long)regs;
+ gdb_regs[KGDB_ARM_LR] = regs->ARM_lr;
+ gdb_regs[KGDB_ARM_PC] = regs->ARM_pc;
+ gdb_regs[KGDB_ARM_CPSR] = regs->ARM_cpsr;
+
+ return KGDB_NUMREGBYTES;
+}
+
+void kgdb_putreg(struct pt_regs *regs, int regno, char *buf, int length)
+{
+ unsigned long *ptr = (unsigned long *)buf;
+
+ if (regno < 0 || regno >= 16)
+ kgdb_error(KGDBERR_BADPARAMS);
+
+ if (length < 4)
+ kgdb_error(KGDBERR_NOSPACE);
+
+ if ((unsigned long)ptr & 3)
+ kgdb_error(KGDBERR_ALIGNFAULT);
+
+ switch (regno) {
+ case KGDB_ARM_R0:
+ regs->ARM_r0 = *ptr;
+ break;
+ case KGDB_ARM_R1:
+ regs->ARM_r1 = *ptr;
+ break;
+ case KGDB_ARM_R2:
+ regs->ARM_r2 = *ptr;
+ break;
+ case KGDB_ARM_R3:
+ regs->ARM_r3 = *ptr;
+ break;
+ case KGDB_ARM_R4:
+ regs->ARM_r4 = *ptr;
+ break;
+ case KGDB_ARM_R5:
+ regs->ARM_r5 = *ptr;
+ break;
+ case KGDB_ARM_R6:
+ regs->ARM_r6 = *ptr;
+ break;
+ case KGDB_ARM_R7:
+ regs->ARM_r7 = *ptr;
+ break;
+ case KGDB_ARM_R8:
+ regs->ARM_r8 = *ptr;
+ break;
+ case KGDB_ARM_R9:
+ regs->ARM_r9 = *ptr;
+ break;
+ case KGDB_ARM_R10:
+ regs->ARM_r10 = *ptr;
+ break;
+ case KGDB_ARM_FP:
+ regs->ARM_fp = *ptr;
+ break;
+ case KGDB_ARM_IP:
+ regs->ARM_ip = *ptr;
+ break;
+ case KGDB_ARM_SP:
+ regs->ARM_sp = *ptr;
+ break;
+ case KGDB_ARM_LR:
+ regs->ARM_lr = *ptr;
+ break;
+ case KGDB_ARM_PC:
+ regs->ARM_pc = *ptr;
+ break;
+ case KGDB_ARM_CPSR:
+ regs->ARM_cpsr = *ptr;
+ break;
+ default:
+ kgdb_error(KGDBERR_BADPARAMS);
+ break;
+ }
+}
+
+void kgdb_putregs(struct pt_regs *regs, char *buf, int length)
+{
+ unsigned long *kgdb_regs = (unsigned long *)buf;
+
+ if (length != KGDB_ARM_MAX_REGS)
+ kgdb_error(KGDBERR_NOSPACE);
+
+ if ((unsigned long)kgdb_regs & 3)
+ kgdb_error(KGDBERR_ALIGNFAULT);
+
+ regs->ARM_r0 = kgdb_regs[KGDB_ARM_R0];
+ regs->ARM_r1 = kgdb_regs[KGDB_ARM_R1];
+ regs->ARM_r2 = kgdb_regs[KGDB_ARM_R2];
+ regs->ARM_r3 = kgdb_regs[KGDB_ARM_R3];
+ regs->ARM_r4 = kgdb_regs[KGDB_ARM_R4];
+ regs->ARM_r5 = kgdb_regs[KGDB_ARM_R5];
+ regs->ARM_r6 = kgdb_regs[KGDB_ARM_R6];
+ regs->ARM_r7 = kgdb_regs[KGDB_ARM_R7];
+ regs->ARM_r8 = kgdb_regs[KGDB_ARM_R8];
+ regs->ARM_r9 = kgdb_regs[KGDB_ARM_R9];
+ regs->ARM_r10 = kgdb_regs[KGDB_ARM_R10];
+ regs->ARM_ip = kgdb_regs[KGDB_ARM_IP];
+ regs->ARM_fp = kgdb_regs[KGDB_ARM_FP];
+ regs->ARM_sp = kgdb_regs[KGDB_ARM_SP];
+ regs->ARM_lr = kgdb_regs[KGDB_ARM_LR];
+ regs->ARM_pc = kgdb_regs[KGDB_ARM_PC];
+ regs->ARM_cpsr = kgdb_regs[KGDB_ARM_CPSR];
+}
+
+void kgdb_flush_cache_all(void)
+{
+ invalidate_icache_all();
+ flush_dcache_all();
+}
+
+/*
+ * 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 * const argv[])
+{
+ asm(".word 0xe7ffdeff");
+}
diff --git a/arch/arm/lib/kgdb/setjmp.S b/arch/arm/lib/kgdb/setjmp.S
new file mode 100644
index 0000000..e17f959
--- /dev/null
+++ b/arch/arm/lib/kgdb/setjmp.S
@@ -0,0 +1,20 @@
+ .text
+ .balign 4
+ .global kgdb_setjmp
+ .type kgdb_setjmp, #function
+kgdb_setjmp:
+ stmia r0, {r4, r5, r6, r7, r8, r9, r10, fp, sp, lr}
+ mov r0, #0
+ mov pc, lr
+ .size kgdb_setjmp,.-kgdb_setjmp
+
+ .text
+ .balign 4
+ .global kgdb_longjmp
+ .type kgdb_longjmp, #function
+kgdb_longjmp:
+ ldmia r0, {r4, r5, r6, r7, r8, r9, r10, fp, sp, lr}
+ movs r0, r1
+ moveq r0, #1
+ mov pc, lr
+ .size kgdb_longjmp,.-kgdb_longjmp
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
index 49238ed..8abad3d 100644
--- a/arch/arm/lib/vectors.S
+++ b/arch/arm/lib/vectors.S
@@ -221,15 +221,43 @@ FIQ_STACK_START:
ldr sp, FIQ_STACK_START
.endm
+ .macro get_und_stack
+ get_bad_stack
+ .endm
+
+ .macro und_save_regs
+ bad_save_user_regs
+ .endm
+
+ /* In svc mode */
+ .macro und_restore_regs
+ add r7, sp, #S_PC
+ ldr r6, [r7, #4] @load spsr into r6
+ msr cpsr, r6 @use und spsr to overwrite svc cpsr
+ ldmdb r7, {r1 - r2} @load sp lr into r1 r2
+ mov lr, r2
+ ldmia sp, {r0 - r12}
+ mov r0, r0
+ add sp, sp, #S_FRAME_SIZE
+ ldr pc, [sp, #-12]
+ .endm
+
/*
* exception handlers
*/
.align 5
undefined_instruction:
+#if defined(CONFIG_CMD_KGDB)
+ get_und_stack
+ und_save_regs
+ bl do_undefined_instruction
+ und_restore_regs
+#else
get_bad_stack
bad_save_user_regs
bl do_undefined_instruction
+#endif
.align 5
software_interrupt:
--
1.8.4.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [RFC PATCH v1 1/2] kgdb: add breakpoint support
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 1/2] kgdb: add breakpoint support Peng Fan
@ 2014-11-04 6:46 ` Simon Glass
2014-11-04 12:23 ` Peng Fan
0 siblings, 1 reply; 7+ messages in thread
From: Simon Glass @ 2014-11-04 6:46 UTC (permalink / raw)
To: u-boot
Hi Peng,
On 31 October 2014 23:12, Peng Fan <Peng.Fan@freescale.com> wrote:
> Add Z/z protocal support for breakpoint set/remove.
>
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
This looks good to me - I just have a few bits below.
> ---
> common/kgdb.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/kgdb.h | 35 ++++++++
> 2 files changed, 308 insertions(+)
>
> diff --git a/common/kgdb.c b/common/kgdb.c
> index d357463..fd83ccd 100644
> --- a/common/kgdb.c
> +++ b/common/kgdb.c
> @@ -92,6 +92,8 @@
> #include <kgdb.h>
> #include <command.h>
>
> +#include <asm-generic/errno.h>
#include <errno.h> would do
> +
> #undef KGDB_DEBUG
Where is this used?
>
> /*
> @@ -111,6 +113,17 @@ static int longjmp_on_fault = 0;
> static int kdebug = 1;
> #endif
>
> +struct kgdb_bkpt kgdb_break[KGDB_MAX_BREAKPOINTS] = {
> + [0 ... KGDB_MAX_BREAKPOINTS-1] = { .state = BP_UNDEFINED }
> +};
> +
> +#ifdef CONFIG_ARM
> +unsigned char gdb_bpt_instr[4] = {0xfe, 0xde, 0xff, 0xe7};
> +#else
> +#error "Please implement gdb_bpt_instr!"
> +#endif
> +
> +
> static const char hexchars[]="0123456789abcdef";
>
> /* Convert ch from a hex digit to an int */
> @@ -309,6 +322,200 @@ putpacket(unsigned char *buffer)
> } while ((recv & 0x7f) != '+');
> }
>
> +int kgdb_validate_break_address(unsigned addr)
> +{
> + /* Need More */
?
> + return 0;
> +}
> +
> +static int probe_kernel_read(unsigned char *dst, void *src, size_t size)
> +{
> + int i;
> + unsigned char *dst_ptr = dst;
> + unsigned char *src_ptr = src;
> +
> + for (i = 0; i < size; i++)
> + *dst_ptr++ = *src_ptr++;
> +
> + return 0;
> +}
> +
> +static int probe_kernel_write(char *dst, void *src, size_t size)
These two above are strange function names - why 'kernel' - what does
it mean in this context? Also could you must use memcpy(), either in
the functions or instead of them?
> +{
> + int i;
> + char *dst_ptr = dst;
> + char *src_ptr = src;
> +
> + for (i = 0; i < size; i++)
> + *dst_ptr++ = *src_ptr++;
> +
> + return 0;
> +}
> +
> +__weak int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
> +{
> + int err;
> +
> + err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
> + BREAK_INSTR_SIZE);
> + if (err)
> + return err;
> +
> + err = probe_kernel_write((char *)bpt->bpt_addr, gdb_bpt_instr,
> + BREAK_INSTR_SIZE);
> +
> + return err;
> +}
> +
> +/*
> + * Set the breakpoints whose state is BP_SET to BP_ACTIVE
> + */
> +int kgdb_active_sw_breakpoints(void)
> +{
> + int err;
> + int ret = 0;
> + int i;
> +
> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
> + if (kgdb_break[i].state != BP_SET)
> + continue;
> +
> + err = kgdb_arch_set_breakpoint(&kgdb_break[i]);
> + if (err) {
> + ret = err;
> + printf("KGDB: BP install failed: %lx\n",
> + kgdb_break[i].bpt_addr);
> + continue;
> + }
> +
> + kgdb_break[i].state = BP_ACTIVE;
> +
> + /*
> + * kgdb_arch_set_breakpoint touched dcache and memory.
> + * cache should be flushed to let icache can see the updated
> + * inst.
instruction
> + */
> + /* flush work is done in do_exit */
> + kgdb_flush_cache_all();
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Set state from BP_SET to BP_REMOVED
> + */
> +int kgdb_remove_sw_breakpoint(unsigned int addr)
> +{
> + int i;
> +
> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
> + if ((kgdb_break[i].state == BP_SET) &&
> + (kgdb_break[i].bpt_addr == addr)) {
> + kgdb_break[i].state = BP_REMOVED;
> + return 0;
> + }
> + }
> +
> + return -ENOENT;
> +}
> +
> +int kgdb_set_sw_breakpoint(unsigned int addr)
> +{
> + int err = kgdb_validate_break_address(addr);
> + int breakno = -1;
> + int i;
> +
> + if (err)
> + return err;
> +
> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
> + if ((kgdb_break[i].state == BP_SET) &&
> + (kgdb_break[i].bpt_addr == addr))
> + return -EEXIST;
> + }
> +
> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
> + /* removed or unused, use it */
> + if ((kgdb_break[i].state == BP_REMOVED) ||
> + (kgdb_break[i].state == BP_UNDEFINED)) {
> + breakno = i;
> + break;
> + }
> + }
> +
> + if (breakno == -1)
> + return -E2BIG;
> +
> + kgdb_break[breakno].state = BP_SET;
> + kgdb_break[breakno].type = BP_BREAKPOINT;
> + kgdb_break[breakno].bpt_addr = addr;
> +
> + return 0;
> +}
> +
> +__weak int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
> +{
> + return probe_kernel_write((char *)bpt->bpt_addr,
> + (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
> +}
> +
> +/*
> + * set breakpoints whose state is BP_ACTIVE to BP_SET
> + */
> +int kgdb_deactivate_sw_breakpoints(void)
> +{
> + int err;
> + int ret = 0;
> + int i;
> +
> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
> + if (kgdb_break[i].state != BP_ACTIVE)
> + continue;
> +
> + err = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
> + if (err) {
> + printf("KGDB: BP remove failed: %lx\n",
> + kgdb_break[i].bpt_addr);
> + ret = err;
> + }
> +
> + kgdb_break[i].state = BP_SET;
> +
> + /*
> + * kgdb_arch_remove_breakpoint touched dcache and memory.
> + * cache should be flushed to let icache can see the updated
> + * inst.
> + */
> + kgdb_flush_cache_all();
> + }
> +
> + return ret;
> +}
> +
> +static int kgdb_remove_all_break(void)
kgdb_remove_all_breakpoints() to be consistent?
> +{
> + int err;
> + int i;
> +
> + /* Clear memory breakpoints. */
> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
> + if (kgdb_break[i].state != BP_ACTIVE)
> + goto setundefined;
> + err = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
> + if (err)
> + printf("KGDB: breakpoint remove failed: %lx\n",
> + kgdb_break[i].bpt_addr);
> +setundefined:
> + kgdb_break[i].state = BP_UNDEFINED;
> + }
> +
> + /* clear hardware breakpoints. */
> + /* ToDo in future. */
/* TODO: clear hardware breakpoints. */
> +
> + return 0;
> +}
> +
> /*
> * This function does all command processing for interfacing to gdb.
> */
> @@ -318,6 +525,7 @@ handle_exception (struct pt_regs *regs)
> int addr;
> int length;
> char *ptr;
> + char *bpt_type;
> kgdb_data kd;
> int i;
>
> @@ -376,6 +584,12 @@ handle_exception (struct pt_regs *regs)
>
> putpacket((unsigned char *)&remcomOutBuffer);
>
> + /*
> + * Each time trigger a kgdb break, first deactive all active
> + * breakpoints.
> + */
> + kgdb_deactivate_sw_breakpoints();
> +
> while (1) {
> volatile int errnum;
>
> @@ -394,6 +608,8 @@ handle_exception (struct pt_regs *regs)
> if (errnum == 0) switch (remcomInBuffer[0]) {
>
> case '?': /* report most recent signal */
> + kgdb_remove_all_break();
> +
> remcomOutBuffer[0] = 'S';
> remcomOutBuffer[1] = hexchars[kd.sigval >> 4];
> remcomOutBuffer[2] = hexchars[kd.sigval & 0xf];
> @@ -465,6 +681,8 @@ handle_exception (struct pt_regs *regs)
> kd.extype |= KGDBEXIT_WITHADDR;
> }
>
> + kgdb_active_sw_breakpoints();
> +
> goto doexit;
>
> case 'S': /* SSS single step with signal SS */
> @@ -479,6 +697,8 @@ handle_exception (struct pt_regs *regs)
> kd.extype |= KGDBEXIT_WITHADDR;
> }
>
> + kgdb_active_sw_breakpoints();
> +
> doexit:
> /* Need to flush the instruction cache here, as we may have deposited a
> * breakpoint, and the icache probably has no way of knowing that a data ref to
> @@ -506,6 +726,59 @@ handle_exception (struct pt_regs *regs)
> kgdb_error(KGDBERR_BADPARAMS);
> }
> break;
> + case 'z':
> + /*
> + * Break point remove
> + * packet: zt,addr,length
> + */
> + case 'Z':
> + /*
> + * Break point set
> + * packet: Zt,addr,length
> + *
> + * t is type: `0' - software breakpoint,
> + * `1' - hardware breakpoint, `2' - write watchpoint,
> + * `3' - read watchpoint, `4' - access watchpoint;
> + * addr is address; length is in bytes. For a software
> + * breakpoint, length specifies the size of the
> + * instruction to be patched. For hardware breakpoints
> + * and watchpoints length specifies the memory region
> + * to be monitored. To avoid potential problems with
> + * duplicate packets, the operations should be
> + * implemented in an idempotent way.
> + */
> + bpt_type = &remcomInBuffer[1];
> + ptr = &remcomInBuffer[2];
> +
> + if (*(ptr++) != ',') {
> + errnum = -EINVAL;
> + break;
> + }
> +
> + if (!hexToInt(&ptr, &addr)) {
> + errnum = -EINVAL;
> + break;
> + }
> +
> + if (*ptr++ != ',') {
> + errnum = -EINVAL;
> + break;
> + }
> +
> + /* only software breakpoint is implemented */
> + if ((remcomInBuffer[0] == 'Z') && (*bpt_type == '0')) {
> + errnum = kgdb_set_sw_breakpoint(addr);
> + } else if ((remcomInBuffer[0] == 'z') &&
> + (*bpt_type == '0')) {
> + errnum = kgdb_remove_sw_breakpoint(addr);
> + } else {
> + /* Unsupported */
> + errnum = -EINVAL;
> + }
> +
> + if (errnum == 0)
> + strcpy(remcomOutBuffer, "OK");
> + break;
> } /* switch */
>
> if (errnum != 0)
> diff --git a/include/kgdb.h b/include/kgdb.h
> index b6ba742..f9152b5 100644
> --- a/include/kgdb.h
> +++ b/include/kgdb.h
> @@ -20,6 +20,12 @@
>
> #define KGDBEXIT_WITHADDR 0x100
>
> +#if defined(CONFIG_ARM)
> +#define BREAK_INSTR_SIZE 4
> +#else
> +#error "BREAK_INSTR_SIZE not set"
> +#endif
> +
> typedef
> struct {
> int num;
> @@ -38,6 +44,29 @@ typedef
> }
> kgdb_data;
>
> +enum kgdb_bptype {
> + BP_BREAKPOINT = 0,
> + BP_HARDWARE_BREAKPOINT,
> + BP_WRITE_WATCHPOINT,
> + BP_READ_WATCHPOINT,
> + BP_ACCESS_WATCHPOINT,
> + BP_POKE_BREAKPOINT,
> +};
> +
> +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;
> +};
> +
> /* these functions are provided by the generic kgdb support */
> extern void kgdb_init(void);
> extern void kgdb_error(int);
> @@ -67,4 +96,10 @@ extern void kgdb_interruptible(int);
> /* this is referenced in the trap handler for the platform */
> extern int (*debugger_exception_handler)(struct pt_regs *);
>
> +int kgdb_set_sw_break(unsigned int addr);
> +int kgdb_remove_sw_break(unsigned int addr);
> +int kgdb_validate_break_address(unsigned int addr);
> +
> +#define KGDB_MAX_BREAKPOINTS 32
> +
> #endif /* __KGDB_H__ */
> --
> 1.8.4.5
>
Regards,
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [RFC PATCH v1 2/2] arm:kgdb add KGDB support for arm platform
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 2/2] arm:kgdb add KGDB support for arm platform Peng Fan
@ 2014-11-04 6:58 ` Simon Glass
2014-11-04 13:09 ` Peng Fan
0 siblings, 1 reply; 7+ messages in thread
From: Simon Glass @ 2014-11-04 6:58 UTC (permalink / raw)
To: u-boot
HI Peng,
On 31 October 2014 23:12, Peng Fan <Peng.Fan@freescale.com> wrote:
> The register save/restore:
> Use get_bad_stack and bad_save_user_regs to save regs.
> Introduce und_restore_regs to restore the previous regs before
> trigger a breakpoint.
>
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
Looks good so far as I understand it. A few nits below and maybe you
can integrate better with the ptrace register numbering.
> ---
> arch/arm/include/asm/proc-armv/ptrace.h | 2 +-
> arch/arm/include/asm/signal.h | 1 +
> arch/arm/lib/Makefile | 3 +
> arch/arm/lib/crt0.S | 4 +
> arch/arm/lib/interrupts.c | 24 ++++
> arch/arm/lib/kgdb/kgdb.c | 231 ++++++++++++++++++++++++++++++++
> arch/arm/lib/kgdb/setjmp.S | 20 +++
> arch/arm/lib/vectors.S | 28 ++++
> 8 files changed, 312 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/proc-armv/ptrace.h b/arch/arm/include/asm/proc-armv/ptrace.h
> index 71df5a9..33fe587 100644
> --- a/arch/arm/include/asm/proc-armv/ptrace.h
> +++ b/arch/arm/include/asm/proc-armv/ptrace.h
> @@ -58,7 +58,7 @@ struct pt_regs {
> stack during a system call. */
>
> struct pt_regs {
> - long uregs[18];
> + unsigned long uregs[18];
> };
>
> #define ARM_cpsr uregs[16]
> diff --git a/arch/arm/include/asm/signal.h b/arch/arm/include/asm/signal.h
> new file mode 100644
> index 0000000..7b1573c
> --- /dev/null
> +++ b/arch/arm/include/asm/signal.h
> @@ -0,0 +1 @@
> +#include <asm-generic/signal.h>
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index 1ef2400..c543563 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -52,3 +52,6 @@ endif
> ifneq (,$(findstring -mabi=aapcs-linux,$(PLATFORM_CPPFLAGS)))
> extra-y += eabi_compat.o
> endif
> +
> +obj-$(CONFIG_CMD_KGDB) += kgdb/setjmp.o
> +obj-$(CONFIG_CMD_KGDB) += kgdb/kgdb.o
> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> index 29cdad0..d96e70b 100644
> --- a/arch/arm/lib/crt0.S
> +++ b/arch/arm/lib/crt0.S
> @@ -99,9 +99,13 @@ clr_gd:
> sub r9, r9, #GD_SIZE /* new GD is below bd */
>
> adr lr, here
> +#ifndef CONFIG_CMD_KGDB
> ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */
> add lr, lr, r0
> ldr r0, [r9, #GD_RELOCADDR] /* r0 = gd->relocaddr */
> +#else
> + ldr r0, =__image_copy_start
> +#endif
> b relocate_code
> here:
>
> diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c
> index 4dacfd9..d16bd58 100644
> --- a/arch/arm/lib/interrupts.c
> +++ b/arch/arm/lib/interrupts.c
> @@ -22,6 +22,7 @@
> #include <common.h>
> #include <asm/proc-armv/ptrace.h>
> #include <asm/u-boot-arm.h>
> +#include <kgdb.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -160,9 +161,32 @@ void show_regs (struct pt_regs *regs)
>
> void do_undefined_instruction (struct pt_regs *pt_regs)
> {
> +#if defined(CONFIG_CMD_KGDB)
> + unsigned long *tmp_pc = NULL;
> +
> + pt_regs->ARM_pc -= 4;
> + tmp_pc = (unsigned long *)pt_regs->ARM_pc;
> +
> + if (*tmp_pc == 0xe7ffdeff) {
> + pt_regs->ARM_pc += 4;
> + if (debugger_exception_handler &&
> + debugger_exception_handler(pt_regs)) {
> + return;
> + }
> + } else if (*tmp_pc == 0xe7ffdefe) {
> + if (debugger_exception_handler &&
> + debugger_exception_handler(pt_regs)) {
> + return;
> + }
> + } else {
> + printf("DCache/ICACHE May need flush\n");
> + return;
> + }
> +#else
> printf ("undefined instruction\n");
> show_regs (pt_regs);
> bad_mode ();
> +#endif
> }
>
> void do_software_interrupt (struct pt_regs *pt_regs)
> diff --git a/arch/arm/lib/kgdb/kgdb.c b/arch/arm/lib/kgdb/kgdb.c
> new file mode 100644
> index 0000000..6b2e542
> --- /dev/null
> +++ b/arch/arm/lib/kgdb/kgdb.c
> @@ -0,0 +1,231 @@
> +#include <common.h>
> +#include <command.h>
> +#include <kgdb.h>
> +#include <asm/signal.h>
> +#include <asm/processor.h>
> +
> +#define KGDB_ARM_GP_REGS 16
> +#define KGDB_ARM_FP_REGS 8
> +#define KGDB_ARM_EXTRA_REGS 2
> +#define KGDB_ARM_MAX_REGS (KGDB_ARM_GP_REGS +\
> + (KGDB_ARM_FP_REGS * 3) +\
> + KGDB_ARM_EXTRA_REGS)
> +#define KGDB_NUMREGBYTES (KGDB_ARM_MAX_REGS << 2)
> +
> +enum arm_regs {
> + KGDB_ARM_R0,
> + KGDB_ARM_R1,
> + KGDB_ARM_R2,
> + KGDB_ARM_R3,
> + KGDB_ARM_R4,
> + KGDB_ARM_R5,
> + KGDB_ARM_R6,
> + KGDB_ARM_R7,
> + KGDB_ARM_R8,
> + KGDB_ARM_R9,
> + KGDB_ARM_R10,
> + KGDB_ARM_FP,
> + KGDB_ARM_IP,
> + KGDB_ARM_SP,
> + KGDB_ARM_LR,
> + KGDB_ARM_PC,
So in here there are the FP and EXTRA regs? If you added them to this
enum it would be clearer.
Also these mirror the numbers in ptrace.h so I wonder if somehow they
could be linked?
> + KGDB_ARM_CPSR = KGDB_ARM_MAX_REGS - 1
> +};
> +
> +void kgdb_enter(struct pt_regs *regs, kgdb_data *kdp)
> +{
> + disable_interrupts();
> +
> + kdp->sigval = kgdb_trap(regs);
> + kdp->nregs = 2;
> + kdp->regs[0].num = KGDB_ARM_PC;
> + kdp->regs[0].val = regs->ARM_pc;
> +
> + kdp->regs[1].num = KGDB_ARM_SP;
> + kdp->regs[1].val = regs->ARM_sp;
> +
> + return;
> +}
> +
> +void kgdb_exit(struct pt_regs *regs, kgdb_data *kdp)
> +{
> + /* Mark Not sure ??? */
What does this mean?
> +
> + if (kdp->extype & KGDBEXIT_WITHADDR) {
> + regs->ARM_pc = kdp->exaddr;
> + printf("KGDBEXIT_WITHADDR 0x%lx\n", regs->ARM_pc);
> + }
> +
> + switch (kdp->extype & KGDBEXIT_TYPEMASK) {
> + case KGDBEXIT_KILL:
> + printf("KGDBEXIT_KILL:\n");
> + break;
> + case KGDBEXIT_CONTINUE:
> + printf("KGDBEXIT_CONTINUE\n");
> + break;
> + case KGDBEXIT_SINGLE:
> + printf("KGDBEXIT_SINGLE\n");
> + break;
> + default:
> + printf("KGDBEXIT : %d\n", kdp->extype);
> + }
> +
> + enable_interrupts();
> +}
> +
> +int kgdb_trap(struct pt_regs *regs)
> +{
> + /* Mark Not sure ??? */
And this?
> + return SIGTRAP;
> +}
> +
> +int kgdb_getregs(struct pt_regs *regs, char *buf, int max)
> +{
> + unsigned long *gdb_regs = (unsigned long *)buf;
> +
> + if (max < KGDB_NUMREGBYTES)
> + kgdb_error(KGDBERR_NOSPACE);
> +
> + if ((unsigned long)gdb_regs & 3)
> + kgdb_error(KGDBERR_ALIGNFAULT);
> +
> + gdb_regs[KGDB_ARM_R0] = regs->ARM_r0;
> + gdb_regs[KGDB_ARM_R1] = regs->ARM_r1;
> + gdb_regs[KGDB_ARM_R2] = regs->ARM_r2;
> + gdb_regs[KGDB_ARM_R3] = regs->ARM_r3;
> + gdb_regs[KGDB_ARM_R4] = regs->ARM_r4;
> + gdb_regs[KGDB_ARM_R5] = regs->ARM_r5;
> + gdb_regs[KGDB_ARM_R6] = regs->ARM_r6;
> + gdb_regs[KGDB_ARM_R7] = regs->ARM_r7;
> + gdb_regs[KGDB_ARM_R8] = regs->ARM_r8;
> + gdb_regs[KGDB_ARM_R9] = regs->ARM_r9;
> + gdb_regs[KGDB_ARM_R10] = regs->ARM_r10;
> + gdb_regs[KGDB_ARM_IP] = regs->ARM_ip;
> + gdb_regs[KGDB_ARM_FP] = regs->ARM_fp;
> + /* set sp */
I think you can remove that comment as it's pretty obvious
> + gdb_regs[KGDB_ARM_SP] = (unsigned long)regs;
> + gdb_regs[KGDB_ARM_LR] = regs->ARM_lr;
> + gdb_regs[KGDB_ARM_PC] = regs->ARM_pc;
> + gdb_regs[KGDB_ARM_CPSR] = regs->ARM_cpsr;
> +
> + return KGDB_NUMREGBYTES;
> +}
> +
> +void kgdb_putreg(struct pt_regs *regs, int regno, char *buf, int length)
> +{
> + unsigned long *ptr = (unsigned long *)buf;
> +
> + if (regno < 0 || regno >= 16)
Should that be regno > KGDB_ARM_PC?
> + kgdb_error(KGDBERR_BADPARAMS);
> +
> + if (length < 4)
> + kgdb_error(KGDBERR_NOSPACE);
> +
> + if ((unsigned long)ptr & 3)
> + kgdb_error(KGDBERR_ALIGNFAULT);
> +
> + switch (regno) {
> + case KGDB_ARM_R0:
> + regs->ARM_r0 = *ptr;
> + break;
Would it be valid to drop this switch and use:
regs->uregs[regno] = *ptr
> + case KGDB_ARM_R1:
> + regs->ARM_r1 = *ptr;
> + break;
> + case KGDB_ARM_R2:
> + regs->ARM_r2 = *ptr;
> + break;
> + case KGDB_ARM_R3:
> + regs->ARM_r3 = *ptr;
> + break;
> + case KGDB_ARM_R4:
> + regs->ARM_r4 = *ptr;
> + break;
> + case KGDB_ARM_R5:
> + regs->ARM_r5 = *ptr;
> + break;
> + case KGDB_ARM_R6:
> + regs->ARM_r6 = *ptr;
> + break;
> + case KGDB_ARM_R7:
> + regs->ARM_r7 = *ptr;
> + break;
> + case KGDB_ARM_R8:
> + regs->ARM_r8 = *ptr;
> + break;
> + case KGDB_ARM_R9:
> + regs->ARM_r9 = *ptr;
> + break;
> + case KGDB_ARM_R10:
> + regs->ARM_r10 = *ptr;
> + break;
> + case KGDB_ARM_FP:
> + regs->ARM_fp = *ptr;
> + break;
> + case KGDB_ARM_IP:
> + regs->ARM_ip = *ptr;
> + break;
> + case KGDB_ARM_SP:
> + regs->ARM_sp = *ptr;
> + break;
> + case KGDB_ARM_LR:
> + regs->ARM_lr = *ptr;
> + break;
> + case KGDB_ARM_PC:
> + regs->ARM_pc = *ptr;
> + break;
> + case KGDB_ARM_CPSR:
> + regs->ARM_cpsr = *ptr;
> + break;
> + default:
> + kgdb_error(KGDBERR_BADPARAMS);
> + break;
> + }
> +}
> +
> +void kgdb_putregs(struct pt_regs *regs, char *buf, int length)
> +{
> + unsigned long *kgdb_regs = (unsigned long *)buf;
> +
> + if (length != KGDB_ARM_MAX_REGS)
Is length the number of registers rather than the number of bytes?
> + kgdb_error(KGDBERR_NOSPACE);
> +
> + if ((unsigned long)kgdb_regs & 3)
> + kgdb_error(KGDBERR_ALIGNFAULT);
> +
> + regs->ARM_r0 = kgdb_regs[KGDB_ARM_R0];
> + regs->ARM_r1 = kgdb_regs[KGDB_ARM_R1];
> + regs->ARM_r2 = kgdb_regs[KGDB_ARM_R2];
> + regs->ARM_r3 = kgdb_regs[KGDB_ARM_R3];
> + regs->ARM_r4 = kgdb_regs[KGDB_ARM_R4];
> + regs->ARM_r5 = kgdb_regs[KGDB_ARM_R5];
> + regs->ARM_r6 = kgdb_regs[KGDB_ARM_R6];
> + regs->ARM_r7 = kgdb_regs[KGDB_ARM_R7];
> + regs->ARM_r8 = kgdb_regs[KGDB_ARM_R8];
> + regs->ARM_r9 = kgdb_regs[KGDB_ARM_R9];
> + regs->ARM_r10 = kgdb_regs[KGDB_ARM_R10];
> + regs->ARM_ip = kgdb_regs[KGDB_ARM_IP];
> + regs->ARM_fp = kgdb_regs[KGDB_ARM_FP];
> + regs->ARM_sp = kgdb_regs[KGDB_ARM_SP];
> + regs->ARM_lr = kgdb_regs[KGDB_ARM_LR];
> + regs->ARM_pc = kgdb_regs[KGDB_ARM_PC];
> + regs->ARM_cpsr = kgdb_regs[KGDB_ARM_CPSR];
Feels like this could be a for() loop.
> +}
> +
> +void kgdb_flush_cache_all(void)
> +{
> + invalidate_icache_all();
> + flush_dcache_all();
Do you need to flush the write buffer here?
> +}
> +
> +/*
> + * 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 * const argv[])
> +{
> + asm(".word 0xe7ffdeff");
> +}
> diff --git a/arch/arm/lib/kgdb/setjmp.S b/arch/arm/lib/kgdb/setjmp.S
> new file mode 100644
> index 0000000..e17f959
> --- /dev/null
> +++ b/arch/arm/lib/kgdb/setjmp.S
> @@ -0,0 +1,20 @@
> + .text
> + .balign 4
> + .global kgdb_setjmp
> + .type kgdb_setjmp, #function
> +kgdb_setjmp:
> + stmia r0, {r4, r5, r6, r7, r8, r9, r10, fp, sp, lr}
{r4-r10, fp, sp, lr}
and below
> + mov r0, #0
> + mov pc, lr
> + .size kgdb_setjmp,.-kgdb_setjmp
> +
> + .text
> + .balign 4
> + .global kgdb_longjmp
> + .type kgdb_longjmp, #function
> +kgdb_longjmp:
> + ldmia r0, {r4, r5, r6, r7, r8, r9, r10, fp, sp, lr}
> + movs r0, r1
> + moveq r0, #1
> + mov pc, lr
> + .size kgdb_longjmp,.-kgdb_longjmp
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index 49238ed..8abad3d 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -221,15 +221,43 @@ FIQ_STACK_START:
> ldr sp, FIQ_STACK_START
> .endm
>
> + .macro get_und_stack
> + get_bad_stack
> + .endm
> +
> + .macro und_save_regs
> + bad_save_user_regs
> + .endm
> +
> + /* In svc mode */
> + .macro und_restore_regs
> + add r7, sp, #S_PC
> + ldr r6, [r7, #4] @load spsr into r6
> + msr cpsr, r6 @use und spsr to overwrite svc cpsr
> + ldmdb r7, {r1 - r2} @load sp lr into r1 r2
> + mov lr, r2
> + ldmia sp, {r0 - r12}
> + mov r0, r0
> + add sp, sp, #S_FRAME_SIZE
> + ldr pc, [sp, #-12]
> + .endm
> +
> /*
> * exception handlers
> */
>
> .align 5
> undefined_instruction:
> +#if defined(CONFIG_CMD_KGDB)
> + get_und_stack
> + und_save_regs
> + bl do_undefined_instruction
> + und_restore_regs
> +#else
> get_bad_stack
> bad_save_user_regs
> bl do_undefined_instruction
> +#endif
>
> .align 5
> software_interrupt:
> --
> 1.8.4.5
>
Regards,
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [RFC PATCH v1 1/2] kgdb: add breakpoint support
2014-11-04 6:46 ` Simon Glass
@ 2014-11-04 12:23 ` Peng Fan
0 siblings, 0 replies; 7+ messages in thread
From: Peng Fan @ 2014-11-04 12:23 UTC (permalink / raw)
To: u-boot
Hi Simon,
? 11/4/2014 2:46 PM, Simon Glass ??:
> Hi Peng,
>
> On 31 October 2014 23:12, Peng Fan <Peng.Fan@freescale.com> wrote:
>> Add Z/z protocal support for breakpoint set/remove.
>>
>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>
> This looks good to me - I just have a few bits below.
>
>> ---
>> common/kgdb.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/kgdb.h | 35 ++++++++
>> 2 files changed, 308 insertions(+)
>>
>> diff --git a/common/kgdb.c b/common/kgdb.c
>> index d357463..fd83ccd 100644
>> --- a/common/kgdb.c
>> +++ b/common/kgdb.c
>> @@ -92,6 +92,8 @@
>> #include <kgdb.h>
>> #include <command.h>
>>
>> +#include <asm-generic/errno.h>
>
> #include <errno.h> would do
Ok.
>
>> +
>> #undef KGDB_DEBUG
>
> Where is this used?
>
You mean KGDB_DEBUG?
>>
>> /*
>> @@ -111,6 +113,17 @@ static int longjmp_on_fault = 0;
>> static int kdebug = 1;
>> #endif
>>
>> +struct kgdb_bkpt kgdb_break[KGDB_MAX_BREAKPOINTS] = {
>> + [0 ... KGDB_MAX_BREAKPOINTS-1] = { .state = BP_UNDEFINED }
>> +};
>> +
>> +#ifdef CONFIG_ARM
>> +unsigned char gdb_bpt_instr[4] = {0xfe, 0xde, 0xff, 0xe7};
>> +#else
>> +#error "Please implement gdb_bpt_instr!"
>> +#endif
>> +
>> +
>> static const char hexchars[]="0123456789abcdef";
>>
>> /* Convert ch from a hex digit to an int */
>> @@ -309,6 +322,200 @@ putpacket(unsigned char *buffer)
>> } while ((recv & 0x7f) != '+');
>> }
>>
>> +int kgdb_validate_break_address(unsigned addr)
>> +{
>> + /* Need More */
>
> ?
I'll remove the comment. Actually i just want to validate whether the
add parameter is fine or not using this function.
>
>> + return 0;
>> +}
>> +
>> +static int probe_kernel_read(unsigned char *dst, void *src, size_t size)
>> +{
>> + int i;
>> + unsigned char *dst_ptr = dst;
>> + unsigned char *src_ptr = src;
>> +
>> + for (i = 0; i < size; i++)
>> + *dst_ptr++ = *src_ptr++;
>> +
>> + return 0;
>> +}
>> +
>> +static int probe_kernel_write(char *dst, void *src, size_t size)
>
> These two above are strange function names - why 'kernel' - what does
> it mean in this context? Also could you must use memcpy(), either in
> the functions or instead of them?
Ok. memcpy is better. I just use the function name in linux kernel,
maybe misleading here. how about probe_mem_read?
>
>> +{
>> + int i;
>> + char *dst_ptr = dst;
>> + char *src_ptr = src;
>> +
>> + for (i = 0; i < size; i++)
>> + *dst_ptr++ = *src_ptr++;
>> +
>> + return 0;
>> +}
>> +
>> +__weak int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
>> +{
>> + int err;
>> +
>> + err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
>> + BREAK_INSTR_SIZE);
>> + if (err)
>> + return err;
>> +
>> + err = probe_kernel_write((char *)bpt->bpt_addr, gdb_bpt_instr,
>> + BREAK_INSTR_SIZE);
>> +
>> + return err;
>> +}
>> +
>> +/*
>> + * Set the breakpoints whose state is BP_SET to BP_ACTIVE
>> + */
>> +int kgdb_active_sw_breakpoints(void)
>> +{
>> + int err;
>> + int ret = 0;
>> + int i;
>> +
>> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
>> + if (kgdb_break[i].state != BP_SET)
>> + continue;
>> +
>> + err = kgdb_arch_set_breakpoint(&kgdb_break[i]);
>> + if (err) {
>> + ret = err;
>> + printf("KGDB: BP install failed: %lx\n",
>> + kgdb_break[i].bpt_addr);
>> + continue;
>> + }
>> +
>> + kgdb_break[i].state = BP_ACTIVE;
>> +
>> + /*
>> + * kgdb_arch_set_breakpoint touched dcache and memory.
>> + * cache should be flushed to let icache can see the updated
>> + * inst.
>
> instruction
>
>> + */
>> + /* flush work is done in do_exit */
>> + kgdb_flush_cache_all();
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * Set state from BP_SET to BP_REMOVED
>> + */
>> +int kgdb_remove_sw_breakpoint(unsigned int addr)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
>> + if ((kgdb_break[i].state == BP_SET) &&
>> + (kgdb_break[i].bpt_addr == addr)) {
>> + kgdb_break[i].state = BP_REMOVED;
>> + return 0;
>> + }
>> + }
>> +
>> + return -ENOENT;
>> +}
>> +
>> +int kgdb_set_sw_breakpoint(unsigned int addr)
>> +{
>> + int err = kgdb_validate_break_address(addr);
>> + int breakno = -1;
>> + int i;
>> +
>> + if (err)
>> + return err;
>> +
>> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
>> + if ((kgdb_break[i].state == BP_SET) &&
>> + (kgdb_break[i].bpt_addr == addr))
>> + return -EEXIST;
>> + }
>> +
>> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
>> + /* removed or unused, use it */
>> + if ((kgdb_break[i].state == BP_REMOVED) ||
>> + (kgdb_break[i].state == BP_UNDEFINED)) {
>> + breakno = i;
>> + break;
>> + }
>> + }
>> +
>> + if (breakno == -1)
>> + return -E2BIG;
>> +
>> + kgdb_break[breakno].state = BP_SET;
>> + kgdb_break[breakno].type = BP_BREAKPOINT;
>> + kgdb_break[breakno].bpt_addr = addr;
>> +
>> + return 0;
>> +}
>> +
>> +__weak int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
>> +{
>> + return probe_kernel_write((char *)bpt->bpt_addr,
>> + (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
>> +}
>> +
>> +/*
>> + * set breakpoints whose state is BP_ACTIVE to BP_SET
>> + */
>> +int kgdb_deactivate_sw_breakpoints(void)
>> +{
>> + int err;
>> + int ret = 0;
>> + int i;
>> +
>> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
>> + if (kgdb_break[i].state != BP_ACTIVE)
>> + continue;
>> +
>> + err = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
>> + if (err) {
>> + printf("KGDB: BP remove failed: %lx\n",
>> + kgdb_break[i].bpt_addr);
>> + ret = err;
>> + }
>> +
>> + kgdb_break[i].state = BP_SET;
>> +
>> + /*
>> + * kgdb_arch_remove_breakpoint touched dcache and memory.
>> + * cache should be flushed to let icache can see the updated
>> + * inst.
>> + */
>> + kgdb_flush_cache_all();
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int kgdb_remove_all_break(void)
>
> kgdb_remove_all_breakpoints() to be consistent?
kgdb_remove_all_breakpoints is better.
>
>> +{
>> + int err;
>> + int i;
>> +
>> + /* Clear memory breakpoints. */
>> + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
>> + if (kgdb_break[i].state != BP_ACTIVE)
>> + goto setundefined;
>> + err = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
>> + if (err)
>> + printf("KGDB: breakpoint remove failed: %lx\n",
>> + kgdb_break[i].bpt_addr);
>> +setundefined:
>> + kgdb_break[i].state = BP_UNDEFINED;
>> + }
>> +
>> + /* clear hardware breakpoints. */
>> + /* ToDo in future. */
>
> /* TODO: clear hardware breakpoints. */
Ok.
>
>> +
>> + return 0;
>> +}
>> +
>> /*
>> * This function does all command processing for interfacing to gdb.
>> */
>> @@ -318,6 +525,7 @@ handle_exception (struct pt_regs *regs)
>> int addr;
>> int length;
>> char *ptr;
>> + char *bpt_type;
>> kgdb_data kd;
>> int i;
>>
>> @@ -376,6 +584,12 @@ handle_exception (struct pt_regs *regs)
>>
>> putpacket((unsigned char *)&remcomOutBuffer);
>>
>> + /*
>> + * Each time trigger a kgdb break, first deactive all active
>> + * breakpoints.
>> + */
>> + kgdb_deactivate_sw_breakpoints();
>> +
>> while (1) {
>> volatile int errnum;
>>
>> @@ -394,6 +608,8 @@ handle_exception (struct pt_regs *regs)
>> if (errnum == 0) switch (remcomInBuffer[0]) {
>>
>> case '?': /* report most recent signal */
>> + kgdb_remove_all_break();
>> +
>> remcomOutBuffer[0] = 'S';
>> remcomOutBuffer[1] = hexchars[kd.sigval >> 4];
>> remcomOutBuffer[2] = hexchars[kd.sigval & 0xf];
>> @@ -465,6 +681,8 @@ handle_exception (struct pt_regs *regs)
>> kd.extype |= KGDBEXIT_WITHADDR;
>> }
>>
>> + kgdb_active_sw_breakpoints();
>> +
>> goto doexit;
>>
>> case 'S': /* SSS single step with signal SS */
>> @@ -479,6 +697,8 @@ handle_exception (struct pt_regs *regs)
>> kd.extype |= KGDBEXIT_WITHADDR;
>> }
>>
>> + kgdb_active_sw_breakpoints();
>> +
>> doexit:
>> /* Need to flush the instruction cache here, as we may have deposited a
>> * breakpoint, and the icache probably has no way of knowing that a data ref to
>> @@ -506,6 +726,59 @@ handle_exception (struct pt_regs *regs)
>> kgdb_error(KGDBERR_BADPARAMS);
>> }
>> break;
>> + case 'z':
>> + /*
>> + * Break point remove
>> + * packet: zt,addr,length
>> + */
>> + case 'Z':
>> + /*
>> + * Break point set
>> + * packet: Zt,addr,length
>> + *
>> + * t is type: `0' - software breakpoint,
>> + * `1' - hardware breakpoint, `2' - write watchpoint,
>> + * `3' - read watchpoint, `4' - access watchpoint;
>> + * addr is address; length is in bytes. For a software
>> + * breakpoint, length specifies the size of the
>> + * instruction to be patched. For hardware breakpoints
>> + * and watchpoints length specifies the memory region
>> + * to be monitored. To avoid potential problems with
>> + * duplicate packets, the operations should be
>> + * implemented in an idempotent way.
>> + */
>> + bpt_type = &remcomInBuffer[1];
>> + ptr = &remcomInBuffer[2];
>> +
>> + if (*(ptr++) != ',') {
>> + errnum = -EINVAL;
>> + break;
>> + }
>> +
>> + if (!hexToInt(&ptr, &addr)) {
>> + errnum = -EINVAL;
>> + break;
>> + }
>> +
>> + if (*ptr++ != ',') {
>> + errnum = -EINVAL;
>> + break;
>> + }
>> +
>> + /* only software breakpoint is implemented */
>> + if ((remcomInBuffer[0] == 'Z') && (*bpt_type == '0')) {
>> + errnum = kgdb_set_sw_breakpoint(addr);
>> + } else if ((remcomInBuffer[0] == 'z') &&
>> + (*bpt_type == '0')) {
>> + errnum = kgdb_remove_sw_breakpoint(addr);
>> + } else {
>> + /* Unsupported */
>> + errnum = -EINVAL;
>> + }
>> +
>> + if (errnum == 0)
>> + strcpy(remcomOutBuffer, "OK");
>> + break;
>> } /* switch */
>>
>> if (errnum != 0)
>> diff --git a/include/kgdb.h b/include/kgdb.h
>> index b6ba742..f9152b5 100644
>> --- a/include/kgdb.h
>> +++ b/include/kgdb.h
>> @@ -20,6 +20,12 @@
>>
>> #define KGDBEXIT_WITHADDR 0x100
>>
>> +#if defined(CONFIG_ARM)
>> +#define BREAK_INSTR_SIZE 4
>> +#else
>> +#error "BREAK_INSTR_SIZE not set"
>> +#endif
>> +
>> typedef
>> struct {
>> int num;
>> @@ -38,6 +44,29 @@ typedef
>> }
>> kgdb_data;
>>
>> +enum kgdb_bptype {
>> + BP_BREAKPOINT = 0,
>> + BP_HARDWARE_BREAKPOINT,
>> + BP_WRITE_WATCHPOINT,
>> + BP_READ_WATCHPOINT,
>> + BP_ACCESS_WATCHPOINT,
>> + BP_POKE_BREAKPOINT,
>> +};
>> +
>> +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;
>> +};
>> +
>> /* these functions are provided by the generic kgdb support */
>> extern void kgdb_init(void);
>> extern void kgdb_error(int);
>> @@ -67,4 +96,10 @@ extern void kgdb_interruptible(int);
>> /* this is referenced in the trap handler for the platform */
>> extern int (*debugger_exception_handler)(struct pt_regs *);
>>
>> +int kgdb_set_sw_break(unsigned int addr);
>> +int kgdb_remove_sw_break(unsigned int addr);
>> +int kgdb_validate_break_address(unsigned int addr);
>> +
>> +#define KGDB_MAX_BREAKPOINTS 32
>> +
>> #endif /* __KGDB_H__ */
>> --
>> 1.8.4.5
>>
>
> Regards,
> Simon
>
Thanks for reviewing.
Regards,
Peng.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [RFC PATCH v1 2/2] arm:kgdb add KGDB support for arm platform
2014-11-04 6:58 ` Simon Glass
@ 2014-11-04 13:09 ` Peng Fan
0 siblings, 0 replies; 7+ messages in thread
From: Peng Fan @ 2014-11-04 13:09 UTC (permalink / raw)
To: u-boot
Hi Simon,
? 11/4/2014 2:58 PM, Simon Glass ??:
> HI Peng,
>
> On 31 October 2014 23:12, Peng Fan <Peng.Fan@freescale.com> wrote:
>> The register save/restore:
>> Use get_bad_stack and bad_save_user_regs to save regs.
>> Introduce und_restore_regs to restore the previous regs before
>> trigger a breakpoint.
>>
>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>
> Looks good so far as I understand it. A few nits below and maybe you
> can integrate better with the ptrace register numbering.
>
>> ---
>> arch/arm/include/asm/proc-armv/ptrace.h | 2 +-
>> arch/arm/include/asm/signal.h | 1 +
>> arch/arm/lib/Makefile | 3 +
>> arch/arm/lib/crt0.S | 4 +
>> arch/arm/lib/interrupts.c | 24 ++++
>> arch/arm/lib/kgdb/kgdb.c | 231 ++++++++++++++++++++++++++++++++
>> arch/arm/lib/kgdb/setjmp.S | 20 +++
>> arch/arm/lib/vectors.S | 28 ++++
>> 8 files changed, 312 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/proc-armv/ptrace.h b/arch/arm/include/asm/proc-armv/ptrace.h
>> index 71df5a9..33fe587 100644
>> --- a/arch/arm/include/asm/proc-armv/ptrace.h
>> +++ b/arch/arm/include/asm/proc-armv/ptrace.h
>> @@ -58,7 +58,7 @@ struct pt_regs {
>> stack during a system call. */
>>
>> struct pt_regs {
>> - long uregs[18];
>> + unsigned long uregs[18];
>> };
>>
>> #define ARM_cpsr uregs[16]
>> diff --git a/arch/arm/include/asm/signal.h b/arch/arm/include/asm/signal.h
>> new file mode 100644
>> index 0000000..7b1573c
>> --- /dev/null
>> +++ b/arch/arm/include/asm/signal.h
>> @@ -0,0 +1 @@
>> +#include <asm-generic/signal.h>
>> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
>> index 1ef2400..c543563 100644
>> --- a/arch/arm/lib/Makefile
>> +++ b/arch/arm/lib/Makefile
>> @@ -52,3 +52,6 @@ endif
>> ifneq (,$(findstring -mabi=aapcs-linux,$(PLATFORM_CPPFLAGS)))
>> extra-y += eabi_compat.o
>> endif
>> +
>> +obj-$(CONFIG_CMD_KGDB) += kgdb/setjmp.o
>> +obj-$(CONFIG_CMD_KGDB) += kgdb/kgdb.o
>> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
>> index 29cdad0..d96e70b 100644
>> --- a/arch/arm/lib/crt0.S
>> +++ b/arch/arm/lib/crt0.S
>> @@ -99,9 +99,13 @@ clr_gd:
>> sub r9, r9, #GD_SIZE /* new GD is below bd */
>>
>> adr lr, here
>> +#ifndef CONFIG_CMD_KGDB
>> ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */
>> add lr, lr, r0
>> ldr r0, [r9, #GD_RELOCADDR] /* r0 = gd->relocaddr */
>> +#else
>> + ldr r0, =__image_copy_start
>> +#endif
>> b relocate_code
>> here:
>>
>> diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c
>> index 4dacfd9..d16bd58 100644
>> --- a/arch/arm/lib/interrupts.c
>> +++ b/arch/arm/lib/interrupts.c
>> @@ -22,6 +22,7 @@
>> #include <common.h>
>> #include <asm/proc-armv/ptrace.h>
>> #include <asm/u-boot-arm.h>
>> +#include <kgdb.h>
>>
>> DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -160,9 +161,32 @@ void show_regs (struct pt_regs *regs)
>>
>> void do_undefined_instruction (struct pt_regs *pt_regs)
>> {
>> +#if defined(CONFIG_CMD_KGDB)
>> + unsigned long *tmp_pc = NULL;
>> +
>> + pt_regs->ARM_pc -= 4;
>> + tmp_pc = (unsigned long *)pt_regs->ARM_pc;
>> +
>> + if (*tmp_pc == 0xe7ffdeff) {
>> + pt_regs->ARM_pc += 4;
>> + if (debugger_exception_handler &&
>> + debugger_exception_handler(pt_regs)) {
>> + return;
>> + }
>> + } else if (*tmp_pc == 0xe7ffdefe) {
>> + if (debugger_exception_handler &&
>> + debugger_exception_handler(pt_regs)) {
>> + return;
>> + }
>> + } else {
>> + printf("DCache/ICACHE May need flush\n");
>> + return;
>> + }
>> +#else
>> printf ("undefined instruction\n");
>> show_regs (pt_regs);
>> bad_mode ();
>> +#endif
>> }
>>
>> void do_software_interrupt (struct pt_regs *pt_regs)
>> diff --git a/arch/arm/lib/kgdb/kgdb.c b/arch/arm/lib/kgdb/kgdb.c
>> new file mode 100644
>> index 0000000..6b2e542
>> --- /dev/null
>> +++ b/arch/arm/lib/kgdb/kgdb.c
>> @@ -0,0 +1,231 @@
>> +#include <common.h>
>> +#include <command.h>
>> +#include <kgdb.h>
>> +#include <asm/signal.h>
>> +#include <asm/processor.h>
>> +
>> +#define KGDB_ARM_GP_REGS 16
>> +#define KGDB_ARM_FP_REGS 8
>> +#define KGDB_ARM_EXTRA_REGS 2
>> +#define KGDB_ARM_MAX_REGS (KGDB_ARM_GP_REGS +\
>> + (KGDB_ARM_FP_REGS * 3) +\
>> + KGDB_ARM_EXTRA_REGS)
>> +#define KGDB_NUMREGBYTES (KGDB_ARM_MAX_REGS << 2)
>> +
>> +enum arm_regs {
>> + KGDB_ARM_R0,
>> + KGDB_ARM_R1,
>> + KGDB_ARM_R2,
>> + KGDB_ARM_R3,
>> + KGDB_ARM_R4,
>> + KGDB_ARM_R5,
>> + KGDB_ARM_R6,
>> + KGDB_ARM_R7,
>> + KGDB_ARM_R8,
>> + KGDB_ARM_R9,
>> + KGDB_ARM_R10,
>> + KGDB_ARM_FP,
>> + KGDB_ARM_IP,
>> + KGDB_ARM_SP,
>> + KGDB_ARM_LR,
>> + KGDB_ARM_PC,
>
> So in here there are the FP and EXTRA regs? If you added them to this
> enum it would be clearer.
I did not test FP and other extra regs. Actually in uboot, the FP and
extra regs are used? I am not sure.
>
> Also these mirror the numbers in ptrace.h so I wonder if somehow they
> could be linked?
hmm, I'll try this.
>
>> + KGDB_ARM_CPSR = KGDB_ARM_MAX_REGS - 1
>> +};
>> +
>> +void kgdb_enter(struct pt_regs *regs, kgdb_data *kdp)
>> +{
>> + disable_interrupts();
>> +
>> + kdp->sigval = kgdb_trap(regs);
>> + kdp->nregs = 2;
>> + kdp->regs[0].num = KGDB_ARM_PC;
>> + kdp->regs[0].val = regs->ARM_pc;
>> +
>> + kdp->regs[1].num = KGDB_ARM_SP;
>> + kdp->regs[1].val = regs->ARM_sp;
>> +
>> + return;
>> +}
>> +
>> +void kgdb_exit(struct pt_regs *regs, kgdb_data *kdp)
>> +{
>> + /* Mark Not sure ??? */
>
> What does this mean?
Missed to remove this. This is marked when I beginning wrote this piece
of code.
>> +
>> + if (kdp->extype & KGDBEXIT_WITHADDR) {
>> + regs->ARM_pc = kdp->exaddr;
>> + printf("KGDBEXIT_WITHADDR 0x%lx\n", regs->ARM_pc);
>> + }
>> +
>> + switch (kdp->extype & KGDBEXIT_TYPEMASK) {
>> + case KGDBEXIT_KILL:
>> + printf("KGDBEXIT_KILL:\n");
>> + break;
>> + case KGDBEXIT_CONTINUE:
>> + printf("KGDBEXIT_CONTINUE\n");
>> + break;
>> + case KGDBEXIT_SINGLE:
>> + printf("KGDBEXIT_SINGLE\n");
>> + break;
>> + default:
>> + printf("KGDBEXIT : %d\n", kdp->extype);
>> + }
>> +
>> + enable_interrupts();
>> +}
>> +
>> +int kgdb_trap(struct pt_regs *regs)
>> +{
>> + /* Mark Not sure ??? */
>
> And this?
I'll remove this.
>
>> + return SIGTRAP;
>> +}
>> +
>> +int kgdb_getregs(struct pt_regs *regs, char *buf, int max)
>> +{
>> + unsigned long *gdb_regs = (unsigned long *)buf;
>> +
>> + if (max < KGDB_NUMREGBYTES)
>> + kgdb_error(KGDBERR_NOSPACE);
>> +
>> + if ((unsigned long)gdb_regs & 3)
>> + kgdb_error(KGDBERR_ALIGNFAULT);
>> +
>> + gdb_regs[KGDB_ARM_R0] = regs->ARM_r0;
>> + gdb_regs[KGDB_ARM_R1] = regs->ARM_r1;
>> + gdb_regs[KGDB_ARM_R2] = regs->ARM_r2;
>> + gdb_regs[KGDB_ARM_R3] = regs->ARM_r3;
>> + gdb_regs[KGDB_ARM_R4] = regs->ARM_r4;
>> + gdb_regs[KGDB_ARM_R5] = regs->ARM_r5;
>> + gdb_regs[KGDB_ARM_R6] = regs->ARM_r6;
>> + gdb_regs[KGDB_ARM_R7] = regs->ARM_r7;
>> + gdb_regs[KGDB_ARM_R8] = regs->ARM_r8;
>> + gdb_regs[KGDB_ARM_R9] = regs->ARM_r9;
>> + gdb_regs[KGDB_ARM_R10] = regs->ARM_r10;
>> + gdb_regs[KGDB_ARM_IP] = regs->ARM_ip;
>> + gdb_regs[KGDB_ARM_FP] = regs->ARM_fp;
>> + /* set sp */
>
> I think you can remove that comment as it's pretty obvious
Ok.
>
>> + gdb_regs[KGDB_ARM_SP] = (unsigned long)regs;
>> + gdb_regs[KGDB_ARM_LR] = regs->ARM_lr;
>> + gdb_regs[KGDB_ARM_PC] = regs->ARM_pc;
>> + gdb_regs[KGDB_ARM_CPSR] = regs->ARM_cpsr;
>> +
>> + return KGDB_NUMREGBYTES;
>> +}
>> +
>> +void kgdb_putreg(struct pt_regs *regs, int regno, char *buf, int length)
>> +{
>> + unsigned long *ptr = (unsigned long *)buf;
>> +
>> + if (regno < 0 || regno >= 16)
>
> Should that be regno > KGDB_ARM_PC?
Yeah.
>
>> + kgdb_error(KGDBERR_BADPARAMS);
>> +
>> + if (length < 4)
>> + kgdb_error(KGDBERR_NOSPACE);
>> +
>> + if ((unsigned long)ptr & 3)
>> + kgdb_error(KGDBERR_ALIGNFAULT);
>> +
>> + switch (regno) {
>> + case KGDB_ARM_R0:
>> + regs->ARM_r0 = *ptr;
>> + break;
>
> Would it be valid to drop this switch and use:
>
> regs->uregs[regno] = *ptr
Hah. regs->uregs[regno] = *ptr is better. Thanks.
>
>> + case KGDB_ARM_R1:
>> + regs->ARM_r1 = *ptr;
>> + break;
>> + case KGDB_ARM_R2:
>> + regs->ARM_r2 = *ptr;
>> + break;
>> + case KGDB_ARM_R3:
>> + regs->ARM_r3 = *ptr;
>> + break;
>> + case KGDB_ARM_R4:
>> + regs->ARM_r4 = *ptr;
>> + break;
>> + case KGDB_ARM_R5:
>> + regs->ARM_r5 = *ptr;
>> + break;
>> + case KGDB_ARM_R6:
>> + regs->ARM_r6 = *ptr;
>> + break;
>> + case KGDB_ARM_R7:
>> + regs->ARM_r7 = *ptr;
>> + break;
>> + case KGDB_ARM_R8:
>> + regs->ARM_r8 = *ptr;
>> + break;
>> + case KGDB_ARM_R9:
>> + regs->ARM_r9 = *ptr;
>> + break;
>> + case KGDB_ARM_R10:
>> + regs->ARM_r10 = *ptr;
>> + break;
>> + case KGDB_ARM_FP:
>> + regs->ARM_fp = *ptr;
>> + break;
>> + case KGDB_ARM_IP:
>> + regs->ARM_ip = *ptr;
>> + break;
>> + case KGDB_ARM_SP:
>> + regs->ARM_sp = *ptr;
>> + break;
>> + case KGDB_ARM_LR:
>> + regs->ARM_lr = *ptr;
>> + break;
>> + case KGDB_ARM_PC:
>> + regs->ARM_pc = *ptr;
>> + break;
>> + case KGDB_ARM_CPSR:
>> + regs->ARM_cpsr = *ptr;
>> + break;
>> + default:
>> + kgdb_error(KGDBERR_BADPARAMS);
>> + break;
>> + }
>> +}
>> +
>> +void kgdb_putregs(struct pt_regs *regs, char *buf, int length)
>> +{
>> + unsigned long *kgdb_regs = (unsigned long *)buf;
>> +
>> + if (length != KGDB_ARM_MAX_REGS)
>
> Is length the number of registers rather than the number of bytes?
you are right, should be the number of bytes.
>
>> + kgdb_error(KGDBERR_NOSPACE);
>> +
>> + if ((unsigned long)kgdb_regs & 3)
>> + kgdb_error(KGDBERR_ALIGNFAULT);
>> +
>> + regs->ARM_r0 = kgdb_regs[KGDB_ARM_R0];
>> + regs->ARM_r1 = kgdb_regs[KGDB_ARM_R1];
>> + regs->ARM_r2 = kgdb_regs[KGDB_ARM_R2];
>> + regs->ARM_r3 = kgdb_regs[KGDB_ARM_R3];
>> + regs->ARM_r4 = kgdb_regs[KGDB_ARM_R4];
>> + regs->ARM_r5 = kgdb_regs[KGDB_ARM_R5];
>> + regs->ARM_r6 = kgdb_regs[KGDB_ARM_R6];
>> + regs->ARM_r7 = kgdb_regs[KGDB_ARM_R7];
>> + regs->ARM_r8 = kgdb_regs[KGDB_ARM_R8];
>> + regs->ARM_r9 = kgdb_regs[KGDB_ARM_R9];
>> + regs->ARM_r10 = kgdb_regs[KGDB_ARM_R10];
>> + regs->ARM_ip = kgdb_regs[KGDB_ARM_IP];
>> + regs->ARM_fp = kgdb_regs[KGDB_ARM_FP];
>> + regs->ARM_sp = kgdb_regs[KGDB_ARM_SP];
>> + regs->ARM_lr = kgdb_regs[KGDB_ARM_LR];
>> + regs->ARM_pc = kgdb_regs[KGDB_ARM_PC];
>> + regs->ARM_cpsr = kgdb_regs[KGDB_ARM_CPSR];
>
> Feels like this could be a for() loop.
I'll try to use for loop or memcpy.
>
>> +}
>> +
>> +void kgdb_flush_cache_all(void)
>> +{
>> + invalidate_icache_all();
>> + flush_dcache_all();
>
> Do you need to flush the write buffer here?
Actually, the inst 'x' is in DRAM, we modify it to a break instruction.
If not flush dcache, the modified instruction may still exists in
dcache. Then icache may not see the break instruction. So flush the
write buffer.
>
>> +}
>> +
>> +/*
>> + * 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 * const argv[])
>> +{
>> + asm(".word 0xe7ffdeff");
>> +}
>> diff --git a/arch/arm/lib/kgdb/setjmp.S b/arch/arm/lib/kgdb/setjmp.S
>> new file mode 100644
>> index 0000000..e17f959
>> --- /dev/null
>> +++ b/arch/arm/lib/kgdb/setjmp.S
>> @@ -0,0 +1,20 @@
>> + .text
>> + .balign 4
>> + .global kgdb_setjmp
>> + .type kgdb_setjmp, #function
>> +kgdb_setjmp:
>> + stmia r0, {r4, r5, r6, r7, r8, r9, r10, fp, sp, lr}
>
> {r4-r10, fp, sp, lr}
>
> and below
Ok.
>
>> + mov r0, #0
>> + mov pc, lr
>> + .size kgdb_setjmp,.-kgdb_setjmp
>> +
>> + .text
>> + .balign 4
>> + .global kgdb_longjmp
>> + .type kgdb_longjmp, #function
>> +kgdb_longjmp:
>> + ldmia r0, {r4, r5, r6, r7, r8, r9, r10, fp, sp, lr}
>> + movs r0, r1
>> + moveq r0, #1
>> + mov pc, lr
>> + .size kgdb_longjmp,.-kgdb_longjmp
>> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
>> index 49238ed..8abad3d 100644
>> --- a/arch/arm/lib/vectors.S
>> +++ b/arch/arm/lib/vectors.S
>> @@ -221,15 +221,43 @@ FIQ_STACK_START:
>> ldr sp, FIQ_STACK_START
>> .endm
>>
>> + .macro get_und_stack
>> + get_bad_stack
>> + .endm
>> +
>> + .macro und_save_regs
>> + bad_save_user_regs
>> + .endm
>> +
>> + /* In svc mode */
>> + .macro und_restore_regs
>> + add r7, sp, #S_PC
>> + ldr r6, [r7, #4] @load spsr into r6
>> + msr cpsr, r6 @use und spsr to overwrite svc cpsr
>> + ldmdb r7, {r1 - r2} @load sp lr into r1 r2
>> + mov lr, r2
>> + ldmia sp, {r0 - r12}
>> + mov r0, r0
>> + add sp, sp, #S_FRAME_SIZE
>> + ldr pc, [sp, #-12]
>> + .endm
>> +
>> /*
>> * exception handlers
>> */
>>
>> .align 5
>> undefined_instruction:
>> +#if defined(CONFIG_CMD_KGDB)
>> + get_und_stack
>> + und_save_regs
>> + bl do_undefined_instruction
>> + und_restore_regs
>> +#else
>> get_bad_stack
>> bad_save_user_regs
>> bl do_undefined_instruction
>> +#endif
>>
>> .align 5
>> software_interrupt:
>> --
>> 1.8.4.5
>>
>
> Regards,
> Simon
>
Thanks for reviewing.
I'll fix and test the patch set and send v2 out for review.
Regards,
Peng.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-11-04 13:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-01 5:12 [U-Boot] [RFC PATCH v1 0/2] kgdb:add breakpoint and arm support Peng Fan
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 1/2] kgdb: add breakpoint support Peng Fan
2014-11-04 6:46 ` Simon Glass
2014-11-04 12:23 ` Peng Fan
2014-11-01 5:12 ` [U-Boot] [RFC PATCH v1 2/2] arm:kgdb add KGDB support for arm platform Peng Fan
2014-11-04 6:58 ` Simon Glass
2014-11-04 13:09 ` Peng Fan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox