Merge pull request #1366 from riscv-software-src/fix-1365

Implement dcsr.v and make DRET use it

riscv/csrs.cc

@@ -13,6 +13,8 @@
 #include "trap.h"
 // For require():
 #include "insn_macros.h"
+// For CSR_DCSR_V:
+#include "debug_defines.h"
 
 // STATE macro used by require_privilege() macro:
 #undef STATE
@@ -1234,6 +1236,7 @@ dcsr_csr_t::dcsr_csr_t(processor_t* const proc, const reg_t addr):
   ebreaks(false),
   ebreaku(false),
   halt(false),
+  v(false),
   cause(0) {
 }
 
@@ -1244,18 +1247,19 @@ void dcsr_csr_t::verify_permissions(insn_t insn, bool write) const {
 }
 
 reg_t dcsr_csr_t::read() const noexcept {
-  uint32_t v = 0;
-  v = set_field(v, DCSR_XDEBUGVER, 1);
-  v = set_field(v, DCSR_EBREAKM, ebreakm);
-  v = set_field(v, DCSR_EBREAKH, ebreakh);
-  v = set_field(v, DCSR_EBREAKS, ebreaks);
-  v = set_field(v, DCSR_EBREAKU, ebreaku);
-  v = set_field(v, DCSR_STOPCYCLE, 0);
-  v = set_field(v, DCSR_STOPTIME, 0);
-  v = set_field(v, DCSR_CAUSE, cause);
-  v = set_field(v, DCSR_STEP, step);
-  v = set_field(v, DCSR_PRV, prv);
-  return v;
+  reg_t result = 0;
+  result = set_field(result, DCSR_XDEBUGVER, 1);
+  result = set_field(result, DCSR_EBREAKM, ebreakm);
+  result = set_field(result, DCSR_EBREAKH, ebreakh);
+  result = set_field(result, DCSR_EBREAKS, ebreaks);
+  result = set_field(result, DCSR_EBREAKU, ebreaku);
+  result = set_field(result, DCSR_STOPCYCLE, 0);
+  result = set_field(result, DCSR_STOPTIME, 0);
+  result = set_field(result, DCSR_CAUSE, cause);
+  result = set_field(result, DCSR_STEP, step);
+  result = set_field(result, DCSR_PRV, prv);
+  result = set_field(result, CSR_DCSR_V, v);
+  return result;
 }
 
 bool dcsr_csr_t::unlogged_write(const reg_t val) noexcept {
@@ -1267,12 +1271,14 @@ bool dcsr_csr_t::unlogged_write(const reg_t val) noexcept {
   ebreaks = get_field(val, DCSR_EBREAKS);
   ebreaku = get_field(val, DCSR_EBREAKU);
   halt = get_field(val, DCSR_HALT);
+  v = proc->extension_enabled('H') ? get_field(val, CSR_DCSR_V) : false;
   return true;
 }
 
-void dcsr_csr_t::write_cause_and_prv(uint8_t cause, reg_t prv) noexcept {
+void dcsr_csr_t::write_cause_and_prv(uint8_t cause, reg_t prv, bool v) noexcept {
   this->cause = cause;
   this->prv = prv;
+  this->v = v;
   log_write();
 }
 

riscv/csrs.h

@@ -656,7 +656,7 @@ class dcsr_csr_t: public csr_t {
   dcsr_csr_t(processor_t* const proc, const reg_t addr);
   virtual void verify_permissions(insn_t insn, bool write) const override;
   virtual reg_t read() const noexcept override;
-  void write_cause_and_prv(uint8_t cause, reg_t prv) noexcept;
+  void write_cause_and_prv(uint8_t cause, reg_t prv, bool v) noexcept;
  protected:
   virtual bool unlogged_write(const reg_t val) noexcept override;
  public:
@@ -667,6 +667,7 @@ class dcsr_csr_t: public csr_t {
   bool ebreaks;
   bool ebreaku;
   bool halt;
+  bool v;
   uint8_t cause;
 };
 

riscv/insns/dret.h

@@ -1,6 +1,6 @@
 require(STATE.debug_mode);
 set_pc_and_serialize(STATE.dpc->read());
-p->set_privilege(STATE.dcsr->prv);
+p->set_privilege(STATE.dcsr->prv, STATE.dcsr->v);
 if (STATE.prv < PRV_M)
   STATE.mstatus->write(STATE.mstatus->read() & ~MSTATUS_MPRV);
 

riscv/insns/mnret.h

@@ -11,5 +11,4 @@ if (prev_prv != PRV_M) {
 }
 s = set_field(s, MNSTATUS_NMIE, 1);
 STATE.mnstatus->write(s);
-p->set_privilege(prev_prv);
-p->set_virt(prev_virt);
+p->set_privilege(prev_prv, prev_virt);

riscv/insns/mret.h

@@ -10,5 +10,4 @@ s = set_field(s, MSTATUS_MPIE, 1);
 s = set_field(s, MSTATUS_MPP, p->extension_enabled('U') ? PRV_U : PRV_M);
 s = set_field(s, MSTATUS_MPV, 0);
 p->put_csr(CSR_MSTATUS, s);
-p->set_privilege(prev_prv);
-p->set_virt(prev_virt);
+p->set_privilege(prev_prv, prev_virt);

riscv/insns/sret.h

@@ -14,14 +14,14 @@ s = set_field(s, MSTATUS_SIE, get_field(s, MSTATUS_SPIE));
 s = set_field(s, MSTATUS_SPIE, 1);
 s = set_field(s, MSTATUS_SPP, PRV_U);
 STATE.sstatus->write(s);
-p->set_privilege(prev_prv);
+bool prev_virt = STATE.v;
 if (!STATE.v) {
   if (p->extension_enabled('H')) {
-    reg_t prev_virt = get_field(prev_hstatus, HSTATUS_SPV);
-    p->set_virt(prev_virt);
+    prev_virt = get_field(prev_hstatus, HSTATUS_SPV);
     reg_t new_hstatus = set_field(prev_hstatus, HSTATUS_SPV, 0);
     STATE.hstatus->write(new_hstatus);
   }
 
   STATE.mstatus->write(set_field(STATE.mstatus->read(), MSTATUS_MPRV, 0));
 }
+p->set_privilege(prev_prv, prev_virt);

riscv/processor.cc

@@ -327,10 +327,10 @@ void state_t::reset(processor_t* const proc, reg_t max_isa)
   mcounteren = std::make_shared<masked_csr_t>(proc, CSR_MCOUNTEREN, counteren_mask, 0);
   if (proc->extension_enabled_const('U')) csrmap[CSR_MCOUNTEREN] = mcounteren;
   csrmap[CSR_SCOUNTEREN] = scounteren = std::make_shared<masked_csr_t>(proc, CSR_SCOUNTEREN, counteren_mask, 0);
-  auto nonvirtual_sepc = std::make_shared<epc_csr_t>(proc, CSR_SEPC);
+  nonvirtual_sepc = std::make_shared<epc_csr_t>(proc, CSR_SEPC);
   csrmap[CSR_VSEPC] = vsepc = std::make_shared<epc_csr_t>(proc, CSR_VSEPC);
   csrmap[CSR_SEPC] = sepc = std::make_shared<virtualized_csr_t>(proc, nonvirtual_sepc, vsepc);
-  auto nonvirtual_stval = std::make_shared<basic_csr_t>(proc, CSR_STVAL, 0);
+  nonvirtual_stval = std::make_shared<basic_csr_t>(proc, CSR_STVAL, 0);
   csrmap[CSR_VSTVAL] = vstval = std::make_shared<basic_csr_t>(proc, CSR_VSTVAL, 0);
   csrmap[CSR_STVAL] = stval = std::make_shared<virtualized_csr_t>(proc, nonvirtual_stval, vstval);
   auto sscratch = std::make_shared<basic_csr_t>(proc, CSR_SSCRATCH, 0);
@@ -338,13 +338,13 @@ void state_t::reset(processor_t* const proc, reg_t max_isa)
   // Note: if max_isa does not include H, we don't really need this virtualized_csr_t at all (though it doesn't hurt):
   csrmap[CSR_SSCRATCH] = std::make_shared<virtualized_csr_t>(proc, sscratch, vsscratch);
   csrmap[CSR_VSSCRATCH] = vsscratch;
-  auto nonvirtual_stvec = std::make_shared<tvec_csr_t>(proc, CSR_STVEC);
+  nonvirtual_stvec = std::make_shared<tvec_csr_t>(proc, CSR_STVEC);
   csrmap[CSR_VSTVEC] = vstvec = std::make_shared<tvec_csr_t>(proc, CSR_VSTVEC);
   csrmap[CSR_STVEC] = stvec = std::make_shared<virtualized_csr_t>(proc, nonvirtual_stvec, vstvec);
   auto nonvirtual_satp = std::make_shared<satp_csr_t>(proc, CSR_SATP);
   csrmap[CSR_VSATP] = vsatp = std::make_shared<base_atp_csr_t>(proc, CSR_VSATP);
   csrmap[CSR_SATP] = satp = std::make_shared<virtualized_satp_csr_t>(proc, nonvirtual_satp, vsatp);
-  auto nonvirtual_scause = std::make_shared<cause_csr_t>(proc, CSR_SCAUSE);
+  nonvirtual_scause = std::make_shared<cause_csr_t>(proc, CSR_SCAUSE);
   csrmap[CSR_VSCAUSE] = vscause = std::make_shared<cause_csr_t>(proc, CSR_VSCAUSE);
   csrmap[CSR_SCAUSE] = scause = std::make_shared<virtualized_csr_t>(proc, nonvirtual_scause, vscause);
   csrmap[CSR_MTVAL2] = mtval2 = std::make_shared<hypervisor_csr_t>(proc, CSR_MTVAL2);
@@ -382,7 +382,7 @@ void state_t::reset(processor_t* const proc, reg_t max_isa)
   csrmap[CSR_HTVAL] = htval = std::make_shared<basic_csr_t>(proc, CSR_HTVAL, 0);
   csrmap[CSR_HTINST] = htinst = std::make_shared<basic_csr_t>(proc, CSR_HTINST, 0);
   csrmap[CSR_HGATP] = hgatp = std::make_shared<hgatp_csr_t>(proc, CSR_HGATP);
-  auto nonvirtual_sstatus = std::make_shared<sstatus_proxy_csr_t>(proc, CSR_SSTATUS, mstatus);
+  nonvirtual_sstatus = std::make_shared<sstatus_proxy_csr_t>(proc, CSR_SSTATUS, mstatus);
   csrmap[CSR_VSSTATUS] = vsstatus = std::make_shared<vsstatus_csr_t>(proc, CSR_VSSTATUS);
   csrmap[CSR_SSTATUS] = sstatus = std::make_shared<sstatus_csr_t>(proc, nonvirtual_sstatus, vsstatus);
 
@@ -714,11 +714,13 @@ reg_t processor_t::legalize_privilege(reg_t prv)
   return prv;
 }
 
-void processor_t::set_privilege(reg_t prv)
+void processor_t::set_privilege(reg_t prv, bool virt)
 {
   mmu->flush_tlb();
   state.prev_prv = state.prv;
+  state.prev_v = state.v;
   state.prv = legalize_privilege(prv);
+  state.v = virt && state.prv != PRV_M;
 }
 
 const char* processor_t::get_privilege_string()
@@ -741,30 +743,11 @@ const char* processor_t::get_privilege_string()
   abort();
 }
 
-void processor_t::set_virt(bool virt)
-{
-  reg_t tmp, mask;
-
-  if (state.prv == PRV_M)
-    return;
-
-  /*
-    * Ideally, we should flush TLB here but we don't need it because
-    * set_virt() is always used in conjucter with set_privilege() and
-    * set_privilege() will flush TLB unconditionally.
-    *
-    * The virtualized sstatus register also relies on this TLB flush,
-    * since changing V might change sstatus.MXR and sstatus.SUM.
-    */
-  state.prev_v = state.v;
-  state.v = virt;
-}
-
 void processor_t::enter_debug_mode(uint8_t cause)
 {
   state.debug_mode = true;
-  state.dcsr->write_cause_and_prv(cause, state.prv);
-  set_privilege(PRV_M);
+  state.dcsr->write_cause_and_prv(cause, state.prv, state.v);
+  set_privilege(PRV_M, false);
   state.dpc->write(state.pc);
   state.pc = DEBUG_ROM_ENTRY;
   in_wfi = false;
@@ -831,23 +814,22 @@ void processor_t::take_trap(trap_t& t, reg_t epc)
     s = set_field(s, MSTATUS_SPP, state.prv);
     s = set_field(s, MSTATUS_SIE, 0);
     state.sstatus->write(s);
-    set_privilege(PRV_S);
+    set_privilege(PRV_S, true);
   } else if (state.prv <= PRV_S && bit < max_xlen && ((hsdeleg >> bit) & 1)) {
     // Handle the trap in HS-mode
-    set_virt(false);
-    reg_t vector = (state.stvec->read() & 1) && interrupt ? 4 * bit : 0;
-    state.pc = (state.stvec->read() & ~(reg_t)1) + vector;
-    state.scause->write(t.cause());
-    state.sepc->write(epc);
-    state.stval->write(t.get_tval());
+    reg_t vector = (state.nonvirtual_stvec->read() & 1) && interrupt ? 4 * bit : 0;
+    state.pc = (state.nonvirtual_stvec->read() & ~(reg_t)1) + vector;
+    state.nonvirtual_scause->write(t.cause());
+    state.nonvirtual_sepc->write(epc);
+    state.nonvirtual_stval->write(t.get_tval());
     state.htval->write(t.get_tval2());
     state.htinst->write(t.get_tinst());
 
-    reg_t s = state.sstatus->read();
+    reg_t s = state.nonvirtual_sstatus->read();
     s = set_field(s, MSTATUS_SPIE, get_field(s, MSTATUS_SIE));
     s = set_field(s, MSTATUS_SPP, state.prv);
     s = set_field(s, MSTATUS_SIE, 0);
-    state.sstatus->write(s);
+    state.nonvirtual_sstatus->write(s);
     if (extension_enabled('H')) {
       s = state.hstatus->read();
       if (curr_virt)
@@ -856,10 +838,9 @@ void processor_t::take_trap(trap_t& t, reg_t epc)
       s = set_field(s, HSTATUS_GVA, t.has_gva());
       state.hstatus->write(s);
     }
-    set_privilege(PRV_S);
+    set_privilege(PRV_S, false);
   } else {
     // Handle the trap in M-mode
-    set_virt(false);
     const reg_t vector = (state.mtvec->read() & 1) && interrupt ? 4 * bit : 0;
     const reg_t trap_handler_address = (state.mtvec->read() & ~(reg_t)1) + vector;
     // RNMI exception vector is implementation-defined.  Since we don't model
@@ -881,7 +862,7 @@ void processor_t::take_trap(trap_t& t, reg_t epc)
     s = set_field(s, MSTATUS_GVA, t.has_gva());
     state.mstatus->write(s);
     if (state.mstatush) state.mstatush->write(s >> 32);  // log mstatush change
-    set_privilege(PRV_M);
+    set_privilege(PRV_M, false);
   }
 }
 

riscv/processor.h

@@ -110,6 +110,13 @@ struct state_t
   virtualized_csr_t_p satp;
   csr_t_p scause;
 
+  // When taking a trap into HS-mode, we must access the nonvirtualized HS-mode CSRs directly:
+  csr_t_p nonvirtual_stvec;
+  csr_t_p nonvirtual_scause;
+  csr_t_p nonvirtual_sepc;
+  csr_t_p nonvirtual_stval;
+  sstatus_proxy_csr_t_p nonvirtual_sstatus;
+
   csr_t_p mtval2;
   csr_t_p mtinst;
   csr_t_p hstatus;
@@ -262,8 +269,7 @@ public:
       throw trap_instruction_address_misaligned(state.v, pc, 0, 0);
   }
   reg_t legalize_privilege(reg_t);
-  void set_privilege(reg_t);
-  void set_virt(bool);
+  void set_privilege(reg_t, bool);
   const char* get_privilege_string();
   void update_histogram(reg_t pc);
   const disassembler_t* get_disassembler() { return disassembler; }