On 10/6/26 22:54, Daniel Henrique Barboza 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 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(-)
@@ -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);
}
Ideally we'd implement riscv_cpu_unrealize() first, so
in this patch you could fill the riscv_trigger_unrealize()
cleaning path. Can be done later, but the sooner the
better IMHO.
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
