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 <[email protected]> --- 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
