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 <npig...@gmail.com>
---

Reviewed-by: Daniel Henrique Barboza <daniel.barb...@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: