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 during realize time in riscv_trigger_realize(), 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.h | 18 ++++++++++++------ target/riscv/csr.c | 2 +- target/riscv/debug.c | 31 +++++++++++++++++++++++-------- target/riscv/machine.c | 12 +++++++++--- 4 files changed, 45 insertions(+), 18 deletions(-) diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index c98652cd62..8345b9a207 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..8b31efdbc6 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,7 +1049,22 @@ 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); } @@ -1061,7 +1076,7 @@ void riscv_trigger_reset_hold(CPURISCVState *env) 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/machine.c b/target/riscv/machine.c index 6e70b145a5..b6b53a4840 100644 --- a/target/riscv/machine.c +++ b/target/riscv/machine.c @@ -247,9 +247,15 @@ static const VMStateDescription vmstate_debug = { .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
