Writing non-existent CSRs, access FPRs with mstatus.FS=0 (#311)

* Don't corrupt s0 when abstract CSR write fails.

* Support abstract FPR access then mstatus.FS=0

Discussion on the spec list leans towards this being a requirement.
Certainly users want their debugger to be able to access all registers
regardless of target state.

debug_rom/debug_rom.S

@@ -43,6 +43,10 @@ entry_loop:
         jal  zero, entry_loop
 
 _exception:
+        // Restore S0, which we always save to dscratch.
+        // We need this in case the user tried an abstract write to a
+        // non-existent CSR.
+        csrr    s0, CSR_DSCRATCH
         sw      zero, DEBUG_ROM_EXCEPTION(zero) // Let debug module know you got an exception.
         ebreak
 

debug_rom/debug_rom.h

@@ -1,13 +1,13 @@
 static const unsigned char debug_rom_raw[] = {
-  0x6f, 0x00, 0xc0, 0x00, 0x6f, 0x00, 0xc0, 0x05, 0x6f, 0x00, 0x80, 0x03,
+  0x6f, 0x00, 0xc0, 0x00, 0x6f, 0x00, 0x00, 0x06, 0x6f, 0x00, 0x80, 0x03,
   0x0f, 0x00, 0xf0, 0x0f, 0x73, 0x10, 0x24, 0x7b, 0x73, 0x24, 0x40, 0xf1,
   0x23, 0x20, 0x80, 0x10, 0x03, 0x44, 0x04, 0x40, 0x13, 0x74, 0x14, 0x00,
-  0x63, 0x12, 0x04, 0x02, 0x73, 0x24, 0x40, 0xf1, 0x03, 0x44, 0x04, 0x40,
-  0x13, 0x74, 0x24, 0x00, 0x63, 0x16, 0x04, 0x02, 0x73, 0x00, 0x50, 0x10,
-  0x6f, 0xf0, 0x9f, 0xfd, 0x23, 0x26, 0x00, 0x10, 0x73, 0x00, 0x10, 0x00,
-  0x73, 0x24, 0x40, 0xf1, 0x23, 0x22, 0x80, 0x10, 0x73, 0x24, 0x20, 0x7b,
-  0x0f, 0x00, 0xf0, 0x0f, 0x0f, 0x10, 0x00, 0x00, 0x67, 0x00, 0x00, 0x30,
-  0x73, 0x24, 0x40, 0xf1, 0x23, 0x24, 0x80, 0x10, 0x73, 0x24, 0x20, 0x7b,
-  0x73, 0x00, 0x20, 0x7b
+  0x63, 0x14, 0x04, 0x02, 0x73, 0x24, 0x40, 0xf1, 0x03, 0x44, 0x04, 0x40,
+  0x13, 0x74, 0x24, 0x00, 0x63, 0x18, 0x04, 0x02, 0x73, 0x00, 0x50, 0x10,
+  0x6f, 0xf0, 0x9f, 0xfd, 0x73, 0x24, 0x20, 0x7b, 0x23, 0x26, 0x00, 0x10,
+  0x73, 0x00, 0x10, 0x00, 0x73, 0x24, 0x40, 0xf1, 0x23, 0x22, 0x80, 0x10,
+  0x73, 0x24, 0x20, 0x7b, 0x0f, 0x00, 0xf0, 0x0f, 0x0f, 0x10, 0x00, 0x00,
+  0x67, 0x00, 0x00, 0x30, 0x73, 0x24, 0x40, 0xf1, 0x23, 0x24, 0x80, 0x10,
+  0x73, 0x24, 0x20, 0x7b, 0x73, 0x00, 0x20, 0x7b
 };
-static const unsigned int debug_rom_raw_len = 112;
+static const unsigned int debug_rom_raw_len = 116;

riscv/debug_module.cc

@@ -557,6 +557,12 @@ void debug_module_t::run_test_idle()
   }
 }
 
+static bool is_fpu_reg(unsigned regno)
+{
+  return (regno >= 0x1020 && regno <= 0x103f) || regno == CSR_FFLAGS ||
+    regno == CSR_FRM || regno == CSR_FCSR;
+}
+
 bool debug_module_t::perform_abstract_command()
 {
   if (abstractcs.cmderr != CMDERR_NONE)
@@ -580,8 +586,22 @@ bool debug_module_t::perform_abstract_command()
     unsigned i = 0;
     if (get_field(command, AC_ACCESS_REGISTER_TRANSFER)) {
 
-      if (regno < 0x1000 && config.support_abstract_csr_access) {
+      if (is_fpu_reg(regno)) {
+        // Save S0
         write32(debug_abstract, i++, csrw(S0, CSR_DSCRATCH));
+        // Save mstatus
+        write32(debug_abstract, i++, csrr(S0, CSR_MSTATUS));
+        write32(debug_abstract, i++, csrw(S0, CSR_DSCRATCH + 1));
+        // Set mstatus.fs
+        assert((MSTATUS_FS & 0xfff) == 0);
+        write32(debug_abstract, i++, lui(S0, MSTATUS_FS >> 12));
+        write32(debug_abstract, i++, csrrs(ZERO, S0, CSR_MSTATUS));
+      }
+
+      if (regno < 0x1000 && config.support_abstract_csr_access) {
+        if (!is_fpu_reg(regno)) {
+          write32(debug_abstract, i++, csrw(S0, CSR_DSCRATCH));
+        }
 
         if (write) {
           switch (size) {
@@ -611,7 +631,9 @@ bool debug_module_t::perform_abstract_command()
               return true;
           }
         }
-        write32(debug_abstract, i++, csrr(S0, CSR_DSCRATCH));
+        if (!is_fpu_reg(regno)) {
+          write32(debug_abstract, i++, csrr(S0, CSR_DSCRATCH));
+        }
 
       } else if (regno >= 0x1000 && regno < 0x1020) {
         unsigned regnum = regno - 0x1000;
@@ -682,6 +704,14 @@ bool debug_module_t::perform_abstract_command()
         abstractcs.cmderr = CMDERR_NOTSUP;
         return true;
       }
+
+      if (is_fpu_reg(regno)) {
+        // restore mstatus
+        write32(debug_abstract, i++, csrr(S0, CSR_DSCRATCH + 1));
+        write32(debug_abstract, i++, csrw(S0, CSR_MSTATUS));
+        // restore s0
+        write32(debug_abstract, i++, csrr(S0, CSR_DSCRATCH));
+      }
     }
 
     if (get_field(command, AC_ACCESS_REGISTER_POSTEXEC)) {

riscv/debug_module.h

@@ -135,7 +135,7 @@ class debug_module_t : public abstract_device_t
     static const unsigned debug_data_start = 0x380;
     unsigned debug_progbuf_start;
 
-    static const unsigned debug_abstract_size = 5;
+    static const unsigned debug_abstract_size = 12;
     unsigned debug_abstract_start;
     // R/W this through custom registers, to allow debuggers to test that
     // functionality.

riscv/opcodes.h

@@ -125,6 +125,11 @@ static uint32_t csrr(unsigned int rd, unsigned int csr) {
   return (csr << 20) | (rd << 7) | MATCH_CSRRS;
 }
 
+static uint32_t csrrs(unsigned int rd, unsigned int rs1, unsigned int csr) __attribute__ ((unused));
+static uint32_t csrrs(unsigned int rd, unsigned int rs1, unsigned int csr) {
+  return (csr << 20) | (rs1 << 15) | (rd << 7) | MATCH_CSRRS;
+}
+
 static uint32_t fsw(unsigned int src, unsigned int base, uint16_t offset) __attribute__ ((unused));
 static uint32_t fsw(unsigned int src, unsigned int base, uint16_t offset)
 {
@@ -177,7 +182,6 @@ static uint32_t fence_i(void)
   return MATCH_FENCE_I;
 }
 
-/*
 static uint32_t lui(unsigned int dest, uint32_t imm) __attribute__ ((unused));
 static uint32_t lui(unsigned int dest, uint32_t imm)
 {
@@ -186,6 +190,7 @@ static uint32_t lui(unsigned int dest, uint32_t imm)
     MATCH_LUI;
 }
 
+/*
 static uint32_t csrci(unsigned int csr, uint16_t imm) __attribute__ ((unused));
 static uint32_t csrci(unsigned int csr, uint16_t imm) {
   return (csr << 20) |

riscv/processor.cc

@@ -651,7 +651,10 @@ void processor_t::set_csr(int which, reg_t val)
       state.dpc = val & ~(reg_t)1;
       break;
     case CSR_DSCRATCH:
-      state.dscratch = val;
+      state.dscratch0 = val;
+      break;
+    case CSR_DSCRATCH + 1:
+      state.dscratch1 = val;
       break;
     case CSR_VSTART:
       VU.vstart = val;
@@ -840,7 +843,9 @@ reg_t processor_t::get_csr(int which)
     case CSR_DPC:
       return state.dpc & pc_alignment_mask();
     case CSR_DSCRATCH:
-      return state.dscratch;
+      return state.dscratch0;
+    case CSR_DSCRATCH + 1:
+      return state.dscratch1;
     case CSR_VSTART:
       return VU.vstart;
     case CSR_VXSAT:

riscv/processor.h

@@ -234,7 +234,7 @@ struct state_t
   reg_t scause;
 
   reg_t dpc;
-  reg_t dscratch;
+  reg_t dscratch0, dscratch1;
   dcsr_t dcsr;
   reg_t tselect;
   mcontrol_t mcontrol[num_triggers];