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

[Nicholas Piggin <npiggin@gmail.com>]

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