@samitouri / QOSamiQemu / commits / 3bcd11adae

target/riscv: dynamic alloc of debug trigger arrays

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/94c772b0-5231-40e4-9cab-6ac39b9e4d45@oss.qualcomm.com/ Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-ID: <20260617131710.1855353-3-daniel.barboza@oss.qualcomm.com> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Daniel Henrique Barboza committed Jun 17, 2026 at 10:17 UTC 3bcd11adae8446de826b340d1c97a392ae90a3c1
6 files changed +69 -20
target/riscv/cpu.c
+6
@@ -1012,7 +1012,13 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
1012 static void riscv_cpu_unrealize(DeviceState *dev)
1013 {
1014 RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
1015 +#ifndef CONFIG_USER_ONLY
1016 + RISCVCPU *cpu = RISCV_CPU(dev);
1017
1018 + if (cpu->cfg.debug) {
1019 + riscv_trigger_unrealize(&cpu->env);
1020 + }
1021 +#endif
1022 mcc->parent_unrealize(dev);
1023 }
1024
target/riscv/cpu.h
+12 -6
@@ -467,12 +467,18 @@ struct CPUArchState {
467 /* trigger module */
468 uint16_t mcontext;
469 uint8_t trigger_cur;
470 - uint64_t tdata1[RV_MAX_TRIGGERS];
471 - uint64_t tdata2[RV_MAX_TRIGGERS];
472 - uint64_t tdata3[RV_MAX_TRIGGERS];
473 - struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
474 - struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
475 - QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
470 + /*
471 + * num_triggers is the length of tdata1, tdata2, tdata3,
472 + * cpu_breakpoint, cpu_watchpoint and itrigger_timer
473 + * arrays.
474 + */
475 + uint32_t num_triggers;
476 + uint64_t *tdata1;
477 + uint64_t *tdata2;
478 + uint64_t *tdata3;
479 + struct CPUBreakpoint **cpu_breakpoint;
480 + struct CPUWatchpoint **cpu_watchpoint;
481 + QEMUTimer **itrigger_timer;
482 int64_t last_icount;
483 bool itrigger_enabled;
484
target/riscv/csr.c
+1 -1
@@ -5441,7 +5441,7 @@ static RISCVException read_tdata(CPURISCVState *env, int csrno,
5441 target_ulong *val)
5442 {
5443 /* return 0 in tdata1 to end the trigger enumeration */
5444 - if (env->trigger_cur >= RV_MAX_TRIGGERS && csrno == CSR_TDATA1) {
5444 + if (env->trigger_cur >= env->num_triggers && csrno == CSR_TDATA1) {
5445 *val = 0;
5446 return RISCV_EXCP_NONE;
5447 }
target/riscv/debug.c
+38 -8
@@ -172,7 +172,7 @@ target_ulong tselect_csr_read(CPURISCVState *env)
172
173 void tselect_csr_write(CPURISCVState *env, target_ulong val)
174 {
175 - if (val < RV_MAX_TRIGGERS) {
175 + if (val < env->num_triggers) {
176 env->trigger_cur = val;
177 }
178 }
@@ -701,7 +701,7 @@ static bool check_itrigger_priv(CPURISCVState *env, int index)
701 bool riscv_itrigger_enabled(CPURISCVState *env)
702 {
703 int count;
704 - for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
704 + for (int i = 0; i < env->num_triggers; i++) {
705 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
706 continue;
707 }
@@ -721,7 +721,7 @@ bool riscv_itrigger_enabled(CPURISCVState *env)
721 void helper_itrigger_match(CPURISCVState *env)
722 {
723 int count;
724 - for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
724 + for (int i = 0; i < env->num_triggers; i++) {
725 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
726 continue;
727 }
@@ -750,7 +750,7 @@ static void riscv_itrigger_update_count(CPURISCVState *env)
750 int64_t last_icount = env->last_icount, current_icount;
751 current_icount = env->last_icount = icount_get_raw();
752
753 - for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
753 + for (int i = 0; i < env->num_triggers; i++) {
754 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
755 continue;
756 }
@@ -950,7 +950,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
950 int i;
951
952 QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
953 - for (i = 0; i < RV_MAX_TRIGGERS; i++) {
953 + for (i = 0; i < env->num_triggers; i++) {
954 trigger_type = get_trigger_type(env, i);
955
956 if (!trigger_common_match(env, trigger_type, i)) {
@@ -996,7 +996,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
996 int flags;
997 int i;
998
999 - for (i = 0; i < RV_MAX_TRIGGERS; i++) {
999 + for (i = 0; i < env->num_triggers; i++) {
1000 trigger_type = get_trigger_type(env, i);
1001
1002 if (!trigger_common_match(env, trigger_type, i)) {
@@ -1049,19 +1049,49 @@ void riscv_trigger_realize(CPURISCVState *env)
1049 {
1050 int i;
1051
1052 - for (i = 0; i < RV_MAX_TRIGGERS; i++) {
1052 + /*
1053 + * Alloc env->tdata1/2/3, cpu_breakpoint, cpu_watchpoint and
1054 + * itrigger_timer dynamically. This is overkill now
1055 + * given that they could be static arrays with RV_MAX_TRIGGERS
1056 + * but we'll parametrize the trigger number later, i.e. the
1057 + * array length won't be static.
1058 + */
1059 + env->num_triggers = RV_MAX_TRIGGERS;
1060 + env->tdata1 = g_new0(uint64_t, env->num_triggers);
1061 + env->tdata2 = g_new0(uint64_t, env->num_triggers);
1062 + env->tdata3 = g_new0(uint64_t, env->num_triggers);
1063 + env->cpu_breakpoint = g_new0(struct CPUBreakpoint *, env->num_triggers);
1064 + env->cpu_watchpoint = g_new0(struct CPUWatchpoint *, env->num_triggers);
1065 + env->itrigger_timer = g_new0(QEMUTimer *, env->num_triggers);
1066 +
1067 + for (i = 0; i < env->num_triggers; i++) {
1068 env->itrigger_timer[i] = timer_new_ns(QEMU_CLOCK_VIRTUAL,
1069 riscv_itrigger_timer_cb, env);
1070 }
1071 }
1072
1073 +void riscv_trigger_unrealize(CPURISCVState *env)
1074 +{
1075 + g_free(env->tdata1);
1076 + g_free(env->tdata2);
1077 + g_free(env->tdata3);
1078 +
1079 + g_free(env->cpu_breakpoint);
1080 + g_free(env->cpu_watchpoint);
1081 +
1082 + for (int i = 0; i < env->num_triggers; i++) {
1083 + timer_del(env->itrigger_timer[i]);
1084 + }
1085 + g_free(env->itrigger_timer);
1086 +}
1087 +
1088 void riscv_trigger_reset_hold(CPURISCVState *env)
1089 {
1090 target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
1091 int i;
1092
1093 /* init to type 2 triggers */
1064 - for (i = 0; i < RV_MAX_TRIGGERS; i++) {
1094 + for (i = 0; i < env->num_triggers; i++) {
1095 /*
1096 * type = TRIGGER_TYPE_AD_MATCH
1097 * dmode = 0 (both debug and M-mode can write tdata)
target/riscv/debug.h
+1
@@ -148,6 +148,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
148 bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
149
150 void riscv_trigger_realize(CPURISCVState *env);
151 +void riscv_trigger_unrealize(CPURISCVState *env);
152 void riscv_trigger_reset_hold(CPURISCVState *env);
153
154 bool riscv_itrigger_enabled(CPURISCVState *env);
target/riscv/machine.c
+11 -5
@@ -240,16 +240,22 @@ static int debug_post_load(void *opaque, int version_id)
240
241 static const VMStateDescription vmstate_debug = {
242 .name = "cpu/debug",
243 - .version_id = 3,
244 - .minimum_version_id = 3,
243 + .version_id = 4,
244 + .minimum_version_id = 4,
245 .needed = debug_needed,
246 .post_load = debug_post_load,
247 .fields = (const VMStateField[]) {
248 VMSTATE_UINT16(env.mcontext, RISCVCPU),
249 VMSTATE_UINT8(env.trigger_cur, RISCVCPU),
250 - VMSTATE_UINT64_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
251 - VMSTATE_UINT64_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
252 - VMSTATE_UINT64_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
250 + VMSTATE_VARRAY_UINT32(env.tdata1, RISCVCPU,
251 + env.num_triggers, 0,
252 + vmstate_info_uint64, uint64_t),
253 + VMSTATE_VARRAY_UINT32(env.tdata2, RISCVCPU,
254 + env.num_triggers, 0,
255 + vmstate_info_uint64, uint64_t),
256 + VMSTATE_VARRAY_UINT32(env.tdata3, RISCVCPU,
257 + env.num_triggers, 0,
258 + vmstate_info_uint64, uint64_t),
259 VMSTATE_END_OF_LIST()
260 }
261 };