Feature/formally parse riscv isa

src/include/buffer_utils.h

@@ -84,4 +84,11 @@ static inline uint64_t read_be8(const uint8_t *const buffer, const size_t offset
 		((uint64_t)buffer[offset + 6] << 8U) | buffer[offset + 7];
 }
 
+static inline size_t write_char(char *const buffer, const size_t buffer_size, const size_t offset, const char c)
+{
+	if (buffer && offset < buffer_size)
+		buffer[offset] = c;
+	return offset + 1U;
+}
+
 #endif /*INCLUDE_BUFFER_UTILS_H*/

src/target/riscv32.c

@@ -40,11 +40,6 @@
 #include "gdb_packet.h"
 #include "adiv5.h"
 
-typedef struct riscv32_regs {
-	uint32_t gprs[32];
-	uint32_t pc;
-} riscv32_regs_s;
-
 /* This defines a match trigger that's for an address or data location */
 #define RV32_MATCH_ADDR_DATA_TRIGGER 0x20000000U
 /* A dmode of 1 restricts the writability of the trigger to debug mode only */
@@ -78,10 +73,11 @@ static int riscv32_breakwatch_clear(target_s *target, breakwatch_s *breakwatch);
 
 bool riscv32_probe(target_s *const target)
 {
+	/* 'E' base ISA has 16 GPRs + PC, 'I' base ISA has 32 GPRs + PC */
+	riscv_hart_s *const hart = riscv_hart_struct(target);
+	target->regs_size = ((hart->extensions & RV_ISA_EXT_EMBEDDED ? 16U : 32U) + 1U) * sizeof(uint32_t);
+
 	/* Finish setting up the target structure with generic rv32 functions */
-	target->core = "rv32";
-	/* Provide the length of a suitable registers structure */
-	target->regs_size = sizeof(riscv32_regs_s);
 	target->regs_read = riscv32_regs_read;
 	target->regs_write = riscv32_regs_write;
 	target->reg_write = riscv32_reg_write;
@@ -115,30 +111,30 @@ static void riscv32_regs_read(target_s *const target, void *const data)
 {
 	/* Grab the hart structure and figure out how many registers need reading out */
 	riscv_hart_s *const hart = riscv_hart_struct(target);
-	riscv32_regs_s *const regs = (riscv32_regs_s *)data;
+	uint32_t *const regs = (uint32_t *)data;
 	const size_t gprs_count = hart->extensions & RV_ISA_EXT_EMBEDDED ? 16U : 32U;
 	/* Loop through reading out the GPRs */
 	for (size_t gpr = 0; gpr < gprs_count; ++gpr) {
 		// TODO: handle when this fails..
-		riscv_csr_read(hart, RV_GPR_BASE + gpr, &regs->gprs[gpr]);
+		riscv_csr_read(hart, RV_GPR_BASE + gpr, &regs[gpr]);
 	}
 	/* Special access to grab the program counter that would be executed on resuming the hart */
-	riscv_csr_read(hart, RV_DPC, &regs->pc);
+	riscv_csr_read(hart, RV_DPC, &regs[gprs_count]);
 }
 
 static void riscv32_regs_write(target_s *const target, const void *const data)
 {
 	/* Grab the hart structure and figure out how many registers need reading out */
 	riscv_hart_s *const hart = riscv_hart_struct(target);
-	const riscv32_regs_s *const regs = (const riscv32_regs_s *)data;
+	const uint32_t *const regs = (const uint32_t *)data;
 	const size_t gprs_count = hart->extensions & RV_ISA_EXT_EMBEDDED ? 16U : 32U;
 	/* Loop through writing out the GPRs, except for the first which is always 0 */
 	for (size_t gpr = 1; gpr < gprs_count; ++gpr) {
 		// TODO: handle when this fails..
-		riscv_csr_write(hart, RV_GPR_BASE + gpr, &regs->gprs[gpr]);
+		riscv_csr_write(hart, RV_GPR_BASE + gpr, &regs[gpr]);
 	}
 	/* Special access to poke in the program counter that will be executed on resuming the hart */
-	riscv_csr_write(hart, RV_DPC, &regs->pc);
+	riscv_csr_write(hart, RV_DPC, &regs[gprs_count]);
 }
 
 static inline size_t riscv32_bool_to_4(const bool ret)

src/target/riscv64.c

@@ -49,9 +49,7 @@ static void riscv64_mem_read(target_s *target, void *dest, target_addr64_t src,
 bool riscv64_probe(target_s *const target)
 {
 	/* Finish setting up the target structure with generic rv64 functions */
-	target->core = "rv64";
-	/* Provide the length of a suitable registers structure */
-	target->regs_size = sizeof(riscv64_regs_s);
+	target->regs_size = sizeof(riscv64_regs_s); /* Provide the length of a suitable registers structure */
 	target->regs_read = riscv64_regs_read;
 	target->regs_write = riscv64_regs_write;
 	target->mem_read = riscv64_mem_read;
@@ -116,7 +114,7 @@ static void riscv64_mem_read(target_s *const target, void *const dest, const tar
 	if (!len)
 		return;
 	riscv_hart_s *const hart = riscv_hart_struct(target);
-	/* Figure out the maxmial width of access to perform, up to the bitness of the target */
+	/* Figure out the maximal width of access to perform, up to the bitness of the target */
 	const uint8_t access_width = riscv_mem_access_width(hart, src, len);
 	const uint8_t access_length = 1U << access_width;
 	/* Build the access command */

src/target/riscv_debug.c

@@ -36,6 +36,7 @@
 #include "target_internal.h"
 #include "gdb_reg.h"
 #include "riscv_debug.h"
+#include "buffer_utils.h"
 
 #include <assert.h>
 
@@ -123,8 +124,6 @@
 #define RV_CSRW_A0 0x00051073U
 #define RV_EBREAK  0x00100073U
 
-#define RV_ISA_EXTENSIONS_MASK 0x03ffffffU
-
 #define RV_VENDOR_JEP106_CONT_MASK 0x7fffff80U
 #define RV_VENDOR_JEP106_CODE_MASK 0x7fU
 
@@ -382,18 +381,17 @@ static void riscv_dm_init(riscv_dm_s *const dbg_module)
 
 static uint8_t riscv_isa_address_width(const uint32_t isa)
 {
-	switch (isa >> 30U) {
-	case 1:
+	switch ((isa & RV_ISA_MXL_MASK) >> RV_ISA_MXL_SHIFT) {
+	case RV_ISA_MXL_32:
 		return 32U;
-	case 2:
+	case RV_ISA_MXL_64:
 		return 64U;
-	case 3:
+	case RV_ISA_MXL_128:
 		return 128U;
 	default:
-		break;
+		DEBUG_INFO("Unknown address width, defaulting to 32\n");
+		return 32U;
 	}
-	DEBUG_INFO("Unknown address width, defaulting to 32\n");
-	return 32U;
 }
 
 static void riscv_hart_read_ids(riscv_hart_s *const hart)
@@ -421,14 +419,66 @@ static void riscv_hart_read_ids(riscv_hart_s *const hart)
 	/* rv128 is unimpl. */
 }
 
+static size_t riscv_snprint_isa_subset(
+	char *const string_buffer, const size_t buffer_size, const uint8_t access_width, const uint32_t extensions)
+{
+	size_t offset = snprintf(string_buffer, buffer_size, "rv%" PRIu8, access_width);
+
+	const bool is_embedded = extensions & RV_ISA_EXT_EMBEDDED;
+
+	offset = write_char(string_buffer, buffer_size, offset, is_embedded ? 'e' : 'i');
+
+	const bool is_general_purpose_isa =
+		!is_embedded && (extensions & RV_ISA_EXT_GENERAL_PURPOSE) == RV_ISA_EXT_GENERAL_PURPOSE;
+
+	if (is_general_purpose_isa) {
+		offset = write_char(string_buffer, buffer_size, offset, 'g');
+		if (extensions & RV_ISA_EXT_QUAD_FLOAT)
+			offset = write_char(string_buffer, buffer_size, offset, 'q');
+	} else {
+		if (extensions & RV_ISA_EXT_MUL_DIV_INT)
+			offset = write_char(string_buffer, buffer_size, offset, 'm');
+		if (extensions & RV_ISA_EXT_ATOMIC)
+			offset = write_char(string_buffer, buffer_size, offset, 'a');
+		if (extensions & RV_ISA_EXT_QUAD_FLOAT)
+			offset = write_char(string_buffer, buffer_size, offset, 'q'); /* Implies d */
+		else if (extensions & RV_ISA_EXT_DOUBLE_FLOAT)
+			offset = write_char(string_buffer, buffer_size, offset, 'd'); /* Implies f */
+		else if (extensions & RV_ISA_EXT_SINGLE_FLOAT)
+			offset = write_char(string_buffer, buffer_size, offset, 'f');
+	}
+	if (extensions & RV_ISA_EXT_DECIMAL_FLOAT)
+		offset = write_char(string_buffer, buffer_size, offset, 'l');
+	if (extensions & RV_ISA_EXT_COMPRESSED)
+		offset = write_char(string_buffer, buffer_size, offset, 'c');
+	if (extensions & RV_ISA_EXT_BIT_MANIP)
+		offset = write_char(string_buffer, buffer_size, offset, 'b');
+	if (extensions & RV_ISA_EXT_DYNAMIC_LANG)
+		offset = write_char(string_buffer, buffer_size, offset, 'j');
+	if (extensions & RV_ISA_EXT_TRANSACT_MEM)
+		offset = write_char(string_buffer, buffer_size, offset, 't');
+	if (extensions & RV_ISA_EXT_PACKED_SIMD)
+		offset = write_char(string_buffer, buffer_size, offset, 'p');
+	if (extensions & RV_ISA_EXT_VECTOR)
+		offset = write_char(string_buffer, buffer_size, offset, 'v');
+	if (extensions & RV_ISA_EXT_USER_INTERRUPTS)
+		offset = write_char(string_buffer, buffer_size, offset, 'n');
+
+	/* null-terminate the string */
+	if (string_buffer && buffer_size > 0)
+		string_buffer[offset < buffer_size ? offset : buffer_size - 1U] = '\0';
+
+	return offset;
+}
+
 static bool riscv_hart_init(riscv_hart_s *const hart)
 {
 	/* Allocate a new target */
 	target_s *target = target_new();
 	if (!target)
 		return false;
 
-	/* Grab a reference to the DMI and DM structurues and do preliminary setup of the target structure */
+	/* Grab a reference to the DMI and DM structures and do preliminary setup of the target structure */
 	riscv_dm_ref(hart->dbg_module);
 	target->driver = "RISC-V";
 	target->priv = hart;
@@ -444,13 +494,16 @@ static bool riscv_hart_init(riscv_hart_s *const hart)
 	/* Then read out the ID registers */
 	riscv_hart_read_ids(hart);
 
-	DEBUG_INFO("Hart %" PRIx32 ": %u-bit RISC-V (arch = %08" PRIx32 "), vendor = %" PRIx32 ", impl = %" PRIx32
-			   ", exts = %08" PRIx32 "\n",
-		hart->hartid, hart->access_width, hart->archid, hart->vendorid, hart->implid, hart->extensions);
+	/* Build the ISA subset string from the Hart */
+	riscv_snprint_isa_subset(hart->isa_name, sizeof(hart->isa_name), hart->access_width, hart->extensions);
+	target->core = hart->isa_name;
+
+	DEBUG_INFO("Hart %" PRIx32 ": %u-bit RISC-V (arch = %08" PRIx32 "), %s ISA (exts = %08" PRIx32
+			   "), vendor = %" PRIx32 ", impl = %" PRIx32 "\n",
+		hart->hartid, hart->access_width, hart->archid, hart->isa_name, hart->extensions, hart->vendorid, hart->implid);
 
 	/* We don't support rv128, so tell the user and fast-quit on this target. */
 	if (hart->access_width == 128U) {
-		target->core = "(unsup) rv128";
 		DEBUG_WARN("rv128 is unsupported, ignoring this hart\n");
 		return true;
 	}
@@ -1091,17 +1144,6 @@ static void riscv_reset(target_s *const target)
 	target_check_error(target);
 }
 
-static const char *riscv_fpu_ext_string(const uint32_t extensions)
-{
-	if (extensions & RV_ISA_EXT_QUAD_FLOAT)
-		return "q";
-	if (extensions & RV_ISA_EXT_DOUBLE_FLOAT)
-		return "d";
-	if (extensions & RV_ISA_EXT_SINGLE_FLOAT)
-		return "f";
-	return "";
-}
-
 /*
  * Generate the FPU section of the description.
  * fpu_size = 32 -> single precision float
@@ -1220,19 +1262,24 @@ static size_t riscv_build_target_fpu_description(char *const buffer, size_t max_
  *  </target>
  */
 static size_t riscv_build_target_description(
-	char *const buffer, size_t max_length, const uint8_t address_width, const uint32_t extensions)
+	char *const buffer, const size_t max_length, const uint8_t address_width, const uint32_t extensions)
 {
-	const bool embedded = extensions & RV_ISA_EXT_EMBEDDED;
-	const uint32_t fpu = extensions & RV_ISA_EXT_ANY_FLOAT;
-
 	size_t print_size = max_length;
 	/* Start with the "preamble" chunks, which are mostly common across targets save for 2 words. */
-	size_t offset =
-		(size_t)snprintf(buffer, print_size, "%s target %sriscv:rv%u%c%s%s <feature name=\"org.gnu.gdb.riscv.cpu\">",
-			gdb_xml_preamble_first, gdb_xml_preamble_second, address_width, embedded ? 'e' : 'i',
-			riscv_fpu_ext_string(fpu), gdb_xml_preamble_third);
+	int offset = snprintf(buffer, print_size, "%s target %sriscv:", gdb_xml_preamble_first, gdb_xml_preamble_second);
+	if (max_length != 0)
+		print_size = max_length - (size_t)offset;
+	/* Write the architecture string, which is the ISA subset */
+	offset += riscv_snprint_isa_subset(buffer + offset, print_size, address_width, extensions);
+	if (max_length != 0)
+		print_size = max_length - (size_t)offset;
+	/* Finally finish the rest of the preamble */
+	offset +=
+		snprintf(buffer + offset, print_size, "%s <feature name=\"org.gnu.gdb.riscv.cpu\">", gdb_xml_preamble_third);
+	if (max_length != 0)
+		print_size = max_length - (size_t)offset;
 
-	const uint8_t gprs = embedded ? 16U : 32U;
+	const uint8_t gprs = extensions & RV_ISA_EXT_EMBEDDED ? 16U : 32U;
 	/* Then build the general purpose register descriptions using the arrays at top of file */
 	/* Note that in a device using the embedded (E) extension, we only generate the first 16. */
 	for (uint8_t i = 0; i < gprs; ++i) {

src/target/riscv_debug.h

@@ -150,6 +150,8 @@ typedef struct riscv_hart {
 	uint32_t implid;
 	uint32_t hartid;
 
+	char isa_name[32U];
+
 	uint32_t triggers;
 	uint32_t trigger_uses[RV_TRIGGERS_MAX];
 } riscv_hart_s;
@@ -201,11 +203,53 @@ typedef struct riscv_hart {
 /* The FP base defines the starting register space address for the floating point registers */
 #define RV_FP_BASE 0x1020U
 
-#define RV_ISA_EXT_EMBEDDED     0x00000010U
-#define RV_ISA_EXT_ANY_FLOAT    0x00010028U
-#define RV_ISA_EXT_SINGLE_FLOAT 0x00000020U
-#define RV_ISA_EXT_DOUBLE_FLOAT 0x00000008U
-#define RV_ISA_EXT_QUAD_FLOAT   0x00010000U
+/**
+ * The MXL (Machine XLEN) field encodes the native base integer ISA width
+ * 
+ * The RISC-V Machine ISA register is MXLEN bits wide so the MXL offset is not fixed
+ * To work around this we convert the register to it's canonical 32-bit form internally
+ */
+#define RV_ISA_MXL_SHIFT 30U                        /* misa Machine XLEN field shift (for 32-bit misa) */
+#define RV_ISA_MXL_MASK  (0x3U << RV_ISA_MXL_SHIFT) /* misa Machine XLEN field mask (for 32-bit misa) */
+#define RV_ISA_MXL_32    0x1U                       /* misa Machine XLEN field value for 32-bit ISA */
+#define RV_ISA_MXL_64    0x2U                       /* misa Machine XLEN field value for 64-bit ISA */
+#define RV_ISA_MXL_128   0x3U                       /* misa Machine XLEN field value for 128-bit ISA */
+
+/* 
+ * The Extensions field encodes the presence of standard extensions, with a single bit per alphabet letter
+ * (bit 0 encodes presence of extension “A” through to bit 25 which encodes “Z”)
+ * 
+ * This list is taken from the RISC-V Instruction Set Manual v2.2
+ * 
+ * The list order is the canonical representation order in the ISA subset string
+ */
+#define RV_ISA_EXTENSIONS_MASK 0x03ffffffU /* misa extensions field mask */
+
+/* Base ISA */
+#define RV_ISA_EXT_INTEGER  (1U << 8U) /* 'I': RV32I/64I/128I integer base ISA */
+#define RV_ISA_EXT_EMBEDDED (1U << 4U) /* 'E': RV32E reduced integer base ISA (Embedded) */
+
+/* Standard general-purpose ISA */
+#define RV_ISA_EXT_MUL_DIV_INT  (1U << 12U) /* 'M': Integer multiplication and division */
+#define RV_ISA_EXT_ATOMIC       (1U << 0U)  /* 'A': Atomic instructions */
+#define RV_ISA_EXT_SINGLE_FLOAT (1U << 5U)  /* 'F': Single-precision floating-point */
+#define RV_ISA_EXT_DOUBLE_FLOAT (1U << 3U)  /* 'D': Double-precision floating-point */
+
+/* 'G' standard general-purpose ISA abbreviation, representing 'IMAFD' */
+#define RV_ISA_EXT_GENERAL_PURPOSE                                                               \
+	(RV_ISA_EXT_INTEGER | RV_ISA_EXT_MUL_DIV_INT | RV_ISA_EXT_ATOMIC | RV_ISA_EXT_SINGLE_FLOAT | \
+		RV_ISA_EXT_DOUBLE_FLOAT)
+
+/* Standard Unprivileged Extensions */
+#define RV_ISA_EXT_QUAD_FLOAT      (1U << 16U) /* 'Q': Quad-precision floating-point */
+#define RV_ISA_EXT_DECIMAL_FLOAT   (1U << 11U) /* 'L': Decimal floating-point */
+#define RV_ISA_EXT_COMPRESSED      (1U << 2U)  /* 'C': 16-bit compressed instructions */
+#define RV_ISA_EXT_BIT_MANIP       (1U << 1U)  /* 'B': Bit manipulation */
+#define RV_ISA_EXT_DYNAMIC_LANG    (1U << 9U)  /* 'J': Dynamic languages  */
+#define RV_ISA_EXT_TRANSACT_MEM    (1U << 19U) /* 'T': Transactional memory */
+#define RV_ISA_EXT_PACKED_SIMD     (1U << 15U) /* 'P': Packed-SIMD */
+#define RV_ISA_EXT_VECTOR          (1U << 21U) /* 'V': Vector extensions */
+#define RV_ISA_EXT_USER_INTERRUPTS (1U << 13U) /* 'N': User-level interrupts */
 
 #define RV_TRIGGER_SUPPORT_MASK       0x0000fffeU
 #define RV_TRIGGER_MODE_MASK          0xffff0000U
@@ -215,16 +259,16 @@ typedef struct riscv_hart {
  * The CSR number when requested by GDB is shifted by RV_CSR_GDB_OFFSET so they cannot collide with
  * the GPRs. As a result, we have to subtract RV_CSR_GDB_OFFSET from the value received from GDB.
  */
-#define RV_CSR_GDB_OFFSET 128
-#define RV_CSR_STATUS     0x300
-#define RV_CSR_MISA       0x301
-#define RV_CSR_MIE        0x304
-#define RV_CSR_MTVEC      0x305
-#define RV_CSR_MSCRATCH   0x340
-#define RV_CSR_MEPC       0x341
-#define RV_CSR_MCAUSE     0x342
-#define RV_CSR_MTVAL      0x343
-#define RV_CSR_MIP        0x344
+#define RV_CSR_GDB_OFFSET 128U
+#define RV_CSR_STATUS     0x300U
+#define RV_CSR_MISA       0x301U
+#define RV_CSR_MIE        0x304U
+#define RV_CSR_MTVEC      0x305U
+#define RV_CSR_MSCRATCH   0x340U
+#define RV_CSR_MEPC       0x341U
+#define RV_CSR_MCAUSE     0x342U
+#define RV_CSR_MTVAL      0x343U
+#define RV_CSR_MIP        0x344U
 
 /*
  * These two lines are about allowing GDB to access FPU registers through fake registers offset by

src/target/target_internal.h

@@ -179,7 +179,7 @@ struct target {
 	/* Other stuff */
 	const char *driver;
 	uint32_t cpuid;
-	char *core;
+	const char *core;
 	char cmdline[MAX_CMDLINE];
 	target_addr_t heapinfo[4];
 	target_command_s *commands;