target/cortex_m: Implement maskisr steponly option 73/4673/9
authorChristopher Head <chead@zaber.com>
Wed, 1 Aug 2018 17:21:15 +0000 (10:21 -0700)
committerFreddie Chopin <freddie.chopin@gmail.com>
Wed, 10 Apr 2019 19:05:32 +0000 (20:05 +0100)
`maskisr steponly` disables interrupts during single-stepping but
enables them during normal execution. This can be used as a partial
workaround for 702596 erratum in Cortex-M7 r0p1. See "Cortex-M7 (AT610)
and Cortex-M7 with FPU (AT611) Software Developer Errata Notice" from
ARM for further details.

Change-Id: I797a14e4d43f6dcb3706528ee4ab452846ebf133
Signed-off-by: Christopher Head <chead@zaber.com>
Reviewed-on: http://openocd.zylin.com/4673
Tested-by: jenkins
Reviewed-by: Freddie Chopin <freddie.chopin@gmail.com>
doc/openocd.texi
src/target/cortex_m.c
src/target/cortex_m.h

index 81a913541d5513fa992435dd15655812e40cf537..a5037b25433fe03a46947e921b2c81073e1d7f77 100644 (file)
@@ -9078,7 +9078,7 @@ Enable or disable trace output for all ITM stimulus ports.
 @subsection Cortex-M specific commands
 @cindex Cortex-M
 
 @subsection Cortex-M specific commands
 @cindex Cortex-M
 
-@deffn Command {cortex_m maskisr} (@option{auto}|@option{on}|@option{off})
+@deffn Command {cortex_m maskisr} (@option{auto}|@option{on}|@option{off}|@option{steponly})
 Control masking (disabling) interrupts during target step/resume.
 
 The @option{auto} option handles interrupts during stepping in a way that they
 Control masking (disabling) interrupts during target step/resume.
 
 The @option{auto} option handles interrupts during stepping in a way that they
@@ -9088,6 +9088,11 @@ the next instruction where the core was halted. After the step interrupts
 are enabled again. If the interrupt handlers don't complete within 500ms,
 the step command leaves with the core running.
 
 are enabled again. If the interrupt handlers don't complete within 500ms,
 the step command leaves with the core running.
 
+The @option{steponly} option disables interrupts during single-stepping but
+enables them during normal execution. This can be used as a partial workaround
+for 702596 erratum in Cortex-M7 r0p1. See "Cortex-M7 (AT610) and Cortex-M7 with
+FPU (AT611) Software Developer Errata Notice" from ARM for further details.
+
 Note that a free hardware (FPB) breakpoint is required for the @option{auto}
 option. If no breakpoint is available at the time of the step, then the step
 is taken with interrupts enabled, i.e. the same way the @option{off} option
 Note that a free hardware (FPB) breakpoint is required for the @option{auto}
 option. If no breakpoint is available at the time of the step, then the step
 is taken with interrupts enabled, i.e. the same way the @option{off} option
index d341d457291de7fd7669cbcb75cf9240df0f8e09..e8839250456e7979890f5aa0f048d26057ac4c2f 100644 (file)
@@ -136,6 +136,83 @@ static int cortex_m_write_debug_halt_mask(struct target *target,
        return mem_ap_write_atomic_u32(armv7m->debug_ap, DCB_DHCSR, cortex_m->dcb_dhcsr);
 }
 
        return mem_ap_write_atomic_u32(armv7m->debug_ap, DCB_DHCSR, cortex_m->dcb_dhcsr);
 }
 
+static int cortex_m_set_maskints(struct target *target, bool mask)
+{
+       struct cortex_m_common *cortex_m = target_to_cm(target);
+       if (!!(cortex_m->dcb_dhcsr & C_MASKINTS) != mask)
+               return cortex_m_write_debug_halt_mask(target, mask ? C_MASKINTS : 0, mask ? 0 : C_MASKINTS);
+       else
+               return ERROR_OK;
+}
+
+static int cortex_m_set_maskints_for_halt(struct target *target)
+{
+       struct cortex_m_common *cortex_m = target_to_cm(target);
+       switch (cortex_m->isrmasking_mode) {
+               case CORTEX_M_ISRMASK_AUTO:
+                       /* interrupts taken at resume, whether for step or run -> no mask */
+                       return cortex_m_set_maskints(target, false);
+
+               case CORTEX_M_ISRMASK_OFF:
+                       /* interrupts never masked */
+                       return cortex_m_set_maskints(target, false);
+
+               case CORTEX_M_ISRMASK_ON:
+                       /* interrupts always masked */
+                       return cortex_m_set_maskints(target, true);
+
+               case CORTEX_M_ISRMASK_STEPONLY:
+                       /* interrupts masked for single step only -> mask now if MASKINTS
+                        * erratum, otherwise only mask before stepping */
+                       return cortex_m_set_maskints(target, cortex_m->maskints_erratum);
+       }
+       return ERROR_OK;
+}
+
+static int cortex_m_set_maskints_for_run(struct target *target)
+{
+       switch (target_to_cm(target)->isrmasking_mode) {
+               case CORTEX_M_ISRMASK_AUTO:
+                       /* interrupts taken at resume, whether for step or run -> no mask */
+                       return cortex_m_set_maskints(target, false);
+
+               case CORTEX_M_ISRMASK_OFF:
+                       /* interrupts never masked */
+                       return cortex_m_set_maskints(target, false);
+
+               case CORTEX_M_ISRMASK_ON:
+                       /* interrupts always masked */
+                       return cortex_m_set_maskints(target, true);
+
+               case CORTEX_M_ISRMASK_STEPONLY:
+                       /* interrupts masked for single step only -> no mask */
+                       return cortex_m_set_maskints(target, false);
+       }
+       return ERROR_OK;
+}
+
+static int cortex_m_set_maskints_for_step(struct target *target)
+{
+       switch (target_to_cm(target)->isrmasking_mode) {
+               case CORTEX_M_ISRMASK_AUTO:
+                       /* the auto-interrupt should already be done -> mask */
+                       return cortex_m_set_maskints(target, true);
+
+               case CORTEX_M_ISRMASK_OFF:
+                       /* interrupts never masked */
+                       return cortex_m_set_maskints(target, false);
+
+               case CORTEX_M_ISRMASK_ON:
+                       /* interrupts always masked */
+                       return cortex_m_set_maskints(target, true);
+
+               case CORTEX_M_ISRMASK_STEPONLY:
+                       /* interrupts masked for single step only -> mask */
+                       return cortex_m_set_maskints(target, true);
+       }
+       return ERROR_OK;
+}
+
 static int cortex_m_clear_halt(struct target *target)
 {
        struct cortex_m_common *cortex_m = target_to_cm(target);
 static int cortex_m_clear_halt(struct target *target)
 {
        struct cortex_m_common *cortex_m = target_to_cm(target);
@@ -237,11 +314,8 @@ static int cortex_m_endreset_event(struct target *target)
                        return retval;
        }
 
                        return retval;
        }
 
-       /* Restore proper interrupt masking setting. */
-       if (cortex_m->isrmasking_mode == CORTEX_M_ISRMASK_ON)
-               cortex_m_write_debug_halt_mask(target, C_MASKINTS, 0);
-       else
-               cortex_m_write_debug_halt_mask(target, 0, C_MASKINTS);
+       /* Restore proper interrupt masking setting for running CPU. */
+       cortex_m_set_maskints_for_run(target);
 
        /* Enable features controlled by ITM and DWT blocks, and catch only
         * the vectors we were told to pay attention to.
 
        /* Enable features controlled by ITM and DWT blocks, and catch only
         * the vectors we were told to pay attention to.
@@ -405,6 +479,10 @@ static int cortex_m_debug_entry(struct target *target)
 
        LOG_DEBUG(" ");
 
 
        LOG_DEBUG(" ");
 
+       /* Do this really early to minimize the window where the MASKINTS erratum
+        * can pile up pending interrupts. */
+       cortex_m_set_maskints_for_halt(target);
+
        cortex_m_clear_halt(target);
        retval = mem_ap_read_atomic_u32(armv7m->debug_ap, DCB_DHCSR, &cortex_m->dcb_dhcsr);
        if (retval != ERROR_OK)
        cortex_m_clear_halt(target);
        retval = mem_ap_read_atomic_u32(armv7m->debug_ap, DCB_DHCSR, &cortex_m->dcb_dhcsr);
        if (retval != ERROR_OK)
@@ -614,6 +692,10 @@ static int cortex_m_halt(struct target *target)
        /* Write to Debug Halting Control and Status Register */
        cortex_m_write_debug_halt_mask(target, C_HALT, 0);
 
        /* Write to Debug Halting Control and Status Register */
        cortex_m_write_debug_halt_mask(target, C_HALT, 0);
 
+       /* Do this really early to minimize the window where the MASKINTS erratum
+        * can pile up pending interrupts. */
+       cortex_m_set_maskints_for_halt(target);
+
        target->debug_reason = DBG_REASON_DBGRQ;
 
        return ERROR_OK;
        target->debug_reason = DBG_REASON_DBGRQ;
 
        return ERROR_OK;
@@ -767,6 +849,7 @@ static int cortex_m_resume(struct target *target, int current,
        }
 
        /* Restart core */
        }
 
        /* Restart core */
+       cortex_m_set_maskints_for_run(target);
        cortex_m_write_debug_halt_mask(target, 0, C_HALT);
 
        target->debug_reason = DBG_REASON_NOTHALTED;
        cortex_m_write_debug_halt_mask(target, 0, C_HALT);
 
        target->debug_reason = DBG_REASON_NOTHALTED;
@@ -829,10 +912,12 @@ static int cortex_m_step(struct target *target, int current,
         * a normal step, otherwise we have to manually step over the bkpt
         * instruction - as such simulate a step */
        if (bkpt_inst_found == false) {
         * a normal step, otherwise we have to manually step over the bkpt
         * instruction - as such simulate a step */
        if (bkpt_inst_found == false) {
-               /* Automatic ISR masking mode off: Just step over the next instruction */
-               if ((cortex_m->isrmasking_mode != CORTEX_M_ISRMASK_AUTO))
+               if ((cortex_m->isrmasking_mode != CORTEX_M_ISRMASK_AUTO)) {
+                       /* Automatic ISR masking mode off: Just step over the next
+                        * instruction, with interrupts on or off as appropriate. */
+                       cortex_m_set_maskints_for_step(target);
                        cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
                        cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
-               else {
+               else {
                        /* Process interrupts during stepping in a way they don't interfere
                         * debugging.
                         *
                        /* Process interrupts during stepping in a way they don't interfere
                         * debugging.
                         *
@@ -871,8 +956,9 @@ static int cortex_m_step(struct target *target, int current,
                                LOG_DEBUG("Stepping over next instruction with interrupts disabled");
                                cortex_m_write_debug_halt_mask(target, C_HALT | C_MASKINTS, 0);
                                cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
                                LOG_DEBUG("Stepping over next instruction with interrupts disabled");
                                cortex_m_write_debug_halt_mask(target, C_HALT | C_MASKINTS, 0);
                                cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
-                               /* Re-enable interrupts */
-                               cortex_m_write_debug_halt_mask(target, C_HALT, C_MASKINTS);
+                               /* Re-enable interrupts if appropriate */
+                               cortex_m_write_debug_halt_mask(target, C_HALT, 0);
+                               cortex_m_set_maskints_for_halt(target);
                        }
                        else {
 
                        }
                        else {
 
@@ -891,12 +977,17 @@ static int cortex_m_step(struct target *target, int current,
                                bool tmp_bp_set = (retval == ERROR_OK);
 
                                /* No more breakpoints left, just do a step */
                                bool tmp_bp_set = (retval == ERROR_OK);
 
                                /* No more breakpoints left, just do a step */
-                               if (!tmp_bp_set)
+                               if (!tmp_bp_set) {
+                                       cortex_m_set_maskints_for_step(target);
                                        cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
                                        cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
-                               else {
+                                       /* Re-enable interrupts if appropriate */
+                                       cortex_m_write_debug_halt_mask(target, C_HALT, 0);
+                                       cortex_m_set_maskints_for_halt(target);
+                               } else {
                                        /* Start the core */
                                        LOG_DEBUG("Starting core to serve pending interrupts");
                                        int64_t t_start = timeval_ms();
                                        /* Start the core */
                                        LOG_DEBUG("Starting core to serve pending interrupts");
                                        int64_t t_start = timeval_ms();
+                                       cortex_m_set_maskints_for_run(target);
                                        cortex_m_write_debug_halt_mask(target, 0, C_HALT | C_STEP);
 
                                        /* Wait for pending handlers to complete or timeout */
                                        cortex_m_write_debug_halt_mask(target, 0, C_HALT | C_STEP);
 
                                        /* Wait for pending handlers to complete or timeout */
@@ -924,12 +1015,14 @@ static int cortex_m_step(struct target *target, int current,
                                                        "leaving target running");
                                        } else {
                                                /* Step over next instruction with interrupts disabled */
                                                        "leaving target running");
                                        } else {
                                                /* Step over next instruction with interrupts disabled */
+                                               cortex_m_set_maskints_for_step(target);
                                                cortex_m_write_debug_halt_mask(target,
                                                        C_HALT | C_MASKINTS,
                                                        0);
                                                cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
                                                cortex_m_write_debug_halt_mask(target,
                                                        C_HALT | C_MASKINTS,
                                                        0);
                                                cortex_m_write_debug_halt_mask(target, C_STEP, C_HALT);
-                                               /* Re-enable interrupts */
-                                               cortex_m_write_debug_halt_mask(target, C_HALT, C_MASKINTS);
+                                               /* Re-enable interrupts if appropriate */
+                                               cortex_m_write_debug_halt_mask(target, C_HALT, 0);
+                                               cortex_m_set_maskints_for_halt(target);
                                        }
                                }
                        }
                                        }
                                }
                        }
@@ -1031,8 +1124,7 @@ static int cortex_m_assert_reset(struct target *target)
 
        if (!target->reset_halt) {
                /* Set/Clear C_MASKINTS in a separate operation */
 
        if (!target->reset_halt) {
                /* Set/Clear C_MASKINTS in a separate operation */
-               if (cortex_m->dcb_dhcsr & C_MASKINTS)
-                       cortex_m_write_debug_halt_mask(target, 0, C_MASKINTS);
+               cortex_m_set_maskints_for_run(target);
 
                /* clear any debug flags before resuming */
                cortex_m_clear_halt(target);
 
                /* clear any debug flags before resuming */
                cortex_m_clear_halt(target);
@@ -2020,12 +2112,15 @@ int cortex_m_examine(struct target *target)
 
                LOG_DEBUG("Cortex-M%d r%" PRId8 "p%" PRId8 " processor detected",
                                i, (uint8_t)((cpuid >> 20) & 0xf), (uint8_t)((cpuid >> 0) & 0xf));
 
                LOG_DEBUG("Cortex-M%d r%" PRId8 "p%" PRId8 " processor detected",
                                i, (uint8_t)((cpuid >> 20) & 0xf), (uint8_t)((cpuid >> 0) & 0xf));
+               cortex_m->maskints_erratum = false;
                if (i == 7) {
                        uint8_t rev, patch;
                        rev = (cpuid >> 20) & 0xf;
                        patch = (cpuid >> 0) & 0xf;
                if (i == 7) {
                        uint8_t rev, patch;
                        rev = (cpuid >> 20) & 0xf;
                        patch = (cpuid >> 0) & 0xf;
-                       if ((rev == 0) && (patch < 2))
-                               LOG_WARNING("Silicon bug: single stepping will enter pending exception handler!");
+                       if ((rev == 0) && (patch < 2)) {
+                               LOG_WARNING("Silicon bug: single stepping may enter pending exception handler!");
+                               cortex_m->maskints_erratum = true;
+                       }
                }
                LOG_DEBUG("cpuid: 0x%8.8" PRIx32 "", cpuid);
 
                }
                LOG_DEBUG("cpuid: 0x%8.8" PRIx32 "", cpuid);
 
@@ -2381,6 +2476,7 @@ COMMAND_HANDLER(handle_cortex_m_mask_interrupts_command)
                { .name = "auto", .value = CORTEX_M_ISRMASK_AUTO },
                { .name = "off", .value = CORTEX_M_ISRMASK_OFF },
                { .name = "on", .value = CORTEX_M_ISRMASK_ON },
                { .name = "auto", .value = CORTEX_M_ISRMASK_AUTO },
                { .name = "off", .value = CORTEX_M_ISRMASK_OFF },
                { .name = "on", .value = CORTEX_M_ISRMASK_ON },
+               { .name = "steponly", .value = CORTEX_M_ISRMASK_STEPONLY },
                { .name = NULL, .value = -1 },
        };
        const Jim_Nvp *n;
                { .name = NULL, .value = -1 },
        };
        const Jim_Nvp *n;
@@ -2400,12 +2496,7 @@ COMMAND_HANDLER(handle_cortex_m_mask_interrupts_command)
                if (n->name == NULL)
                        return ERROR_COMMAND_SYNTAX_ERROR;
                cortex_m->isrmasking_mode = n->value;
                if (n->name == NULL)
                        return ERROR_COMMAND_SYNTAX_ERROR;
                cortex_m->isrmasking_mode = n->value;
-
-
-               if (cortex_m->isrmasking_mode == CORTEX_M_ISRMASK_ON)
-                       cortex_m_write_debug_halt_mask(target, C_HALT | C_MASKINTS, 0);
-               else
-                       cortex_m_write_debug_halt_mask(target, C_HALT, C_MASKINTS);
+               cortex_m_set_maskints_for_halt(target);
        }
 
        n = Jim_Nvp_value2name_simple(nvp_maskisr_modes, cortex_m->isrmasking_mode);
        }
 
        n = Jim_Nvp_value2name_simple(nvp_maskisr_modes, cortex_m->isrmasking_mode);
@@ -2465,7 +2556,7 @@ static const struct command_registration cortex_m_exec_command_handlers[] = {
                .handler = handle_cortex_m_mask_interrupts_command,
                .mode = COMMAND_EXEC,
                .help = "mask cortex_m interrupts",
                .handler = handle_cortex_m_mask_interrupts_command,
                .mode = COMMAND_EXEC,
                .help = "mask cortex_m interrupts",
-               .usage = "['auto'|'on'|'off']",
+               .usage = "['auto'|'on'|'off'|'steponly']",
        },
        {
                .name = "vector_catch",
        },
        {
                .name = "vector_catch",
index 4b207467eaf996cc944939fd776f3df1d6b7c123..c33486273fc08f44eae89db6a2336238f69bd444 100644 (file)
@@ -159,6 +159,7 @@ enum cortex_m_isrmasking_mode {
        CORTEX_M_ISRMASK_AUTO,
        CORTEX_M_ISRMASK_OFF,
        CORTEX_M_ISRMASK_ON,
        CORTEX_M_ISRMASK_AUTO,
        CORTEX_M_ISRMASK_OFF,
        CORTEX_M_ISRMASK_ON,
+       CORTEX_M_ISRMASK_STEPONLY,
 };
 
 struct cortex_m_common {
 };
 
 struct cortex_m_common {
@@ -190,6 +191,10 @@ struct cortex_m_common {
        struct armv7m_common armv7m;
 
        int apsel;
        struct armv7m_common armv7m;
 
        int apsel;
+
+       /* Whether this target has the erratum that makes C_MASKINTS not apply to
+        * already pending interrupts */
+       bool maskints_erratum;
 };
 
 static inline struct cortex_m_common *
 };
 
 static inline struct cortex_m_common *

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)