Merge pull request #69 from riscv/multi-gdb

Fix the multi-GDB mode bugs

Author: palmer-dabbelt

src/target/riscv/riscv-013.c

@@ -706,7 +706,7 @@ static int init_target(struct command_context *cmd_ctx,
 	LOG_DEBUG("init");
 	riscv_info_t *generic_info = (riscv_info_t *) target->arch_info;
 
-	riscv_info_init(generic_info);
+	riscv_info_init(target, generic_info);
 	generic_info->get_register = &riscv013_get_register;
 	generic_info->set_register = &riscv013_set_register;
 	generic_info->select_current_hart = &riscv013_select_current_hart;
@@ -832,6 +832,9 @@ static int add_trigger(struct target *target, struct trigger *trigger)
 
 		uint64_t tdata1_rb;
 		for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) {
+			if (!riscv_hart_enabled(target, hartid))
+				continue;
+
 			riscv_set_current_hartid(target, hartid);
 
 			if (hartid > 0) {
@@ -918,6 +921,9 @@ static int remove_trigger(struct target *target, struct trigger *trigger)
 	}
 	LOG_DEBUG("Stop using resource %d for bp %d", i, trigger->unique_id);
 	for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) {
+		if (!riscv_hart_enabled(target, hartid))
+			continue;
+
 		riscv_set_current_hartid(target, hartid);
 		register_write_direct(target, GDB_REGNO_TSELECT, i);
 		register_write_direct(target, GDB_REGNO_TDATA1, 0);
@@ -1123,17 +1129,23 @@ static int examine(struct target *target)
 
 	/* Before doing anything else we must first enumerate the harts. */
 	RISCV_INFO(r);
-	if (riscv_rtos_enabled(target)) {
-		for (int i = 0; i < RISCV_MAX_HARTS; ++i) {
-			riscv_set_current_hartid(target, i);
-			uint32_t s = dmi_read(target, DMI_DMSTATUS);
-			if (get_field(s, DMI_DMSTATUS_ANYNONEXISTENT))
-				break;
-			r->hart_count = i + 1;
+	int original_coreid = target->coreid;
+	for (int i = 0; i < RISCV_MAX_HARTS; ++i) {
+		/* Fake being a non-RTOS targeted to this core so we can see if
+		 * it exists.  This avoids the assertion in
+		 * riscv_set_current_hartid() that ensures non-RTOS targets
+		 * don't touch the harts they're not assigned to.  */
+		target->coreid = i;
+		r->hart_count = i + 1;
+		riscv_set_current_hartid(target, i);
+
+		uint32_t s = dmi_read(target, DMI_DMSTATUS);
+		if (get_field(s, DMI_DMSTATUS_ANYNONEXISTENT)) {
+			r->hart_count--;
+			break;
 		}
-	} else {
-		r->hart_count = 1;
 	}
+	target->coreid = original_coreid;
 
 	LOG_DEBUG("Enumerated %d harts", r->hart_count);
 
@@ -1143,6 +1155,9 @@ static int examine(struct target *target)
 	/* Find the address of the program buffer, which must be done without
 	 * knowing anything about the target. */
 	for (int i = 0; i < riscv_count_harts(target); ++i) {
+		if (!riscv_hart_enabled(target, i))
+			continue;
+
 		riscv_set_current_hartid(target, i);
 
 		/* Without knowing anything else we can at least mess with the
@@ -1225,6 +1240,9 @@ static int examine(struct target *target)
 
 	/* Then we check the number of triggers availiable to each hart. */
 	for (int i = 0; i < riscv_count_harts(target); ++i) {
+		if (!riscv_hart_enabled(target, i))
+			continue;
+
 		for (uint32_t t = 0; t < RISCV_MAX_TRIGGERS; ++t) {
 			riscv_set_current_hartid(target, i);
 
@@ -1239,6 +1257,7 @@ static int examine(struct target *target)
 
 	/* Resumes all the harts, so the debugger can later pause them. */
 	riscv_resume_all_harts(target);
+	target->state = TARGET_RUNNING;
 	target_set_examined(target);
 
 	if (target->rtos) {
@@ -1333,9 +1352,6 @@ static int read_memory(struct target *target, target_addr_t address,
 			size, address);
 
 	select_dmi(target);
-	/* There was a bug in the memory system and only accesses from hart 0 actually
-	 * worked correctly.  This should be obselete now. -palmer */
-	riscv_set_current_hartid(target, 0);
 
 	/* This program uses two temporary registers.  A word of data and the
 	 * associated address are stored at some location in memory.  The
@@ -1531,9 +1547,6 @@ static int write_memory(struct target *target, target_addr_t address,
 	LOG_DEBUG("writing %d words of %d bytes to 0x%08lx", count, size, (long)address);
 
 	select_dmi(target);
-	/* There was a bug in the memory system and only accesses from hart 0 actually
-	 * worked correctly.  This should be obselete now. -palmer */
-	riscv_set_current_hartid(target, 0);
 
 	/* This program uses two temporary registers.  A word of data and the
 	 * associated address are stored at some location in memory.  The

src/target/riscv/riscv.c

@@ -448,11 +448,12 @@ static int riscv_get_gdb_reg_list(struct target *target,
 {
 	RISCV_INFO(r);
 	LOG_DEBUG("reg_class=%d", reg_class);
-	LOG_DEBUG("riscv_get_gdb_reg_list: rtos_hartid=%d current_hartid=%d", r->rtos_hartid, r->current_hartid);
-	if (r->rtos_hartid != -1)
+	LOG_DEBUG("rtos_hartid=%d current_hartid=%d", r->rtos_hartid, r->current_hartid);
+
+	if (r->rtos_hartid != -1 && riscv_rtos_enabled(target))
 		riscv_set_current_hartid(target, r->rtos_hartid);
 	else
-		riscv_set_current_hartid(target, 0);
+		riscv_set_current_hartid(target, target->coreid);
 
 	switch (reg_class) {
 		case REG_CLASS_GENERAL:
@@ -872,11 +873,12 @@ struct target_type riscv_target =
 
 /*** RISC-V Interface ***/
 
-void riscv_info_init(riscv_info_t *r)
+void riscv_info_init(struct target *target, riscv_info_t *r)
 {
 	memset(r, 0, sizeof(*r));
 	r->dtm_version = 1;
 	r->registers_initialized = false;
+	r->current_hartid = target->coreid;
 
 	for (size_t h = 0; h < RISCV_MAX_HARTS; ++h) {
 		r->xlen[h] = -1;
@@ -889,11 +891,11 @@ void riscv_info_init(riscv_info_t *r)
 
 int riscv_halt_all_harts(struct target *target)
 {
-	if (riscv_rtos_enabled(target)) {
-		for (int i = 0; i < riscv_count_harts(target); ++i)
-			riscv_halt_one_hart(target, i);
-	} else {
-		riscv_halt_one_hart(target, riscv_current_hartid(target));
+	for (int i = 0; i < riscv_count_harts(target); ++i) {
+		if (!riscv_hart_enabled(target, i))
+			continue;
+
+		riscv_halt_one_hart(target, i);
 	}
 
 	return ERROR_OK;
@@ -915,11 +917,11 @@ int riscv_halt_one_hart(struct target *target, int hartid)
 
 int riscv_resume_all_harts(struct target *target)
 {
-	if (riscv_rtos_enabled(target)) {
-		for (int i = 0; i < riscv_count_harts(target); ++i)
-			riscv_resume_one_hart(target, i);
-	} else {
-		riscv_resume_one_hart(target, riscv_current_hartid(target));
+	for (int i = 0; i < riscv_count_harts(target); ++i) {
+		if (!riscv_hart_enabled(target, i))
+			continue;
+
+		riscv_resume_one_hart(target, i);
 	}
 
 	riscv_invalidate_register_cache(target);
@@ -943,11 +945,11 @@ int riscv_resume_one_hart(struct target *target, int hartid)
 
 int riscv_reset_all_harts(struct target *target)
 {
-	if (riscv_rtos_enabled(target)) {
-		for (int i = 0; i < riscv_count_harts(target); ++i)
-			riscv_reset_one_hart(target, i);
-	} else {
-		riscv_reset_one_hart(target, riscv_current_hartid(target));
+	for (int i = 0; i < riscv_count_harts(target); ++i) {
+		if (!riscv_hart_enabled(target, i))
+			continue;
+
+		riscv_reset_one_hart(target, i);
 	}
 
 	riscv_invalidate_register_cache(target);
@@ -1017,10 +1019,9 @@ void riscv_set_current_hartid(struct target *target, int hartid)
 
 	int previous_hartid = riscv_current_hartid(target);
 	r->current_hartid = hartid;
-	assert(riscv_rtos_enabled(target) || target->coreid == hartid);
+	assert(riscv_hart_enabled(target, hartid));
 	LOG_DEBUG("setting hartid to %d, was %d", hartid, previous_hartid);
-	if (riscv_rtos_enabled(target))
-		r->select_current_hart(target);
+	r->select_current_hart(target);
 
 	/* This might get called during init, in which case we shouldn't be
 	 * setting up the register cache. */
@@ -1068,10 +1069,7 @@ void riscv_invalidate_register_cache(struct target *target)
 int riscv_current_hartid(const struct target *target)
 {
 	RISCV_INFO(r);
-	if (riscv_rtos_enabled(target))
-		return r->current_hartid;
-	else
-		return target->coreid;
+	return r->current_hartid;
 }
 
 void riscv_set_all_rtos_harts(struct target *target)
@@ -1236,3 +1234,12 @@ int riscv_dmi_write_u64_bits(struct target *target)
 	RISCV_INFO(r);
 	return r->dmi_write_u64_bits(target);
 }
+
+bool riscv_hart_enabled(struct target *target, int hartid)
+{
+	/* FIXME: Add a hart mask to the RTOS. */
+	if (riscv_rtos_enabled(target))
+		return hartid < riscv_count_harts(target);
+
+	return hartid == target->coreid;
+}

src/target/riscv/riscv.h

@@ -134,7 +134,7 @@ int riscv_openocd_deassert_reset(struct target *target);
 /*** RISC-V Interface ***/
 
 /* Initializes the shared RISC-V structure. */
-void riscv_info_init(riscv_info_t *r);
+void riscv_info_init(struct target *target, riscv_info_t *r);
 
 /* Run control, possibly for multiple harts.  The _all_harts versions resume
  * all the enabled harts, which when running in RTOS mode is all the harts on
@@ -215,4 +215,7 @@ int riscv_dmi_write_u64_bits(struct target *target);
 /* Invalidates the register cache. */
 void riscv_invalidate_register_cache(struct target *target);
 
+/* Returns TRUE when a hart is enabled in this target. */
+bool riscv_hart_enabled(struct target *target, int hartid);
+
 #endif