Only implement one solution for native triggers.

When S-mode is present, use option 1 (disable triggers in M-mode unless MIE is set) from the Debug Spec. When S-mode is not present, use option 2 (implement mte and mpte bits in tcontrol). See discussion in #1777.
riscv-software-src · Sep 4, 2024 · cfcbffd · cfcbffd
1 parent cb78f09
commit cfcbffd
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 19 deletions.
diff --git a/riscv/csr_init.cc b/riscv/csr_init.cc
@@ -205,13 +205,14 @@ void state_t::csr_init(processor_t* const proc, reg_t max_isa)
     add_csr(CSR_TDATA2, tdata2 = std::make_shared<tdata2_csr_t>(proc, CSR_TDATA2));
     add_csr(CSR_TDATA3, std::make_shared<tdata3_csr_t>(proc, CSR_TDATA3));
     add_csr(CSR_TINFO, std::make_shared<tinfo_csr_t>(proc, CSR_TINFO));
-    add_csr(CSR_TCONTROL, tcontrol = std::make_shared<masked_csr_t>(proc, CSR_TCONTROL, CSR_TCONTROL_MPTE | CSR_TCONTROL_MTE, 0));
+    if (!proc->extension_enabled_const('S')) {
+      add_csr(CSR_TCONTROL, tcontrol = std::make_shared<masked_csr_t>(proc, CSR_TCONTROL, CSR_TCONTROL_MPTE | CSR_TCONTROL_MTE, 0));
+    }
   } else {
     add_csr(CSR_TDATA1, std::make_shared<const_csr_t>(proc, CSR_TDATA1, 0));
     add_csr(CSR_TDATA2, tdata2 = std::make_shared<const_csr_t>(proc, CSR_TDATA2, 0));
     add_csr(CSR_TDATA3, std::make_shared<const_csr_t>(proc, CSR_TDATA3, 0));
     add_csr(CSR_TINFO, std::make_shared<const_csr_t>(proc, CSR_TINFO, 0));
-    add_csr(CSR_TCONTROL, tcontrol = std::make_shared<const_csr_t>(proc, CSR_TCONTROL, 0));
   }
   unsigned scontext_length = (xlen == 32 ? 16 : 32); // debug spec suggests 16-bit for RV32 and 32-bit for RV64
   add_supervisor_csr(CSR_SCONTEXT, scontext = std::make_shared<masked_csr_t>(proc, CSR_SCONTEXT, (reg_t(1) << scontext_length) - 1, 0));

diff --git a/riscv/insns/mret.h b/riscv/insns/mret.h
@@ -20,5 +20,5 @@ if (prev_virt && prev_prv == PRV_U)
   STATE.vsstatus->write(STATE.vsstatus->read() & ~SSTATUS_SDT);
 STATE.mstatus->write(s);
 if (STATE.mstatush) STATE.mstatush->write(s >> 32); // log mstatush change
-STATE.tcontrol->write((STATE.tcontrol->read() & CSR_TCONTROL_MPTE) ? (CSR_TCONTROL_MPTE | CSR_TCONTROL_MTE) : 0);
+if (STATE.tcontrol) STATE.tcontrol->write((STATE.tcontrol->read() & CSR_TCONTROL_MPTE) ? (CSR_TCONTROL_MPTE | CSR_TCONTROL_MTE) : 0);
 p->set_privilege(prev_prv, prev_virt);
diff --git a/riscv/processor.cc b/riscv/processor.cc
@@ -535,7 +535,7 @@ void processor_t::take_trap(trap_t& t, reg_t epc)
     state.elp = elp_t::NO_LP_EXPECTED;
     state.mstatus->write(s);
     if (state.mstatush) state.mstatush->write(s >> 32);  // log mstatush change
-    state.tcontrol->write((state.tcontrol->read() & CSR_TCONTROL_MTE) ? CSR_TCONTROL_MPTE : 0);
+    if (state.tcontrol) state.tcontrol->write((state.tcontrol->read() & CSR_TCONTROL_MTE) ? CSR_TCONTROL_MPTE : 0);
     set_privilege(PRV_M, false);
   }
 }

diff --git a/riscv/triggers.cc b/riscv/triggers.cc
@@ -55,11 +55,18 @@ void trigger_t::tdata3_write(processor_t * const proc, const reg_t val) noexcept
   sselect = (sselect_t)((proc->extension_enabled_const('S') && get_field(val, CSR_TEXTRA_SSELECT(xlen)) <= SSELECT_MAXVAL) ? get_field(val, CSR_TEXTRA_SSELECT(xlen)) : SSELECT_IGNORE);
 }
 
+static reg_t tcontrol_value(const state_t * state) {
+  if (state->tcontrol)
+    return state->tcontrol->read();
+  else
+    return 0;
+}
+
 bool trigger_t::common_match(processor_t * const proc, bool use_prev_prv) const noexcept {
   auto state = proc->get_state();
   auto prv = use_prev_prv ? state->prev_prv : state->prv;
   auto v = use_prev_prv ? state->prev_v : state->v;
-  auto m_enabled = get_action() != 0 || (state->tcontrol->read() & CSR_TCONTROL_MTE);
+  auto m_enabled = get_action() != 0 || (tcontrol_value(state) & CSR_TCONTROL_MTE);
   return (prv < PRV_M || m_enabled) && mode_match(prv, v) && textra_match(proc);
 }
 
@@ -110,17 +117,29 @@ bool trigger_t::textra_match(processor_t * const proc) const noexcept
   return true;
 }
 
-bool trigger_t::allow_action(const state_t * const state) const
+bool trigger_t::allow_action(processor_t * const proc) const
 {
+  const state_t *state = proc->get_state();
   if (get_action() == ACTION_DEBUG_EXCEPTION) {
-    const bool mstatus_mie = state->mstatus->read() & MSTATUS_MIE;
-    const bool sstatus_sie = state->sstatus->read() & MSTATUS_SIE;
-    const bool vsstatus_sie = state->vsstatus->read() & MSTATUS_SIE;
-    const bool medeleg_breakpoint = (state->medeleg->read() >> CAUSE_BREAKPOINT) & 1;
-    const bool hedeleg_breakpoint = (state->hedeleg->read() >> CAUSE_BREAKPOINT) & 1;
-    return (state->prv != PRV_M || mstatus_mie) &&
-           (state->prv != PRV_S || state->v || !medeleg_breakpoint || sstatus_sie) &&
-           (state->prv != PRV_S || !state->v || !medeleg_breakpoint || !hedeleg_breakpoint || vsstatus_sie);
+    if (proc->extension_enabled('S')) {
+      // The hardware prevents triggers with action=0 from matching or firing
+      // while in M-mode and while MIE in mstatus is 0. If medeleg [3]=1 then it
+      // prevents triggers with action=0 from matching or firing while in S-mode
+      // and while SIE in sstatus is 0. If medeleg [3]=1 and hedeleg [3]=1 then
+      // it prevents triggers with action=0 from matching or firing while in
+      // VS-mode and while SIE in vstatus is 0.
+      const bool mstatus_mie = state->mstatus->read() & MSTATUS_MIE;
+      const bool sstatus_sie = state->sstatus->read() & MSTATUS_SIE;
+      const bool vsstatus_sie = state->vsstatus->read() & MSTATUS_SIE;
+      const bool medeleg_breakpoint = (state->medeleg->read() >> CAUSE_BREAKPOINT) & 1;
+      const bool hedeleg_breakpoint = (state->hedeleg->read() >> CAUSE_BREAKPOINT) & 1;
+      return (state->prv != PRV_M || mstatus_mie) &&
+            (state->prv != PRV_S || state->v || !medeleg_breakpoint || sstatus_sie) &&
+            (state->prv != PRV_S || !state->v || !medeleg_breakpoint || !hedeleg_breakpoint || vsstatus_sie);
+    } else {
+      // mte and mpte in tcontrol is implemented. medeleg [3] is hard-wired to 0.
+      return (state->prv != PRV_M) || (tcontrol_value(state) & CSR_TCONTROL_MTE);
+    }
   }
   return true;
 }
@@ -235,7 +254,7 @@ std::optional<match_result_t> mcontrol_common_t::detect_memory_access_match(proc
     value &= 0xffffffff;
   }
 
-  if (simple_match(xlen, value) && allow_action(proc->get_state())) {
+  if (simple_match(xlen, value) && allow_action(proc)) {
     /* This is OK because this function is only called if the trigger was not
      * inhibited by the previous trigger in the chain. */
     set_hit(timing ? HIT_IMMEDIATELY_AFTER : HIT_BEFORE);
@@ -324,7 +343,7 @@ void mcontrol6_t::tdata1_write(processor_t * const proc, const reg_t val, const
 
 std::optional<match_result_t> icount_t::detect_icount_fire(processor_t * const proc) noexcept
 {
-  if (!common_match(proc) || !allow_action(proc->get_state()))
+  if (!common_match(proc) || !allow_action(proc))
     return std::nullopt;
 
   std::optional<match_result_t> ret = std::nullopt;
@@ -339,7 +358,7 @@ std::optional<match_result_t> icount_t::detect_icount_fire(processor_t * const p
 
 void icount_t::detect_icount_decrement(processor_t * const proc) noexcept
 {
-  if (!common_match(proc) || !allow_action(proc->get_state()))
+  if (!common_match(proc) || !allow_action(proc))
     return;
 
   if (count >= 1) {
@@ -431,7 +450,7 @@ std::optional<match_result_t> trap_common_t::detect_trap_match(processor_t * con
   bool interrupt = (t.cause() & ((reg_t)1 << (xlen - 1))) != 0;
   reg_t bit = t.cause() & ~((reg_t)1 << (xlen - 1));
   assert(bit < xlen);
-  if (simple_match(interrupt, bit) && allow_action(proc->get_state())) {
+  if (simple_match(interrupt, bit) && allow_action(proc)) {
     hit = true;
     return match_result_t(TIMING_AFTER, action);
   }

diff --git a/riscv/triggers.h b/riscv/triggers.h
@@ -99,7 +99,7 @@ class trigger_t {
 protected:
   static action_t legalize_action(reg_t val, reg_t action_mask, reg_t dmode_mask) noexcept;
   bool common_match(processor_t * const proc, bool use_prev_prv = false) const noexcept;
-  bool allow_action(const state_t * const state) const;
+  bool allow_action(processor_t * const proc) const;
   reg_t tdata2;
 
   bool vs = false;