On 1/30/2026 3:00 AM, Chao Liu wrote:
RISC-V Debug Specification:
https://github.com/riscv/riscv-debug-spec/releases/tag/1.0

Allow mcontrol/mcontrol6 action=1 when Sdext is enabled. When such a
trigger hits, enter Debug Mode with cause=trigger and stop with
EXCP_DEBUG.

Also report inst-count triggers in tinfo and read their action field.

Signed-off-by: Chao Liu <[email protected]>
---
  target/riscv/debug.c | 53 ++++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 5877a60c50..4e30d42905 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -110,6 +110,8 @@ static trigger_action_t get_trigger_action(CPURISCVState 
*env,
          action = (tdata1 & TYPE6_ACTION) >> 12;
          break;
      case TRIGGER_TYPE_INST_CNT:
+        action = tdata1 & ITRIGGER_ACTION;
+        break;
      case TRIGGER_TYPE_INT:
      case TRIGGER_TYPE_EXCP:
      case TRIGGER_TYPE_EXT_SRC:
@@ -280,6 +282,7 @@ static target_ulong textra_validate(CPURISCVState *env, 
target_ulong tdata3)
static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
  {
+    CPUState *cs = env_cpu(env);
      trigger_action_t action = get_trigger_action(env, trigger_index);
switch (action) {
@@ -289,6 +292,21 @@ static void do_trigger_action(CPURISCVState *env, 
target_ulong trigger_index)
          riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
          break;
      case DBG_ACTION_DBG_MODE:
+        if (!env_archcpu(env)->cfg.ext_sdext) {
+            qemu_log_mask(LOG_UNIMP,
+                          "trigger action=debug mode requires Sdext\n");


I believe we want LOG_GUEST_ERROR for invalid actions like this one. You can check examples in trigger_priv_match and other places in debug.c

LOG_UNIMP is reserved for cases where QEMU does not implement something at all and we want to warn the user about it. For instance, again in debug.c:trigger_priv_match():


    case TRIGGER_TYPE_INT:
    case TRIGGER_TYPE_EXCP:
    case TRIGGER_TYPE_EXT_SRC:
qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n", type);
        break;


This is the intended use for LOG_UNIMP.

Everything else LGTM.

Thanks,
Daniel



+            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+        }
+        riscv_cpu_enter_debug_mode(env, env->pc, DCSR_CAUSE_TRIGGER);
+        /*
+         * If this came from the Trigger Module's CPU breakpoint/watchpoint,
+         * we're already returning via EXCP_DEBUG. Otherwise, stop now.
+         */
+        if (cs->exception_index != EXCP_DEBUG) {
+            cs->exception_index = EXCP_DEBUG;
+            cpu_loop_exit_restore(cs, GETPC());
+        }
+        break;
      case DBG_ACTION_TRACE0:
      case DBG_ACTION_TRACE1:
      case DBG_ACTION_TRACE2:
@@ -441,6 +459,7 @@ static target_ulong type2_mcontrol_validate(CPURISCVState 
*env,
  {
      target_ulong val;
      uint32_t size;
+    uint32_t action;
/* validate the generic part first */
      val = tdata1_validate(env, ctrl, TRIGGER_TYPE_AD_MATCH);
@@ -448,11 +467,25 @@ static target_ulong type2_mcontrol_validate(CPURISCVState 
*env,
      /* validate unimplemented (always zero) bits */
      warn_always_zero_bit(ctrl, TYPE2_MATCH, "match");
      warn_always_zero_bit(ctrl, TYPE2_CHAIN, "chain");
-    warn_always_zero_bit(ctrl, TYPE2_ACTION, "action");
      warn_always_zero_bit(ctrl, TYPE2_TIMING, "timing");
      warn_always_zero_bit(ctrl, TYPE2_SELECT, "select");
      warn_always_zero_bit(ctrl, TYPE2_HIT, "hit");
+ action = (ctrl & TYPE2_ACTION) >> 12;
+    if (action == DBG_ACTION_BP) {
+        val |= ctrl & TYPE2_ACTION;
+    } else if (action == DBG_ACTION_DBG_MODE) {
+        if (env_archcpu(env)->cfg.ext_sdext) {
+            val |= ctrl & TYPE2_ACTION;
+        } else {
+            qemu_log_mask(LOG_UNIMP,
+                          "trigger action=debug mode requires Sdext\n");
+        }
+    } else {
+        qemu_log_mask(LOG_UNIMP, "trigger action: %u is not supported\n",
+                      action);
+    }
+
      /* validate size encoding */
      size = type2_breakpoint_size(env, ctrl);
      if (access_size[size] == -1) {
@@ -569,6 +602,7 @@ static target_ulong type6_mcontrol6_validate(CPURISCVState 
*env,
  {
      target_ulong val;
      uint32_t size;
+    uint32_t action;
/* validate the generic part first */
      val = tdata1_validate(env, ctrl, TRIGGER_TYPE_AD_MATCH6);
@@ -576,11 +610,25 @@ static target_ulong 
type6_mcontrol6_validate(CPURISCVState *env,
      /* validate unimplemented (always zero) bits */
      warn_always_zero_bit(ctrl, TYPE6_MATCH, "match");
      warn_always_zero_bit(ctrl, TYPE6_CHAIN, "chain");
-    warn_always_zero_bit(ctrl, TYPE6_ACTION, "action");
      warn_always_zero_bit(ctrl, TYPE6_TIMING, "timing");
      warn_always_zero_bit(ctrl, TYPE6_SELECT, "select");
      warn_always_zero_bit(ctrl, TYPE6_HIT, "hit");
+ action = (ctrl & TYPE6_ACTION) >> 12;
+    if (action == DBG_ACTION_BP) {
+        val |= ctrl & TYPE6_ACTION;
+    } else if (action == DBG_ACTION_DBG_MODE) {
+        if (env_archcpu(env)->cfg.ext_sdext) {
+            val |= ctrl & TYPE6_ACTION;
+        } else {
+            qemu_log_mask(LOG_UNIMP,
+                          "trigger action=debug mode requires Sdext\n");
+        }
+    } else {
+        qemu_log_mask(LOG_UNIMP, "trigger action: %u is not supported\n",
+                      action);
+    }
+
      /* validate size encoding */
      size = extract32(ctrl, 16, 4);
      if (access_size[size] == -1) {
@@ -919,6 +967,7 @@ target_ulong tinfo_csr_read(CPURISCVState *env)
  {
      /* assume all triggers support the same types of triggers */
      return BIT(TRIGGER_TYPE_AD_MATCH) |
+           BIT(TRIGGER_TYPE_INST_CNT) |
             BIT(TRIGGER_TYPE_AD_MATCH6);
  }

Reply via email to