Remove the hart_state variablenohartstatus

The new code always keeps harts in consistant states, so it's no longer
necessary to track their states manually.  There's still some flakyness
with halt/resume over breakpoints.

4 files changed, 47 insertions, 88 deletions

diff --git a/src/rtos/riscv_debug.c b/src/rtos/riscv_debug.c
index 3677466..b2ef66e 100644
--- a/src/rtos/riscv_debug.c
+++ b/src/rtos/riscv_debug.c
@@ -248,10 +248,12 @@ static int riscv_gdb_v_packet(struct connection *connection, const char *packet,
 	}
 
 	if (strcmp(packet_stttrr, "vCont;c") == 0) {
-		riscv_resume_all_harts(target);
 		target->state = TARGET_RUNNING;
-		target_call_event_callbacks(target, TARGET_EVENT_GDB_START);
 		gdb_set_frontend_state_running(connection);
+		target_call_event_callbacks(target, TARGET_EVENT_GDB_START);
+		target_call_event_callbacks(target, TARGET_EVENT_RESUME_START);
+		riscv_resume_all_harts(target);
+		target_call_event_callbacks(target, TARGET_EVENT_RESUME_END);
 		return JIM_OK;
 	}
 
diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c
index 58ad937..105a860 100644
--- a/src/target/riscv/riscv.c
+++ b/src/target/riscv/riscv.c
@@ -616,29 +616,19 @@ struct target_type riscv_target =
 static int riscv_poll_hart(struct target *target, int hartid)
 {
 	RISCV_INFO(r);
-	LOG_DEBUG("polling hart %d", hartid);
-
-	/* Polling can only detect one state change: a hart that was previously
-	 * running but has gone to sleep.  A state change in the other
-	 * direction is invalid and indicates that one of the previous calls
-	 * didn't correctly block. */
 	riscv_set_current_hartid(target, hartid);
-	if (riscv_was_halted(target) && !riscv_is_halted(target)) {
-		LOG_ERROR("unexpected wakeup on hart %d", hartid);
-		abort();
-	}
 
-	/* If there's no new event then there's nothing to do. */
-	if (riscv_was_halted(target) || !riscv_is_halted(target))
-		return 0;
+	LOG_INFO("polling hart %d, target->state=%d (TARGET_HALTED=%d)", hartid, target->state, TARGET_HALTED);
 
-	/* If we got here then this must be the first poll during which this
-	 * hart halted.  We need to synchronize the hart's state with the
-	 * debugger, and inform the outer polling loop that there's something
-	 * to do. */
-	r->hart_state[hartid] = RISCV_HART_HALTED;
-	r->on_halt(target);
-	return 1;
+	/* If OpenOCD this we're running but this hart is halted then it's time
+	 * to raise an event. */
+	if (target->state != TARGET_HALTED && riscv_is_halted(target)) {
+		LOG_INFO("  triggered a halt");
+		r->on_halt(target);
+		return 1;
+	}
+
+	return 0;
 }
 
 /*** OpenOCD Interface ***/
@@ -661,7 +651,7 @@ int riscv_openocd_poll(struct target *target)
 			}
 		}
 		if (triggered_hart == -1) {
-			LOG_DEBUG("  no harts halted");
+			LOG_DEBUG("  no harts just halted, target->state=%d", target->state);
 			return ERROR_OK;
 		}
 		LOG_DEBUG("  hart %d halted", triggered_hart);
@@ -691,6 +681,7 @@ int riscv_openocd_poll(struct target *target)
 		target->rtos->current_threadid = triggered_hart + 1;
 		target->rtos->current_thread = triggered_hart + 1;
 
+		target->state = TARGET_HALTED;
 		target_call_event_callbacks(target, TARGET_EVENT_HALTED);
 		return ERROR_OK;
 	} else {
@@ -700,13 +691,16 @@ int riscv_openocd_poll(struct target *target)
 
 int riscv_openocd_halt(struct target *target)
 {
+	LOG_DEBUG("halting all harts");
+
 	int out = riscv_halt_all_harts(target);
-	if (out != ERROR_OK)
+	if (out != ERROR_OK) {
+		LOG_ERROR("Unable to halt all harts");
 		return out;
+	}
 
-	/* Don't change the target state right here, it'll get updated by the
-	 * poll. */
-	riscv_openocd_poll(target);
+	target->state = TARGET_HALTED;
+	target_call_event_callbacks(target, TARGET_EVENT_HALTED);
 	return out;
 }
 
@@ -717,17 +711,21 @@ int riscv_openocd_resume(
         int handle_breakpoints,
         int debug_execution
 ) {
+	LOG_DEBUG("resuming all harts");
+
 	if (!current) {
 		LOG_ERROR("resume-at-pc unimplemented");
 		return ERROR_FAIL;
 	}
 
 	int out = riscv_resume_all_harts(target);
-	if (out != ERROR_OK)
+	if (out != ERROR_OK) {
+		LOG_ERROR("unable to resume all harts");
 		return out;
+	}
 
 	target->state = TARGET_RUNNING;
-	riscv_openocd_poll(target);
+	target_call_event_callbacks(target, TARGET_EVENT_RESUMED);
 	return out;
 }
 
@@ -737,6 +735,8 @@ int riscv_openocd_step(
         uint32_t address,
         int handle_breakpoints
 ) {
+	LOG_DEBUG("stepping rtos hart");
+
 	RISCV_INFO(r);
 
 	if (!current) {
@@ -745,13 +745,10 @@ int riscv_openocd_step(
 	}
 
 	int out = riscv_step_rtos_hart(target);
-	if (out != ERROR_OK)
+	if (out != ERROR_OK) {
+		LOG_ERROR("unable to step rtos hart");
 		return out;
-
-	/* step_rtos_hart blocks until the hart has actually stepped, but we
-	 * need to cycle through OpenOCD to actually get this to trigger. */
-	target->state = TARGET_RUNNING;
-	riscv_openocd_poll(target);
+	}
 
 	return out;
 }
@@ -768,7 +765,6 @@ void riscv_info_init(riscv_info_t *r)
 
 	for (size_t h = 0; h < RISCV_MAX_HARTS; ++h) {
 		r->xlen[h] = -1;
-		r->hart_state[h] = RISCV_HART_UNKNOWN;
 		r->debug_buffer_addr[h] = -1;
 
 		for (size_t e = 0; e < RISCV_MAX_REGISTERS; ++e)
@@ -793,23 +789,12 @@ int riscv_halt_one_hart(struct target *target, int hartid)
 	RISCV_INFO(r);
 	LOG_DEBUG("halting hart %d", hartid);
 	riscv_set_current_hartid(target, hartid);
-
-	if (r->hart_state[hartid] == RISCV_HART_UNKNOWN) {
-		r->hart_state[hartid] = riscv_is_halted(target) ? RISCV_HART_HALTED : RISCV_HART_RUNNING;
-		if (riscv_was_halted(target)) {
-			LOG_WARNING("Connected to hart %d, which was halted.  s0, s1, and pc were overwritten by your previous debugger session and cannot be restored.", hartid);
-			r->on_halt(target);
-		}
-	}
-
-	if (riscv_was_halted(target)) {
+	if (riscv_is_halted(target)) {
 		LOG_DEBUG("  hart %d requested halt, but was already halted", hartid);
 		return ERROR_OK;
 	}
 
 	r->halt_current_hart(target);
-	/* Here we don't actually update 'hart_state' because we want poll to
-	 * pick that up.  We can't actually wait until */
 	return ERROR_OK;
 }
 
@@ -830,23 +815,13 @@ int riscv_resume_one_hart(struct target *target, int hartid)
 	RISCV_INFO(r);
 	LOG_DEBUG("resuming hart %d", hartid);
 	riscv_set_current_hartid(target, hartid);
-
-	if (r->hart_state[hartid] == RISCV_HART_UNKNOWN) {
-		r->hart_state[hartid] = riscv_is_halted(target) ? RISCV_HART_HALTED : RISCV_HART_RUNNING;
-		if (!riscv_was_halted(target)) {
-			LOG_ERROR("Asked to resume hart %d, which was in an unknown state", hartid);
-			r->on_resume(target);
-		}
-	}
-
-	if (!riscv_was_halted(target)) {
+	if (!riscv_is_halted(target)) {
 		LOG_DEBUG("  hart %d requested resume, but was already resumed", hartid);
 		return ERROR_OK;
 	}
 
 	r->on_resume(target);
 	r->resume_current_hart(target);
-	r->hart_state[hartid] = RISCV_HART_RUNNING;
 	return ERROR_OK;
 }
 
@@ -864,12 +839,10 @@ int riscv_step_rtos_hart(struct target *target)
 	riscv_set_current_hartid(target, hartid);
 	LOG_DEBUG("stepping hart %d", hartid);
 
-	assert(r->hart_state[hartid] == RISCV_HART_HALTED);
+	assert(riscv_is_halted(target));
 	r->on_step(target);
 	r->step_current_hart(target);
-	r->hart_state[hartid] = RISCV_HART_RUNNING;
 	r->on_halt(target);
-	r->hart_state[hartid] = RISCV_HART_HALTED;
 	assert(riscv_is_halted(target));
 	return ERROR_OK;
 }
@@ -991,13 +964,6 @@ bool riscv_is_halted(struct target *target)
 	return r->is_halted(target);
 }
 
-bool riscv_was_halted(struct target *target)
-{
-	RISCV_INFO(r);
-	assert(r->hart_state[r->current_hartid] != RISCV_HART_UNKNOWN);
-	return r->hart_state[r->current_hartid] == RISCV_HART_HALTED;
-}
-
 enum riscv_halt_reason riscv_halt_reason(struct target *target, int hartid)
 {
 	RISCV_INFO(r);
diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h
index 84ed411..3aedf2d 100644
--- a/src/target/riscv/riscv.h
+++ b/src/target/riscv/riscv.h
@@ -22,12 +22,6 @@ typedef uint64_t riscv_reg_t;
 typedef uint32_t riscv_insn_t;
 typedef  int64_t riscv_addr_t;
 
-enum riscv_hart_state {
-	RISCV_HART_UNKNOWN,
-	RISCV_HART_HALTED,
-	RISCV_HART_RUNNING,
-};
-
 enum riscv_halt_reason {
 	RISCV_HALT_INTERRUPT,
 	RISCV_HALT_BREAKPOINT,
@@ -67,9 +61,6 @@ typedef struct {
 	/* It's possible that each core has a different supported ISA set. */
 	int xlen[RISCV_MAX_HARTS];
 
-	/* The state of every hart. */
-	enum riscv_hart_state hart_state[RISCV_MAX_HARTS];
-
 	/* The number of triggers per hart. */
 	int trigger_count[RISCV_MAX_HARTS];
 
@@ -188,9 +179,8 @@ riscv_reg_t riscv_get_register(struct target *target, enum gdb_regno i);
 riscv_reg_t riscv_get_register_on_hart(struct target *target, int hid, enum gdb_regno rid);
 
 /* Checks the state of the current hart -- "is_halted" checks the actual
- * on-device register, while "was_halted" checks the machine's state. */
+ * on-device register. */
 bool riscv_is_halted(struct target *target);
-bool riscv_was_halted(struct target *target);
 enum riscv_halt_reason riscv_halt_reason(struct target *target, int hartid);
 
 /* Returns the number of triggers availiable to either the current hart or to
diff --git a/src/target/target.c b/src/target/target.c
index 42a8cac..9c0bbcf 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -1088,7 +1088,7 @@ int target_add_breakpoint(struct target *target,
 		struct breakpoint *breakpoint)
 {
 	if ((target->state != TARGET_HALTED) && (breakpoint->type != BKPT_HARD)) {
-		LOG_WARNING("target %s is not halted", target_name(target));
+		LOG_WARNING("target %s is not halted (add breakpoint)", target_name(target));
 		return ERROR_TARGET_NOT_HALTED;
 	}
 	return target->type->add_breakpoint(target, breakpoint);
@@ -1098,7 +1098,7 @@ int target_add_context_breakpoint(struct target *target,
 		struct breakpoint *breakpoint)
 {
 	if (target->state != TARGET_HALTED) {
-		LOG_WARNING("target %s is not halted", target_name(target));
+		LOG_WARNING("target %s is not halted (add context breakpoint)", target_name(target));
 		return ERROR_TARGET_NOT_HALTED;
 	}
 	return target->type->add_context_breakpoint(target, breakpoint);
@@ -1108,7 +1108,7 @@ int target_add_hybrid_breakpoint(struct target *target,
 		struct breakpoint *breakpoint)
 {
 	if (target->state != TARGET_HALTED) {
-		LOG_WARNING("target %s is not halted", target_name(target));
+		LOG_WARNING("target %s is not halted (add hybrid breakpoint)", target_name(target));
 		return ERROR_TARGET_NOT_HALTED;
 	}
 	return target->type->add_hybrid_breakpoint(target, breakpoint);
@@ -1124,7 +1124,7 @@ int target_add_watchpoint(struct target *target,
 		struct watchpoint *watchpoint)
 {
 	if (target->state != TARGET_HALTED) {
-		LOG_WARNING("target %s is not halted", target_name(target));
+		LOG_WARNING("target %s is not halted (add watchpoint)", target_name(target));
 		return ERROR_TARGET_NOT_HALTED;
 	}
 	return target->type->add_watchpoint(target, watchpoint);
@@ -1138,7 +1138,7 @@ int target_hit_watchpoint(struct target *target,
 		struct watchpoint **hit_watchpoint)
 {
 	if (target->state != TARGET_HALTED) {
-		LOG_WARNING("target %s is not halted", target->cmd_name);
+		LOG_WARNING("target %s is not halted (hit watchpoint)", target->cmd_name);
 		return ERROR_TARGET_NOT_HALTED;
 	}
 
@@ -1167,7 +1167,8 @@ int target_step(struct target *target,
 int target_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fileio_info)
 {
 	if (target->state != TARGET_HALTED) {
-		LOG_WARNING("target %s is not halted", target->cmd_name);
+		LOG_WARNING("target %s is not halted (gdb fileio)", target->cmd_name);
+		abort();
 		return ERROR_TARGET_NOT_HALTED;
 	}
 	return target->type->get_gdb_fileio_info(target, fileio_info);
@@ -1176,7 +1177,7 @@ int target_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fi
 int target_gdb_fileio_end(struct target *target, int retcode, int fileio_errno, bool ctrl_c)
 {
 	if (target->state != TARGET_HALTED) {
-		LOG_WARNING("target %s is not halted", target->cmd_name);
+		LOG_WARNING("target %s is not halted (gdb fileio end)", target->cmd_name);
 		return ERROR_TARGET_NOT_HALTED;
 	}
 	return target->type->gdb_fileio_end(target, retcode, fileio_errno, ctrl_c);
@@ -1186,7 +1187,7 @@ int target_profiling(struct target *target, uint32_t *samples,
 			uint32_t max_num_samples, uint32_t *num_samples, uint32_t seconds)
 {
 	if (target->state != TARGET_HALTED) {
-		LOG_WARNING("target %s is not halted", target->cmd_name);
+		LOG_WARNING("target %s is not halted (profiling)", target->cmd_name);
 		return ERROR_TARGET_NOT_HALTED;
 	}
 	return target->type->profiling(target, samples, max_num_samples,