target/cortex_m: Implement maskisr steponly option

`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

@@ -9078,7 +9078,7 @@ Enable or disable trace output for all ITM stimulus ports.
 @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
@@ -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.
 
+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

src/target/cortex_m.c

@@ -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);
 }
 
+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);
@@ -237,11 +314,8 @@ static int cortex_m_endreset_event(struct target *target)
 			return retval;
 	}
 
-	/* Restore proper interrupt masking setting. */
+	/* Restore proper interrupt masking setting for running CPU. */
-	if (cortex_m->isrmasking_mode == CORTEX_M_ISRMASK_ON)
+	cortex_m_set_maskints_for_run(target);
-		cortex_m_write_debug_halt_mask(target, C_MASKINTS, 0);
-	else
-		cortex_m_write_debug_halt_mask(target, 0, C_MASKINTS);
 
 	/* 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(" ");
 
+	/* 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)
@@ -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);
 
+	/* 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;
@@ -767,6 +849,7 @@ static int cortex_m_resume(struct target *target, int current,
 	}
 
 	/* 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;
@@ -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) {
-		/* 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);
-		else {
+		} else {
 			/* 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);
-				/* Re-enable interrupts */
+				/* Re-enable interrupts if appropriate */
-				cortex_m_write_debug_halt_mask(target, C_HALT, C_MASKINTS);
+				cortex_m_write_debug_halt_mask(target, C_HALT, 0);
+				cortex_m_set_maskints_for_halt(target);
 			}
 			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 */
-				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);
-				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();
+					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 */
@@ -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 */
+						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);
-						/* Re-enable interrupts */
+						/* Re-enable interrupts if appropriate */
-						cortex_m_write_debug_halt_mask(target, C_HALT, C_MASKINTS);
+						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 (cortex_m->dcb_dhcsr & C_MASKINTS)
+		cortex_m_set_maskints_for_run(target);
-			cortex_m_write_debug_halt_mask(target, 0, C_MASKINTS);
 
 		/* 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));
+		cortex_m->maskints_erratum = false;
 		if (i == 7) {
 			uint8_t rev, patch;
 			rev = (cpuid >> 20) & 0xf;
 			patch = (cpuid >> 0) & 0xf;
-			if ((rev == 0) && (patch < 2))
+			if ((rev == 0) && (patch < 2)) {
-				LOG_WARNING("Silicon bug: single stepping will enter pending exception handler!");
+				LOG_WARNING("Silicon bug: single stepping may enter pending exception handler!");
+				cortex_m->maskints_erratum = true;
+			}
 		}
 		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 = "steponly", .value = CORTEX_M_ISRMASK_STEPONLY },
 		{ .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;
-
+		cortex_m_set_maskints_for_halt(target);
-
-		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);
 	}
 
 	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",
-		.usage = "['auto'|'on'|'off']",
+		.usage = "['auto'|'on'|'off'|'steponly']",
 	},
 	{
 		.name = "vector_catch",

src/target/cortex_m.h

@@ -159,6 +159,7 @@ enum cortex_m_isrmasking_mode {
 	CORTEX_M_ISRMASK_AUTO,
 	CORTEX_M_ISRMASK_OFF,
 	CORTEX_M_ISRMASK_ON,
+	CORTEX_M_ISRMASK_STEPONLY,
 };
 
 struct cortex_m_common {
@@ -190,6 +191,10 @@ struct cortex_m_common {
 	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 *