[RFC PATCH 00/25] target/riscv/debug: Sdtrig fixes and TT Ascalon support

[RFC PATCH 00/25] target/riscv/debug: Sdtrig fixes and TT Ascalon support

[Nicholas Piggin]

Hi,

Sorry for the big series. The Ascalon CPU implements Sdtrig with 2
different types of mcontrol6 trigger and the icount trigger, so in
the course of testing and bringing up OpenSBI and Linux support for
this, I've accumulated quite a lot.

My new year resolution is to start being better upstream contributor,
it's taken me a while with changing jobs and architectures. So I don't
expect others to drop everything to review this! Joel has been
prodding me, and noted there is some other Sdtrig work going on
with the v1.0 support patches.

I think the debug v1.0 patches are somewhat orthogonal to this series,
but both are addressing aspects of a common problem of Sdtrig
implementation specifics. I wonder if these should be reconciled or
left separate. Sdtrig v1.00/v0.13 configuration is a single boolean
which is feasible as a CPU property. Whereas the entire space of
Sdtrig implementation seems like too much to make configurable in that
way.

Any thoughts would be welcome.

Thanks,
Nick

Nicholas Piggin (25):
  target/riscv/debug: Check only mcontrol triggers for break/watchpoint
    matching
  target/riscv/debug: Handle changing trigger types
  target/riscv/debug: Implement permissive type unavailable trigger
  target/riscv/debug: Fix icount trigger privilege check
  target/riscv/debug: Update itrigger_enabled after changing privilege
  target/riscv/debug: Implement get_trigger_action for icount type
    trigger
  target/riscv/debug: Fix migration post_load icount_enabled() test
  target/riscv/debug: Fix icount privilege matching icount_enabled()
    test
  target/riscv/debug: Implement icount trigger textra matching
  target/riscv/debug: Maintain itrigger_enabled in
    helper_itrigger_match()
  target/riscv/debug: Fix breakpoint matching action
  target/riscv/debug: Put mcontrol load/store match address into tval
  target/riscv/debug: Remove breakpoints on reset
  target/riscv/debug: Move debug CPU post_load details into debug.c
  target/riscv/debug: Insert breakpoints after migration
  target/riscv/debug: Remove itrigger icount-enabled mode
  target/riscv/debug: Advertise icount trigger type in tinfo
  target/riscv/debug: Reset trigger type to unavailable
  target/riscv/debug: Add new debug state format
  target/riscv/debug: Migrate mcontext using new sdtrig vmstate
  target/riscv/debug: Implementation specific Sdtrig configuration
  target/riscv/debug: Support heterogeneous trigger types
  target/riscv/debug: Support heterogeneous mcontrol access types
  target/riscv/debug: Emulate TT Ascalon Sdtrig
  target/riscv/debug: Fix minor comment typos

 target/riscv/cpu.c         |  65 ++++-
 target/riscv/cpu.h         |  41 ++-
 target/riscv/cpu_helper.c  |  10 +-
 target/riscv/csr.c         |   7 +-
 target/riscv/debug.c       | 571 ++++++++++++++++++++-----------------
 target/riscv/debug.h       |  19 +-
 target/riscv/machine.c     |  96 ++++++-
 target/riscv/tcg/tcg-cpu.c |   5 +-
 8 files changed, 510 insertions(+), 304 deletions(-)

-- 
2.51.0

[RFC PATCH 01/25] target/riscv/debug: Check only mcontrol triggers for break/watchpoint matching

[Nicholas Piggin]

trigger_common_match() has some warning log messages in it for
unsupported triggers. Don't call it in BP/WP checking except for
mcontrol/mcontrol6 triggers applicable to BP/WPs.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 5664466749..4273ab7a8d 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -952,6 +952,14 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
         for (i = 0; i < RV_MAX_TRIGGERS; i++) {
             trigger_type = get_trigger_type(env, i);
 
+            switch (trigger_type) {
+            case TRIGGER_TYPE_AD_MATCH:
+            case TRIGGER_TYPE_AD_MATCH6:
+                break;
+            default:
+                continue; /* No other types match breakpoint */
+            }
+
             if (!trigger_common_match(env, trigger_type, i)) {
                 continue;
             }
@@ -976,8 +984,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
                 }
                 break;
             default:
-                /* other trigger types are not supported or irrelevant */
-                break;
+                g_assert_not_reached();
             }
         }
     }
@@ -998,6 +1005,14 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
     for (i = 0; i < RV_MAX_TRIGGERS; i++) {
         trigger_type = get_trigger_type(env, i);
 
+        switch (trigger_type) {
+        case TRIGGER_TYPE_AD_MATCH:
+        case TRIGGER_TYPE_AD_MATCH6:
+            break;
+        default:
+            continue; /* No other types match watchpoint */
+        }
+
         if (!trigger_common_match(env, trigger_type, i)) {
             continue;
         }
@@ -1036,8 +1051,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
             }
             break;
         default:
-            /* other trigger types are not supported */
-            break;
+            g_assert_not_reached();
         }
     }
 
-- 
2.51.0

Re: [RFC PATCH 01/25] target/riscv/debug: Check only mcontrol triggers for break/watchpoint matching

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> trigger_common_match() has some warning log messages in it for
> unsupported triggers. Don't call it in BP/WP checking except for
> mcontrol/mcontrol6 triggers applicable to BP/WPs.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 5664466749..4273ab7a8d 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -952,6 +952,14 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
>           for (i = 0; i < RV_MAX_TRIGGERS; i++) {
>               trigger_type = get_trigger_type(env, i);
>   
> +            switch (trigger_type) {
> +            case TRIGGER_TYPE_AD_MATCH:
> +            case TRIGGER_TYPE_AD_MATCH6:
> +                break;
> +            default:
> +                continue; /* No other types match breakpoint */
> +            }
> +
>               if (!trigger_common_match(env, trigger_type, i)) {
>                   continue;
>               }
> @@ -976,8 +984,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
>                   }
>                   break;
>               default:
> -                /* other trigger types are not supported or irrelevant */
> -                break;
> +                g_assert_not_reached();
>               }
>           }
>       }
> @@ -998,6 +1005,14 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>       for (i = 0; i < RV_MAX_TRIGGERS; i++) {
>           trigger_type = get_trigger_type(env, i);
>   
> +        switch (trigger_type) {
> +        case TRIGGER_TYPE_AD_MATCH:
> +        case TRIGGER_TYPE_AD_MATCH6:
> +            break;
> +        default:
> +            continue; /* No other types match watchpoint */
> +        }
> +
>           if (!trigger_common_match(env, trigger_type, i)) {
>               continue;
>           }
> @@ -1036,8 +1051,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>               }
>               break;
>           default:
> -            /* other trigger types are not supported */
> -            break;
> +            g_assert_not_reached();
>           }
>       }
>   

Re: [RFC PATCH 01/25] target/riscv/debug: Check only mcontrol triggers for break/watchpoint matching

[Chao Liu]

On Wed, Jan 14, 2026 at 02:46:34PM +0800, Nicholas Piggin wrote:
> trigger_common_match() has some warning log messages in it for
> unsupported triggers. Don't call it in BP/WP checking except for
> mcontrol/mcontrol6 triggers applicable to BP/WPs.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

> ---
>  target/riscv/debug.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 5664466749..4273ab7a8d 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -952,6 +952,14 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
>          for (i = 0; i < RV_MAX_TRIGGERS; i++) {
>              trigger_type = get_trigger_type(env, i);
>  
> +            switch (trigger_type) {
> +            case TRIGGER_TYPE_AD_MATCH:
> +            case TRIGGER_TYPE_AD_MATCH6:
> +                break;
> +            default:
> +                continue; /* No other types match breakpoint */
> +            }
> +
>              if (!trigger_common_match(env, trigger_type, i)) {
>                  continue;
>              }
> @@ -976,8 +984,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
>                  }
>                  break;
>              default:
> -                /* other trigger types are not supported or irrelevant */
> -                break;
> +                g_assert_not_reached();
>              }
>          }
>      }
> @@ -998,6 +1005,14 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>      for (i = 0; i < RV_MAX_TRIGGERS; i++) {
>          trigger_type = get_trigger_type(env, i);
>  
> +        switch (trigger_type) {
> +        case TRIGGER_TYPE_AD_MATCH:
> +        case TRIGGER_TYPE_AD_MATCH6:
> +            break;
> +        default:
> +            continue; /* No other types match watchpoint */
> +        }
> +
>          if (!trigger_common_match(env, trigger_type, i)) {
>              continue;
>          }
> @@ -1036,8 +1051,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>              }
>              break;
>          default:
> -            /* other trigger types are not supported */
> -            break;
> +            g_assert_not_reached();
>          }
>      }
>  
> -- 
> 2.51.0
> 
> 

[RFC PATCH 02/25] target/riscv/debug: Handle changing trigger types

[Nicholas Piggin]

Updating debug registers will first remove the existing TCG breakpoint /
watchpoint, then adds it back with new values.

Writing TDATA1 with a value that changes the trigger type attempts to
remove the facility for the new trigger type rather than the existing
one. That is, it will not remove a breakpoint if the type is changed to
a non-breakpoint type.

Fix this by removing based on the old trigger type, then inserting based
on the new type.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 64 +++++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 4273ab7a8d..2190c25f23 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -528,23 +528,12 @@ static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
 static void type2_reg_write(CPURISCVState *env, target_ulong index,
                             int tdata_index, target_ulong val)
 {
-    target_ulong new_val;
-
     switch (tdata_index) {
     case TDATA1:
-        new_val = type2_mcontrol_validate(env, val);
-        if (new_val != env->tdata1[index]) {
-            env->tdata1[index] = new_val;
-            type2_breakpoint_remove(env, index);
-            type2_breakpoint_insert(env, index);
-        }
+        env->tdata1[index] = type2_mcontrol_validate(env, val);
         break;
     case TDATA2:
-        if (val != env->tdata2[index]) {
-            env->tdata2[index] = val;
-            type2_breakpoint_remove(env, index);
-            type2_breakpoint_insert(env, index);
-        }
+        env->tdata2[index] = val;
         break;
     case TDATA3:
         env->tdata3[index] = textra_validate(env, val);
@@ -552,6 +541,8 @@ static void type2_reg_write(CPURISCVState *env, target_ulong index,
     default:
         g_assert_not_reached();
     }
+
+    type2_breakpoint_insert(env, index);
 }
 
 /* type 6 trigger */
@@ -642,23 +633,12 @@ static void type6_breakpoint_remove(CPURISCVState *env, target_ulong index)
 static void type6_reg_write(CPURISCVState *env, target_ulong index,
                             int tdata_index, target_ulong val)
 {
-    target_ulong new_val;
-
     switch (tdata_index) {
     case TDATA1:
-        new_val = type6_mcontrol6_validate(env, val);
-        if (new_val != env->tdata1[index]) {
-            env->tdata1[index] = new_val;
-            type6_breakpoint_remove(env, index);
-            type6_breakpoint_insert(env, index);
-        }
+        env->tdata1[index] = type6_mcontrol6_validate(env, val);
         break;
     case TDATA2:
-        if (val != env->tdata2[index]) {
-            env->tdata2[index] = val;
-            type6_breakpoint_remove(env, index);
-            type6_breakpoint_insert(env, index);
-        }
+        env->tdata2[index] = val;
         break;
     case TDATA3:
         env->tdata3[index] = textra_validate(env, val);
@@ -666,6 +646,7 @@ static void type6_reg_write(CPURISCVState *env, target_ulong index,
     default:
         g_assert_not_reached();
     }
+    type6_breakpoint_insert(env, index);
 }
 
 /* icount trigger type */
@@ -831,8 +812,6 @@ static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
                 /* set the count to timer */
                 timer_mod(env->itrigger_timer[index],
                           env->last_icount + itrigger_get_count(env, index));
-            } else {
-                env->itrigger_enabled = riscv_itrigger_enabled(env);
             }
         }
         break;
@@ -881,12 +860,30 @@ target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
 
 void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
 {
-    int trigger_type;
+    int trigger_type = get_trigger_type(env, env->trigger_cur);
+    bool check_itrigger = false;
+
+    switch (trigger_type) {
+    case TRIGGER_TYPE_AD_MATCH:
+        type2_breakpoint_remove(env, env->trigger_cur);
+        break;
+    case TRIGGER_TYPE_AD_MATCH6:
+        type6_breakpoint_remove(env, env->trigger_cur);
+        break;
+    case TRIGGER_TYPE_INST_CNT:
+        /*
+         * itrigger_enabled is the union of all enabled icount triggers,
+         * so it's easiest to recheck all if any have changed (removed or
+         * added or modified).
+         */
+        check_itrigger = true;
+        break;
+    default:
+        break;
+    }
 
     if (tdata_index == TDATA1) {
         trigger_type = extract_trigger_type(env, val);
-    } else {
-        trigger_type = get_trigger_type(env, env->trigger_cur);
     }
 
     switch (trigger_type) {
@@ -898,6 +895,7 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
         break;
     case TRIGGER_TYPE_INST_CNT:
         itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
+        check_itrigger = true;
         break;
     case TRIGGER_TYPE_INT:
     case TRIGGER_TYPE_EXCP:
@@ -913,6 +911,10 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
     default:
         g_assert_not_reached();
     }
+
+    if (check_itrigger && !icount_enabled()) {
+        env->itrigger_enabled = riscv_itrigger_enabled(env);
+    }
 }
 
 target_ulong tinfo_csr_read(CPURISCVState *env)
-- 
2.51.0

Re: [RFC PATCH 02/25] target/riscv/debug: Handle changing trigger types

[Chao Liu]

On Wed, Jan 14, 2026 at 02:46:35PM +0800, Nicholas Piggin wrote:
> Updating debug registers will first remove the existing TCG breakpoint /
> watchpoint, then adds it back with new values.
> 
> Writing TDATA1 with a value that changes the trigger type attempts to
> remove the facility for the new trigger type rather than the existing
> one. That is, it will not remove a breakpoint if the type is changed to
> a non-breakpoint type.
> 
> Fix this by removing based on the old trigger type, then inserting based
> on the new type.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

> ---
>  target/riscv/debug.c | 64 +++++++++++++++++++++++---------------------
>  1 file changed, 33 insertions(+), 31 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 4273ab7a8d..2190c25f23 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -528,23 +528,12 @@ static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
>  static void type2_reg_write(CPURISCVState *env, target_ulong index,
>                              int tdata_index, target_ulong val)
>  {
> -    target_ulong new_val;
> -
>      switch (tdata_index) {
>      case TDATA1:
> -        new_val = type2_mcontrol_validate(env, val);
> -        if (new_val != env->tdata1[index]) {
> -            env->tdata1[index] = new_val;
> -            type2_breakpoint_remove(env, index);
> -            type2_breakpoint_insert(env, index);
> -        }
> +        env->tdata1[index] = type2_mcontrol_validate(env, val);
>          break;
>      case TDATA2:
> -        if (val != env->tdata2[index]) {
> -            env->tdata2[index] = val;
> -            type2_breakpoint_remove(env, index);
> -            type2_breakpoint_insert(env, index);
> -        }
> +        env->tdata2[index] = val;
>          break;
>      case TDATA3:
>          env->tdata3[index] = textra_validate(env, val);
> @@ -552,6 +541,8 @@ static void type2_reg_write(CPURISCVState *env, target_ulong index,
>      default:
>          g_assert_not_reached();
>      }
> +
> +    type2_breakpoint_insert(env, index);
>  }
>  
>  /* type 6 trigger */
> @@ -642,23 +633,12 @@ static void type6_breakpoint_remove(CPURISCVState *env, target_ulong index)
>  static void type6_reg_write(CPURISCVState *env, target_ulong index,
>                              int tdata_index, target_ulong val)
>  {
> -    target_ulong new_val;
> -
>      switch (tdata_index) {
>      case TDATA1:
> -        new_val = type6_mcontrol6_validate(env, val);
> -        if (new_val != env->tdata1[index]) {
> -            env->tdata1[index] = new_val;
> -            type6_breakpoint_remove(env, index);
> -            type6_breakpoint_insert(env, index);
> -        }
> +        env->tdata1[index] = type6_mcontrol6_validate(env, val);
>          break;
>      case TDATA2:
> -        if (val != env->tdata2[index]) {
> -            env->tdata2[index] = val;
> -            type6_breakpoint_remove(env, index);
> -            type6_breakpoint_insert(env, index);
> -        }
> +        env->tdata2[index] = val;
>          break;
>      case TDATA3:
>          env->tdata3[index] = textra_validate(env, val);
> @@ -666,6 +646,7 @@ static void type6_reg_write(CPURISCVState *env, target_ulong index,
>      default:
>          g_assert_not_reached();
>      }
> +    type6_breakpoint_insert(env, index);
>  }
>  
>  /* icount trigger type */
> @@ -831,8 +812,6 @@ static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
>                  /* set the count to timer */
>                  timer_mod(env->itrigger_timer[index],
>                            env->last_icount + itrigger_get_count(env, index));
> -            } else {
> -                env->itrigger_enabled = riscv_itrigger_enabled(env);
>              }
>          }
>          break;
> @@ -881,12 +860,30 @@ target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
>  
>  void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>  {
> -    int trigger_type;
> +    int trigger_type = get_trigger_type(env, env->trigger_cur);
> +    bool check_itrigger = false;
> +
> +    switch (trigger_type) {
> +    case TRIGGER_TYPE_AD_MATCH:
> +        type2_breakpoint_remove(env, env->trigger_cur);
> +        break;
> +    case TRIGGER_TYPE_AD_MATCH6:
> +        type6_breakpoint_remove(env, env->trigger_cur);
> +        break;
> +    case TRIGGER_TYPE_INST_CNT:
> +        /*
> +         * itrigger_enabled is the union of all enabled icount triggers,
> +         * so it's easiest to recheck all if any have changed (removed or
> +         * added or modified).
> +         */
> +        check_itrigger = true;
> +        break;
> +    default:
> +        break;
> +    }
>  
>      if (tdata_index == TDATA1) {
>          trigger_type = extract_trigger_type(env, val);
> -    } else {
> -        trigger_type = get_trigger_type(env, env->trigger_cur);
>      }
>  
>      switch (trigger_type) {
> @@ -898,6 +895,7 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>          break;
>      case TRIGGER_TYPE_INST_CNT:
>          itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
> +        check_itrigger = true;
>          break;
>      case TRIGGER_TYPE_INT:
>      case TRIGGER_TYPE_EXCP:
> @@ -913,6 +911,10 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>      default:
>          g_assert_not_reached();
>      }
> +
> +    if (check_itrigger && !icount_enabled()) {
> +        env->itrigger_enabled = riscv_itrigger_enabled(env);
> +    }
>  }
>  
>  target_ulong tinfo_csr_read(CPURISCVState *env)
> -- 
> 2.51.0
> 
> 

[RFC PATCH 03/25] target/riscv/debug: Implement permissive type unavailable trigger

[Nicholas Piggin]

When a trigger type is "disabled", tdata2 and tdata3 must accept values
that are valid for some supported trigger type. Additionally, writing 0
to tdata1 must result in the type becoming "disabled". This is important
for the prescribed sequences for updating triggers.

Implement write tdata=0 -> disabled behaviour and permissive accepting
of tdata2/3 values in disabled state. This implementation could be
improved by checking tdata2/3 values against supported trigger types,
but it is good enough to be usable by software.

From the RISC-V Debug Specification for tdata1:

  Writing 0 to this register must result in a trigger that is disabled.
  If this trigger supports multiple types, then the hardware should
  disable it by changing type to 15.

and, when type=15:

  This trigger is disabled. In this state, tdata2 and tdata3 can be
  written with any value that is supported for any of the types this
  trigger implements.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 2190c25f23..c92bd9860e 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -827,6 +827,28 @@ static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
     }
 }
 
+static void anytype_reg_write(CPURISCVState *env, target_ulong index,
+                              int tdata_index, target_ulong val)
+{
+    /*
+     * This should check the value is valid for at least one of the supported
+     * trigger types.
+     */
+    switch (tdata_index) {
+    case TDATA1:
+        env->tdata1[env->trigger_cur] = val;
+        break;
+    case TDATA2:
+        env->tdata2[env->trigger_cur] = val;
+        break;
+    case TDATA3:
+        env->tdata3[env->trigger_cur] = val;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+}
+
 static int itrigger_get_adjust_count(CPURISCVState *env)
 {
     int count = itrigger_get_count(env, env->trigger_cur), executed;
@@ -883,6 +905,10 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
     }
 
     if (tdata_index == TDATA1) {
+        if (val == 0) {
+            /* special case, writing 0 results in disabled trigger */
+            val = build_tdata1(env, TRIGGER_TYPE_UNAVAIL, 0, 0);
+        }
         trigger_type = extract_trigger_type(env, val);
     }
 
@@ -897,6 +923,9 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
         itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
         check_itrigger = true;
         break;
+    case TRIGGER_TYPE_UNAVAIL:
+        anytype_reg_write(env, env->trigger_cur, tdata_index, val);
+        break;
     case TRIGGER_TYPE_INT:
     case TRIGGER_TYPE_EXCP:
     case TRIGGER_TYPE_EXT_SRC:
@@ -904,7 +933,6 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
                       trigger_type);
         break;
     case TRIGGER_TYPE_NO_EXIST:
-    case TRIGGER_TYPE_UNAVAIL:
         qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
                       trigger_type);
         break;
-- 
2.51.0

Re: [RFC PATCH 03/25] target/riscv/debug: Implement permissive type unavailable trigger

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> When a trigger type is "disabled", tdata2 and tdata3 must accept values
> that are valid for some supported trigger type. Additionally, writing 0
> to tdata1 must result in the type becoming "disabled". This is important
> for the prescribed sequences for updating triggers.
> 
> Implement write tdata=0 -> disabled behaviour and permissive accepting
> of tdata2/3 values in disabled state. This implementation could be
> improved by checking tdata2/3 values against supported trigger types,
> but it is good enough to be usable by software.
> 
>  From the RISC-V Debug Specification for tdata1:
> 
>    Writing 0 to this register must result in a trigger that is disabled.
>    If this trigger supports multiple types, then the hardware should
>    disable it by changing type to 15.
> 
> and, when type=15:
> 
>    This trigger is disabled. In this state, tdata2 and tdata3 can be
>    written with any value that is supported for any of the types this
>    trigger implements.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 30 +++++++++++++++++++++++++++++-
>   1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 2190c25f23..c92bd9860e 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -827,6 +827,28 @@ static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
>       }
>   }
>   
> +static void anytype_reg_write(CPURISCVState *env, target_ulong index,
> +                              int tdata_index, target_ulong val)
> +{
> +    /*
> +     * This should check the value is valid for at least one of the supported
> +     * trigger types.
> +     */
> +    switch (tdata_index) {
> +    case TDATA1:
> +        env->tdata1[env->trigger_cur] = val;
> +        break;
> +    case TDATA2:
> +        env->tdata2[env->trigger_cur] = val;
> +        break;
> +    case TDATA3:
> +        env->tdata3[env->trigger_cur] = val;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
>   static int itrigger_get_adjust_count(CPURISCVState *env)
>   {
>       int count = itrigger_get_count(env, env->trigger_cur), executed;
> @@ -883,6 +905,10 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>       }
>   
>       if (tdata_index == TDATA1) {
> +        if (val == 0) {
> +            /* special case, writing 0 results in disabled trigger */
> +            val = build_tdata1(env, TRIGGER_TYPE_UNAVAIL, 0, 0);
> +        }
>           trigger_type = extract_trigger_type(env, val);
>       }
>   
> @@ -897,6 +923,9 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>           itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
>           check_itrigger = true;
>           break;
> +    case TRIGGER_TYPE_UNAVAIL:
> +        anytype_reg_write(env, env->trigger_cur, tdata_index, val);
> +        break;
>       case TRIGGER_TYPE_INT:
>       case TRIGGER_TYPE_EXCP:
>       case TRIGGER_TYPE_EXT_SRC:
> @@ -904,7 +933,6 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>                         trigger_type);
>           break;
>       case TRIGGER_TYPE_NO_EXIST:
> -    case TRIGGER_TYPE_UNAVAIL:
>           qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
>                         trigger_type);
>           break;

Re: [RFC PATCH 03/25] target/riscv/debug: Implement permissive type unavailable trigger

[Chao Liu]

On Wed, Jan 14, 2026 at 02:46:36PM +0800, Nicholas Piggin wrote:
> When a trigger type is "disabled", tdata2 and tdata3 must accept values
> that are valid for some supported trigger type. Additionally, writing 0
> to tdata1 must result in the type becoming "disabled". This is important
> for the prescribed sequences for updating triggers.
> 
> Implement write tdata=0 -> disabled behaviour and permissive accepting
> of tdata2/3 values in disabled state. This implementation could be
> improved by checking tdata2/3 values against supported trigger types,
> but it is good enough to be usable by software.
> 
> From the RISC-V Debug Specification for tdata1:
> 
>   Writing 0 to this register must result in a trigger that is disabled.
>   If this trigger supports multiple types, then the hardware should
>   disable it by changing type to 15.
> 
> and, when type=15:
> 
>   This trigger is disabled. In this state, tdata2 and tdata3 can be
>   written with any value that is supported for any of the types this
>   trigger implements.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

> ---
>  target/riscv/debug.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 2190c25f23..c92bd9860e 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -827,6 +827,28 @@ static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
>      }
>  }
>  
> +static void anytype_reg_write(CPURISCVState *env, target_ulong index,
> +                              int tdata_index, target_ulong val)
> +{
> +    /*
> +     * This should check the value is valid for at least one of the supported
> +     * trigger types.
> +     */
> +    switch (tdata_index) {
> +    case TDATA1:
> +        env->tdata1[env->trigger_cur] = val;
> +        break;
> +    case TDATA2:
> +        env->tdata2[env->trigger_cur] = val;
> +        break;
> +    case TDATA3:
> +        env->tdata3[env->trigger_cur] = val;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
>  static int itrigger_get_adjust_count(CPURISCVState *env)
>  {
>      int count = itrigger_get_count(env, env->trigger_cur), executed;
> @@ -883,6 +905,10 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>      }
>  
>      if (tdata_index == TDATA1) {
> +        if (val == 0) {
> +            /* special case, writing 0 results in disabled trigger */
> +            val = build_tdata1(env, TRIGGER_TYPE_UNAVAIL, 0, 0);
> +        }
>          trigger_type = extract_trigger_type(env, val);
>      }
>  
> @@ -897,6 +923,9 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>          itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
>          check_itrigger = true;
>          break;
> +    case TRIGGER_TYPE_UNAVAIL:
> +        anytype_reg_write(env, env->trigger_cur, tdata_index, val);
> +        break;
>      case TRIGGER_TYPE_INT:
>      case TRIGGER_TYPE_EXCP:
>      case TRIGGER_TYPE_EXT_SRC:
> @@ -904,7 +933,6 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>                        trigger_type);
>          break;
>      case TRIGGER_TYPE_NO_EXIST:
> -    case TRIGGER_TYPE_UNAVAIL:
>          qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
>                        trigger_type);
>          break;
> -- 
> 2.51.0
> 
> 

[RFC PATCH 04/25] target/riscv/debug: Fix icount trigger privilege check

[Nicholas Piggin]

The check_itrigger_priv() did not check privilege bits properly. Move
all priv checks into functions and have the icount check follow the same
form as the others.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 82 ++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index c92bd9860e..2effbb49af 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -306,48 +306,50 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
  * Check the privilege level of specific trigger matches CPU's current privilege
  * level.
  */
+static bool type2_priv_match(CPURISCVState *env, target_ulong tdata1)
+{
+    /* type 2 trigger cannot be fired in VU/VS mode */
+    if (env->virt_enabled) {
+        return false;
+    }
+    /* check U/S/M bit against current privilege level */
+    return (((tdata1 >> 3) & 0b1011) & BIT(env->priv));
+}
+
+static bool type6_priv_match(CPURISCVState *env, target_ulong tdata1)
+{
+    if (env->virt_enabled) {
+        /* check VU/VS bit against current privilege level */
+        return (((tdata1 >> 23) & 0b11) & BIT(env->priv));
+    } else {
+        /* check U/S/M bit against current privilege level */
+        return (((tdata1 >> 3) & 0b1011) & BIT(env->priv));
+    }
+}
+
+static bool icount_priv_match(CPURISCVState *env, target_ulong tdata1)
+{
+    if (env->virt_enabled) {
+        /* check VU/VS bit against current privilege level */
+        return (((tdata1 >> 25) & 0b11) & BIT(env->priv));
+    } else {
+        /* check U/S/M bit against current privilege level */
+        return (((tdata1 >> 6) & 0b1011) & BIT(env->priv));
+    }
+}
+
 static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
                                int trigger_index)
 {
-    target_ulong ctrl = env->tdata1[trigger_index];
+    target_ulong tdata1 = env->tdata1[trigger_index];
 
     switch (type) {
     case TRIGGER_TYPE_AD_MATCH:
-        /* type 2 trigger cannot be fired in VU/VS mode */
-        if (env->virt_enabled) {
-            return false;
-        }
-        /* check U/S/M bit against current privilege level */
-        if ((ctrl >> 3) & BIT(env->priv)) {
-            return true;
-        }
-        break;
+        return type2_priv_match(env, tdata1);
     case TRIGGER_TYPE_AD_MATCH6:
-        if (env->virt_enabled) {
-            /* check VU/VS bit against current privilege level */
-            if ((ctrl >> 23) & BIT(env->priv)) {
-                return true;
-            }
-        } else {
-            /* check U/S/M bit against current privilege level */
-            if ((ctrl >> 3) & BIT(env->priv)) {
-                return true;
-            }
-        }
-        break;
+        return type6_priv_match(env, tdata1);
     case TRIGGER_TYPE_INST_CNT:
-        if (env->virt_enabled) {
-            /* check VU/VS bit against current privilege level */
-            if ((ctrl >> 25) & BIT(env->priv)) {
-                return true;
-            }
-        } else {
-            /* check U/S/M bit against current privilege level */
-            if ((ctrl >> 6) & BIT(env->priv)) {
-                return true;
-            }
-        }
-        break;
+        return icount_priv_match(env, tdata1);
     case TRIGGER_TYPE_INT:
     case TRIGGER_TYPE_EXCP:
     case TRIGGER_TYPE_EXT_SRC:
@@ -665,17 +667,7 @@ itrigger_set_count(CPURISCVState *env, int index, int value)
 
 static bool check_itrigger_priv(CPURISCVState *env, int index)
 {
-    target_ulong tdata1 = env->tdata1[index];
-    if (env->virt_enabled) {
-        /* check VU/VS bit against current privilege level */
-        return (get_field(tdata1, ITRIGGER_VS) == env->priv) ||
-               (get_field(tdata1, ITRIGGER_VU) == env->priv);
-    } else {
-        /* check U/S/M bit against current privilege level */
-        return (get_field(tdata1, ITRIGGER_M) == env->priv) ||
-               (get_field(tdata1, ITRIGGER_S) == env->priv) ||
-               (get_field(tdata1, ITRIGGER_U) == env->priv);
-    }
+    return icount_priv_match(env, index);
 }
 
 bool riscv_itrigger_enabled(CPURISCVState *env)
-- 
2.51.0

Re: [RFC PATCH 04/25] target/riscv/debug: Fix icount trigger privilege check

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> The check_itrigger_priv() did not check privilege bits properly. Move
> all priv checks into functions and have the icount check follow the same
> form as the others.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 82 ++++++++++++++++++++------------------------
>   1 file changed, 37 insertions(+), 45 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index c92bd9860e..2effbb49af 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -306,48 +306,50 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
>    * Check the privilege level of specific trigger matches CPU's current privilege
>    * level.
>    */
> +static bool type2_priv_match(CPURISCVState *env, target_ulong tdata1)
> +{
> +    /* type 2 trigger cannot be fired in VU/VS mode */
> +    if (env->virt_enabled) {
> +        return false;
> +    }
> +    /* check U/S/M bit against current privilege level */
> +    return (((tdata1 >> 3) & 0b1011) & BIT(env->priv));
> +}
> +
> +static bool type6_priv_match(CPURISCVState *env, target_ulong tdata1)
> +{
> +    if (env->virt_enabled) {
> +        /* check VU/VS bit against current privilege level */
> +        return (((tdata1 >> 23) & 0b11) & BIT(env->priv));
> +    } else {
> +        /* check U/S/M bit against current privilege level */
> +        return (((tdata1 >> 3) & 0b1011) & BIT(env->priv));
> +    }
> +}
> +
> +static bool icount_priv_match(CPURISCVState *env, target_ulong tdata1)
> +{
> +    if (env->virt_enabled) {
> +        /* check VU/VS bit against current privilege level */
> +        return (((tdata1 >> 25) & 0b11) & BIT(env->priv));
> +    } else {
> +        /* check U/S/M bit against current privilege level */
> +        return (((tdata1 >> 6) & 0b1011) & BIT(env->priv));
> +    }
> +}
> +
>   static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
>                                  int trigger_index)
>   {
> -    target_ulong ctrl = env->tdata1[trigger_index];
> +    target_ulong tdata1 = env->tdata1[trigger_index];
>   
>       switch (type) {
>       case TRIGGER_TYPE_AD_MATCH:
> -        /* type 2 trigger cannot be fired in VU/VS mode */
> -        if (env->virt_enabled) {
> -            return false;
> -        }
> -        /* check U/S/M bit against current privilege level */
> -        if ((ctrl >> 3) & BIT(env->priv)) {
> -            return true;
> -        }
> -        break;
> +        return type2_priv_match(env, tdata1);
>       case TRIGGER_TYPE_AD_MATCH6:
> -        if (env->virt_enabled) {
> -            /* check VU/VS bit against current privilege level */
> -            if ((ctrl >> 23) & BIT(env->priv)) {
> -                return true;
> -            }
> -        } else {
> -            /* check U/S/M bit against current privilege level */
> -            if ((ctrl >> 3) & BIT(env->priv)) {
> -                return true;
> -            }
> -        }
> -        break;
> +        return type6_priv_match(env, tdata1);
>       case TRIGGER_TYPE_INST_CNT:
> -        if (env->virt_enabled) {
> -            /* check VU/VS bit against current privilege level */
> -            if ((ctrl >> 25) & BIT(env->priv)) {
> -                return true;
> -            }
> -        } else {
> -            /* check U/S/M bit against current privilege level */
> -            if ((ctrl >> 6) & BIT(env->priv)) {
> -                return true;
> -            }
> -        }
> -        break;
> +        return icount_priv_match(env, tdata1);
>       case TRIGGER_TYPE_INT:
>       case TRIGGER_TYPE_EXCP:
>       case TRIGGER_TYPE_EXT_SRC:
> @@ -665,17 +667,7 @@ itrigger_set_count(CPURISCVState *env, int index, int value)
>   
>   static bool check_itrigger_priv(CPURISCVState *env, int index)
>   {
> -    target_ulong tdata1 = env->tdata1[index];
> -    if (env->virt_enabled) {
> -        /* check VU/VS bit against current privilege level */
> -        return (get_field(tdata1, ITRIGGER_VS) == env->priv) ||
> -               (get_field(tdata1, ITRIGGER_VU) == env->priv);
> -    } else {
> -        /* check U/S/M bit against current privilege level */
> -        return (get_field(tdata1, ITRIGGER_M) == env->priv) ||
> -               (get_field(tdata1, ITRIGGER_S) == env->priv) ||
> -               (get_field(tdata1, ITRIGGER_U) == env->priv);
> -    }
> +    return icount_priv_match(env, index);
>   }
>   
>   bool riscv_itrigger_enabled(CPURISCVState *env)

Re: [RFC PATCH 04/25] target/riscv/debug: Fix icount trigger privilege check

[Chao Liu]

On Wed, Jan 14, 2026 at 02:46:37PM +0800, Nicholas Piggin wrote:
> The check_itrigger_priv() did not check privilege bits properly. Move
> all priv checks into functions and have the icount check follow the same
> form as the others.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  target/riscv/debug.c | 82 ++++++++++++++++++++------------------------
>  1 file changed, 37 insertions(+), 45 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index c92bd9860e..2effbb49af 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -306,48 +306,50 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
>   * Check the privilege level of specific trigger matches CPU's current privilege
>   * level.
>   */
> +static bool type2_priv_match(CPURISCVState *env, target_ulong tdata1)
> +{
> +    /* type 2 trigger cannot be fired in VU/VS mode */
> +    if (env->virt_enabled) {
> +        return false;
> +    }
> +    /* check U/S/M bit against current privilege level */
> +    return (((tdata1 >> 3) & 0b1011) & BIT(env->priv));
> +}
> +
> +static bool type6_priv_match(CPURISCVState *env, target_ulong tdata1)
> +{
> +    if (env->virt_enabled) {
> +        /* check VU/VS bit against current privilege level */
> +        return (((tdata1 >> 23) & 0b11) & BIT(env->priv));
> +    } else {
> +        /* check U/S/M bit against current privilege level */
> +        return (((tdata1 >> 3) & 0b1011) & BIT(env->priv));
> +    }
> +}
The new logic looks correct to me, but now it uses open-coded shifts
and masks to check the privilege bits.

Since we already have these bits defined in debug.h, like `TYPE2_U/S/M`,
`TYPE6_VU/VS/U/S/M` and `ITRIGGER_VU/VS/U/S/M`, I think it would be
better to use those masks through a small helper instead of spelling
out `>> 3`, `>> 6`, `>> 23` and `0b1011` here.

That would make the code easier to read and maintain later.

like:

```
static bool trigger_priv_bit_match(target_ulong tdata1,
                                   target_ulong u_mask,
                                   target_ulong s_mask,
                                   target_ulong m_mask,
                                   target_ulong priv)
{
    switch (priv) {
    case PRV_U:
        return (tdata1 & u_mask) != 0;
    case PRV_S:
        return (tdata1 & s_mask) != 0;
    case PRV_M:
        return (tdata1 & m_mask) != 0;
    default:
        g_assert_not_reached();
    }
}

static bool trigger_vpriv_bit_match(target_ulong tdata1,
                                    target_ulong vu_mask,
                                    target_ulong vs_mask,
                                    target_ulong priv)
{
    switch (priv) {
    case PRV_U:
        return (tdata1 & vu_mask) != 0;
    case PRV_S:
        return (tdata1 & vs_mask) != 0;
    default:
        return false;
    }
}
```

So we can call it this way:

```
static bool type2_priv_match(CPURISCVState *env, target_ulong tdata1)
{
    /* type 2 trigger cannot be fired in VU/VS mode */
    if (env->virt_enabled) {
        return false;
    }

    return trigger_priv_bit_match(tdata1, TYPE2_U, TYPE2_S, TYPE2_M,
                                  env->priv);
}

static bool type6_priv_match(CPURISCVState *env, target_ulong tdata1)
{
    if (env->virt_enabled) {
        return trigger_vpriv_bit_match(tdata1, TYPE6_VU, TYPE6_VS,
                                       env->priv);
    }

    return trigger_priv_bit_match(tdata1, TYPE6_U, TYPE6_S, TYPE6_M,
                                  env->priv);
}

static bool icount_priv_match(CPURISCVState *env, target_ulong tdata1)
{
    if (env->virt_enabled) {
        return trigger_vpriv_bit_match(tdata1, ITRIGGER_VU, ITRIGGER_VS,
                                       env->priv);
    }

    return trigger_priv_bit_match(tdata1, ITRIGGER_U, ITRIGGER_S, ITRIGGER_M,
                                  env->priv);
}
```

And I don't mean going back to `get_field(mask) == env->priv`; that pattern
is easy to get wrong for `PRV_U == 0`. The helper should just test the mask
selected by the current privilege.

Thanks,
Chao
> +
> +static bool icount_priv_match(CPURISCVState *env, target_ulong tdata1)
> +{
> +    if (env->virt_enabled) {
> +        /* check VU/VS bit against current privilege level */
> +        return (((tdata1 >> 25) & 0b11) & BIT(env->priv));
> +    } else {
> +        /* check U/S/M bit against current privilege level */
> +        return (((tdata1 >> 6) & 0b1011) & BIT(env->priv));
> +    }
> +}
> +
>  static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
>                                 int trigger_index)
>  {
> -    target_ulong ctrl = env->tdata1[trigger_index];
> +    target_ulong tdata1 = env->tdata1[trigger_index];
>  
>      switch (type) {
>      case TRIGGER_TYPE_AD_MATCH:
> -        /* type 2 trigger cannot be fired in VU/VS mode */
> -        if (env->virt_enabled) {
> -            return false;
> -        }
> -        /* check U/S/M bit against current privilege level */
> -        if ((ctrl >> 3) & BIT(env->priv)) {
> -            return true;
> -        }
> -        break;
> +        return type2_priv_match(env, tdata1);
>      case TRIGGER_TYPE_AD_MATCH6:
> -        if (env->virt_enabled) {
> -            /* check VU/VS bit against current privilege level */
> -            if ((ctrl >> 23) & BIT(env->priv)) {
> -                return true;
> -            }
> -        } else {
> -            /* check U/S/M bit against current privilege level */
> -            if ((ctrl >> 3) & BIT(env->priv)) {
> -                return true;
> -            }
> -        }
> -        break;
> +        return type6_priv_match(env, tdata1);
>      case TRIGGER_TYPE_INST_CNT:
> -        if (env->virt_enabled) {
> -            /* check VU/VS bit against current privilege level */
> -            if ((ctrl >> 25) & BIT(env->priv)) {
> -                return true;
> -            }
> -        } else {
> -            /* check U/S/M bit against current privilege level */
> -            if ((ctrl >> 6) & BIT(env->priv)) {
> -                return true;
> -            }
> -        }
> -        break;
> +        return icount_priv_match(env, tdata1);
>      case TRIGGER_TYPE_INT:
>      case TRIGGER_TYPE_EXCP:
>      case TRIGGER_TYPE_EXT_SRC:
> @@ -665,17 +667,7 @@ itrigger_set_count(CPURISCVState *env, int index, int value)
>  
>  static bool check_itrigger_priv(CPURISCVState *env, int index)
>  {
> -    target_ulong tdata1 = env->tdata1[index];
> -    if (env->virt_enabled) {
> -        /* check VU/VS bit against current privilege level */
> -        return (get_field(tdata1, ITRIGGER_VS) == env->priv) ||
> -               (get_field(tdata1, ITRIGGER_VU) == env->priv);
> -    } else {
> -        /* check U/S/M bit against current privilege level */
> -        return (get_field(tdata1, ITRIGGER_M) == env->priv) ||
> -               (get_field(tdata1, ITRIGGER_S) == env->priv) ||
> -               (get_field(tdata1, ITRIGGER_U) == env->priv);
> -    }
> +    return icount_priv_match(env, index);
>  }
>  
>  bool riscv_itrigger_enabled(CPURISCVState *env)
> -- 
> 2.51.0
> 
> 

[RFC PATCH 05/25] target/riscv/debug: Update itrigger_enabled after changing privilege

[Nicholas Piggin]

The !icount_enabled() path should update itrigger_enabled after changing
privilege, which can change whether or not an icount trigger is enabled.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/cpu_helper.c | 7 +++++++
 target/riscv/debug.c      | 7 +++++++
 target/riscv/debug.h      | 3 +++
 3 files changed, 17 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index dd6c861a90..e096da939b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1030,9 +1030,12 @@ void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
 
 void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
 {
+    bool change = false;
+
     g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
 
     if (newpriv != env->priv || env->virt_enabled != virt_en) {
+        change = true;
         if (icount_enabled()) {
             riscv_itrigger_update_priv(env);
         }
@@ -1075,6 +1078,10 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
             riscv_cpu_update_mip(env, 0, 0);
         }
     }
+
+    if (change) {
+        riscv_cpu_debug_change_priv(env);
+    }
 }
 
 /*
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 2effbb49af..51c5934bfa 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -1090,6 +1090,13 @@ void riscv_trigger_realize(CPURISCVState *env)
     }
 }
 
+void riscv_cpu_debug_change_priv(CPURISCVState *env)
+{
+    if (!icount_enabled()) {
+        env->itrigger_enabled = riscv_itrigger_enabled(env);
+    }
+}
+
 void riscv_trigger_reset_hold(CPURISCVState *env)
 {
     target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index f76b8f944a..5a14b7894e 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -153,4 +153,7 @@ void riscv_trigger_reset_hold(CPURISCVState *env);
 
 bool riscv_itrigger_enabled(CPURISCVState *env);
 void riscv_itrigger_update_priv(CPURISCVState *env);
+
+void riscv_cpu_debug_change_priv(CPURISCVState *env);
+
 #endif /* RISCV_DEBUG_H */
-- 
2.51.0

Re: [RFC PATCH 05/25] target/riscv/debug: Update itrigger_enabled after changing privilege

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> The !icount_enabled() path should update itrigger_enabled after changing
> privilege, which can change whether or not an icount trigger is enabled.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   target/riscv/cpu_helper.c | 7 +++++++
>   target/riscv/debug.c      | 7 +++++++
>   target/riscv/debug.h      | 3 +++
>   3 files changed, 17 insertions(+)
> 
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index dd6c861a90..e096da939b 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1030,9 +1030,12 @@ void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
>   
>   void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
>   {
> +    bool change = false;
> +
>       g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
>   
>       if (newpriv != env->priv || env->virt_enabled != virt_en) {
> +        change = true;
>           if (icount_enabled()) {
>               riscv_itrigger_update_priv(env);
>           }
> @@ -1075,6 +1078,10 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
>               riscv_cpu_update_mip(env, 0, 0);
>           }
>       }
> +
> +    if (change) {
> +        riscv_cpu_debug_change_priv(env);
> +    }

Is there a particular reason to do this change at the end of cpu_set_mode()?
Because if there isn't, we can remove the 'change' flag, then just do a:

 >       if (newpriv != env->priv || env->virt_enabled != virt_en) {
 > +        if (!icount_enabled()) {
 > +            env->itrigger_enabled = riscv_itrigger_enabled(env);
 > +        }
 >           if (icount_enabled()) {
 >               riscv_itrigger_update_priv(env);
 >           }



You'll remove the icount_enabled() branch in patch 16 anyway so I wouldn't bother
do a if/then/else with icount_enabled(), but can be done too.


This will also get rid of the riscv_cpu_debug_change_priv() function added here.
Which is fine: from what I can see in the patches this is the only place that
it would be used, so no harm done.


Thanks,
Daniel



>   }
>   
>   /*
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 2effbb49af..51c5934bfa 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -1090,6 +1090,13 @@ void riscv_trigger_realize(CPURISCVState *env)
>       }
>   }
>   
> +void riscv_cpu_debug_change_priv(CPURISCVState *env)
> +{
> +    if (!icount_enabled()) {
> +        env->itrigger_enabled = riscv_itrigger_enabled(env);
> +    }
> +}
> +
>   void riscv_trigger_reset_hold(CPURISCVState *env)
>   {
>       target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index f76b8f944a..5a14b7894e 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -153,4 +153,7 @@ void riscv_trigger_reset_hold(CPURISCVState *env);
>   
>   bool riscv_itrigger_enabled(CPURISCVState *env);
>   void riscv_itrigger_update_priv(CPURISCVState *env);
> +
> +void riscv_cpu_debug_change_priv(CPURISCVState *env);
> +
>   #endif /* RISCV_DEBUG_H */

[RFC PATCH 06/25] target/riscv/debug: Implement get_trigger_action for icount type trigger

[Nicholas Piggin]

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 51c5934bfa..af9c4e37cd 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:
-- 
2.51.0

Re: [RFC PATCH 06/25] target/riscv/debug: Implement get_trigger_action for icount type trigger

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 51c5934bfa..af9c4e37cd 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:

[RFC PATCH 07/25] target/riscv/debug: Fix migration post_load icount_enabled() test

[Nicholas Piggin]

itrigger_enabled is relevant to the !icount_enabled() path, the test in
debug_post_load() is inverted.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 09c032a879..34fd73c920 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -230,7 +230,7 @@ static int debug_post_load(void *opaque, int version_id)
     RISCVCPU *cpu = opaque;
     CPURISCVState *env = &cpu->env;
 
-    if (icount_enabled()) {
+    if (!icount_enabled()) {
         env->itrigger_enabled = riscv_itrigger_enabled(env);
     }
 
-- 
2.51.0

Re: [RFC PATCH 07/25] target/riscv/debug: Fix migration post_load icount_enabled() test

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> itrigger_enabled is relevant to the !icount_enabled() path, the test in
> debug_post_load() is inverted.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/machine.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 09c032a879..34fd73c920 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -230,7 +230,7 @@ static int debug_post_load(void *opaque, int version_id)
>       RISCVCPU *cpu = opaque;
>       CPURISCVState *env = &cpu->env;
>   
> -    if (icount_enabled()) {
> +    if (!icount_enabled()) {
>           env->itrigger_enabled = riscv_itrigger_enabled(env);
>       }
>   

[RFC PATCH 08/25] target/riscv/debug: Fix icount privilege matching icount_enabled() test

[Nicholas Piggin]

itrigger_enabled should be set if the privilege does match. The test is
inverted.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index af9c4e37cd..ecedffd3b9 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -679,7 +679,7 @@ bool riscv_itrigger_enabled(CPURISCVState *env)
         if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
             continue;
         }
-        if (check_itrigger_priv(env, i)) {
+        if (!check_itrigger_priv(env, i)) {
             continue;
         }
         count = itrigger_get_count(env, i);
-- 
2.51.0

Re: [RFC PATCH 08/25] target/riscv/debug: Fix icount privilege matching icount_enabled() test

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> itrigger_enabled should be set if the privilege does match. The test is
> inverted.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index af9c4e37cd..ecedffd3b9 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -679,7 +679,7 @@ bool riscv_itrigger_enabled(CPURISCVState *env)
>           if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
>               continue;
>           }
> -        if (check_itrigger_priv(env, i)) {
> +        if (!check_itrigger_priv(env, i)) {
>               continue;
>           }
>           count = itrigger_get_count(env, i);

[RFC PATCH 09/25] target/riscv/debug: Implement icount trigger textra matching

[Nicholas Piggin]

icount triggers are subject to textra checks, per spec.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index ecedffd3b9..d835c0e79a 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -675,11 +675,12 @@ static bool check_itrigger_priv(CPURISCVState *env, int index)
 bool riscv_itrigger_enabled(CPURISCVState *env)
 {
     int count;
+
     for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
         if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
             continue;
         }
-        if (!check_itrigger_priv(env, i)) {
+        if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
             continue;
         }
         count = itrigger_get_count(env, i);
-- 
2.51.0

Re: [RFC PATCH 09/25] target/riscv/debug: Implement icount trigger textra matching

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> icount triggers are subject to textra checks, per spec.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index ecedffd3b9..d835c0e79a 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -675,11 +675,12 @@ static bool check_itrigger_priv(CPURISCVState *env, int index)
>   bool riscv_itrigger_enabled(CPURISCVState *env)
>   {
>       int count;
> +
>       for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
>           if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
>               continue;
>           }
> -        if (!check_itrigger_priv(env, i)) {
> +        if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
>               continue;
>           }
>           count = itrigger_get_count(env, i);

[RFC PATCH 10/25] target/riscv/debug: Maintain itrigger_enabled in helper_itrigger_match()

[Nicholas Piggin]

helper_itrigger_match() iterates over all triggers so it can maintain
itrigger_enabled rather than calling riscv_itrigger_enabled() which
becomes O(N^2) where N is the number of icount triggers.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index d835c0e79a..7ae02fe2d2 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -693,9 +693,18 @@ bool riscv_itrigger_enabled(CPURISCVState *env)
     return false;
 }
 
+/*
+ * This is called by TCG when an instruction completes.
+ * TCG runs in single-step mode when itrigger_enabled = true, so
+ * it can call after each insn.
+ */
 void helper_itrigger_match(CPURISCVState *env)
 {
     int count;
+    bool enabled = false;
+
+    g_assert(env->itrigger_enabled);
+
     for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
         if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
             continue;
@@ -709,10 +718,12 @@ void helper_itrigger_match(CPURISCVState *env)
         }
         itrigger_set_count(env, i, count--);
         if (!count) {
-            env->itrigger_enabled = riscv_itrigger_enabled(env);
             do_trigger_action(env, i);
+        } else {
+            enabled = true;
         }
     }
+    env->itrigger_enabled = enabled;
 }
 
 static void riscv_itrigger_update_count(CPURISCVState *env)
-- 
2.51.0

Re: [RFC PATCH 10/25] target/riscv/debug: Maintain itrigger_enabled in helper_itrigger_match()

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> helper_itrigger_match() iterates over all triggers so it can maintain
> itrigger_enabled rather than calling riscv_itrigger_enabled() which
> becomes O(N^2) where N is the number of icount triggers.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index d835c0e79a..7ae02fe2d2 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -693,9 +693,18 @@ bool riscv_itrigger_enabled(CPURISCVState *env)
>       return false;
>   }
>   
> +/*
> + * This is called by TCG when an instruction completes.
> + * TCG runs in single-step mode when itrigger_enabled = true, so
> + * it can call after each insn.
> + */
>   void helper_itrigger_match(CPURISCVState *env)
>   {
>       int count;
> +    bool enabled = false;
> +
> +    g_assert(env->itrigger_enabled);
> +
>       for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
>           if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
>               continue;
> @@ -709,10 +718,12 @@ void helper_itrigger_match(CPURISCVState *env)
>           }
>           itrigger_set_count(env, i, count--);
>           if (!count) {
> -            env->itrigger_enabled = riscv_itrigger_enabled(env);
>               do_trigger_action(env, i);
> +        } else {
> +            enabled = true;
>           }
>       }
> +    env->itrigger_enabled = enabled;
>   }
>   
>   static void riscv_itrigger_update_count(CPURISCVState *env)

[RFC PATCH 11/25] target/riscv/debug: Fix breakpoint matching action

[Nicholas Piggin]

The debug exception callback is too late to find the action for the
trigger(s) which caused it, and it is actually passing the wrong thing
to do_trigger_action(), which expects a trigger index but is given
DBG_ACTION_BP (which will be interpreted as trigger 0 and use the
action set for that trigger).

It could be possible to derive the trigger index from the bp/wp address,
but that is clunky and it is really the action that determines whether
an exception should be raised, also multiple triggers may perform their
actions in the same cycle, so it is more consistent to check action
during the breakpoint matching phase. If a breakpoint exception is to be
taken then that is signaled at that time.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 7ae02fe2d2..bd61b7ff02 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -280,7 +280,8 @@ static target_ulong textra_validate(CPURISCVState *env, target_ulong tdata3)
     return textra;
 }
 
-static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
+/* Return true if an exception should be raised */
+static bool do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
 {
     trigger_action_t action = get_trigger_action(env, trigger_index);
 
@@ -288,8 +289,7 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
     case DBG_ACTION_NONE:
         break;
     case DBG_ACTION_BP:
-        riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
-        break;
+        return true;
     case DBG_ACTION_DBG_MODE:
     case DBG_ACTION_TRACE0:
     case DBG_ACTION_TRACE1:
@@ -302,6 +302,7 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
     default:
         g_assert_not_reached();
     }
+    return false;
 }
 
 /*
@@ -718,7 +719,9 @@ void helper_itrigger_match(CPURISCVState *env)
         }
         itrigger_set_count(env, i, count--);
         if (!count) {
-            do_trigger_action(env, i);
+            if (do_trigger_action(env, i)) {
+                riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+            }
         } else {
             enabled = true;
         }
@@ -965,11 +968,11 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
 
     if (cs->watchpoint_hit) {
         if (cs->watchpoint_hit->flags & BP_CPU) {
-            do_trigger_action(env, DBG_ACTION_BP);
+            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
         }
     } else {
         if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
-            do_trigger_action(env, DBG_ACTION_BP);
+            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
         }
     }
 }
@@ -1006,8 +1009,10 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
                 pc = env->tdata2[i];
 
                 if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
-                    env->badaddr = pc;
-                    return true;
+                    if (do_trigger_action(env, i)) {
+                        env->badaddr = pc;
+                        return true;
+                    }
                 }
                 break;
             case TRIGGER_TYPE_AD_MATCH6:
@@ -1015,8 +1020,10 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
                 pc = env->tdata2[i];
 
                 if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
-                    env->badaddr = pc;
-                    return true;
+                    if (do_trigger_action(env, i)) {
+                        env->badaddr = pc;
+                        return true;
+                    }
                 }
                 break;
             default:
@@ -1067,7 +1074,9 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
             }
 
             if ((wp->flags & flags) && (wp->vaddr == addr)) {
-                return true;
+                if (do_trigger_action(env, i)) {
+                    return true;
+                }
             }
             break;
         case TRIGGER_TYPE_AD_MATCH6:
@@ -1083,7 +1092,9 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
             }
 
             if ((wp->flags & flags) && (wp->vaddr == addr)) {
-                return true;
+                if (do_trigger_action(env, i)) {
+                    return true;
+                }
             }
             break;
         default:
-- 
2.51.0

Re: [RFC PATCH 11/25] target/riscv/debug: Fix breakpoint matching action

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> The debug exception callback is too late to find the action for the
> trigger(s) which caused it, and it is actually passing the wrong thing
> to do_trigger_action(), which expects a trigger index but is given
> DBG_ACTION_BP (which will be interpreted as trigger 0 and use the
> action set for that trigger).
> 
> It could be possible to derive the trigger index from the bp/wp address,
> but that is clunky and it is really the action that determines whether
> an exception should be raised, also multiple triggers may perform their
> actions in the same cycle, so it is more consistent to check action
> during the breakpoint matching phase. If a breakpoint exception is to be
> taken then that is signaled at that time.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 35 +++++++++++++++++++++++------------
>   1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 7ae02fe2d2..bd61b7ff02 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -280,7 +280,8 @@ static target_ulong textra_validate(CPURISCVState *env, target_ulong tdata3)
>       return textra;
>   }
>   
> -static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
> +/* Return true if an exception should be raised */
> +static bool do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
>   {
>       trigger_action_t action = get_trigger_action(env, trigger_index);
>   
> @@ -288,8 +289,7 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
>       case DBG_ACTION_NONE:
>           break;
>       case DBG_ACTION_BP:
> -        riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> -        break;
> +        return true;
>       case DBG_ACTION_DBG_MODE:
>       case DBG_ACTION_TRACE0:
>       case DBG_ACTION_TRACE1:
> @@ -302,6 +302,7 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
>       default:
>           g_assert_not_reached();
>       }
> +    return false;
>   }
>   
>   /*
> @@ -718,7 +719,9 @@ void helper_itrigger_match(CPURISCVState *env)
>           }
>           itrigger_set_count(env, i, count--);
>           if (!count) {
> -            do_trigger_action(env, i);
> +            if (do_trigger_action(env, i)) {
> +                riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> +            }
>           } else {
>               enabled = true;
>           }
> @@ -965,11 +968,11 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
>   
>       if (cs->watchpoint_hit) {
>           if (cs->watchpoint_hit->flags & BP_CPU) {
> -            do_trigger_action(env, DBG_ACTION_BP);
> +            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
>           }
>       } else {
>           if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
> -            do_trigger_action(env, DBG_ACTION_BP);
> +            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
>           }
>       }
>   }
> @@ -1006,8 +1009,10 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
>                   pc = env->tdata2[i];
>   
>                   if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
> -                    env->badaddr = pc;
> -                    return true;
> +                    if (do_trigger_action(env, i)) {
> +                        env->badaddr = pc;
> +                        return true;
> +                    }
>                   }
>                   break;
>               case TRIGGER_TYPE_AD_MATCH6:
> @@ -1015,8 +1020,10 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
>                   pc = env->tdata2[i];
>   
>                   if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
> -                    env->badaddr = pc;
> -                    return true;
> +                    if (do_trigger_action(env, i)) {
> +                        env->badaddr = pc;
> +                        return true;
> +                    }
>                   }
>                   break;
>               default:
> @@ -1067,7 +1074,9 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>               }
>   
>               if ((wp->flags & flags) && (wp->vaddr == addr)) {
> -                return true;
> +                if (do_trigger_action(env, i)) {
> +                    return true;
> +                }
>               }
>               break;
>           case TRIGGER_TYPE_AD_MATCH6:
> @@ -1083,7 +1092,9 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>               }
>   
>               if ((wp->flags & flags) && (wp->vaddr == addr)) {
> -                return true;
> +                if (do_trigger_action(env, i)) {
> +                    return true;
> +                }
>               }
>               break;
>           default:

[RFC PATCH 12/25] target/riscv/debug: Put mcontrol load/store match address into tval

[Nicholas Piggin]

When an mc/mc6 trigger triggers on a load/store and raises a breakpoint
exception, put the matching memory address into tval, as is already
done for exec addresses matching.

Tval for breakpoint exceptions is optional in the spec, but it seems
more consistent to implement it for load/store if exec is implemented.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index bd61b7ff02..19c45fb13c 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -1075,6 +1075,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
 
             if ((wp->flags & flags) && (wp->vaddr == addr)) {
                 if (do_trigger_action(env, i)) {
+                    env->badaddr = wp->vaddr;
                     return true;
                 }
             }
@@ -1093,6 +1094,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
 
             if ((wp->flags & flags) && (wp->vaddr == addr)) {
                 if (do_trigger_action(env, i)) {
+                    env->badaddr = wp->vaddr;
                     return true;
                 }
             }
-- 
2.51.0

Re: [RFC PATCH 12/25] target/riscv/debug: Put mcontrol load/store match address into tval

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> When an mc/mc6 trigger triggers on a load/store and raises a breakpoint
> exception, put the matching memory address into tval, as is already
> done for exec addresses matching.
> 
> Tval for breakpoint exceptions is optional in the spec, but it seems
> more consistent to implement it for load/store if exec is implemented.

There is a non-written standard that we follow in qemu-riscv where we always
tend to supply these tval style addresses for exceptions/interrupts, even if
the spec says it's s optional.  So yeah, you are correct.


> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---


Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index bd61b7ff02..19c45fb13c 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -1075,6 +1075,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>   
>               if ((wp->flags & flags) && (wp->vaddr == addr)) {
>                   if (do_trigger_action(env, i)) {
> +                    env->badaddr = wp->vaddr;
>                       return true;
>                   }
>               }
> @@ -1093,6 +1094,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>   
>               if ((wp->flags & flags) && (wp->vaddr == addr)) {
>                   if (do_trigger_action(env, i)) {
> +                    env->badaddr = wp->vaddr;
>                       return true;
>                   }
>               }

[RFC PATCH 13/25] target/riscv/debug: Remove breakpoints on reset

[Nicholas Piggin]

Remove TCG breakpoints/watchpoints when the CPU is reset.
Currently they get lost.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 19c45fb13c..973de3b2e2 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -1131,6 +1131,19 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
 
     /* init to type 2 triggers */
     for (i = 0; i < RV_MAX_TRIGGERS; i++) {
+        int trigger_type = get_trigger_type(env, i);
+
+        switch (trigger_type) {
+        case TRIGGER_TYPE_AD_MATCH:
+            type2_breakpoint_remove(env, i);
+            break;
+        case TRIGGER_TYPE_AD_MATCH6:
+            type6_breakpoint_remove(env, i);
+            break;
+        default:
+            break;
+        }
+
         /*
          * type = TRIGGER_TYPE_AD_MATCH
          * dmode = 0 (both debug and M-mode can write tdata)
@@ -1147,8 +1160,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
         env->tdata1[i] = tdata1;
         env->tdata2[i] = 0;
         env->tdata3[i] = 0;
-        env->cpu_breakpoint[i] = NULL;
-        env->cpu_watchpoint[i] = NULL;
         timer_del(env->itrigger_timer[i]);
     }
 
-- 
2.51.0

Re: [RFC PATCH 13/25] target/riscv/debug: Remove breakpoints on reset

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> Remove TCG breakpoints/watchpoints when the CPU is reset.
> Currently they get lost.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 19c45fb13c..973de3b2e2 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -1131,6 +1131,19 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
>   
>       /* init to type 2 triggers */
>       for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> +        int trigger_type = get_trigger_type(env, i);
> +
> +        switch (trigger_type) {
> +        case TRIGGER_TYPE_AD_MATCH:
> +            type2_breakpoint_remove(env, i);
> +            break;
> +        case TRIGGER_TYPE_AD_MATCH6:
> +            type6_breakpoint_remove(env, i);
> +            break;
> +        default:
> +            break;
> +        }
> +
>           /*
>            * type = TRIGGER_TYPE_AD_MATCH
>            * dmode = 0 (both debug and M-mode can write tdata)
> @@ -1147,8 +1160,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
>           env->tdata1[i] = tdata1;
>           env->tdata2[i] = 0;
>           env->tdata3[i] = 0;
> -        env->cpu_breakpoint[i] = NULL;
> -        env->cpu_watchpoint[i] = NULL;
>           timer_del(env->itrigger_timer[i]);
>       }
>   

[RFC PATCH 14/25] target/riscv/debug: Move debug CPU post_load details into debug.c

[Nicholas Piggin]

No need for machine.c to know internals of how debug.c maintains debug
state.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c   | 9 ++++++++-
 target/riscv/debug.h   | 2 +-
 target/riscv/machine.c | 4 +---
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 973de3b2e2..24161fd46b 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -673,7 +673,7 @@ static bool check_itrigger_priv(CPURISCVState *env, int index)
     return icount_priv_match(env, index);
 }
 
-bool riscv_itrigger_enabled(CPURISCVState *env)
+static bool riscv_itrigger_enabled(CPURISCVState *env)
 {
     int count;
 
@@ -1124,6 +1124,13 @@ void riscv_cpu_debug_change_priv(CPURISCVState *env)
     }
 }
 
+void riscv_cpu_debug_post_load(CPURISCVState *env)
+{
+    if (!icount_enabled()) {
+        env->itrigger_enabled = riscv_itrigger_enabled(env);
+    }
+}
+
 void riscv_trigger_reset_hold(CPURISCVState *env)
 {
     target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 5a14b7894e..400c023943 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -151,9 +151,9 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
 void riscv_trigger_realize(CPURISCVState *env);
 void riscv_trigger_reset_hold(CPURISCVState *env);
 
-bool riscv_itrigger_enabled(CPURISCVState *env);
 void riscv_itrigger_update_priv(CPURISCVState *env);
 
 void riscv_cpu_debug_change_priv(CPURISCVState *env);
+void riscv_cpu_debug_post_load(CPURISCVState *env);
 
 #endif /* RISCV_DEBUG_H */
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 34fd73c920..bee1445a31 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -230,9 +230,7 @@ static int debug_post_load(void *opaque, int version_id)
     RISCVCPU *cpu = opaque;
     CPURISCVState *env = &cpu->env;
 
-    if (!icount_enabled()) {
-        env->itrigger_enabled = riscv_itrigger_enabled(env);
-    }
+    riscv_cpu_debug_post_load(env);
 
     return 0;
 }
-- 
2.51.0

Re: [RFC PATCH 14/25] target/riscv/debug: Move debug CPU post_load details into debug.c

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> No need for machine.c to know internals of how debug.c maintains debug
> state.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c   | 9 ++++++++-
>   target/riscv/debug.h   | 2 +-
>   target/riscv/machine.c | 4 +---
>   3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 973de3b2e2..24161fd46b 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -673,7 +673,7 @@ static bool check_itrigger_priv(CPURISCVState *env, int index)
>       return icount_priv_match(env, index);
>   }
>   
> -bool riscv_itrigger_enabled(CPURISCVState *env)
> +static bool riscv_itrigger_enabled(CPURISCVState *env)
>   {
>       int count;
>   
> @@ -1124,6 +1124,13 @@ void riscv_cpu_debug_change_priv(CPURISCVState *env)
>       }
>   }
>   
> +void riscv_cpu_debug_post_load(CPURISCVState *env)
> +{
> +    if (!icount_enabled()) {
> +        env->itrigger_enabled = riscv_itrigger_enabled(env);
> +    }
> +}
> +
>   void riscv_trigger_reset_hold(CPURISCVState *env)
>   {
>       target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index 5a14b7894e..400c023943 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -151,9 +151,9 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
>   void riscv_trigger_realize(CPURISCVState *env);
>   void riscv_trigger_reset_hold(CPURISCVState *env);
>   
> -bool riscv_itrigger_enabled(CPURISCVState *env);
>   void riscv_itrigger_update_priv(CPURISCVState *env);
>   
>   void riscv_cpu_debug_change_priv(CPURISCVState *env);
> +void riscv_cpu_debug_post_load(CPURISCVState *env);
>   
>   #endif /* RISCV_DEBUG_H */
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 34fd73c920..bee1445a31 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -230,9 +230,7 @@ static int debug_post_load(void *opaque, int version_id)
>       RISCVCPU *cpu = opaque;
>       CPURISCVState *env = &cpu->env;
>   
> -    if (!icount_enabled()) {
> -        env->itrigger_enabled = riscv_itrigger_enabled(env);
> -    }
> +    riscv_cpu_debug_post_load(env);
>   
>       return 0;
>   }

[RFC PATCH 15/25] target/riscv/debug: Insert breakpoints after migration

[Nicholas Piggin]

After migration, TCG breakpoints should be re-established based
on debug register state.

icount trigger timer in the icount_enabled() path is also missing,
but that path will be removed subsequently so don't bother to fix
it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 24161fd46b..a30b345b25 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -1126,6 +1126,20 @@ void riscv_cpu_debug_change_priv(CPURISCVState *env)
 
 void riscv_cpu_debug_post_load(CPURISCVState *env)
 {
+    for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
+        int trigger_type = get_trigger_type(env, i);
+
+        switch (trigger_type) {
+        case TRIGGER_TYPE_AD_MATCH:
+            type2_breakpoint_insert(env, i);
+            break;
+        case TRIGGER_TYPE_AD_MATCH6:
+            type6_breakpoint_insert(env, i);
+            break;
+        default:
+            break;
+        }
+    }
     if (!icount_enabled()) {
         env->itrigger_enabled = riscv_itrigger_enabled(env);
     }
-- 
2.51.0

Re: [RFC PATCH 15/25] target/riscv/debug: Insert breakpoints after migration

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> After migration, TCG breakpoints should be re-established based
> on debug register state.
> 
> icount trigger timer in the icount_enabled() path is also missing,
> but that path will be removed subsequently so don't bother to fix
> it.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 24161fd46b..a30b345b25 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -1126,6 +1126,20 @@ void riscv_cpu_debug_change_priv(CPURISCVState *env)
>   
>   void riscv_cpu_debug_post_load(CPURISCVState *env)
>   {
> +    for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
> +        int trigger_type = get_trigger_type(env, i);
> +
> +        switch (trigger_type) {
> +        case TRIGGER_TYPE_AD_MATCH:
> +            type2_breakpoint_insert(env, i);
> +            break;
> +        case TRIGGER_TYPE_AD_MATCH6:
> +            type6_breakpoint_insert(env, i);
> +            break;
> +        default:
> +            break;
> +        }
> +    }
>       if (!icount_enabled()) {
>           env->itrigger_enabled = riscv_itrigger_enabled(env);
>       }

[RFC PATCH 16/25] target/riscv/debug: Remove itrigger icount-enabled mode

[Nicholas Piggin]

When QEMU icount is enabled, the riscv icount trigger facility is
implemented cleverly using precise counting timers rather than
single-stepping TCG.

I found this possibly has some bugs, it is a bit complicated and
splits testing between icount and !icount, and icount enabled is
not the important case for performance. Therefore remove the
separate icount enabled path.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/cpu.c         |   6 --
 target/riscv/cpu.h         |   2 -
 target/riscv/cpu_helper.c  |   3 -
 target/riscv/debug.c       | 115 ++-----------------------------------
 target/riscv/debug.h       |   3 -
 target/riscv/tcg/tcg-cpu.c |   2 +-
 6 files changed, 5 insertions(+), 126 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ffd98e8eed..057e221808 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -942,12 +942,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 
     riscv_cpu_register_gdb_regs_for_features(cs);
 
-#ifndef CONFIG_USER_ONLY
-    if (cpu->cfg.debug) {
-        riscv_trigger_realize(&cpu->env);
-    }
-#endif
-
     qemu_init_vcpu(cs);
     cpu_reset(cs);
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 35d1f6362c..a718287d41 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -450,8 +450,6 @@ struct CPUArchState {
     target_ulong mcontext;
     struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
     struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
-    QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
-    int64_t last_icount;
     bool itrigger_enabled;
 
     /* machine specific rdtime callback */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e096da939b..55518cad86 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1036,9 +1036,6 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
 
     if (newpriv != env->priv || env->virt_enabled != virt_en) {
         change = true;
-        if (icount_enabled()) {
-            riscv_itrigger_update_priv(env);
-        }
 
         riscv_pmu_update_fixed_ctrs(env, newpriv, virt_en);
     }
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index a30b345b25..69e7037fac 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -30,8 +30,6 @@
 #include "trace.h"
 #include "exec/helper-proto.h"
 #include "exec/watchpoint.h"
-#include "system/cpu-timers.h"
-#include "exec/icount.h"
 
 /*
  * The following M-mode trigger CSRs are implemented:
@@ -668,11 +666,6 @@ itrigger_set_count(CPURISCVState *env, int index, int value)
                                    ITRIGGER_COUNT, value);
 }
 
-static bool check_itrigger_priv(CPURISCVState *env, int index)
-{
-    return icount_priv_match(env, index);
-}
-
 static bool riscv_itrigger_enabled(CPURISCVState *env)
 {
     int count;
@@ -729,62 +722,6 @@ void helper_itrigger_match(CPURISCVState *env)
     env->itrigger_enabled = enabled;
 }
 
-static void riscv_itrigger_update_count(CPURISCVState *env)
-{
-    int count, executed;
-    /*
-     * Record last icount, so that we can evaluate the executed instructions
-     * since last privilege mode change or timer expire.
-     */
-    int64_t last_icount = env->last_icount, current_icount;
-    current_icount = env->last_icount = icount_get_raw();
-
-    for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
-        if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
-            continue;
-        }
-        count = itrigger_get_count(env, i);
-        if (!count) {
-            continue;
-        }
-        /*
-         * Only when privilege is changed or itrigger timer expires,
-         * the count field in itrigger tdata1 register is updated.
-         * And the count field in itrigger only contains remaining value.
-         */
-        if (check_itrigger_priv(env, i)) {
-            /*
-             * If itrigger enabled in this privilege mode, the number of
-             * executed instructions since last privilege change
-             * should be reduced from current itrigger count.
-             */
-            executed = current_icount - last_icount;
-            itrigger_set_count(env, i, count - executed);
-            if (count == executed) {
-                do_trigger_action(env, i);
-            }
-        } else {
-            /*
-             * If itrigger is not enabled in this privilege mode,
-             * the number of executed instructions will be discard and
-             * the count field in itrigger will not change.
-             */
-            timer_mod(env->itrigger_timer[i],
-                      current_icount + count);
-        }
-    }
-}
-
-static void riscv_itrigger_timer_cb(void *opaque)
-{
-    riscv_itrigger_update_count((CPURISCVState *)opaque);
-}
-
-void riscv_itrigger_update_priv(CPURISCVState *env)
-{
-    riscv_itrigger_update_count(env);
-}
-
 static target_ulong itrigger_validate(CPURISCVState *env,
                                       target_ulong ctrl)
 {
@@ -808,21 +745,9 @@ static target_ulong itrigger_validate(CPURISCVState *env,
 static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
                                int tdata_index, target_ulong val)
 {
-    target_ulong new_val;
-
     switch (tdata_index) {
     case TDATA1:
-        /* set timer for icount */
-        new_val = itrigger_validate(env, val);
-        if (new_val != env->tdata1[index]) {
-            env->tdata1[index] = new_val;
-            if (icount_enabled()) {
-                env->last_icount = icount_get_raw();
-                /* set the count to timer */
-                timer_mod(env->itrigger_timer[index],
-                          env->last_icount + itrigger_get_count(env, index));
-            }
-        }
+        env->tdata1[index] = itrigger_validate(env, val);
         break;
     case TDATA2:
         qemu_log_mask(LOG_UNIMP,
@@ -858,27 +783,10 @@ static void anytype_reg_write(CPURISCVState *env, target_ulong index,
     }
 }
 
-static int itrigger_get_adjust_count(CPURISCVState *env)
-{
-    int count = itrigger_get_count(env, env->trigger_cur), executed;
-    if ((count != 0) && check_itrigger_priv(env, env->trigger_cur)) {
-        executed = icount_get_raw() - env->last_icount;
-        count += executed;
-    }
-    return count;
-}
-
 target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
 {
-    int trigger_type;
     switch (tdata_index) {
     case TDATA1:
-        trigger_type = extract_trigger_type(env,
-                                            env->tdata1[env->trigger_cur]);
-        if ((trigger_type == TRIGGER_TYPE_INST_CNT) && icount_enabled()) {
-            return deposit64(env->tdata1[env->trigger_cur], 10, 14,
-                             itrigger_get_adjust_count(env));
-        }
         return env->tdata1[env->trigger_cur];
     case TDATA2:
         return env->tdata2[env->trigger_cur];
@@ -949,7 +857,7 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
         g_assert_not_reached();
     }
 
-    if (check_itrigger && !icount_enabled()) {
+    if (check_itrigger) {
         env->itrigger_enabled = riscv_itrigger_enabled(env);
     }
 }
@@ -1107,21 +1015,9 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
     return false;
 }
 
-void riscv_trigger_realize(CPURISCVState *env)
-{
-    int i;
-
-    for (i = 0; i < RV_MAX_TRIGGERS; i++) {
-        env->itrigger_timer[i] = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-                                              riscv_itrigger_timer_cb, env);
-    }
-}
-
 void riscv_cpu_debug_change_priv(CPURISCVState *env)
 {
-    if (!icount_enabled()) {
-        env->itrigger_enabled = riscv_itrigger_enabled(env);
-    }
+    env->itrigger_enabled = riscv_itrigger_enabled(env);
 }
 
 void riscv_cpu_debug_post_load(CPURISCVState *env)
@@ -1140,9 +1036,7 @@ void riscv_cpu_debug_post_load(CPURISCVState *env)
             break;
         }
     }
-    if (!icount_enabled()) {
-        env->itrigger_enabled = riscv_itrigger_enabled(env);
-    }
+    env->itrigger_enabled = riscv_itrigger_enabled(env);
 }
 
 void riscv_trigger_reset_hold(CPURISCVState *env)
@@ -1181,7 +1075,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
         env->tdata1[i] = tdata1;
         env->tdata2[i] = 0;
         env->tdata3[i] = 0;
-        timer_del(env->itrigger_timer[i]);
     }
 
     env->mcontext = 0;
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 400c023943..bee42b8593 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -148,11 +148,8 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
 bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
 bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
 
-void riscv_trigger_realize(CPURISCVState *env);
 void riscv_trigger_reset_hold(CPURISCVState *env);
 
-void riscv_itrigger_update_priv(CPURISCVState *env);
-
 void riscv_cpu_debug_change_priv(CPURISCVState *env);
 void riscv_cpu_debug_post_load(CPURISCVState *env);
 
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 988b2d905f..677172ae2d 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -177,7 +177,7 @@ static TCGTBCPUState riscv_get_tb_cpu_state(CPUState *cs)
              ? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED;
     }
 
-    if (cpu->cfg.debug && !icount_enabled()) {
+    if (cpu->cfg.debug) {
         flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
     }
 #endif
-- 
2.51.0

[RFC PATCH 17/25] target/riscv/debug: Advertise icount trigger type in tinfo

[Nicholas Piggin]

icount triggers are supported so tinfo should advertise them.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 69e7037fac..605ed95b14 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -865,7 +865,9 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
 target_ulong tinfo_csr_read(CPURISCVState *env)
 {
     /* assume all triggers support the same types of triggers */
+    /* XXX: should we set 1 (version 1.0) in the version field? */
     return BIT(TRIGGER_TYPE_AD_MATCH) |
+           BIT(TRIGGER_TYPE_INST_CNT) |
            BIT(TRIGGER_TYPE_AD_MATCH6);
 }
 
-- 
2.51.0

Re: [RFC PATCH 17/25] target/riscv/debug: Advertise icount trigger type in tinfo

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> icount triggers are supported so tinfo should advertise them.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   target/riscv/debug.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 69e7037fac..605ed95b14 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -865,7 +865,9 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>   target_ulong tinfo_csr_read(CPURISCVState *env)
>   {
>       /* assume all triggers support the same types of triggers */
> +    /* XXX: should we set 1 (version 1.0) in the version field? */

For the 'debug' flag, no.  For sdtrig and others, yes.

>       return BIT(TRIGGER_TYPE_AD_MATCH) |
> +           BIT(TRIGGER_TYPE_INST_CNT) |
>              BIT(TRIGGER_TYPE_AD_MATCH6);
>   }

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   

[RFC PATCH 18/25] target/riscv/debug: Reset trigger type to unavailable

[Nicholas Piggin]

Reset triggers to type 15 (unavailable). The reset value for tdata1 type
field is implementation specific, legal values are permitted.
Unavailable is a legal value, and some implementations may not support
type2 triggers so use that instead.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 605ed95b14..96b00193e2 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -1043,10 +1043,10 @@ void riscv_cpu_debug_post_load(CPURISCVState *env)
 
 void riscv_trigger_reset_hold(CPURISCVState *env)
 {
-    target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
+    target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_UNAVAIL, 0, 0);
     int i;
 
-    /* init to type 2 triggers */
+    /* init to type 15 (unavailable) triggers */
     for (i = 0; i < RV_MAX_TRIGGERS; i++) {
         int trigger_type = get_trigger_type(env, i);
 
@@ -1061,19 +1061,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
             break;
         }
 
-        /*
-         * type = TRIGGER_TYPE_AD_MATCH
-         * dmode = 0 (both debug and M-mode can write tdata)
-         * maskmax = 0 (unimplemented, always 0)
-         * sizehi = 0 (match against any size, RV64 only)
-         * hit = 0 (unimplemented, always 0)
-         * select = 0 (always 0, perform match on address)
-         * timing = 0 (always 0, trigger before instruction)
-         * sizelo = 0 (match against any size)
-         * action = 0 (always 0, raise a breakpoint exception)
-         * chain = 0 (unimplemented, always 0)
-         * match = 0 (always 0, when any compare value equals tdata2)
-         */
         env->tdata1[i] = tdata1;
         env->tdata2[i] = 0;
         env->tdata3[i] = 0;
-- 
2.51.0

Re: [RFC PATCH 18/25] target/riscv/debug: Reset trigger type to unavailable

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> Reset triggers to type 15 (unavailable). The reset value for tdata1 type
> field is implementation specific, legal values are permitted.
> Unavailable is a legal value, and some implementations may not support
> type2 triggers so use that instead.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 17 ++---------------
>   1 file changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 605ed95b14..96b00193e2 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -1043,10 +1043,10 @@ void riscv_cpu_debug_post_load(CPURISCVState *env)
>   
>   void riscv_trigger_reset_hold(CPURISCVState *env)
>   {
> -    target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
> +    target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_UNAVAIL, 0, 0);
>       int i;
>   
> -    /* init to type 2 triggers */
> +    /* init to type 15 (unavailable) triggers */
>       for (i = 0; i < RV_MAX_TRIGGERS; i++) {
>           int trigger_type = get_trigger_type(env, i);
>   
> @@ -1061,19 +1061,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
>               break;
>           }
>   
> -        /*
> -         * type = TRIGGER_TYPE_AD_MATCH
> -         * dmode = 0 (both debug and M-mode can write tdata)
> -         * maskmax = 0 (unimplemented, always 0)
> -         * sizehi = 0 (match against any size, RV64 only)
> -         * hit = 0 (unimplemented, always 0)
> -         * select = 0 (always 0, perform match on address)
> -         * timing = 0 (always 0, trigger before instruction)
> -         * sizelo = 0 (match against any size)
> -         * action = 0 (always 0, raise a breakpoint exception)
> -         * chain = 0 (unimplemented, always 0)
> -         * match = 0 (always 0, when any compare value equals tdata2)
> -         */
>           env->tdata1[i] = tdata1;
>           env->tdata2[i] = 0;
>           env->tdata3[i] = 0;

[RFC PATCH 19/25] target/riscv/debug: Add new debug state format

[Nicholas Piggin]

Some Sdtrig implementations support more than 2 debug triggers,
but the Sdtrig vmstate is tied to 2 triggers. Extending this will
be a compatibility issue anyway, so move to a nicer layout and
provide state for all 32 architecturally possible triggers.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/cpu.h         |  35 ++++++--
 target/riscv/csr.c         |   7 +-
 target/riscv/debug.c       | 167 +++++++++++++++++++++----------------
 target/riscv/debug.h       |   3 +-
 target/riscv/machine.c     |  31 ++++++-
 target/riscv/tcg/tcg-cpu.c |   3 +-
 6 files changed, 157 insertions(+), 89 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a718287d41..44ed1665e2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -194,6 +194,28 @@ FIELD(VTYPE, VMA, 7, 1)
 FIELD(VTYPE, VEDIV, 8, 2)
 FIELD(VTYPE, RESERVED, 10, sizeof(target_ulong) * 8 - 11)
 
+#ifndef CONFIG_USER_ONLY
+typedef struct SdtrigTrigger {
+    target_ulong tdata1;
+    target_ulong tdata2;
+    target_ulong tdata3;
+} SdtrigTrigger;
+
+typedef struct SdtrigState {
+    /* Architected state */
+    target_ulong trigger_cur; /* tselect */
+    SdtrigTrigger triggers[RV_MAX_SDTRIG_TRIGGERS];
+    target_ulong tcontrol;
+    target_ulong mcontext; /* hcontext */
+    target_ulong scontext;
+
+    /* QEMU state */
+    struct CPUBreakpoint *cpu_breakpoint[RV_MAX_SDTRIG_TRIGGERS];
+    struct CPUWatchpoint *cpu_watchpoint[RV_MAX_SDTRIG_TRIGGERS];
+    bool itrigger_enabled;
+} SdtrigState;
+#endif
+
 typedef struct PMUCTRState {
     /* Current value of a counter */
     uint64_t mhpmcounter_val;
@@ -443,14 +465,11 @@ struct CPUArchState {
     target_ulong mseccfg;
 
     /* trigger module */
-    target_ulong trigger_cur;
-    target_ulong tdata1[RV_MAX_TRIGGERS];
-    target_ulong tdata2[RV_MAX_TRIGGERS];
-    target_ulong tdata3[RV_MAX_TRIGGERS];
-    target_ulong mcontext;
-    struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
-    struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
-    bool itrigger_enabled;
+    SdtrigState sdtrig_state;
+    /* migration compat */
+    target_ulong old_tdata1[RV_DEFAULT_TRIGGERS];
+    target_ulong old_tdata2[RV_DEFAULT_TRIGGERS];
+    target_ulong old_tdata3[RV_DEFAULT_TRIGGERS];
 
     /* machine specific rdtime callback */
     uint64_t (*rdtime_fn)(void *);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 05c7ec8352..ec70a47d0b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -5311,7 +5311,8 @@ static RISCVException read_tdata(CPURISCVState *env, int csrno,
                                  target_ulong *val)
 {
     /* return 0 in tdata1 to end the trigger enumeration */
-    if (env->trigger_cur >= RV_MAX_TRIGGERS && csrno == CSR_TDATA1) {
+    if (env->sdtrig_state.trigger_cur >= RV_MAX_SDTRIG_TRIGGERS &&
+        csrno == CSR_TDATA1) {
         *val = 0;
         return RISCV_EXCP_NONE;
     }
@@ -5345,7 +5346,7 @@ static RISCVException read_tinfo(CPURISCVState *env, int csrno,
 static RISCVException read_mcontext(CPURISCVState *env, int csrno,
                                     target_ulong *val)
 {
-    *val = env->mcontext;
+    *val = env->sdtrig_state.mcontext;
     return RISCV_EXCP_NONE;
 }
 
@@ -5363,7 +5364,7 @@ static RISCVException write_mcontext(CPURISCVState *env, int csrno,
         mask = rv32 ? MCONTEXT32 : MCONTEXT64;
     }
 
-    env->mcontext = val & mask;
+    env->sdtrig_state.mcontext = val & mask;
     return RISCV_EXCP_NONE;
 }
 
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 96b00193e2..22f7958a79 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -90,13 +90,15 @@ static inline target_ulong extract_trigger_type(CPURISCVState *env,
 static inline target_ulong get_trigger_type(CPURISCVState *env,
                                             target_ulong trigger_index)
 {
-    return extract_trigger_type(env, env->tdata1[trigger_index]);
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[trigger_index];
+    return extract_trigger_type(env, trigger->tdata1);
 }
 
 static trigger_action_t get_trigger_action(CPURISCVState *env,
                                            target_ulong trigger_index)
 {
-    target_ulong tdata1 = env->tdata1[trigger_index];
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[trigger_index];
+    target_ulong tdata1 = trigger->tdata1;
     int trigger_type = get_trigger_type(env, trigger_index);
     trigger_action_t action = DBG_ACTION_NONE;
 
@@ -155,7 +157,7 @@ static inline target_ulong build_tdata1(CPURISCVState *env,
 
 bool tdata_available(CPURISCVState *env, int tdata_index)
 {
-    int trigger_type = get_trigger_type(env, env->trigger_cur);
+    int trigger_type = get_trigger_type(env, env->sdtrig_state.trigger_cur);
 
     if (unlikely(tdata_index >= TDATA_NUM)) {
         return false;
@@ -166,13 +168,13 @@ bool tdata_available(CPURISCVState *env, int tdata_index)
 
 target_ulong tselect_csr_read(CPURISCVState *env)
 {
-    return env->trigger_cur;
+    return env->sdtrig_state.trigger_cur;
 }
 
 void tselect_csr_write(CPURISCVState *env, target_ulong val)
 {
-    if (val < RV_MAX_TRIGGERS) {
-        env->trigger_cur = val;
+    if (val < RV_DEFAULT_TRIGGERS) {
+        env->sdtrig_state.trigger_cur = val;
     }
 }
 
@@ -342,7 +344,8 @@ static bool icount_priv_match(CPURISCVState *env, target_ulong tdata1)
 static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
                                int trigger_index)
 {
-    target_ulong tdata1 = env->tdata1[trigger_index];
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[trigger_index];
+    target_ulong tdata1 = trigger->tdata1;
 
     switch (type) {
     case TRIGGER_TYPE_AD_MATCH:
@@ -371,7 +374,8 @@ static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
 static bool trigger_textra_match(CPURISCVState *env, trigger_type_t type,
                                  int trigger_index)
 {
-    target_ulong textra = env->tdata3[trigger_index];
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[trigger_index];
+    target_ulong textra = trigger->tdata3;
     target_ulong mhvalue, mhselect;
 
     if (type < TRIGGER_TYPE_AD_MATCH || type > TRIGGER_TYPE_AD_MATCH6) {
@@ -399,7 +403,7 @@ static bool trigger_textra_match(CPURISCVState *env, trigger_type_t type,
         break;
     case MHSELECT_MCONTEXT:
         /* Match if the low bits of mcontext/hcontext equal mhvalue. */
-        if (mhvalue != env->mcontext) {
+        if (mhvalue != env->sdtrig_state.mcontext) {
             return false;
         }
         break;
@@ -477,8 +481,9 @@ static target_ulong type2_mcontrol_validate(CPURISCVState *env,
 
 static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
 {
-    target_ulong ctrl = env->tdata1[index];
-    target_ulong addr = env->tdata2[index];
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
+    target_ulong ctrl = trigger->tdata1;
+    target_ulong addr = trigger->tdata2;
     bool enabled = type2_breakpoint_enabled(ctrl);
     CPUState *cs = env_cpu(env);
     int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
@@ -489,7 +494,8 @@ static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
     }
 
     if (ctrl & TYPE2_EXEC) {
-        cpu_breakpoint_insert(cs, addr, flags, &env->cpu_breakpoint[index]);
+        cpu_breakpoint_insert(cs, addr, flags,
+                              &env->sdtrig_state.cpu_breakpoint[index]);
     }
 
     if (ctrl & TYPE2_LOAD) {
@@ -503,12 +509,12 @@ static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
         size = type2_breakpoint_size(env, ctrl);
         if (size != 0) {
             cpu_watchpoint_insert(cs, addr, size, flags,
-                                  &env->cpu_watchpoint[index]);
+                                  &env->sdtrig_state.cpu_watchpoint[index]);
         } else {
             def_size = riscv_cpu_mxl(env) == MXL_RV64 ? 8 : 4;
 
             cpu_watchpoint_insert(cs, addr, def_size, flags,
-                                  &env->cpu_watchpoint[index]);
+                                  &env->sdtrig_state.cpu_watchpoint[index]);
         }
     }
 }
@@ -517,29 +523,32 @@ static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
 {
     CPUState *cs = env_cpu(env);
 
-    if (env->cpu_breakpoint[index]) {
-        cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
-        env->cpu_breakpoint[index] = NULL;
+    if (env->sdtrig_state.cpu_breakpoint[index]) {
+        cpu_breakpoint_remove_by_ref(cs,
+                                     env->sdtrig_state.cpu_breakpoint[index]);
+        env->sdtrig_state.cpu_breakpoint[index] = NULL;
     }
 
-    if (env->cpu_watchpoint[index]) {
-        cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
-        env->cpu_watchpoint[index] = NULL;
+    if (env->sdtrig_state.cpu_watchpoint[index]) {
+        cpu_watchpoint_remove_by_ref(cs,
+                                     env->sdtrig_state.cpu_watchpoint[index]);
+        env->sdtrig_state.cpu_watchpoint[index] = NULL;
     }
 }
 
 static void type2_reg_write(CPURISCVState *env, target_ulong index,
                             int tdata_index, target_ulong val)
 {
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
     switch (tdata_index) {
     case TDATA1:
-        env->tdata1[index] = type2_mcontrol_validate(env, val);
+        trigger->tdata1 = type2_mcontrol_validate(env, val);
         break;
     case TDATA2:
-        env->tdata2[index] = val;
+        trigger->tdata2 = val;
         break;
     case TDATA3:
-        env->tdata3[index] = textra_validate(env, val);
+        trigger->tdata3 = textra_validate(env, val);
         break;
     default:
         g_assert_not_reached();
@@ -593,8 +602,9 @@ static target_ulong type6_mcontrol6_validate(CPURISCVState *env,
 
 static void type6_breakpoint_insert(CPURISCVState *env, target_ulong index)
 {
-    target_ulong ctrl = env->tdata1[index];
-    target_ulong addr = env->tdata2[index];
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
+    target_ulong ctrl = trigger->tdata1;
+    target_ulong addr = trigger->tdata2;
     bool enabled = type6_breakpoint_enabled(ctrl);
     CPUState *cs = env_cpu(env);
     int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
@@ -605,7 +615,8 @@ static void type6_breakpoint_insert(CPURISCVState *env, target_ulong index)
     }
 
     if (ctrl & TYPE6_EXEC) {
-        cpu_breakpoint_insert(cs, addr, flags, &env->cpu_breakpoint[index]);
+        cpu_breakpoint_insert(cs, addr, flags,
+                              &env->sdtrig_state.cpu_breakpoint[index]);
     }
 
     if (ctrl & TYPE6_LOAD) {
@@ -620,10 +631,10 @@ static void type6_breakpoint_insert(CPURISCVState *env, target_ulong index)
         size = extract32(ctrl, 16, 4);
         if (size != 0) {
             cpu_watchpoint_insert(cs, addr, size, flags,
-                                  &env->cpu_watchpoint[index]);
+                                  &env->sdtrig_state.cpu_watchpoint[index]);
         } else {
             cpu_watchpoint_insert(cs, addr, 8, flags,
-                                  &env->cpu_watchpoint[index]);
+                                  &env->sdtrig_state.cpu_watchpoint[index]);
         }
     }
 }
@@ -636,15 +647,16 @@ static void type6_breakpoint_remove(CPURISCVState *env, target_ulong index)
 static void type6_reg_write(CPURISCVState *env, target_ulong index,
                             int tdata_index, target_ulong val)
 {
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
     switch (tdata_index) {
     case TDATA1:
-        env->tdata1[index] = type6_mcontrol6_validate(env, val);
+        trigger->tdata1 = type6_mcontrol6_validate(env, val);
         break;
     case TDATA2:
-        env->tdata2[index] = val;
+        trigger->tdata2 = val;
         break;
     case TDATA3:
-        env->tdata3[index] = textra_validate(env, val);
+        trigger->tdata3 = textra_validate(env, val);
         break;
     default:
         g_assert_not_reached();
@@ -656,21 +668,22 @@ static void type6_reg_write(CPURISCVState *env, target_ulong index,
 static inline int
 itrigger_get_count(CPURISCVState *env, int index)
 {
-    return get_field(env->tdata1[index], ITRIGGER_COUNT);
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
+    return get_field(trigger->tdata1, ITRIGGER_COUNT);
 }
 
 static inline void
 itrigger_set_count(CPURISCVState *env, int index, int value)
 {
-    env->tdata1[index] = set_field(env->tdata1[index],
-                                   ITRIGGER_COUNT, value);
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
+    trigger->tdata1 = set_field(trigger->tdata1, ITRIGGER_COUNT, value);
 }
 
 static bool riscv_itrigger_enabled(CPURISCVState *env)
 {
     int count;
 
-    for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
+    for (int i = 0; i < RV_MAX_SDTRIG_TRIGGERS; i++) {
         if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
             continue;
         }
@@ -697,9 +710,9 @@ void helper_itrigger_match(CPURISCVState *env)
     int count;
     bool enabled = false;
 
-    g_assert(env->itrigger_enabled);
+    g_assert(env->sdtrig_state.itrigger_enabled);
 
-    for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
+    for (int i = 0; i < RV_MAX_SDTRIG_TRIGGERS; i++) {
         if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
             continue;
         }
@@ -719,7 +732,7 @@ void helper_itrigger_match(CPURISCVState *env)
             enabled = true;
         }
     }
-    env->itrigger_enabled = enabled;
+    env->sdtrig_state.itrigger_enabled = enabled;
 }
 
 static target_ulong itrigger_validate(CPURISCVState *env,
@@ -745,16 +758,17 @@ static target_ulong itrigger_validate(CPURISCVState *env,
 static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
                                int tdata_index, target_ulong val)
 {
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
     switch (tdata_index) {
     case TDATA1:
-        env->tdata1[index] = itrigger_validate(env, val);
+        trigger->tdata1 = itrigger_validate(env, val);
         break;
     case TDATA2:
         qemu_log_mask(LOG_UNIMP,
                       "tdata2 is not supported for icount trigger\n");
         break;
     case TDATA3:
-        env->tdata3[index] = textra_validate(env, val);
+        trigger->tdata3 = textra_validate(env, val);
         break;
     default:
         g_assert_not_reached();
@@ -764,19 +778,21 @@ static void itrigger_reg_write(CPURISCVState *env, target_ulong index,
 static void anytype_reg_write(CPURISCVState *env, target_ulong index,
                               int tdata_index, target_ulong val)
 {
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
+
     /*
      * This should check the value is valid for at least one of the supported
      * trigger types.
      */
     switch (tdata_index) {
     case TDATA1:
-        env->tdata1[env->trigger_cur] = val;
+        trigger->tdata1 = val;
         break;
     case TDATA2:
-        env->tdata2[env->trigger_cur] = val;
+        trigger->tdata2 = val;
         break;
     case TDATA3:
-        env->tdata3[env->trigger_cur] = val;
+        trigger->tdata3 = val;
         break;
     default:
         g_assert_not_reached();
@@ -785,13 +801,16 @@ static void anytype_reg_write(CPURISCVState *env, target_ulong index,
 
 target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
 {
+    target_ulong index = env->sdtrig_state.trigger_cur;
+    SdtrigTrigger *trigger = &env->sdtrig_state.triggers[index];
+
     switch (tdata_index) {
     case TDATA1:
-        return env->tdata1[env->trigger_cur];
+        return trigger->tdata1;
     case TDATA2:
-        return env->tdata2[env->trigger_cur];
+        return trigger->tdata2;
     case TDATA3:
-        return env->tdata3[env->trigger_cur];
+        return trigger->tdata3;
     default:
         g_assert_not_reached();
     }
@@ -799,15 +818,16 @@ target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
 
 void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
 {
-    int trigger_type = get_trigger_type(env, env->trigger_cur);
+    target_ulong index = env->sdtrig_state.trigger_cur;
+    int trigger_type = get_trigger_type(env, index);
     bool check_itrigger = false;
 
     switch (trigger_type) {
     case TRIGGER_TYPE_AD_MATCH:
-        type2_breakpoint_remove(env, env->trigger_cur);
+        type2_breakpoint_remove(env, index);
         break;
     case TRIGGER_TYPE_AD_MATCH6:
-        type6_breakpoint_remove(env, env->trigger_cur);
+        type6_breakpoint_remove(env, index);
         break;
     case TRIGGER_TYPE_INST_CNT:
         /*
@@ -831,17 +851,17 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
 
     switch (trigger_type) {
     case TRIGGER_TYPE_AD_MATCH:
-        type2_reg_write(env, env->trigger_cur, tdata_index, val);
+        type2_reg_write(env, index, tdata_index, val);
         break;
     case TRIGGER_TYPE_AD_MATCH6:
-        type6_reg_write(env, env->trigger_cur, tdata_index, val);
+        type6_reg_write(env, index, tdata_index, val);
         break;
     case TRIGGER_TYPE_INST_CNT:
-        itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
+        itrigger_reg_write(env, index, tdata_index, val);
         check_itrigger = true;
         break;
     case TRIGGER_TYPE_UNAVAIL:
-        anytype_reg_write(env, env->trigger_cur, tdata_index, val);
+        anytype_reg_write(env, index, tdata_index, val);
         break;
     case TRIGGER_TYPE_INT:
     case TRIGGER_TYPE_EXCP:
@@ -858,7 +878,7 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
     }
 
     if (check_itrigger) {
-        env->itrigger_enabled = riscv_itrigger_enabled(env);
+        env->sdtrig_state.itrigger_enabled = riscv_itrigger_enabled(env);
     }
 }
 
@@ -898,7 +918,8 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
     int i;
 
     QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
-        for (i = 0; i < RV_MAX_TRIGGERS; i++) {
+        for (i = 0; i < RV_MAX_SDTRIG_TRIGGERS; i++) {
+            SdtrigTrigger *trigger = &env->sdtrig_state.triggers[i];
             trigger_type = get_trigger_type(env, i);
 
             switch (trigger_type) {
@@ -915,8 +936,8 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 
             switch (trigger_type) {
             case TRIGGER_TYPE_AD_MATCH:
-                ctrl = env->tdata1[i];
-                pc = env->tdata2[i];
+                ctrl = trigger->tdata1;
+                pc = trigger->tdata2;
 
                 if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
                     if (do_trigger_action(env, i)) {
@@ -926,8 +947,8 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
                 }
                 break;
             case TRIGGER_TYPE_AD_MATCH6:
-                ctrl = env->tdata1[i];
-                pc = env->tdata2[i];
+                ctrl = trigger->tdata1;
+                pc = trigger->tdata2;
 
                 if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
                     if (do_trigger_action(env, i)) {
@@ -955,7 +976,8 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
     int flags;
     int i;
 
-    for (i = 0; i < RV_MAX_TRIGGERS; i++) {
+    for (i = 0; i < RV_MAX_SDTRIG_TRIGGERS; i++) {
+        SdtrigTrigger *trigger = &env->sdtrig_state.triggers[i];
         trigger_type = get_trigger_type(env, i);
 
         switch (trigger_type) {
@@ -972,8 +994,8 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
 
         switch (trigger_type) {
         case TRIGGER_TYPE_AD_MATCH:
-            ctrl = env->tdata1[i];
-            addr = env->tdata2[i];
+            ctrl = trigger->tdata1;
+            addr = trigger->tdata2;
             flags = 0;
 
             if (ctrl & TYPE2_LOAD) {
@@ -991,8 +1013,8 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
             }
             break;
         case TRIGGER_TYPE_AD_MATCH6:
-            ctrl = env->tdata1[i];
-            addr = env->tdata2[i];
+            ctrl = trigger->tdata1;
+            addr = trigger->tdata2;
             flags = 0;
 
             if (ctrl & TYPE6_LOAD) {
@@ -1019,12 +1041,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
 
 void riscv_cpu_debug_change_priv(CPURISCVState *env)
 {
-    env->itrigger_enabled = riscv_itrigger_enabled(env);
+    env->sdtrig_state.itrigger_enabled = riscv_itrigger_enabled(env);
 }
 
 void riscv_cpu_debug_post_load(CPURISCVState *env)
 {
-    for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
+    for (int i = 0; i < RV_MAX_SDTRIG_TRIGGERS; i++) {
         int trigger_type = get_trigger_type(env, i);
 
         switch (trigger_type) {
@@ -1038,7 +1060,7 @@ void riscv_cpu_debug_post_load(CPURISCVState *env)
             break;
         }
     }
-    env->itrigger_enabled = riscv_itrigger_enabled(env);
+    env->sdtrig_state.itrigger_enabled = riscv_itrigger_enabled(env);
 }
 
 void riscv_trigger_reset_hold(CPURISCVState *env)
@@ -1047,7 +1069,8 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
     int i;
 
     /* init to type 15 (unavailable) triggers */
-    for (i = 0; i < RV_MAX_TRIGGERS; i++) {
+    for (i = 0; i < RV_MAX_SDTRIG_TRIGGERS; i++) {
+        SdtrigTrigger *trigger = &env->sdtrig_state.triggers[i];
         int trigger_type = get_trigger_type(env, i);
 
         switch (trigger_type) {
@@ -1061,10 +1084,10 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
             break;
         }
 
-        env->tdata1[i] = tdata1;
-        env->tdata2[i] = 0;
-        env->tdata3[i] = 0;
+        trigger->tdata1 = tdata1;
+        trigger->tdata2 = 0;
+        trigger->tdata3 = 0;
     }
 
-    env->mcontext = 0;
+    env->sdtrig_state.mcontext = 0;
 }
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index bee42b8593..8a047c8073 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -24,7 +24,8 @@
 
 #include "exec/breakpoint.h"
 
-#define RV_MAX_TRIGGERS         2
+#define RV_MAX_SDTRIG_TRIGGERS        32
+#define RV_DEFAULT_TRIGGERS            2
 
 /* register index of tdata CSRs */
 enum {
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index bee1445a31..fdd5e8b67b 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -225,10 +225,32 @@ static bool debug_needed(void *opaque)
     return cpu->cfg.debug;
 }
 
+static int debug_pre_save(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+    int i;
+
+    for (i = 0; i < RV_DEFAULT_TRIGGERS; i++) {
+        env->old_tdata1[i] = env->sdtrig_state.triggers[i].tdata1;
+        env->old_tdata2[i] = env->sdtrig_state.triggers[i].tdata2;
+        env->old_tdata3[i] = env->sdtrig_state.triggers[i].tdata3;
+    }
+
+    return 0;
+}
+
 static int debug_post_load(void *opaque, int version_id)
 {
     RISCVCPU *cpu = opaque;
     CPURISCVState *env = &cpu->env;
+    int i;
+
+    for (i = 0; i < RV_DEFAULT_TRIGGERS; i++) {
+        env->sdtrig_state.triggers[i].tdata1 = env->old_tdata1[i];
+        env->sdtrig_state.triggers[i].tdata2 = env->old_tdata2[i];
+        env->sdtrig_state.triggers[i].tdata3 = env->old_tdata3[i];
+    }
 
     riscv_cpu_debug_post_load(env);
 
@@ -240,12 +262,13 @@ static const VMStateDescription vmstate_debug = {
     .version_id = 2,
     .minimum_version_id = 2,
     .needed = debug_needed,
+    .pre_save = debug_pre_save,
     .post_load = debug_post_load,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
-        VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
-        VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
-        VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
+        VMSTATE_UINTTL(env.sdtrig_state.trigger_cur, RISCVCPU),
+        VMSTATE_UINTTL_ARRAY(env.old_tdata1, RISCVCPU, RV_DEFAULT_TRIGGERS),
+        VMSTATE_UINTTL_ARRAY(env.old_tdata2, RISCVCPU, RV_DEFAULT_TRIGGERS),
+        VMSTATE_UINTTL_ARRAY(env.old_tdata3, RISCVCPU, RV_DEFAULT_TRIGGERS),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 677172ae2d..bcc8cfcece 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -178,7 +178,8 @@ static TCGTBCPUState riscv_get_tb_cpu_state(CPUState *cs)
     }
 
     if (cpu->cfg.debug) {
-        flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
+        flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER,
+                           env->sdtrig_state.itrigger_enabled);
     }
 #endif
 
-- 
2.51.0

[RFC PATCH 20/25] target/riscv/debug: Migrate mcontext using new sdtrig vmstate

[Nicholas Piggin]

The debug migration state does not include mcontext. Change to using
a new sdtrig vmstate format that allows for all architectural state
if mcontext != 0.

This means a machine is sometimes-migratable, which on second
thoughts is probably bad. Is there a better way to do this, or could
we just break compat and move to the new vmstate?

Not yet signed off.
---
 target/riscv/machine.c | 58 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index fdd5e8b67b..23a5f60d2a 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -221,8 +221,10 @@ static const VMStateDescription vmstate_kvmtimer = {
 static bool debug_needed(void *opaque)
 {
     RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
 
-    return cpu->cfg.debug;
+    return cpu->cfg.debug &&
+           env->sdtrig_state.mcontext == 0;
 }
 
 static int debug_pre_save(void *opaque)
@@ -273,6 +275,59 @@ static const VMStateDescription vmstate_debug = {
     }
 };
 
+/*
+ * This is a newer version of the debug (sdtrig) state, required
+ * to migrate hcontext/mcontext.
+ */
+static bool sdtrig_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    return cpu->cfg.debug &&
+           env->sdtrig_state.mcontext != 0;
+}
+
+static int sdtrig_post_load(void *opaque, int version_id)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    riscv_cpu_debug_post_load(env);
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_sdtrig_trigger = {
+    .name = "cpu/sdtrig/trigger",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINTTL(tdata1, SdtrigTrigger),
+        VMSTATE_UINTTL(tdata2, SdtrigTrigger),
+        VMSTATE_UINTTL(tdata3, SdtrigTrigger),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_sdtrig = {
+    .name = "cpu/sdtrig",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = sdtrig_needed,
+    .post_load = sdtrig_post_load,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINTTL(env.sdtrig_state.trigger_cur, RISCVCPU),
+        VMSTATE_UINTTL(env.sdtrig_state.tcontrol, RISCVCPU),
+        VMSTATE_UINTTL(env.sdtrig_state.mcontext, RISCVCPU),
+        VMSTATE_UINTTL(env.sdtrig_state.scontext, RISCVCPU),
+        VMSTATE_STRUCT_ARRAY(env.sdtrig_state.triggers, RISCVCPU,
+                             RV_MAX_SDTRIG_TRIGGERS,
+                             0, vmstate_sdtrig_trigger, SdtrigTrigger),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int riscv_cpu_post_load(void *opaque, int version_id)
 {
     RISCVCPU *cpu = opaque;
@@ -514,6 +569,7 @@ const VMStateDescription vmstate_riscv_cpu = {
 #endif
         &vmstate_envcfg,
         &vmstate_debug,
+        &vmstate_sdtrig,
         &vmstate_smstateen,
         &vmstate_jvt,
         &vmstate_elp,
-- 
2.51.0

[RFC PATCH 21/25] target/riscv/debug: Implementation specific Sdtrig configuration

[Nicholas Piggin]

Implement a Sdtrig CPU configuration class that provides
details about the Sdtrig configuration.

It is stored as a pointer in RISCVCPUClass to avoid issues with
nested compound literals inside static initialisers with GCC 11.

For now, the number of supported triggers is configurable. This
requires some logic

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/cpu.c     | 25 +++++++++++++++++++++++++
 target/riscv/cpu.h     |  4 ++++
 target/riscv/debug.c   |  5 ++++-
 target/riscv/debug.h   |  4 ++++
 target/riscv/machine.c | 11 ++++++++---
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 057e221808..6f7a327fc7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -23,6 +23,7 @@
 #include "qemu/log.h"
 #include "cpu.h"
 #include "cpu_vendorid.h"
+#include "debug.h"
 #include "internals.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
@@ -2816,6 +2817,11 @@ static void riscv_cpu_class_base_init(ObjectClass *c, const void *data)
             mcc->def->vext_spec = def->vext_spec;
         }
         mcc->def->misa_ext |= def->misa_ext;
+#if !defined(CONFIG_USER_ONLY)
+        if (def->debug_cfg) {
+            mcc->def->debug_cfg = def->debug_cfg;
+        }
+#endif
 
         riscv_cpu_cfg_merge(&mcc->def->cfg, &def->cfg);
 
@@ -2951,6 +2957,18 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char *nodename)
     DEFINE_RISCV_CPU(type_name, parent_type_name,             \
         .profile = &(profile_))
 
+#if !defined(CONFIG_USER_ONLY)
+/* Sdtrig implementation has 2 triggers that support match, match6, icount */
+static const RISCVSdtrigConfig default_sdtrig_config = {
+    .nr_triggers = 2,
+};
+
+bool riscv_sdtrig_default_implementation(const RISCVSdtrigConfig *config)
+{
+    return config == &default_sdtrig_config;
+}
+#endif
+
 static const TypeInfo riscv_cpu_type_infos[] = {
     {
         .name = TYPE_RISCV_CPU,
@@ -2968,6 +2986,9 @@ static const TypeInfo riscv_cpu_type_infos[] = {
         .cfg.mmu = true,
         .cfg.pmp = true,
         .priv_spec = PRIV_VERSION_LATEST,
+#if !defined(CONFIG_USER_ONLY)
+        .debug_cfg = &default_sdtrig_config,
+#endif
     ),
 
     DEFINE_ABSTRACT_RISCV_CPU(TYPE_RISCV_VENDOR_CPU, TYPE_RISCV_CPU),
@@ -2995,6 +3016,10 @@ static const TypeInfo riscv_cpu_type_infos[] = {
 #else
         .cfg.max_satp_mode = VM_1_10_SV57,
 #endif
+
+#if !defined(CONFIG_USER_ONLY)
+        .debug_cfg = &default_sdtrig_config,
+#endif
     ),
 
     DEFINE_RISCV_CPU(TYPE_RISCV_CPU_MAX, TYPE_RISCV_DYNAMIC_CPU,
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 44ed1665e2..c4f1cb0a9d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -571,6 +571,9 @@ typedef struct RISCVCSR RISCVCSR;
 typedef struct RISCVCPUDef {
     RISCVMXL misa_mxl_max;  /* max mxl for this cpu */
     RISCVCPUProfile *profile;
+#if !defined(CONFIG_USER_ONLY)
+    const RISCVSdtrigConfig *debug_cfg;
+#endif
     uint32_t misa_ext;
     int priv_spec;
     int32_t vext_spec;
@@ -666,6 +669,7 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
                                    void *rmw_fn_arg);
 
 RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
+bool riscv_sdtrig_default_implementation(const RISCVSdtrigConfig *config);
 #endif /* !CONFIG_USER_ONLY */
 
 void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en);
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 22f7958a79..93615b43fb 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -173,7 +173,10 @@ target_ulong tselect_csr_read(CPURISCVState *env)
 
 void tselect_csr_write(CPURISCVState *env, target_ulong val)
 {
-    if (val < RV_DEFAULT_TRIGGERS) {
+    CPUState *cs = env_cpu(env);
+    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
+
+    if (val < mcc->def->debug_cfg->nr_triggers) {
         env->sdtrig_state.trigger_cur = val;
     }
 }
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 8a047c8073..3ba12f95cd 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -135,6 +135,10 @@ enum {
 #define MHSELECT_IGNORE       0
 #define MHSELECT_MCONTEXT     4
 
+typedef struct RISCVSdtrigConfig {
+    unsigned int nr_triggers;
+} RISCVSdtrigConfig;
+
 bool tdata_available(CPURISCVState *env, int tdata_index);
 
 target_ulong tselect_csr_read(CPURISCVState *env);
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 23a5f60d2a..9f65bdca9b 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -221,10 +221,12 @@ static const VMStateDescription vmstate_kvmtimer = {
 static bool debug_needed(void *opaque)
 {
     RISCVCPU *cpu = opaque;
+    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
     CPURISCVState *env = &cpu->env;
 
     return cpu->cfg.debug &&
-           env->sdtrig_state.mcontext == 0;
+           (riscv_sdtrig_default_implementation(mcc->def->debug_cfg) &&
+            env->sdtrig_state.mcontext == 0);
 }
 
 static int debug_pre_save(void *opaque)
@@ -277,15 +279,18 @@ static const VMStateDescription vmstate_debug = {
 
 /*
  * This is a newer version of the debug (sdtrig) state, required
- * to migrate hcontext/mcontext.
+ * to migrate hcontext/mcontext, or machines with non-default
+ * sdtrig implementation.
  */
 static bool sdtrig_needed(void *opaque)
 {
     RISCVCPU *cpu = opaque;
+    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
     CPURISCVState *env = &cpu->env;
 
     return cpu->cfg.debug &&
-           env->sdtrig_state.mcontext != 0;
+           !(riscv_sdtrig_default_implementation(mcc->def->debug_cfg) &&
+             env->sdtrig_state.mcontext == 0);
 }
 
 static int sdtrig_post_load(void *opaque, int version_id)
-- 
2.51.0

[RFC PATCH 22/25] target/riscv/debug: Support heterogeneous trigger types

[Nicholas Piggin]

Not all Sdtrig implementations have heterogeneous triggers. Add
support to the CPU class Sdtrig config to specify the trigger
types that each trigger supports.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/cpu.c   |  8 ++++++++
 target/riscv/debug.c | 17 +++++++++++++----
 target/riscv/debug.h |  5 +++++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6f7a327fc7..5708da5054 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2961,6 +2961,14 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char *nodename)
 /* Sdtrig implementation has 2 triggers that support match, match6, icount */
 static const RISCVSdtrigConfig default_sdtrig_config = {
     .nr_triggers = 2,
+    .triggers = {
+        [0 ... 1] = {
+            .type_mask = (1 << TRIGGER_TYPE_AD_MATCH) |
+                         (1 << TRIGGER_TYPE_AD_MATCH6) |
+                         (1 << TRIGGER_TYPE_INST_CNT) |
+                         (1 << TRIGGER_TYPE_UNAVAIL),
+        },
+    },
 };
 
 bool riscv_sdtrig_default_implementation(const RISCVSdtrigConfig *config)
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 93615b43fb..e8d343bf42 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -845,11 +845,19 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
     }
 
     if (tdata_index == TDATA1) {
+        CPUState *cs = env_cpu(env);
+        RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
+
         if (val == 0) {
             /* special case, writing 0 results in disabled trigger */
             val = build_tdata1(env, TRIGGER_TYPE_UNAVAIL, 0, 0);
         }
         trigger_type = extract_trigger_type(env, val);
+        if (!(mcc->def->debug_cfg->triggers[index].type_mask &
+              (1 << trigger_type))) {
+            val = build_tdata1(env, TRIGGER_TYPE_UNAVAIL, 0, 0);
+            trigger_type = extract_trigger_type(env, val);
+        }
     }
 
     switch (trigger_type) {
@@ -887,11 +895,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
 
 target_ulong tinfo_csr_read(CPURISCVState *env)
 {
-    /* assume all triggers support the same types of triggers */
+    CPUState *cs = env_cpu(env);
+    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
+    target_ulong index = env->sdtrig_state.trigger_cur;
+
     /* XXX: should we set 1 (version 1.0) in the version field? */
-    return BIT(TRIGGER_TYPE_AD_MATCH) |
-           BIT(TRIGGER_TYPE_INST_CNT) |
-           BIT(TRIGGER_TYPE_AD_MATCH6);
+    return mcc->def->debug_cfg->triggers[index].type_mask;
 }
 
 void riscv_cpu_debug_excp_handler(CPUState *cs)
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 3ba12f95cd..f9e840d615 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -135,8 +135,13 @@ enum {
 #define MHSELECT_IGNORE       0
 #define MHSELECT_MCONTEXT     4
 
+struct trigger_properties {
+    uint16_t type_mask; /* Trigger types supported (0 = no trigger here) */
+};
+
 typedef struct RISCVSdtrigConfig {
     unsigned int nr_triggers;
+    struct trigger_properties triggers[RV_MAX_SDTRIG_TRIGGERS];
 } RISCVSdtrigConfig;
 
 bool tdata_available(CPURISCVState *env, int tdata_index);
-- 
2.51.0

[RFC PATCH 23/25] target/riscv/debug: Support heterogeneous mcontrol access types

[Nicholas Piggin]

Similarly to the last patch, mcontrol/mcontrol6 trigger types may
not implement the same read/write/execute match capability. Add
configuration to describe what access type matches are supported.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/cpu.c   |  1 +
 target/riscv/debug.c | 26 ++++++++++++++++++++------
 target/riscv/debug.h |  1 +
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5708da5054..d349457c87 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2967,6 +2967,7 @@ static const RISCVSdtrigConfig default_sdtrig_config = {
                          (1 << TRIGGER_TYPE_AD_MATCH6) |
                          (1 << TRIGGER_TYPE_INST_CNT) |
                          (1 << TRIGGER_TYPE_UNAVAIL),
+            .mcontrol_rwx_mask = 0x7, /* WP/BP */
         },
     },
 };
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e8d343bf42..d7c171736f 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -449,7 +449,11 @@ static inline bool type2_breakpoint_enabled(target_ulong ctrl)
 static target_ulong type2_mcontrol_validate(CPURISCVState *env,
                                             target_ulong ctrl)
 {
+    CPUState *cs = env_cpu(env);
+    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
+    target_ulong index = env->sdtrig_state.trigger_cur;
     target_ulong val;
+    target_ulong rwx_mask;
     uint32_t size;
 
     /* validate the generic part first */
@@ -475,9 +479,12 @@ static target_ulong type2_mcontrol_validate(CPURISCVState *env,
         }
     }
 
-    /* keep the mode and attribute bits */
-    val |= (ctrl & (TYPE2_U | TYPE2_S | TYPE2_M |
-                    TYPE2_LOAD | TYPE2_STORE | TYPE2_EXEC));
+    /* only set supported access (load/store/exec) bits */
+    rwx_mask = mcc->def->debug_cfg->triggers[index].mcontrol_rwx_mask;
+    val |= ctrl & rwx_mask;
+
+    /* keep the mode bits */
+    val |= ctrl & (TYPE2_U | TYPE2_S | TYPE2_M);
 
     return val;
 }
@@ -573,7 +580,11 @@ static inline bool type6_breakpoint_enabled(target_ulong ctrl)
 static target_ulong type6_mcontrol6_validate(CPURISCVState *env,
                                              target_ulong ctrl)
 {
+    CPUState *cs = env_cpu(env);
+    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
+    target_ulong index = env->sdtrig_state.trigger_cur;
     target_ulong val;
+    target_ulong rwx_mask;
     uint32_t size;
 
     /* validate the generic part first */
@@ -596,9 +607,12 @@ static target_ulong type6_mcontrol6_validate(CPURISCVState *env,
         val |= (ctrl & TYPE6_SIZE);
     }
 
-    /* keep the mode and attribute bits */
-    val |= (ctrl & (TYPE6_VU | TYPE6_VS | TYPE6_U | TYPE6_S | TYPE6_M |
-                    TYPE6_LOAD | TYPE6_STORE | TYPE6_EXEC));
+    /* only set supported access (load/store/exec) bits */
+    rwx_mask = mcc->def->debug_cfg->triggers[index].mcontrol_rwx_mask;
+    val |= ctrl & rwx_mask;
+
+    /* keep the mode bits */
+    val |= (ctrl & (TYPE6_VU | TYPE6_VS | TYPE6_U | TYPE6_S | TYPE6_M));
 
     return val;
 }
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index f9e840d615..c9f7225954 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -137,6 +137,7 @@ enum {
 
 struct trigger_properties {
     uint16_t type_mask; /* Trigger types supported (0 = no trigger here) */
+    uint8_t mcontrol_rwx_mask; /* mc/mc6 rwx access match supported */
 };
 
 typedef struct RISCVSdtrigConfig {
-- 
2.51.0

[RFC PATCH 24/25] target/riscv/debug: Emulate TT Ascalon Sdtrig

[Nicholas Piggin]

This adds initial Tenstorrent Ascalon Sdtrig implementation details.
Ascalon has 9 triggers: 4 mcontrol6 triggers that can match exec access,
4 mcontrol6 triggers that can match load / store access, and 1 icount
trigger.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/cpu.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d349457c87..bdc33bb746 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2972,6 +2972,28 @@ static const RISCVSdtrigConfig default_sdtrig_config = {
     },
 };
 
+#if defined(TARGET_RISCV64)
+static const RISCVSdtrigConfig tt_ascalon_sdtrig_config = {
+    .nr_triggers = 9,
+    .triggers = {
+        [0 ... 3] = {
+            .type_mask = (1 << TRIGGER_TYPE_AD_MATCH6) |
+                         (1 << TRIGGER_TYPE_UNAVAIL),
+            .mcontrol_rwx_mask = 0x4, /* BP */
+        },
+        [4 ... 7] = {
+            .type_mask = (1 << TRIGGER_TYPE_AD_MATCH6) |
+                         (1 << TRIGGER_TYPE_UNAVAIL),
+            .mcontrol_rwx_mask = 0x3, /* WP */
+        },
+        [8]       = {
+            .type_mask = (1 << TRIGGER_TYPE_INST_CNT) |
+                         (1 << TRIGGER_TYPE_UNAVAIL),
+        },
+    },
+};
+#endif
+
 bool riscv_sdtrig_default_implementation(const RISCVSdtrigConfig *config)
 {
     return config == &default_sdtrig_config;
@@ -3166,6 +3188,9 @@ static const TypeInfo riscv_cpu_type_infos[] = {
         .misa_ext = RVG | RVC | RVS | RVU | RVH | RVV,
         .priv_spec = PRIV_VERSION_1_13_0,
         .vext_spec = VEXT_VERSION_1_00_0,
+#if !defined(CONFIG_USER_ONLY)
+        .debug_cfg = &tt_ascalon_sdtrig_config,
+#endif
 
         /* ISA extensions */
         .cfg.mmu = true,
-- 
2.51.0

[RFC PATCH 25/25] target/riscv/debug: Fix minor comment typos

[Nicholas Piggin]

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/riscv/debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index d7c171736f..32aa46262a 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -120,7 +120,7 @@ static trigger_action_t get_trigger_action(CPURISCVState *env,
         break;
     case TRIGGER_TYPE_NO_EXIST:
     case TRIGGER_TYPE_UNAVAIL:
-        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
+        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
                       trigger_type);
         break;
     default:
@@ -895,7 +895,7 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
                       trigger_type);
         break;
     case TRIGGER_TYPE_NO_EXIST:
-        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
+        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
                       trigger_type);
         break;
     default:
-- 
2.51.0

Re: [RFC PATCH 25/25] target/riscv/debug: Fix minor comment typos

[Daniel Henrique Barboza]

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/debug.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index d7c171736f..32aa46262a 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -120,7 +120,7 @@ static trigger_action_t get_trigger_action(CPURISCVState *env,
>           break;
>       case TRIGGER_TYPE_NO_EXIST:
>       case TRIGGER_TYPE_UNAVAIL:
> -        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> +        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
>                         trigger_type);
>           break;
>       default:
> @@ -895,7 +895,7 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>                         trigger_type);
>           break;
>       case TRIGGER_TYPE_NO_EXIST:
> -        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> +        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
>                         trigger_type);
>           break;
>       default:

Re: [RFC PATCH 00/25] target/riscv/debug: Sdtrig fixes and TT Ascalon support

[Daniel Henrique Barboza]

Salutations,


Sorry for the late review.  Just got informed in another thread that this
was sent back in January and I missed it.

I believe patches 1-18 and 25 can be sent in a separated "debug related fixes"
series.  They seem nice to have regardless of the other patches.

Patches 19-24 start to touch into a more sensible area that we're discussing in
other threads, most notably this one:

("[PATCH v3 0/1] target/riscv: deprecate 'debug' CPU property and Debug 0.13")
https://lore.kernel.org/qemu-devel/20260528131214.2251075-1-daniel.barboza@oss.qualcomm.com/


The lore behind that change is that we want to support Debug 1.0 extensions, in
particular sdext, because that is a requirement for the Server Reference Board.
But 'debug' is based on spec 0.13, and unfortunately these 2 versions can't play
ball, so my initial reaction was to move away for 0.13.

 From what I can see in this series there is a desire to keep the existing debug
0.13 code base around, and that's fine.  I also believe that despite the
incompatibility the 0.13 code base can be re-used for the 1.0 sdtrig as well,
with the appropriate "if-then-else" to split between the behaviors we want, so
it's feasible to do both.

IIUC what we seem to be missing in patches 19-24 is a separated flag, i.e. a
new extension flag, for the 1.0 sdtrig.  So we would end up with 'debug' being
an alias for 'sdtrig 0.13' and a new 'sdtrig' flag that implements debug 1.0.

Chao can then base his 'sdext' work on top of the new sdtrig flag, and then
we can upstream the riscv-server-ref board.  At least this is how I'm seeing
all this pieces playing out ....


Cheers,
Daniel




On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> Hi,
> 
> Sorry for the big series. The Ascalon CPU implements Sdtrig with 2
> different types of mcontrol6 trigger and the icount trigger, so in
> the course of testing and bringing up OpenSBI and Linux support for
> this, I've accumulated quite a lot.
> 
> My new year resolution is to start being better upstream contributor,
> it's taken me a while with changing jobs and architectures. So I don't
> expect others to drop everything to review this! Joel has been
> prodding me, and noted there is some other Sdtrig work going on
> with the v1.0 support patches.
> 
> I think the debug v1.0 patches are somewhat orthogonal to this series,
> but both are addressing aspects of a common problem of Sdtrig
> implementation specifics. I wonder if these should be reconciled or
> left separate. Sdtrig v1.00/v0.13 configuration is a single boolean
> which is feasible as a CPU property. Whereas the entire space of
> Sdtrig implementation seems like too much to make configurable in that
> way.
> 
> Any thoughts would be welcome.
> 
> Thanks,
> Nick
> 
> Nicholas Piggin (25):
>    target/riscv/debug: Check only mcontrol triggers for break/watchpoint
>      matching
>    target/riscv/debug: Handle changing trigger types
>    target/riscv/debug: Implement permissive type unavailable trigger
>    target/riscv/debug: Fix icount trigger privilege check
>    target/riscv/debug: Update itrigger_enabled after changing privilege
>    target/riscv/debug: Implement get_trigger_action for icount type
>      trigger
>    target/riscv/debug: Fix migration post_load icount_enabled() test
>    target/riscv/debug: Fix icount privilege matching icount_enabled()
>      test
>    target/riscv/debug: Implement icount trigger textra matching
>    target/riscv/debug: Maintain itrigger_enabled in
>      helper_itrigger_match()
>    target/riscv/debug: Fix breakpoint matching action
>    target/riscv/debug: Put mcontrol load/store match address into tval
>    target/riscv/debug: Remove breakpoints on reset
>    target/riscv/debug: Move debug CPU post_load details into debug.c
>    target/riscv/debug: Insert breakpoints after migration
>    target/riscv/debug: Remove itrigger icount-enabled mode
>    target/riscv/debug: Advertise icount trigger type in tinfo
>    target/riscv/debug: Reset trigger type to unavailable
>    target/riscv/debug: Add new debug state format
>    target/riscv/debug: Migrate mcontext using new sdtrig vmstate
>    target/riscv/debug: Implementation specific Sdtrig configuration
>    target/riscv/debug: Support heterogeneous trigger types
>    target/riscv/debug: Support heterogeneous mcontrol access types
>    target/riscv/debug: Emulate TT Ascalon Sdtrig
>    target/riscv/debug: Fix minor comment typos
> 
>   target/riscv/cpu.c         |  65 ++++-
>   target/riscv/cpu.h         |  41 ++-
>   target/riscv/cpu_helper.c  |  10 +-
>   target/riscv/csr.c         |   7 +-
>   target/riscv/debug.c       | 571 ++++++++++++++++++++-----------------
>   target/riscv/debug.h       |  19 +-
>   target/riscv/machine.c     |  96 ++++++-
>   target/riscv/tcg/tcg-cpu.c |   5 +-
>   8 files changed, 510 insertions(+), 304 deletions(-)
> 

Re: [RFC PATCH 00/25] target/riscv/debug: Sdtrig fixes and TT Ascalon support

[Daniel Henrique Barboza]

Greetings,


My current understanding is that this series is an official dependency of
the riscv-server-ref board, currently on v8:

[PATCH v8 0/7] hw/riscv: Server Platform Reference Board

Because we decided that we want to keep 0.13 and 1.0 instead of deprecating
debug 1.0 (see [1]).

We missed the 11.1 boat with this work and with riscv-server-ref.  No big
deal TBH since the server-ref board can use improvements before upstreaming.
But I would like to not miss the 11.2 train in the end of the year.

Nick, I already reviewed a bunch of patches that seems to be fixes that we
can use right away.  If you're ok with it I'll pick them all and re-send in
your behalf, rebased and with acks.  My hope is that this will reduce
the amount of work needed for the rest of the 0.13/1.0 debug code and
we can get that sorted it out.  Then Chao can use the new 1.0 infrastructure
for his 'sdext' patches, and then we can lend the riscv-server-ref board on
top of it all.

Let me know if that works for you.  Chao, not sure if you were aware of this
series so feel free to comment.


[1] https://lore.kernel.org/qemu-devel/383936e5-fcdf-488b-8f77-17fdee41243e@oss.qualcomm.com/


Thanks,

Daniel

On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> Hi,
> 
> Sorry for the big series. The Ascalon CPU implements Sdtrig with 2
> different types of mcontrol6 trigger and the icount trigger, so in
> the course of testing and bringing up OpenSBI and Linux support for
> this, I've accumulated quite a lot.
> 
> My new year resolution is to start being better upstream contributor,
> it's taken me a while with changing jobs and architectures. So I don't
> expect others to drop everything to review this! Joel has been
> prodding me, and noted there is some other Sdtrig work going on
> with the v1.0 support patches.
> 
> I think the debug v1.0 patches are somewhat orthogonal to this series,
> but both are addressing aspects of a common problem of Sdtrig
> implementation specifics. I wonder if these should be reconciled or
> left separate. Sdtrig v1.00/v0.13 configuration is a single boolean
> which is feasible as a CPU property. Whereas the entire space of
> Sdtrig implementation seems like too much to make configurable in that
> way.
> 
> Any thoughts would be welcome.
> 
> Thanks,
> Nick
> 
> Nicholas Piggin (25):
>    target/riscv/debug: Check only mcontrol triggers for break/watchpoint
>      matching
>    target/riscv/debug: Handle changing trigger types
>    target/riscv/debug: Implement permissive type unavailable trigger
>    target/riscv/debug: Fix icount trigger privilege check
>    target/riscv/debug: Update itrigger_enabled after changing privilege
>    target/riscv/debug: Implement get_trigger_action for icount type
>      trigger
>    target/riscv/debug: Fix migration post_load icount_enabled() test
>    target/riscv/debug: Fix icount privilege matching icount_enabled()
>      test
>    target/riscv/debug: Implement icount trigger textra matching
>    target/riscv/debug: Maintain itrigger_enabled in
>      helper_itrigger_match()
>    target/riscv/debug: Fix breakpoint matching action
>    target/riscv/debug: Put mcontrol load/store match address into tval
>    target/riscv/debug: Remove breakpoints on reset
>    target/riscv/debug: Move debug CPU post_load details into debug.c
>    target/riscv/debug: Insert breakpoints after migration
>    target/riscv/debug: Remove itrigger icount-enabled mode
>    target/riscv/debug: Advertise icount trigger type in tinfo
>    target/riscv/debug: Reset trigger type to unavailable
>    target/riscv/debug: Add new debug state format
>    target/riscv/debug: Migrate mcontext using new sdtrig vmstate
>    target/riscv/debug: Implementation specific Sdtrig configuration
>    target/riscv/debug: Support heterogeneous trigger types
>    target/riscv/debug: Support heterogeneous mcontrol access types
>    target/riscv/debug: Emulate TT Ascalon Sdtrig
>    target/riscv/debug: Fix minor comment typos
> 
>   target/riscv/cpu.c         |  65 ++++-
>   target/riscv/cpu.h         |  41 ++-
>   target/riscv/cpu_helper.c  |  10 +-
>   target/riscv/csr.c         |   7 +-
>   target/riscv/debug.c       | 571 ++++++++++++++++++++-----------------
>   target/riscv/debug.h       |  19 +-
>   target/riscv/machine.c     |  96 ++++++-
>   target/riscv/tcg/tcg-cpu.c |   5 +-
>   8 files changed, 510 insertions(+), 304 deletions(-)
> 

Re: [RFC PATCH 00/25] target/riscv/debug: Sdtrig fixes and TT Ascalon support

[Chao Liu]

On Fri, Jul 03, 2026 at 06:36:13PM +0800, Daniel Henrique Barboza wrote:
> Greetings,
> 
> 
> My current understanding is that this series is an official dependency of
> the riscv-server-ref board, currently on v8:
> 
> [PATCH v8 0/7] hw/riscv: Server Platform Reference Board
> 
> Because we decided that we want to keep 0.13 and 1.0 instead of deprecating
> debug 1.0 (see [1]).
> 
> We missed the 11.1 boat with this work and with riscv-server-ref.  No big
> deal TBH since the server-ref board can use improvements before upstreaming.
> But I would like to not miss the 11.2 train in the end of the year.
> 
> Nick, I already reviewed a bunch of patches that seems to be fixes that we
> can use right away.  If you're ok with it I'll pick them all and re-send in
> your behalf, rebased and with acks.  My hope is that this will reduce
> the amount of work needed for the rest of the 0.13/1.0 debug code and
> we can get that sorted it out.  Then Chao can use the new 1.0 infrastructure
> for his 'sdext' patches, and then we can lend the riscv-server-ref board on
That's great news. I'll follow up on this series next week and start updating
my 'sdext' patches.

> top of it all.
> 
> Let me know if that works for you.  Chao, not sure if you were aware of this
> series so feel free to comment.
I'm no longer being blocked by anything, and I feel like our work is basically
nearing completion. 

The main thing missing right now relates to the debug spec features, but I don't
think those will be an issue. We should be able to push them upstream before
the 11.2 release.

Thanks,
Chao
> 
> 
> [1] https://lore.kernel.org/qemu-devel/383936e5-fcdf-488b-8f77-17fdee41243e@oss.qualcomm.com/
> 
> 
> Thanks,
> 
> Daniel
> 
> On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
> > Hi,
> > 
> > Sorry for the big series. The Ascalon CPU implements Sdtrig with 2
> > different types of mcontrol6 trigger and the icount trigger, so in
> > the course of testing and bringing up OpenSBI and Linux support for
> > this, I've accumulated quite a lot.
> > 
> > My new year resolution is to start being better upstream contributor,
> > it's taken me a while with changing jobs and architectures. So I don't
> > expect others to drop everything to review this! Joel has been
> > prodding me, and noted there is some other Sdtrig work going on
> > with the v1.0 support patches.
> > 
> > I think the debug v1.0 patches are somewhat orthogonal to this series,
> > but both are addressing aspects of a common problem of Sdtrig
> > implementation specifics. I wonder if these should be reconciled or
> > left separate. Sdtrig v1.00/v0.13 configuration is a single boolean
> > which is feasible as a CPU property. Whereas the entire space of
> > Sdtrig implementation seems like too much to make configurable in that
> > way.
> > 
> > Any thoughts would be welcome.
> > 
> > Thanks,
> > Nick
> > 
> > Nicholas Piggin (25):
> >    target/riscv/debug: Check only mcontrol triggers for break/watchpoint
> >      matching
> >    target/riscv/debug: Handle changing trigger types
> >    target/riscv/debug: Implement permissive type unavailable trigger
> >    target/riscv/debug: Fix icount trigger privilege check
> >    target/riscv/debug: Update itrigger_enabled after changing privilege
> >    target/riscv/debug: Implement get_trigger_action for icount type
> >      trigger
> >    target/riscv/debug: Fix migration post_load icount_enabled() test
> >    target/riscv/debug: Fix icount privilege matching icount_enabled()
> >      test
> >    target/riscv/debug: Implement icount trigger textra matching
> >    target/riscv/debug: Maintain itrigger_enabled in
> >      helper_itrigger_match()
> >    target/riscv/debug: Fix breakpoint matching action
> >    target/riscv/debug: Put mcontrol load/store match address into tval
> >    target/riscv/debug: Remove breakpoints on reset
> >    target/riscv/debug: Move debug CPU post_load details into debug.c
> >    target/riscv/debug: Insert breakpoints after migration
> >    target/riscv/debug: Remove itrigger icount-enabled mode
> >    target/riscv/debug: Advertise icount trigger type in tinfo
> >    target/riscv/debug: Reset trigger type to unavailable
> >    target/riscv/debug: Add new debug state format
> >    target/riscv/debug: Migrate mcontext using new sdtrig vmstate
> >    target/riscv/debug: Implementation specific Sdtrig configuration
> >    target/riscv/debug: Support heterogeneous trigger types
> >    target/riscv/debug: Support heterogeneous mcontrol access types
> >    target/riscv/debug: Emulate TT Ascalon Sdtrig
> >    target/riscv/debug: Fix minor comment typos
> > 
> >   target/riscv/cpu.c         |  65 ++++-
> >   target/riscv/cpu.h         |  41 ++-
> >   target/riscv/cpu_helper.c  |  10 +-
> >   target/riscv/csr.c         |   7 +-
> >   target/riscv/debug.c       | 571 ++++++++++++++++++++-----------------
> >   target/riscv/debug.h       |  19 +-
> >   target/riscv/machine.c     |  96 ++++++-
> >   target/riscv/tcg/tcg-cpu.c |   5 +-
> >   8 files changed, 510 insertions(+), 304 deletions(-)
> > 
>