On Thu, Jun 11, 2026 at 10:35 PM Daniel Henrique Barboza <[email protected]> wrote: > > 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]>
Reviewed-by: Alistair Francis <[email protected]> Alistair > --- > 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 > >
