The debug trigger facility consists of a set of arrays: tdata1-3, cpu_breakpoint, cpu_watchpoint and itrigger_timer. All of them are static allocated with RV_MAX_TRIGGERS (2). This means that all RISC-V cpus will have 2 triggers per hart.
The RISC-V Server Ref demands at least 11 triggers per hart, and several CPUs in the wild works with 4+ triggers. We need more flexibility, ergo we need to parametrize the amount of triggers and make it configurable. Before doing that we need to handle a situation faced in a previous attempt [1]. We were unable to set the tdataN array length in vmstate_debug, meaning that we would always migrate RV_MAX_TRIGGERS regardless of the actual amount of triggers in play. To fix that we need to change the tdata arrays from static to dynamic, allowing us to use VMSTATE_VARRAY_UINT32(). This also means that, in contrast with [1], we have the opportunity to turn all trigger arrays into dynamic allocation and reduce the amount of stuff being carried by CPURISCVState, or in other words, we can carry just what we're using instead of a static max value. All the forementioned trigger facility arrays are now dynamic. They are allocated and freed during realize/unrealize, and their size is expressed by env->num_triggers. All relevant code is changed to use env->num_triggers instead of the RV_MAX_TRIGGERS to loop through each array. This will make it easier for the next patch to parametrize env->num_triggers. [1] https://lore.kernel.org/qemu-devel/[email protected]/ Signed-off-by: Daniel Henrique Barboza <[email protected]> --- target/riscv/cpu.c | 6 ++++++ target/riscv/cpu.h | 18 +++++++++++------ target/riscv/csr.c | 2 +- target/riscv/debug.c | 46 ++++++++++++++++++++++++++++++++++-------- target/riscv/debug.h | 1 + target/riscv/machine.c | 16 ++++++++++----- 6 files changed, 69 insertions(+), 20 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 5b2133d811..b20271a220 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1015,7 +1015,13 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) static void riscv_cpu_unrealize(DeviceState *dev) { RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); +#ifndef CONFIG_USER_ONLY + RISCVCPU *cpu = RISCV_CPU(dev); + if (cpu->cfg.debug) { + riscv_trigger_unrealize(&cpu->env); + } +#endif mcc->parent_unrealize(dev); } diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 11cd710990..ea92e2c68c 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -467,12 +467,18 @@ struct CPUArchState { /* trigger module */ uint16_t mcontext; uint8_t trigger_cur; - uint64_t tdata1[RV_MAX_TRIGGERS]; - uint64_t tdata2[RV_MAX_TRIGGERS]; - uint64_t tdata3[RV_MAX_TRIGGERS]; - struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS]; - struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS]; - QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS]; + /* + * num_triggers is the length of tdata1, tdata2, tdata3, + * cpu_breakpoint, cpu_watchpoint and itrigger_timer + * arrays. + */ + uint32_t num_triggers; + uint64_t *tdata1; + uint64_t *tdata2; + uint64_t *tdata3; + struct CPUBreakpoint **cpu_breakpoint; + struct CPUWatchpoint **cpu_watchpoint; + QEMUTimer **itrigger_timer; int64_t last_icount; bool itrigger_enabled; diff --git a/target/riscv/csr.c b/target/riscv/csr.c index ec931a8c3d..82afad82db 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -5436,7 +5436,7 @@ 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->trigger_cur >= env->num_triggers && csrno == CSR_TDATA1) { *val = 0; return RISCV_EXCP_NONE; } diff --git a/target/riscv/debug.c b/target/riscv/debug.c index 30d39ee5cd..9a4910c431 100644 --- a/target/riscv/debug.c +++ b/target/riscv/debug.c @@ -172,7 +172,7 @@ target_ulong tselect_csr_read(CPURISCVState *env) void tselect_csr_write(CPURISCVState *env, target_ulong val) { - if (val < RV_MAX_TRIGGERS) { + if (val < env->num_triggers) { env->trigger_cur = val; } } @@ -701,7 +701,7 @@ 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++) { + for (int i = 0; i < env->num_triggers; i++) { if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) { continue; } @@ -721,7 +721,7 @@ bool riscv_itrigger_enabled(CPURISCVState *env) void helper_itrigger_match(CPURISCVState *env) { int count; - for (int i = 0; i < RV_MAX_TRIGGERS; i++) { + for (int i = 0; i < env->num_triggers; i++) { if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) { continue; } @@ -750,7 +750,7 @@ static void riscv_itrigger_update_count(CPURISCVState *env) 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++) { + for (int i = 0; i < env->num_triggers; i++) { if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) { continue; } @@ -950,7 +950,7 @@ 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 < env->num_triggers; i++) { trigger_type = get_trigger_type(env, i); if (!trigger_common_match(env, trigger_type, i)) { @@ -996,7 +996,7 @@ 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 < env->num_triggers; i++) { trigger_type = get_trigger_type(env, i); if (!trigger_common_match(env, trigger_type, i)) { @@ -1049,19 +1049,49 @@ void riscv_trigger_realize(CPURISCVState *env) { int i; - for (i = 0; i < RV_MAX_TRIGGERS; i++) { + /* + * Alloc env->tdata1/2/3, cpu_breakpoint, cpu_watchpoint and + * itrigger_timer dynamically. This is overkill now + * given that they could be static arrays with RV_MAX_TRIGGERS + * but we'll parametrize the trigger number later, i.e. the + * array length won't be static. + */ + env->num_triggers = RV_MAX_TRIGGERS; + env->tdata1 = g_new0(uint64_t, env->num_triggers); + env->tdata2 = g_new0(uint64_t, env->num_triggers); + env->tdata3 = g_new0(uint64_t, env->num_triggers); + env->cpu_breakpoint = g_new0(struct CPUBreakpoint *, env->num_triggers); + env->cpu_watchpoint = g_new0(struct CPUWatchpoint *, env->num_triggers); + env->itrigger_timer = g_new0(QEMUTimer *, env->num_triggers); + + for (i = 0; i < env->num_triggers; i++) { env->itrigger_timer[i] = timer_new_ns(QEMU_CLOCK_VIRTUAL, riscv_itrigger_timer_cb, env); } } +void riscv_trigger_unrealize(CPURISCVState *env) +{ + g_free(env->tdata1); + g_free(env->tdata2); + g_free(env->tdata3); + + g_free(env->cpu_breakpoint); + g_free(env->cpu_watchpoint); + + for (int i = 0; i < env->num_triggers; i++) { + timer_del(env->itrigger_timer[i]); + } + g_free(env->itrigger_timer); +} + void riscv_trigger_reset_hold(CPURISCVState *env) { target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0); int i; /* init to type 2 triggers */ - for (i = 0; i < RV_MAX_TRIGGERS; i++) { + for (i = 0; i < env->num_triggers; i++) { /* * type = TRIGGER_TYPE_AD_MATCH * dmode = 0 (both debug and M-mode can write tdata) diff --git a/target/riscv/debug.h b/target/riscv/debug.h index 55a3ac72e6..a25d099b37 100644 --- a/target/riscv/debug.h +++ b/target/riscv/debug.h @@ -148,6 +148,7 @@ 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_unrealize(CPURISCVState *env); void riscv_trigger_reset_hold(CPURISCVState *env); bool riscv_itrigger_enabled(CPURISCVState *env); diff --git a/target/riscv/machine.c b/target/riscv/machine.c index 6e70b145a5..ba96ceceef 100644 --- a/target/riscv/machine.c +++ b/target/riscv/machine.c @@ -240,16 +240,22 @@ static int debug_post_load(void *opaque, int version_id) static const VMStateDescription vmstate_debug = { .name = "cpu/debug", - .version_id = 3, - .minimum_version_id = 3, + .version_id = 4, + .minimum_version_id = 4, .needed = debug_needed, .post_load = debug_post_load, .fields = (const VMStateField[]) { VMSTATE_UINT16(env.mcontext, RISCVCPU), VMSTATE_UINT8(env.trigger_cur, RISCVCPU), - VMSTATE_UINT64_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS), - VMSTATE_UINT64_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS), - VMSTATE_UINT64_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS), + VMSTATE_VARRAY_UINT32(env.tdata1, RISCVCPU, + env.num_triggers, 0, + vmstate_info_uint64, uint64_t), + VMSTATE_VARRAY_UINT32(env.tdata2, RISCVCPU, + env.num_triggers, 0, + vmstate_info_uint64, uint64_t), + VMSTATE_VARRAY_UINT32(env.tdata3, RISCVCPU, + env.num_triggers, 0, + vmstate_info_uint64, uint64_t), VMSTATE_END_OF_LIST() } }; -- 2.43.0
