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