Hi Daniel, On Mon, Feb 02, 2026 at 02:48:11PM -0300, Daniel Henrique Barboza wrote:
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 <chao.liu.zevorn@gmail.com> --- 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.
Thank you for the review and the clarification on the logging macros. I will replace the sdext-related LOG_UNIMP macros with LOG_GUEST_ERROR for this case. Will send v5 with this fix. Thanks, Chao
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); }