ARM: core DPM support for watchpoints
authorDavid Brownell <dbrownell@users.sourceforge.net>
Wed, 2 Dec 2009 05:47:45 +0000 (21:47 -0800)
committerDavid Brownell <dbrownell@users.sourceforge.net>
Wed, 2 Dec 2009 05:47:45 +0000 (21:47 -0800)
This is a NOP unless the underlying core exposes two new methods, and
neither of the two cores using this (ARM11xx, Cortex-A8) do so yet.

This patch only updates those cores so they pass a flag saying whether
or not to update breakpoint and watchpoint status before resuming; and
removing some now-needless anti-segfault code from ARM11.  Cortex-A8
didn't have that code ... yes, it segfaulted when setting watchpoints.

NOTE:  this uses a slightly different strategy for setting/clearing
breakpoints than the ARM7/ARM9/etc code uses.  It leaves them alone
unless it's *got* to change something, to speed halt/resume cycles
(including single stepping).

ALSO NOTE:  this under-delivers for Cortex-A8, where regions with size
up to 2 GBytes can be watched ... it handles watchpoints which ARM11 can
also handle (size 1/2/4 bytes).  Should get fixed later.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
src/target/arm11.c
src/target/arm_dpm.c
src/target/arm_dpm.h
src/target/cortex_a8.c

index fd9b4655562563857ce5b41d0bb944e6b83be614..605e741b3db0819c5ea1d9168a4345f9edd1d35f 100644 (file)
@@ -293,12 +293,11 @@ static int arm11_on_enter_debug_state(struct arm11_common *arm11)
        return ERROR_OK;
 }
 
-/** Restore processor state
-  *
-  * This is called in preparation for the RESTART function.
-  *
-  */
-static int arm11_leave_debug_state(struct arm11_common *arm11)
+/**
+ * Restore processor state.  This is called in preparation for
+ * the RESTART function.
+ */
+static int arm11_leave_debug_state(struct arm11_common *arm11, bool bpwp)
 {
        int retval;
 
@@ -354,7 +353,7 @@ static int arm11_leave_debug_state(struct arm11_common *arm11)
        /* restore CPSR, PC, and R0 ... after flushing any modified
         * registers.
         */
-       retval = arm_dpm_write_dirty_registers(&arm11->dpm);
+       retval = arm_dpm_write_dirty_registers(&arm11->dpm, bpwp);
 
        register_cache_invalidate(arm11->arm.core_cache);
 
@@ -598,7 +597,7 @@ static int arm11_resume(struct target *target, int current,
                arm11_sc7_set_vcr(arm11, arm11_vcr);
        }
 
-       arm11_leave_debug_state(arm11);
+       arm11_leave_debug_state(arm11, handle_breakpoints);
 
        arm11_add_IR(arm11, ARM11_RESTART, TAP_IDLE);
 
@@ -762,7 +761,7 @@ static int arm11_step(struct target *target, int current,
                        R(DSCR) |= ARM11_DSCR_INTERRUPTS_DISABLE;
 
 
-               CHECK_RETVAL(arm11_leave_debug_state(arm11));
+               CHECK_RETVAL(arm11_leave_debug_state(arm11, handle_breakpoints));
 
                arm11_add_IR(arm11, ARM11_RESTART, TAP_IDLE);
 
@@ -1203,22 +1202,6 @@ static int arm11_remove_breakpoint(struct target *target,
        return ERROR_OK;
 }
 
-static int arm11_add_watchpoint(struct target *target,
-               struct watchpoint *watchpoint)
-{
-       LOG_WARNING("Not implemented: %s", __func__);
-
-       return ERROR_FAIL;
-}
-
-static int arm11_remove_watchpoint(struct target *target,
-               struct watchpoint *watchpoint)
-{
-       LOG_WARNING("Not implemented: %s", __func__);
-
-       return ERROR_FAIL;
-}
-
 static int arm11_target_create(struct target *target, Jim_Interp *interp)
 {
        struct arm11_common *arm11;
@@ -1605,8 +1588,6 @@ struct target_type arm11_target = {
 
        .add_breakpoint =       arm11_add_breakpoint,
        .remove_breakpoint =    arm11_remove_breakpoint,
-       .add_watchpoint =       arm11_add_watchpoint,
-       .remove_watchpoint =    arm11_remove_watchpoint,
 
        .run_algorithm =        armv4_5_run_algorithm,
 
index 127f87b53648c403f9ea30bdced193482eacedc5..f94fcc4e0962b900b68c367edb3def8cf1c93a43 100644 (file)
@@ -25,6 +25,8 @@
 #include "arm_dpm.h"
 #include "jtag.h"
 #include "register.h"
+#include "breakpoints.h"
+#include "target_type.h"
 
 
 /**
@@ -34,6 +36,8 @@
  * implementation differences between cores like ARM1136 and Cortex-A8.
  */
 
+/*----------------------------------------------------------------------*/
+
 /*
  * Coprocessor support
  */
@@ -85,6 +89,8 @@ static int dpm_mcr(struct target *target, int cpnum,
        return retval;
 }
 
+/*----------------------------------------------------------------------*/
+
 /*
  * Register access utilities
  */
@@ -178,7 +184,7 @@ static int dpm_write_reg(struct arm_dpm *dpm, struct reg *r, unsigned regnum)
 
        switch (regnum) {
        case 0 ... 14:
-               /* load register from DCC:  "MCR p14, 0, Rnum, c0, c5, 0" */
+               /* load register from DCC:  "MRC p14, 0, Rnum, c0, c5, 0" */
                retval = dpm->instr_write_data_dcc(dpm,
                                ARMV4_5_MRC(14, 0, regnum, 0, 5, 0),
                                value);
@@ -256,6 +262,11 @@ int arm_dpm_read_current_registers(struct arm_dpm *dpm)
 
        /* NOTE: SPSR ignored (if it's even relevant). */
 
+       /* REVISIT the debugger can trigger various exceptions.  See the
+        * ARMv7A architecture spec, section C5.7, for more info about
+        * what defenses are needed; v6 debug has the most issues.
+        */
+
 fail:
        /* (void) */ dpm->finish(dpm);
        return retval;
@@ -264,8 +275,11 @@ fail:
 /**
  * Writes all modified core registers for all processor modes.  In normal
  * operation this is called on exit from halting debug state.
+ *
+ * @param bpwp: true ensures breakpoints and watchpoints are set,
+ *     false ensures they are cleared
  */
-int arm_dpm_write_dirty_registers(struct arm_dpm *dpm)
+int arm_dpm_write_dirty_registers(struct arm_dpm *dpm, bool bpwp)
 {
        struct arm *arm = dpm->arm;
        struct reg_cache *cache = arm->core_cache;
@@ -276,6 +290,53 @@ int arm_dpm_write_dirty_registers(struct arm_dpm *dpm)
        if (retval != ERROR_OK)
                goto done;
 
+       /* enable/disable watchpoints */
+       for (unsigned i = 0; i < dpm->nwp; i++) {
+               struct dpm_wp *dwp = dpm->dwp + i;
+               struct watchpoint *wp = dwp->wp;
+               bool disable;
+
+               /* Avoid needless I/O ... leave watchpoints alone
+                * unless they're removed, or need updating because
+                * of single-stepping or running debugger code.
+                */
+               if (!wp) {
+                       if (!dwp->dirty)
+                               continue;
+                       dwp->dirty = false;
+                       /* removed or startup; we must disable it */
+                       disable = true;
+               } else if (bpwp) {
+                       if (!dwp->dirty)
+                               continue;
+                       /* disabled, but we must set it */
+                       dwp->dirty = disable = false;
+                       wp->set = true;
+               } else {
+                       if (!wp->set)
+                               continue;
+                       /* set, but we must temporarily disable it */
+                       dwp->dirty = disable = true;
+                       wp->set = false;
+               }
+
+               if (disable)
+                       retval = dpm->bpwp_disable(dpm, 16 + i);
+               else
+                       retval = dpm->bpwp_enable(dpm, 16 + i,
+                                       wp->address, dwp->control);
+
+               if (retval != ERROR_OK)
+                       LOG_ERROR("%s: can't %s HW watchpoint %d",
+                                       target_name(arm->target),
+                                       disable ? "disable" : "enable",
+                                       i);
+       }
+
+       /* NOTE:  writes to breakpoint and watchpoint registers might
+        * be queued, and need (efficient/batched) flushing later.
+        */
+
        /* Scan the registers until we find one that's both dirty and
         * eligible for flushing.  Flush that and everything else that
         * shares the same core mode setting.  Typically this won't
@@ -399,6 +460,13 @@ static enum armv4_5_mode dpm_mapmode(struct arm *arm,
        return ARMV4_5_MODE_ANY;
 }
 
+
+/*
+ * Standard ARM register accessors ... there are three methods
+ * in "struct arm", to support individual read/write and bulk read
+ * of registers.
+ */
+
 static int arm_dpm_read_core_reg(struct target *target, struct reg *r,
                int regnum, enum armv4_5_mode mode)
 {
@@ -544,9 +612,141 @@ done:
        return retval;
 }
 
+
+/*----------------------------------------------------------------------*/
+
+/*
+ * Breakpoint and Watchpoint support.
+ *
+ * Hardware {break,watch}points are usually left active, to minimize
+ * debug entry/exit costs.  When they are set or cleared, it's done in
+ * batches.  Also, DPM-conformant hardware can update debug registers
+ * regardless of whether the CPU is running or halted ... though that
+ * fact isn't currently leveraged.
+ */
+
+static int dpm_watchpoint_setup(struct arm_dpm *dpm, unsigned index,
+               struct watchpoint *wp)
+{
+       uint32_t addr = wp->address;
+       uint32_t control;
+
+       /* this hardware doesn't support data value matching or masking */
+       if (wp->value || wp->mask != ~(uint32_t)0) {
+               LOG_DEBUG("watchpoint values and masking not supported");
+               return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
+       }
+
+       control = (1 << 0)      /* enable */
+               | (3 << 1);     /* both user and privileged access */
+
+       switch (wp->rw) {
+       case WPT_READ:
+               control |= 1 << 3;
+               break;
+       case WPT_WRITE:
+               control |= 2 << 3;
+               break;
+       case WPT_ACCESS:
+               control |= 3 << 3;
+               break;
+       }
+
+       /* Match 1, 2, or all 4 byte addresses in this word.
+        *
+        * FIXME:  v7 hardware allows lengths up to 2 GB, and has eight
+        * byte address select bits.  Support larger wp->length, if addr
+        * is suitably aligned.
+        */
+       switch (wp->length) {
+       case 1:
+               control |= (1 << (addr & 3)) << 5;
+               addr &= ~3;
+               break;
+       case 2:
+               /* require 2-byte alignment */
+               if (!(addr & 1)) {
+                       control |= (3 << (addr & 2)) << 5;
+                       break;
+               }
+               /* FALL THROUGH */
+       case 4:
+               /* require 4-byte alignment */
+               if (!(addr & 3)) {
+                       control |= 0xf << 5;
+                       break;
+               }
+               /* FALL THROUGH */
+       default:
+               LOG_DEBUG("bad watchpoint length or alignment");
+               return ERROR_INVALID_ARGUMENTS;
+       }
+
+       /* other control bits:
+        * bits 9:12 == 0 ... only checking up to four byte addresses (v7 only)
+        * bits 15:14 == 0 ... both secure and nonsecure states (v6.1+ only)
+        * bit 20 == 0 ... not linked to a context ID
+        * bit 28:24 == 0 ... not ignoring N LSBs (v7 only)
+        */
+
+       dpm->dwp[index].wp = wp;
+       dpm->dwp[index].control = control;
+       dpm->dwp[index].dirty = true;
+
+       /* hardware is updated in write_dirty_registers() */
+       return ERROR_OK;
+}
+
+
+static int dpm_add_watchpoint(struct target *target, struct watchpoint *wp)
+{
+       struct arm *arm = target_to_arm(target);
+       struct arm_dpm *dpm = arm->dpm;
+       int retval = ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
+
+       if (dpm->bpwp_enable) {
+               for (unsigned i = 0; i < dpm->nwp; i++) {
+                       if (!dpm->dwp[i].wp) {
+                               retval = dpm_watchpoint_setup(dpm, i, wp);
+                               break;
+                       }
+               }
+       }
+
+       return retval;
+}
+
+static int dpm_remove_watchpoint(struct target *target, struct watchpoint *wp)
+{
+       struct arm *arm = target_to_arm(target);
+       struct arm_dpm *dpm = arm->dpm;
+       int retval = ERROR_INVALID_ARGUMENTS;
+
+       for (unsigned i = 0; i < dpm->nwp; i++) {
+               if (dpm->dwp[i].wp == wp) {
+                       dpm->dwp[i].wp = NULL;
+                       dpm->dwp[i].dirty = true;
+
+                       /* hardware is updated in write_dirty_registers() */
+                       retval = ERROR_OK;
+                       break;
+               }
+       }
+
+       return retval;
+}
+
+/*----------------------------------------------------------------------*/
+
+/*
+ * Setup and management support.
+ */
+
 /**
  * Hooks up this DPM to its associated target; call only once.
  * Initially this only covers the register cache.
+ *
+ * Oh, and watchpoints.  Yeah.
  */
 int arm_dpm_setup(struct arm_dpm *dpm)
 {
@@ -556,6 +756,7 @@ int arm_dpm_setup(struct arm_dpm *dpm)
 
        arm->dpm = dpm;
 
+       /* register access setup */
        arm->full_context = arm_dpm_full_context;
        arm->read_core_reg = arm_dpm_read_core_reg;
        arm->write_core_reg = arm_dpm_write_core_reg;
@@ -566,9 +767,48 @@ int arm_dpm_setup(struct arm_dpm *dpm)
 
        *register_get_last_cache_p(&target->reg_cache) = cache;
 
+       /* coprocessor access setup */
        arm->mrc = dpm_mrc;
        arm->mcr = dpm_mcr;
 
+       /* breakpoint and watchpoint setup */
+       target->type->add_watchpoint = dpm_add_watchpoint;
+       target->type->remove_watchpoint = dpm_remove_watchpoint;
+
+       /* FIXME add breakpoint support */
+       /* FIXME add vector catch support */
+
+       dpm->nbp = 1 + ((dpm->didr >> 24) & 0xf);
+       dpm->dbp = calloc(dpm->nbp, sizeof *dpm->dbp);
+
+       dpm->nwp = 1 + ((dpm->didr >> 28) & 0xf);
+       dpm->dwp = calloc(dpm->nwp, sizeof *dpm->dwp);
+
+       if (!dpm->dbp || !dpm->dwp) {
+               free(dpm->dbp);
+               free(dpm->dwp);
+               return ERROR_FAIL;
+       }
+
+       /* Disable all breakpoints and watchpoints at startup. */
+       if (dpm->bpwp_disable) {
+               unsigned i;
+
+               for (i = 0; i < dpm->nbp; i++)
+                       (void) dpm->bpwp_disable(dpm, i);
+               for (i = 0; i < dpm->nwp; i++)
+                       (void) dpm->bpwp_disable(dpm, 16 + i);
+       } else
+               LOG_WARNING("%s: can't disable breakpoints and watchpoints",
+                       target_name(target));
+
+       LOG_INFO("%s: hardware has %d breakpoints, %d watchpoints",
+                       target_name(target), dpm->nbp, dpm->nwp);
+
+       /* REVISIT ... and some of those breakpoints could match
+        * execution context IDs...
+        */
+
        return ERROR_OK;
 }
 
index 67ce2180edd65360b555cd409b10b5538c868a98..5d665a866f29ae6ced8f992a343086acc0eb6b34 100644 (file)
  * registers are compatible.
  */
 
+struct dpm_bp {
+       struct breakpoint *bp;
+       /* bp->address == breakpoint value register
+        * control == breakpoint control register
+        */
+       uint32_t control;
+       /* true if hardware state needs flushing */
+       bool dirty;
+};
+
+struct dpm_wp {
+       struct watchpoint *wp;
+       /* wp->address == watchpoint value register
+        * control == watchpoint control register
+        */
+       uint32_t control;
+       /* true if hardware state needs flushing */
+       bool dirty;
+};
+
 /**
  * This wraps an implementation of DPM primitives.  Each interface
  * provider supplies a structure like this, which is the glue between
@@ -74,9 +94,33 @@ struct arm_dpm {
        int (*instr_read_data_r0)(struct arm_dpm *,
                        uint32_t opcode, uint32_t *data);
 
-       // FIXME -- add breakpoint support
-
-       // FIXME -- add watchpoint support (including context-sensitive ones)
+       /* BREAKPOINT/WATCHPOINT SUPPORT */
+
+       /**
+        * Enables one breakpoint or watchpoint by writing to the
+        * hardware registers.  The specified breakpoint/watchpoint
+        * must currently be disabled.  Indices 0..15 are used for
+        * breakpoints; indices 16..31 are for watchpoints.
+        */
+       int (*bpwp_enable)(struct arm_dpm *, unsigned index,
+                       uint32_t addr, uint32_t control);
+
+       /**
+        * Disables one breakpoint or watchpoint by clearing its
+        * hardware control registers.  Indices are the same ones
+        * accepted by bpwp_enable().
+        */
+       int (*bpwp_disable)(struct arm_dpm *, unsigned index);
+
+       /* The breakpoint and watchpoint arrays are private to the
+        * DPM infrastructure.  There are nbp indices in the dbp
+        * array.  There are nwp indices in the dwp array.
+        */
+
+       unsigned nbp;
+       unsigned nwp;
+       struct dpm_bp *dbp;
+       struct dpm_wp *dwp;
 
        // FIXME -- read/write DCSR methods and symbols
 };
@@ -85,6 +129,6 @@ int arm_dpm_setup(struct arm_dpm *dpm);
 int arm_dpm_reinitialize(struct arm_dpm *dpm);
 
 int arm_dpm_read_current_registers(struct arm_dpm *);
-int arm_dpm_write_dirty_registers(struct arm_dpm *);
+int arm_dpm_write_dirty_registers(struct arm_dpm *, bool bpwp);
 
 #endif /* __ARM_DPM_H */
index e312e547615842bd45259705a7064f3320530de5..81402d79d422c62d91b5db484127f71f1dde34b7 100644 (file)
@@ -41,7 +41,7 @@
 
 static int cortex_a8_poll(struct target *target);
 static int cortex_a8_debug_entry(struct target *target);
-static int cortex_a8_restore_context(struct target *target);
+static int cortex_a8_restore_context(struct target *target, bool bpwp);
 static int cortex_a8_set_breakpoint(struct target *target,
                struct breakpoint *breakpoint, uint8_t matchmode);
 static int cortex_a8_unset_breakpoint(struct target *target,
@@ -602,11 +602,7 @@ static int cortex_a8_resume(struct target *target, int current,
        dap_ap_select(swjdp, swjdp_debugap);
 
        if (!debug_execution)
-       {
                target_free_all_working_areas(target);
-//             cortex_m3_enable_breakpoints(target);
-//             cortex_m3_enable_watchpoints(target);
-       }
 
 #if 0
        if (debug_execution)
@@ -661,7 +657,7 @@ static int cortex_a8_resume(struct target *target, int current,
        armv4_5->core_cache->reg_list[15].dirty = 1;
        armv4_5->core_cache->reg_list[15].valid = 1;
 
-       cortex_a8_restore_context(target);
+       cortex_a8_restore_context(target, handle_breakpoints);
 
 #if 0
        /* the front-end may request us not to handle breakpoints */
@@ -952,7 +948,7 @@ static int cortex_a8_step(struct target *target, int current, uint32_t address,
        return ERROR_OK;
 }
 
-static int cortex_a8_restore_context(struct target *target)
+static int cortex_a8_restore_context(struct target *target, bool bpwp)
 {
        struct armv7a_common *armv7a = target_to_armv7a(target);
 
@@ -961,7 +957,7 @@ static int cortex_a8_restore_context(struct target *target)
        if (armv7a->pre_restore_context)
                armv7a->pre_restore_context(target);
 
-       arm_dpm_write_dirty_registers(&armv7a->dpm);
+       arm_dpm_write_dirty_registers(&armv7a->dpm, bpwp);
 
        if (armv7a->post_restore_context)
                armv7a->post_restore_context(target);