Merge pull request #67 from riscv/cosmetics

Various cosmetic improvements

3 files changed, 147 insertions, 77 deletions

diff --git a/src/target/riscv/debug_defines.h b/src/target/riscv/debug_defines.h
index 2fb541b..e5f9291 100644
--- a/src/target/riscv/debug_defines.h
+++ b/src/target/riscv/debug_defines.h
@@ -84,7 +84,7 @@
 /*
 * 0: Version described in spec version 0.11.
 *
-* 1: Version described in spec version 0.12 (and later?), which
+* 1: Version described in spec version 0.13 (and later?), which
 * reduces the DMI data width to 32 bits.
 *
 * Other values are reserved for future use.
@@ -110,7 +110,12 @@
 /*
 * When the debugger writes this field, it has the following meaning:
 *
-* 0: Ignore \Fdata. (nop)
+* 0: Ignore \Fdata and \Faddress. (nop)
+*
+* Don't send anything over the DMI during Update-DR.
+* This operation should never result in a busy or error response.
+* The address and data reported in the following Capture-DR
+* are undefined.
 *
 * 1: Read from \Faddress. (read)
 *
@@ -151,13 +156,11 @@
 /*
 * 0: There is no external debug support.
 *
-* 1: External debug support exists as it is described in this document.
-*
-* Other values are reserved for future standards.
+* 4: External debug support exists as it is described in this document.
  */
-#define CSR_DCSR_XDEBUGVER_OFFSET           30
-#define CSR_DCSR_XDEBUGVER_LENGTH           2
-#define CSR_DCSR_XDEBUGVER                  (0x3 << CSR_DCSR_XDEBUGVER_OFFSET)
+#define CSR_DCSR_XDEBUGVER_OFFSET           28
+#define CSR_DCSR_XDEBUGVER_LENGTH           4
+#define CSR_DCSR_XDEBUGVER                  (0xf << CSR_DCSR_XDEBUGVER_OFFSET)
 /*
 * When 1, {\tt ebreak} instructions in Machine Mode enter Debug Mode.
  */
@@ -230,8 +233,11 @@
 #define CSR_DCSR_CAUSE                      (0x7 << CSR_DCSR_CAUSE_OFFSET)
 /*
 * When set and not in Debug Mode, the hart will only execute a single
-* instruction, and then enter Debug Mode. Interrupts are disabled
-* when this bit is set.
+* instruction and then enter Debug Mode.
+* Interrupts are disabled when this bit is set.
+* If the instruction does not complete due to an exception,
+* the hart will immediately enter Debug Mode before executing
+* the trap handler, with appropriate exception registers set.
  */
 #define CSR_DCSR_STEP_OFFSET                2
 #define CSR_DCSR_STEP_LENGTH                1
@@ -255,16 +261,6 @@
 #define CSR_DPC_DPC                         (((1L<<XLEN)-1) << CSR_DPC_DPC_OFFSET)
 #define CSR_DSCRATCH0                       0x7b2
 #define CSR_DSCRATCH1                       0x7b3
-#define CSR_PRIV                            virtual
-/*
-* Contains the privilege level the hart was operating in when Debug
-* Mode was entered. The encoding is described in Table
-* \ref{tab:privlevel}. A user can write this value to change the
-* hart's privilege level when exiting Debug Mode.
- */
-#define CSR_PRIV_PRV_OFFSET                 0
-#define CSR_PRIV_PRV_LENGTH                 2
-#define CSR_PRIV_PRV                        (0x3 << CSR_PRIV_PRV_OFFSET)
 #define CSR_TSELECT                         0x7a0
 #define CSR_TSELECT_INDEX_OFFSET            0
 #define CSR_TSELECT_INDEX_LENGTH            XLEN
@@ -485,28 +481,28 @@
 #define CSR_ICOUNT_COUNT_LENGTH             14
 #define CSR_ICOUNT_COUNT                    (0x3fffL << CSR_ICOUNT_COUNT_OFFSET)
 /*
-* When set, every instruction completed in M mode decrements \Fcount
+* When set, every instruction completed or exception taken in M mode decrements \Fcount
 * by 1.
  */
 #define CSR_ICOUNT_M_OFFSET                 9
 #define CSR_ICOUNT_M_LENGTH                 1
 #define CSR_ICOUNT_M                        (0x1L << CSR_ICOUNT_M_OFFSET)
 /*
-* When set, every instruction completed in H mode decrements \Fcount
+* When set, every instruction completed or exception taken in in H mode decrements \Fcount
 * by 1.
  */
 #define CSR_ICOUNT_H_OFFSET                 8
 #define CSR_ICOUNT_H_LENGTH                 1
 #define CSR_ICOUNT_H                        (0x1L << CSR_ICOUNT_H_OFFSET)
 /*
-* When set, every instruction completed in S mode decrements \Fcount
+* When set, every instruction completed or exception taken in S mode decrements \Fcount
 * by 1.
  */
 #define CSR_ICOUNT_S_OFFSET                 7
 #define CSR_ICOUNT_S_LENGTH                 1
 #define CSR_ICOUNT_S                        (0x1L << CSR_ICOUNT_S_OFFSET)
 /*
-* When set, every instruction completed in U mode decrements \Fcount
+* When set, every instruction completed or exception taken in U mode decrements \Fcount
 * by 1.
  */
 #define CSR_ICOUNT_U_OFFSET                 6
@@ -619,29 +615,24 @@
 #define DMI_DMSTATUS_CFGSTRVALID_LENGTH     1
 #define DMI_DMSTATUS_CFGSTRVALID            (0x1 << DMI_DMSTATUS_CFGSTRVALID_OFFSET)
 /*
-* Reserved for future use. Reads as 0.
- */
-#define DMI_DMSTATUS_VERSIONHI_OFFSET       2
-#define DMI_DMSTATUS_VERSIONHI_LENGTH       2
-#define DMI_DMSTATUS_VERSIONHI              (0x3 << DMI_DMSTATUS_VERSIONHI_OFFSET)
-/*
-* 00: There is no Debug Module present.
+* 0: There is no Debug Module present.
 *
-* 01: There is a Debug Module and it conforms to version 0.11 of this
+* 1: There is a Debug Module and it conforms to version 0.11 of this
 * specification.
 *
-* 10: There is a Debug Module and it conforms to version 0.13 of this
+* 2: There is a Debug Module and it conforms to version 0.13 of this
 * specification.
-*
-* 11: Reserved for future use.
  */
-#define DMI_DMSTATUS_VERSIONLO_OFFSET       0
-#define DMI_DMSTATUS_VERSIONLO_LENGTH       2
-#define DMI_DMSTATUS_VERSIONLO              (0x3 << DMI_DMSTATUS_VERSIONLO_OFFSET)
+#define DMI_DMSTATUS_VERSION_OFFSET         0
+#define DMI_DMSTATUS_VERSION_LENGTH         4
+#define DMI_DMSTATUS_VERSION                (0xf << DMI_DMSTATUS_VERSION_OFFSET)
 #define DMI_DMCONTROL                       0x10
 /*
-* Halt request signal for all currently selected harts. When set to 1, the
-* hart will halt if it is not currently halted.
+* Halt request signal for all currently selected harts. When set to
+* 1, each selected hart will halt if it is not currently halted.
+*
+* Writing 1 or 0 has no effect on a hart which is already halted, but
+* the bit should be cleared to 0 before the hart is resumed.
 * Setting both \Fhaltreq and \Fresumereq leads to undefined behavior.
 *
 * Writes apply to the new value of \Fhartsel and \Fhasel.
@@ -651,7 +642,7 @@
 #define DMI_DMCONTROL_HALTREQ               (0x1 << DMI_DMCONTROL_HALTREQ_OFFSET)
 /*
 * Resume request signal for all currently selected harts. When set to 1,
-* the hart will resume if it is currently halted.
+* each selected hart will resume if it is currently halted.
 * Setting both \Fhaltreq and \Fresumereq leads to undefined behavior.
 *
 * Writes apply to the new value of \Fhartsel and \Fhasel.
@@ -698,8 +689,11 @@
 #define DMI_DMCONTROL_HARTSEL               (0x3ff << DMI_DMCONTROL_HARTSEL_OFFSET)
 /*
 * This bit controls the reset signal from the DM to the rest of the
-* system. To perform a reset the debugger writes 1, and then writes 0
-* to deassert the reset.
+* system. To perform a system reset the debugger writes 1,
+* and then writes 0
+* to deassert the reset. This bit must not reset the Debug Module
+* registers. What it does reset is platform-specific (it may
+* reset nothing).
  */
 #define DMI_DMCONTROL_NDMRESET_OFFSET       1
 #define DMI_DMCONTROL_NDMRESET_LENGTH       1
@@ -717,8 +711,8 @@
 * Debug Module after power up, including the platform's system reset
 * or Debug Transport reset signals.
 *
-* A debugger should pulse this bit low to ensure that the Debug
-* Module is fully reset and ready to use.
+* A debugger may pulse this bit low to get the debug module into a
+* known state.
 *
 * Implementations may use this bit to aid debugging, for example by
 * preventing the Debug Module from being power gated while debugging
@@ -899,8 +893,9 @@
 *
 * 0 (none): No error.
 *
-* 1 (busy): An abstract command was executing while \Rcommand or one
-* of the {\tt data} registers was accessed.
+* 1 (busy): An abstract command was executing while \Rcommand,
+* \Rabstractcs, \Rabstractauto was written, or when one
+* of the {\tt data} or {\tt progbuf} registers was read or written.
 *
 * 2 (not supported): The requested command is not supported. A
 * command that is not supported while the hart is running may be
@@ -919,7 +914,7 @@
 #define DMI_ABSTRACTCS_CMDERR               (0x7 << DMI_ABSTRACTCS_CMDERR_OFFSET)
 /*
 * Number of {\tt data} registers that are implemented as part of the
-* abstract command interface. Valid sizes are 0 - 8.
+* abstract command interface. Valid sizes are 0 - 12.
  */
 #define DMI_ABSTRACTCS_DATACOUNT_OFFSET     0
 #define DMI_ABSTRACTCS_DATACOUNT_LENGTH     5
@@ -1138,7 +1133,7 @@
 *
 * 3: There was some other error (eg. alignment).
 *
-* 4: The system bus master was busy when a one of the
+* 4: The system bus master was busy when one of the
 * {\tt sbaddress} or {\tt sbdata} registers was written,
 * or the {\tt sbdata} register was read when it had
 * stale data.
@@ -1396,7 +1391,10 @@
 #define AC_ACCESS_REGISTER_WRITE_LENGTH     1
 #define AC_ACCESS_REGISTER_WRITE            (0x1 << AC_ACCESS_REGISTER_WRITE_OFFSET)
 /*
-* Number of the register to access, as described in Table~\ref{tab:regno}.
+* Number of the register to access, as described in
+* Table~\ref{tab:regno}.
+* \Rdpc may be used as an alias for PC if this command is
+* supported on a non-halted hart.
  */
 #define AC_ACCESS_REGISTER_REGNO_OFFSET     0
 #define AC_ACCESS_REGISTER_REGNO_LENGTH     16
@@ -1408,3 +1406,13 @@
 #define AC_QUICK_ACCESS_CMDTYPE_OFFSET      24
 #define AC_QUICK_ACCESS_CMDTYPE_LENGTH      8
 #define AC_QUICK_ACCESS_CMDTYPE             (0xff << AC_QUICK_ACCESS_CMDTYPE_OFFSET)
+#define VIRT_PRIV                           virtual
+/*
+* Contains the privilege level the hart was operating in when Debug
+* Mode was entered. The encoding is described in Table
+* \ref{tab:privlevel}. A user can write this value to change the
+* hart's privilege level when exiting Debug Mode.
+ */
+#define VIRT_PRIV_PRV_OFFSET                0
+#define VIRT_PRIV_PRV_LENGTH                2
+#define VIRT_PRIV_PRV                       (0x3 << VIRT_PRIV_PRV_OFFSET)
diff --git a/src/target/riscv/program.c b/src/target/riscv/program.c
index 6f7b10b..04edf6a 100644
--- a/src/target/riscv/program.c
+++ b/src/target/riscv/program.c
@@ -17,8 +17,6 @@ int riscv_program_lal(struct riscv_program *p, enum gdb_regno d, riscv_addr_t ad
 /* Program interface. */
 int riscv_program_init(struct riscv_program *p, struct target *target)
 {
-	LOG_DEBUG("riscv_program_init: p=%p", p);
-
 	memset(p, 0, sizeof(*p));
 	p->target = target;
 	p->instruction_count = 0;
@@ -72,14 +70,19 @@ int riscv_program_exec(struct riscv_program *p, struct target *t)
 		return ERROR_FAIL;
 	}
 
-	for (size_t i = 0; i < riscv_debug_buffer_size(p->target); ++i) {
-		LOG_DEBUG("Executing program %p: debug_buffer[%02x] = DASM(0x%08lx)", p, (int)i, (long)p->debug_buffer[i]);
-		if (i <= p->instruction_count || i >= riscv_debug_buffer_size(p->target) - p->data_count)
+	for (unsigned i = 0; i < riscv_debug_buffer_size(p->target); ++i) {
+		if (i < p->instruction_count) {
+			LOG_DEBUG("%p: debug_buffer[%02x] = DASM(0x%08x)", p, i, p->debug_buffer[i]);
+			riscv_write_debug_buffer(t, i, p->debug_buffer[i]);
+		}
+		if (i >= riscv_debug_buffer_size(p->target) - p->data_count) {
+			LOG_DEBUG("%p: debug_buffer[%02x] = 0x%08x", p, i, p->debug_buffer[i]);
 			riscv_write_debug_buffer(t, i, p->debug_buffer[i]);
+		}
 	}
 
 	if (riscv_execute_debug_buffer(t) != ERROR_OK) {
-		LOG_DEBUG("Unable to execute program %p", p);
+		LOG_ERROR("Unable to execute program %p", p);
 		return ERROR_FAIL;
 	}
 
@@ -96,8 +99,6 @@ int riscv_program_exec(struct riscv_program *p, struct target *t)
 
 riscv_addr_t riscv_program_alloc_data(struct riscv_program *p, size_t bytes)
 {
-	LOG_DEBUG("allocating %d bytes of data", (int)bytes);
-
 	riscv_addr_t addr = 
 		riscv_debug_buffer_addr(p->target) 
 		+ riscv_debug_buffer_size(p->target) * sizeof(p->debug_buffer[0])
@@ -110,11 +111,10 @@ riscv_addr_t riscv_program_alloc_data(struct riscv_program *p, size_t bytes)
 		+ p->instruction_count * sizeof(p->debug_buffer[0]);
 
 	if (addr <= ptop) {
-		LOG_DEBUG("unable to allocate %d bytes", (int)bytes);
+		LOG_ERROR("unable to allocate %d bytes", (int)bytes);
 		return RISCV_PROGRAM_ALLOC_FAIL;
 	}
 
-	LOG_DEBUG("allocated %d bytes at 0x%08lx", (int)bytes, (long)addr);
 	p->data_count = 
 		+ riscv_debug_buffer_size(p->target)
 		- (addr - riscv_debug_buffer_addr(p->target)) / sizeof(p->debug_buffer[0]);
@@ -474,17 +474,14 @@ int riscv_program_lal(struct riscv_program *p, enum gdb_regno d, riscv_addr_t ad
 
 int riscv_program_insert(struct riscv_program *p, riscv_insn_t i)
 {
-	LOG_DEBUG("instruction_count: %d (p=%p)", (int)p->instruction_count, p);
-
 	if (p->instruction_count + p->data_count + 1 > riscv_debug_buffer_size(p->target)) {
-		LOG_DEBUG("Unable to insert instruction:");
-		LOG_DEBUG("  instruction_count=%d", (int)p->instruction_count);
-		LOG_DEBUG("  data_count       =%d", (int)p->data_count);
-		LOG_DEBUG("  buffer size      =%d", (int)riscv_debug_buffer_size(p->target));
+		LOG_ERROR("Unable to insert instruction:");
+		LOG_ERROR("  instruction_count=%d", (int)p->instruction_count);
+		LOG_ERROR("  data_count       =%d", (int)p->data_count);
+		LOG_ERROR("  buffer size      =%d", (int)riscv_debug_buffer_size(p->target));
 		return ERROR_FAIL;
 	}
 
-	LOG_DEBUG("PROGBUF[%d] = DASM(0x%08x) [0x%08x]", (int)p->instruction_count, i, i);
 	p->debug_buffer[p->instruction_count] = i;
 	p->instruction_count++;
 	return ERROR_OK;
diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c
index fa240df..f1d4cfb 100644
--- a/src/target/riscv/riscv-013.c
+++ b/src/target/riscv/riscv-013.c
@@ -188,6 +188,56 @@ typedef struct {
 	int progbuf_size, progbuf_addr, data_addr, data_size;
 } riscv013_info_t;
 
+static void decode_dmi(char *text, unsigned address, unsigned data)
+{
+	text[0] = 0;
+	switch (address) {
+		case DMI_DMSTATUS:
+			if (get_field(data, DMI_DMSTATUS_ALLRESUMEACK)) {
+				strcat(text, " allresumeack");
+			}
+			if (get_field(data, DMI_DMSTATUS_ANYRESUMEACK)) {
+				strcat(text, " anyresumeack");
+			}
+			if (get_field(data, DMI_DMSTATUS_ALLNONEXISTENT)) {
+				strcat(text, " allnonexistent");
+			}
+			if (get_field(data, DMI_DMSTATUS_ANYNONEXISTENT)) {
+				strcat(text, " anynonexistent");
+			}
+			if (get_field(data, DMI_DMSTATUS_ALLUNAVAIL)) {
+				strcat(text, " allunavail");
+			}
+			if (get_field(data, DMI_DMSTATUS_ANYUNAVAIL)) {
+				strcat(text, " anyunavail");
+			}
+			if (get_field(data, DMI_DMSTATUS_ALLRUNNING)) {
+				strcat(text, " allrunning");
+			}
+			if (get_field(data, DMI_DMSTATUS_ANYRUNNING)) {
+				strcat(text, " anyrunning");
+			}
+			if (get_field(data, DMI_DMSTATUS_ALLHALTED)) {
+				strcat(text, " allhalted");
+			}
+			if (get_field(data, DMI_DMSTATUS_ANYHALTED)) {
+				strcat(text, " anyhalted");
+			}
+			if (get_field(data, DMI_DMSTATUS_AUTHENTICATED)) {
+				strcat(text, " authenticated");
+			}
+			if (get_field(data, DMI_DMSTATUS_AUTHBUSY)) {
+				strcat(text, " authbusy");
+			}
+			if (get_field(data, DMI_DMSTATUS_CFGSTRVALID)) {
+				strcat(text, " cfgstrvalid");
+			}
+			sprintf(text + strlen(text), " version=%d", get_field(data,
+						DMI_DMSTATUS_VERSION));
+			break;
+	}
+}
+
 static void dump_field(const struct scan_field *field)
 {
 	static const char *op_string[] = {"-", "r", "w", "?"};
@@ -213,6 +263,14 @@ static void dump_field(const struct scan_field *field)
 			op_string[out_op], out_data, out_address,
 			status_string[in_op], in_data, in_address);
 
+	char out_text[500];
+	char in_text[500];
+	decode_dmi(out_text, out_address, out_data);
+	decode_dmi(in_text, in_address, in_data);
+	if (in_text[0] || out_text[0]) {
+		log_printf_lf(LOG_LVL_DEBUG, __FILE__, __LINE__, "scan", "%s -> %s",
+				out_text, in_text);
+	}
 }
 
 static riscv013_info_t *get_info(const struct target *target)
@@ -985,9 +1043,9 @@ static int examine(struct target *target)
 
 	uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL);
 	uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS);
-	if (get_field(dmstatus, DMI_DMSTATUS_VERSIONLO) != 2) {
+	if (get_field(dmstatus, DMI_DMSTATUS_VERSION) != 2) {
 		LOG_ERROR("OpenOCD only supports Debug Module version 2, not %d "
-				"(dmstatus=0x%x)", get_field(dmstatus, DMI_DMSTATUS_VERSIONLO), dmstatus);
+				"(dmstatus=0x%x)", get_field(dmstatus, DMI_DMSTATUS_VERSION), dmstatus);
 		return ERROR_FAIL;
 	}
 
@@ -1097,15 +1155,17 @@ static int examine(struct target *target)
 			r->xlen[i] = 64;
 		}
 
-		LOG_DEBUG("hart %d has XLEN=%d", i, r->xlen[i]);
-		LOG_DEBUG("found program buffer at 0x%08lx", (long)(r->debug_buffer_addr[i]));
+		/* Display this as early as possible to help people who are using
+		 * really slow simulators. */
+		LOG_DEBUG(" hart %d: XLEN=%d, program buffer at 0x%" PRIx64, i,
+				r->xlen[i], r->debug_buffer_addr[i]);
 
 		if (riscv_program_gah(&program64, r->debug_buffer_addr[i])) {
 			LOG_ERROR("This implementation will not work with hart %d with debug_buffer_addr of 0x%lx\n", i, 
 					(long)r->debug_buffer_addr[i]);
 			abort();
 		}
-		
+
 		/* Check to see if we can use the data words as an extended
 		 * program buffer or not. */
 		if (r->debug_buffer_addr[i] + (4 * r->debug_buffer_size[i]) == riscv013_data_addr(target)) {
@@ -1132,10 +1192,15 @@ static int examine(struct target *target)
 	riscv_resume_all_harts(target);
 	target_set_examined(target);
 
-	// This print is used by some regression suites to know when
-	// they can connect with gdb/telnet.
-	// We will need to update those suites if we want to remove this line.
-	LOG_INFO("Examined RISC-V core");
+	// Some regression suites rely on seeing 'Examined RISC-V core' to know
+	// when they can connect with gdb/telnet.
+	// We will need to update those suites if we want to change that text.
+	LOG_INFO("Examined RISC-V core; found %d harts",
+			riscv_count_harts(target));
+	for (int i = 0; i < riscv_count_harts(target); ++i) {
+		LOG_INFO(" hart %d: XLEN=%d, program buffer at 0x%" PRIx64, i,
+				r->xlen[i], r->debug_buffer_addr[i]);
+	}
 	return ERROR_OK;
 }
 
@@ -1893,7 +1958,7 @@ static void riscv013_step_or_resume_current_hart(struct target *target, bool ste
 	if (riscv_program_exec(&program, target) != ERROR_OK)
 		abort();
 
-	/* Issue the halt command, and then wait for the current hart to halt. */
+	/* Issue the resume command, and then wait for the current hart to resume. */
 	uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL);
 	dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 1);
 	dmi_write(target, DMI_DMCONTROL, dmcontrol);