1. Patchew
  2. QEMU
  3. [RFC PATCH v4 0/7] riscv: add initial sdext support
  4. View patch

[RFC PATCH v4 7/7] target/riscv: add sdtrig trigger action=debug mode

Chao Liu posted 7 patches 1 week, 3 days ago
There is a newer version of this series
[RFC PATCH v4 7/7] target/riscv: add sdtrig trigger action=debug mode
Posted by Chao Liu 1 week, 3 days ago 
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");
+            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);
 }
 
-- 
2.52.0
Re: [RFC PATCH v4 7/7] target/riscv: add sdtrig trigger action=debug mode
Posted by Daniel Henrique Barboza 6 days, 18 hours ago 

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.

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);
>   }
>
Re: [RFC PATCH v4 7/7] target/riscv: add sdtrig trigger action=debug mode
Posted by Chao Liu 6 days, 4 hours ago 
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);
> >   }
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.