Hi, i'd like to have this fixed, as i have some usecases i'd like to use the imm16 with BKPT-instruction, which is definately supported by everything sys/arch/arm is meant to support.
how to reproduce on latest current GENERIC: av7cubieb# cat bkpt.c int main(void) { asm volatile("bkpt #0\n" ::: "memory"); return 0; } av7cubieb# cc -o bkpt bkpt.c av7cubieb# ./bkpt load: 0.59 cmd: bkpt 86476 [runnable] 0.51u 28.72s 89% 116k load: 0.90 cmd: bkpt 86476 [runnable] 0.77u 45.25s 116% 116k load: 1.14 cmd: bkpt 86476 [runnable] 1.16u 67.34s 132% 116k load: 1.27 cmd: bkpt 86476 [runnable] 1.39u 86.34s 139% 116k load: 1.43 cmd: bkpt 86476 [runnable] 1.70u 105.73s 143% 116k load: 1.59 cmd: bkpt 86476 [runnable] 2.08u 133.62s 146% 116k load: 1.75 cmd: bkpt 86476 [runnable] 2.66u 177.41s 148% 116k ^C av7cubieb# top(1) did show it like this: CPU: 1.9% user, 0.0% nice, 98.1% sys, 0.0% spin, 0.0% intr, 0.0% idle Memory: Real: 25M/233M act/tot Free: 762M Cache: 160M Swap: 0K/745M PID USERNAME PRI NICE SIZE RES STATE WAIT TIME CPU COMMAND 86476 root 64 0 184K 464K run - 1:44 147.41% bkpt 25432 root 28 0 752K 1680K onproc - 0:19 0.00% top lazy untested diff offering direction below, better than nothing, i guess. (so to make it clear, i'm not sure if this might cause mishandling in the undefinedinstruction handling, but it'd be easy to fix:)) -Artturi diff --git a/sys/arch/arm/arm/db_interface.c b/sys/arch/arm/arm/db_interface.c index 8534dda55c4..64121ec4e34 100644 --- a/sys/arch/arm/arm/db_interface.c +++ b/sys/arch/arm/arm/db_interface.c @@ -340,7 +340,7 @@ db_write_bytes(vaddr_t addr, size_t size, char *data) void db_enter(void) { - asm(".word 0xe7ffffff"); + asm volatile(KBPT_ASM ::: "memory"); } struct db_command db_machine_command_table[] = { @@ -356,7 +356,7 @@ db_trapper(u_int addr, u_int inst, trapframe_t *frame, int fault_code) { if (fault_code == 0) { - if ((inst & ~INSN_COND_MASK) == (BKPT_INST & ~INSN_COND_MASK)) { + if (inst_bkpt(inst)) { db_ktrap(T_BREAKPOINT, frame); frame->tf_pc += INSN_SIZE; } else diff --git a/sys/arch/arm/include/db_machdep.h b/sys/arch/arm/include/db_machdep.h index 381eaf80061..d09f425f887 100644 --- a/sys/arch/arm/include/db_machdep.h +++ b/sys/arch/arm/include/db_machdep.h @@ -64,6 +64,11 @@ extern db_regs_t ddb_regs; /* register state */ #define IS_BREAKPOINT_TRAP(type, code) ((type) == T_BREAKPOINT) #define IS_WATCHPOINT_TRAP(type, code) (0) +#define inst_nocond(ins) ((ins) & ~INSN_COND_MASK) + +#define inst_bkpt(ins) (((inst_nocond((ins)) & ~0x000fff0f) == \ + inst_nocond(BKPT_INST)) + #define inst_trap_return(ins) (0) /* ldmxx reg, {..., pc} 01800000 stack mode diff --git a/sys/arch/arm/include/trap.h b/sys/arch/arm/include/trap.h index d6346a42ef4..59216d9b986 100644 --- a/sys/arch/arm/include/trap.h +++ b/sys/arch/arm/include/trap.h @@ -50,9 +50,6 @@ * Ideally ARM would define several standard instruction sequences for * use as breakpoints. * - * The BKPT instruction isn't much use to us, since its behaviour is - * unpredictable on ARMv3 and lower. - * * The ARM ARM says that for maximum compatibility, we should use undefined * instructions that look like 0x.7f...f. . */ @@ -60,12 +57,14 @@ #define GDB_BREAKPOINT 0xe6000011 /* Used by GDB 4.x */ #define IPKDB_BREAKPOINT 0xe6000010 /* Used by IPKDB */ #define GDB5_BREAKPOINT 0xe7ffdefe /* Used by GDB 5.0 */ -#define KERNEL_BREAKPOINT 0xe7ffffff /* Used by DDB */ +#define KERNEL_BREAKPOINT 0xe1200070 /* Used by DDB */ -#define KBPT_ASM ".word 0xe7ffdefe" +#define KBPT_ASM "bkpt #0\n" +#define KBPT_IMM16(ins) ((((ins) >> 4) & 0xfff0) | ((ins) & 0xf)) #define USER_BREAKPOINT GDB_BREAKPOINT #define T_FAULT 1 + /* XXX ^ == T_BREAKPOINT in db_machdep.h?? */ /* End of trap.h */