target/riscv/cpu.c | 11 +++++++++++ target/riscv/cpu.h | 23 ++++++++++++++++++++++- target/riscv/csr.c | 2 +- target/riscv/debug.c | 16 ++++++++-------- target/riscv/machine.c | 4 ++-- target/riscv/tcg/tcg-cpu.c | 7 +++++++ 6 files changed, 51 insertions(+), 12 deletions(-)
All CPUs have the same amount of triggers: 2 triggers per hart, set via
RV_MAX_TRIGGERS. This is not enough anymore: we'll have at least one
future CPU that will demand more triggers per hart when implementing the
RISC-V Server Ref Platform, requiring at least 11 triggers per hart.
Parametrize the trigger amount using a new 'trigger_count' env property.
The default amount is kept at 2. Set the new maximum to an educated
guess of 128 triggers per hart (which is less than 2^XLEN theoretical
maximum by the spec). All relevant callers (csr.c and debug.c) will now
use env->trigger_count instead of RV_MAX_TRIGGERS.
We're also adding two ways of setting the property:
- a '.trigger_count' CPU definition flag, allowing CPUs to set a custom
amount inside the CPU def in DEFINE_RISCV_CPU();
- a new 'trigger-count' user property. The user property has a higher
priority than an existing '.trigger_count' CPU def setting.
Assuming a hypotetical case where a CPU 'X' is defined with
'.trigger_count = 8':
- -cpu X,trigger-count=30 => trigger_count set to 30
- -cpu X (...) => trigger_count set to 8
For a CPU that doesn't set '.trigger_count':
- -cpu rv64,trigger-count=30 => trigger_count set to 30
- -cpu rv64 (...) => trigger_count set to 2
Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
target/riscv/cpu.c | 11 +++++++++++
target/riscv/cpu.h | 23 ++++++++++++++++++++++-
target/riscv/csr.c | 2 +-
target/riscv/debug.c | 16 ++++++++--------
target/riscv/machine.c | 4 ++--
target/riscv/tcg/tcg-cpu.c | 7 +++++++
6 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c1b10cc7ef..56dc9ad644 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1154,6 +1154,11 @@ static void riscv_cpu_init(Object *obj)
IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
qdev_init_gpio_in_named(DEVICE(cpu), riscv_cpu_set_nmi,
"riscv.cpu.rnmi", RNMI_MAX);
+
+ if (mcc->def->trigger_count) {
+ env->trigger_count = mcc->def->trigger_count;
+ }
+
#endif /* CONFIG_USER_ONLY */
general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
@@ -2606,6 +2611,8 @@ static const Property riscv_cpu_properties[] = {
DEFAULT_RNMI_IRQVEC),
DEFINE_PROP_UINT64("rnmi-exception-vector", RISCVCPU, env.rnmi_excpvec,
DEFAULT_RNMI_EXCPVEC),
+ DEFINE_PROP_UINT16("trigger-count", RISCVCPU, env.trigger_count,
+ RV_DEFAULT_TRIGGER_COUNT),
#endif
DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
@@ -2749,6 +2756,10 @@ static void riscv_cpu_class_base_init(ObjectClass *c, const void *data)
!valid_vm_1_10_32[mcc->def->cfg.max_satp_mode]) {
mcc->def->cfg.max_satp_mode = VM_1_10_SV32;
}
+
+ if (def->trigger_count) {
+ mcc->def->trigger_count = def->trigger_count;
+ }
#endif
}
if (def->priv_spec != RISCV_PROFILE_ATTR_UNUSED) {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c98652cd62..361f3cecf3 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -189,7 +189,26 @@ extern RISCVCPUImpliedExtsRule *riscv_multi_ext_implied_rules[];
#define RV_VLEN_MAX 1024
#define RV_MAX_MHPMEVENTS 32
#define RV_MAX_MHPMCOUNTERS 32
-#define RV_MAX_TRIGGERS 2
+
+/*
+ * The Debug 1.0 spec allows a humongous amount of triggers. Section
+ * "Enumeration" says: "The above algorithm reads back tselect so that
+ * implementations which have 2^n triggers only need to implement n
+ * bits of tselect.". tselect can have up to XLEN bits, so the max
+ * theoretical RV_MAX_TRIGGERS value is 2^XLEN.
+ *
+ * Alas, we need to carry over migration state for all these folks,
+ * even if not utilized, so setting 2^XLEN isn't an option.
+ *
+ * We're setting a smaller default based on what we see current
+ * hardware use (most chips uses 2-4 triggers per hart, RISC-V
+ * Server Ref requires at least 11).
+ *
+ * 128 seems like a good amount for current and future usage without
+ * a lot of footprint on our side.
+ */
+#define RV_MAX_TRIGGERS 128
+#define RV_DEFAULT_TRIGGER_COUNT 2
FIELD(VTYPE, VLMUL, 0, 3)
FIELD(VTYPE, VSEW, 3, 3)
@@ -475,6 +494,7 @@ struct CPUArchState {
QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
int64_t last_icount;
bool itrigger_enabled;
+ uint16_t trigger_count;
/* machine specific rdtime callback */
uint64_t (*rdtime_fn)(void *);
@@ -570,6 +590,7 @@ typedef struct RISCVCPUDef {
RISCVCPUConfig cfg;
bool bare;
const RISCVCSR *custom_csrs;
+ uint16_t trigger_count;
} RISCVCPUDef;
/**
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ec931a8c3d..6aa4a321c7 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->trigger_count && csrno == CSR_TDATA1) {
*val = 0;
return RISCV_EXCP_NONE;
}
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 30d39ee5cd..78b84591b4 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->trigger_count) {
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->trigger_count; 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->trigger_count; 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->trigger_count; 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->trigger_count; 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->trigger_count; i++) {
trigger_type = get_trigger_type(env, i);
if (!trigger_common_match(env, trigger_type, i)) {
@@ -1049,7 +1049,7 @@ void riscv_trigger_realize(CPURISCVState *env)
{
int i;
- for (i = 0; i < RV_MAX_TRIGGERS; i++) {
+ for (i = 0; i < env->trigger_count; i++) {
env->itrigger_timer[i] = timer_new_ns(QEMU_CLOCK_VIRTUAL,
riscv_itrigger_timer_cb, env);
}
@@ -1061,7 +1061,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->trigger_count; 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..a667c06b17 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -240,8 +240,8 @@ 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[]) {
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index b73e3e9dd4..9b657a7544 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1211,6 +1211,13 @@ void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
riscv_pmu_timer_cb, cpu);
}
}
+
+ if (env->trigger_count > RV_MAX_TRIGGERS) {
+ error_setg(errp,
+ "Invalid configuration: 'trigger-count' must be less than %u",
+ RV_MAX_TRIGGERS);
+ return;
+ }
#endif
}
--
2.43.0Hi Daniel,
On 9/6/26 23:39, Daniel Henrique Barboza wrote:
> All CPUs have the same amount of triggers: 2 triggers per hart, set via
> RV_MAX_TRIGGERS. This is not enough anymore: we'll have at least one
> future CPU that will demand more triggers per hart when implementing the
> RISC-V Server Ref Platform, requiring at least 11 triggers per hart.
>
> Parametrize the trigger amount using a new 'trigger_count' env property.
> The default amount is kept at 2. Set the new maximum to an educated
> guess of 128 triggers per hart (which is less than 2^XLEN theoretical
> maximum by the spec). All relevant callers (csr.c and debug.c) will now
> use env->trigger_count instead of RV_MAX_TRIGGERS.
>
> We're also adding two ways of setting the property:
>
> - a '.trigger_count' CPU definition flag, allowing CPUs to set a custom
> amount inside the CPU def in DEFINE_RISCV_CPU();
> - a new 'trigger-count' user property. The user property has a higher
> priority than an existing '.trigger_count' CPU def setting.
>
> Assuming a hypotetical case where a CPU 'X' is defined with
> '.trigger_count = 8':
>
> - -cpu X,trigger-count=30 => trigger_count set to 30
> - -cpu X (...) => trigger_count set to 8
>
> For a CPU that doesn't set '.trigger_count':
>
> - -cpu rv64,trigger-count=30 => trigger_count set to 30
> - -cpu rv64 (...) => trigger_count set to 2
>
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> ---
> target/riscv/cpu.c | 11 +++++++++++
> target/riscv/cpu.h | 23 ++++++++++++++++++++++-
> target/riscv/csr.c | 2 +-
> target/riscv/debug.c | 16 ++++++++--------
> target/riscv/machine.c | 4 ++--
> target/riscv/tcg/tcg-cpu.c | 7 +++++++
> 6 files changed, 51 insertions(+), 12 deletions(-)
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 6e70b145a5..a667c06b17 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -240,8 +240,8 @@ 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,
Since you bump the migration version, we can also change the array size
to only serialize the number of trigger used:
@@ -246,9 +246,9 @@ static const VMStateDescription vmstate_debug = {
.post_load = debug_post_load,
.fields = (const VMStateField[]) {
VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
- VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
- VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
- VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
+ VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
+ VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, env.trigger_count),
+ VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, env.trigger_count),
VMSTATE_END_OF_LIST()
}
};
Orthogonal, we'd need to replace these target_ulong at some point
(likely bumping the version number again).
On 6/10/2026 6:04 AM, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
>
> On 9/6/26 23:39, Daniel Henrique Barboza wrote:
>> All CPUs have the same amount of triggers: 2 triggers per hart, set via
>> RV_MAX_TRIGGERS. This is not enough anymore: we'll have at least one
>> future CPU that will demand more triggers per hart when implementing the
>> RISC-V Server Ref Platform, requiring at least 11 triggers per hart.
>>
>> Parametrize the trigger amount using a new 'trigger_count' env property.
>> The default amount is kept at 2. Set the new maximum to an educated
>> guess of 128 triggers per hart (which is less than 2^XLEN theoretical
>> maximum by the spec). All relevant callers (csr.c and debug.c) will now
>> use env->trigger_count instead of RV_MAX_TRIGGERS.
>>
>> We're also adding two ways of setting the property:
>>
>> - a '.trigger_count' CPU definition flag, allowing CPUs to set a custom
>> amount inside the CPU def in DEFINE_RISCV_CPU();
>> - a new 'trigger-count' user property. The user property has a higher
>> priority than an existing '.trigger_count' CPU def setting.
>>
>> Assuming a hypotetical case where a CPU 'X' is defined with
>> '.trigger_count = 8':
>>
>> - -cpu X,trigger-count=30 => trigger_count set to 30
>> - -cpu X (...) => trigger_count set to 8
>>
>> For a CPU that doesn't set '.trigger_count':
>>
>> - -cpu rv64,trigger-count=30 => trigger_count set to 30
>> - -cpu rv64 (...) => trigger_count set to 2
>>
>> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
>> ---
>> target/riscv/cpu.c | 11 +++++++++++
>> target/riscv/cpu.h | 23 ++++++++++++++++++++++-
>> target/riscv/csr.c | 2 +-
>> target/riscv/debug.c | 16 ++++++++--------
>> target/riscv/machine.c | 4 ++--
>> target/riscv/tcg/tcg-cpu.c | 7 +++++++
>> 6 files changed, 51 insertions(+), 12 deletions(-)
>
>
>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> index 6e70b145a5..a667c06b17 100644
>> --- a/target/riscv/machine.c
>> +++ b/target/riscv/machine.c
>> @@ -240,8 +240,8 @@ 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,
>
> Since you bump the migration version, we can also change the array size
> to only serialize the number of trigger used:
>
> @@ -246,9 +246,9 @@ static const VMStateDescription vmstate_debug = {
> .post_load = debug_post_load,
> .fields = (const VMStateField[]) {
> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
> - VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
> - VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
> - VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
> + VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
> + VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, env.trigger_count),
> + VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, env.trigger_count),
This doesn't seem to work:
.p/target_riscv_machine.c.o -c ../target/riscv/machine.c
In file included from /home/dbarboza/work/qemu/include/migration/cpu.h:7,
from ../target/riscv/machine.c:23:
../target/riscv/machine.c:250:52: error: ‘env’ undeclared here (not in a function)
250 | VMSTATE_UINT64_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
| ^~~
/home/dbarboza/work/qemu/include/migration/vmstate.h:427:20: note: in definition of macro ‘VMSTATE_ARRAY’
427 | .num = (_num), \
| ^~~~
/home/dbarboza/work/qemu/include/migration/vmstate.h:1228:5: note: in expansion of macro ‘VMSTATE_UINT64_ARRAY_V’
1228 | VMSTATE_UINT64_ARRAY_V(_f, _s, _n, 0)
| ^~~~~~~~~~~~~~~~~~~~~~
../target/riscv/machine.c:250:9: note: in expansion of macro ‘VMSTATE_UINT64_ARRAY’
250 | VMSTATE_UINT64_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
| ^~~~~~~~~~~~~~~~~~~~
[72/908] Compiling C object libqemu-riscv64-softmmu.a.p/target_riscv_translate.c.o
From what I'm gathering the macro accepts an actual number/value in '_num'. It can't
deal with a field type like env.trigger_count.
It seems like there's a "VARRAY" macro that is supposed to deal with variable
arrays, e.g. in riscv_aplic.c:
VMSTATE_VARRAY_UINT32(sourcecfg, RISCVAPLICState,
num_irqs, 0,
vmstate_info_uint32, uint32_t),
But, for some reason, we don't have VMSTATE_VARRAY_(INT64/UINT64), and the fields we're
dealing with happen to be 64 bits.
There's an alternative that ppc64 uses called VMSTATE_STRUCT_VARRAY_ALLOC(). It would be
used like this:
VMSTATE_STRUCT_VARRAY_ALLOC(env.tdata1, RISCVCPU,
env.trigger_count, 0,
vmstate_info_uint64, uint64_t),
But then we have another set of errors:
In file included from /home/dbarboza/work/qemu/include/migration/cpu.h:7,
from ../target/riscv/machine.c:23:
/home/dbarboza/work/qemu/include/migration/vmstate.h:736:19: error: initialization of ‘const VMStateDescription *’ from inco
mpatible pointer type ‘const VMStateInfo *’ [-Werror=incompatible-pointer-types]
736 | .vmsd = &(_vmsd), \ | ^
../target/riscv/machine.c:250:9: note: in expansion of macro ‘VMSTATE_STRUCT_VARRAY_ALLOC’
250 | VMSTATE_STRUCT_VARRAY_ALLOC(env.tdata1, RISCVCPU,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/dbarboza/work/qemu/include/migration/vmstate.h:736:19: note: (near initialization for ‘(anonymous)[2].vmsd’)
736 | .vmsd = &(_vmsd), \
| ^
../target/riscv/machine.c:250:9: note: in expansion of macro ‘VMSTATE_STRUCT_VARRAY_ALLOC’
250 | VMSTATE_STRUCT_VARRAY_ALLOC(env.tdata1, RISCVCPU,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/dbarboza/work/qemu/include/migration/vmstate.h:319:44: error: invalid operands to binary - (have ‘uint64_t **’ {aka ‘l
ong unsigned int **’} and ‘uint64_t (*)[128]’ {aka ‘long unsigned int (*)[128]’})
319 | #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0) | ^
(...)
It really seems to me that I need to turn the static arrays into pointers
and alloc them during init(). Then maybe we can use this macro to pass
env.trigger_count as a size.
I'll give it a go. If it works I'll add an extra patch converting stuff
first before introducing trigger_count.
Cheers,
Daniel
> VMSTATE_END_OF_LIST()
> }
> };
>
> Orthogonal, we'd need to replace these target_ulong at some point
> (likely bumping the version number again).
On 6/10/2026 9:28 AM, Daniel Henrique Barboza wrote:
>
>
> On 6/10/2026 6:04 AM, Philippe Mathieu-Daudé wrote:
>> Hi Daniel,
>>
>> On 9/6/26 23:39, Daniel Henrique Barboza wrote:
>>> All CPUs have the same amount of triggers: 2 triggers per hart, set via
>>> RV_MAX_TRIGGERS. This is not enough anymore: we'll have at least one
>>> future CPU that will demand more triggers per hart when implementing the
>>> RISC-V Server Ref Platform, requiring at least 11 triggers per hart.
>>>
>>> Parametrize the trigger amount using a new 'trigger_count' env property.
>>> The default amount is kept at 2. Set the new maximum to an educated
>>> guess of 128 triggers per hart (which is less than 2^XLEN theoretical
>>> maximum by the spec). All relevant callers (csr.c and debug.c) will now
>>> use env->trigger_count instead of RV_MAX_TRIGGERS.
>>>
>>> We're also adding two ways of setting the property:
>>>
>>> - a '.trigger_count' CPU definition flag, allowing CPUs to set a custom
>>> amount inside the CPU def in DEFINE_RISCV_CPU();
>>> - a new 'trigger-count' user property. The user property has a higher
>>> priority than an existing '.trigger_count' CPU def setting.
>>>
>>> Assuming a hypotetical case where a CPU 'X' is defined with
>>> '.trigger_count = 8':
>>>
>>> - -cpu X,trigger-count=30 => trigger_count set to 30
>>> - -cpu X (...) => trigger_count set to 8
>>>
>>> For a CPU that doesn't set '.trigger_count':
>>>
>>> - -cpu rv64,trigger-count=30 => trigger_count set to 30
>>> - -cpu rv64 (...) => trigger_count set to 2
>>>
>>> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
>>> ---
>>> target/riscv/cpu.c | 11 +++++++++++
>>> target/riscv/cpu.h | 23 ++++++++++++++++++++++-
>>> target/riscv/csr.c | 2 +-
>>> target/riscv/debug.c | 16 ++++++++--------
>>> target/riscv/machine.c | 4 ++--
>>> target/riscv/tcg/tcg-cpu.c | 7 +++++++
>>> 6 files changed, 51 insertions(+), 12 deletions(-)
>>
>>
>>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>>> index 6e70b145a5..a667c06b17 100644
>>> --- a/target/riscv/machine.c
>>> +++ b/target/riscv/machine.c
>>> @@ -240,8 +240,8 @@ 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,
>>
>> Since you bump the migration version, we can also change the array size
>> to only serialize the number of trigger used:
>>
>> @@ -246,9 +246,9 @@ static const VMStateDescription vmstate_debug = {
>> .post_load = debug_post_load,
>> .fields = (const VMStateField[]) {
>> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
>> - VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
>> - VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
>> - VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
>> + VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
>> + VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, env.trigger_count),
>> + VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, env.trigger_count),
>
> This doesn't seem to work:
>
>
> .p/target_riscv_machine.c.o -c ../target/riscv/machine.c
> In file included from /home/dbarboza/work/qemu/include/migration/cpu.h:7,
> from ../target/riscv/machine.c:23:
> ../target/riscv/machine.c:250:52: error: ‘env’ undeclared here (not in a function)
> 250 | VMSTATE_UINT64_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
> | ^~~
> /home/dbarboza/work/qemu/include/migration/vmstate.h:427:20: note: in definition of macro ‘VMSTATE_ARRAY’
> 427 | .num = (_num), \
> | ^~~~
> /home/dbarboza/work/qemu/include/migration/vmstate.h:1228:5: note: in expansion of macro ‘VMSTATE_UINT64_ARRAY_V’
> 1228 | VMSTATE_UINT64_ARRAY_V(_f, _s, _n, 0)
> | ^~~~~~~~~~~~~~~~~~~~~~
> ../target/riscv/machine.c:250:9: note: in expansion of macro ‘VMSTATE_UINT64_ARRAY’
> 250 | VMSTATE_UINT64_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
> | ^~~~~~~~~~~~~~~~~~~~
> [72/908] Compiling C object libqemu-riscv64-softmmu.a.p/target_riscv_translate.c.o
>
>
>
> From what I'm gathering the macro accepts an actual number/value in '_num'. It can't
> deal with a field type like env.trigger_count.
>
> It seems like there's a "VARRAY" macro that is supposed to deal with variable
> arrays, e.g. in riscv_aplic.c:
>
> VMSTATE_VARRAY_UINT32(sourcecfg, RISCVAPLICState,
> num_irqs, 0,
> vmstate_info_uint32, uint32_t),
>
>
> But, for some reason, we don't have VMSTATE_VARRAY_(INT64/UINT64), and the fields we're
> dealing with happen to be 64 bits.
So, quick update. VMSTATE_VARRAY_UINT32() doesn't mean "this is a variable length
array of uint32_t elements". This means "this is a variable length array, in
which 'length' is an uint32_t variable, and it has elements described as vmstateinfo
and type".
This means that changing 'trigger_count' to a supported type (we don't have
VMSTATE_VARRAY_UINT16()) and changing the static arrays to pointers make
VMSTATE_VARRAY_UINT32() work.
I'll see if I can put this in a preliminary patch. Thanks,
Daniel
>
> There's an alternative that ppc64 uses called VMSTATE_STRUCT_VARRAY_ALLOC(). It would be
> used like this:
>
>
> VMSTATE_STRUCT_VARRAY_ALLOC(env.tdata1, RISCVCPU,
> env.trigger_count, 0,
> vmstate_info_uint64, uint64_t),
>
>
> But then we have another set of errors:
>
>
> In file included from /home/dbarboza/work/qemu/include/migration/cpu.h:7,
> from ../target/riscv/machine.c:23:
> /home/dbarboza/work/qemu/include/migration/vmstate.h:736:19: error: initialization of ‘const VMStateDescription *’ from inco
> mpatible pointer type ‘const VMStateInfo *’ [-Werror=incompatible-pointer-types]
> 736 | .vmsd = &(_vmsd), \ | ^
> ../target/riscv/machine.c:250:9: note: in expansion of macro ‘VMSTATE_STRUCT_VARRAY_ALLOC’
> 250 | VMSTATE_STRUCT_VARRAY_ALLOC(env.tdata1, RISCVCPU,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/dbarboza/work/qemu/include/migration/vmstate.h:736:19: note: (near initialization for ‘(anonymous)[2].vmsd’)
> 736 | .vmsd = &(_vmsd), \
> | ^
> ../target/riscv/machine.c:250:9: note: in expansion of macro ‘VMSTATE_STRUCT_VARRAY_ALLOC’
> 250 | VMSTATE_STRUCT_VARRAY_ALLOC(env.tdata1, RISCVCPU,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/dbarboza/work/qemu/include/migration/vmstate.h:319:44: error: invalid operands to binary - (have ‘uint64_t **’ {aka ‘l
> ong unsigned int **’} and ‘uint64_t (*)[128]’ {aka ‘long unsigned int (*)[128]’})
> 319 | #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0) | ^
> (...)
>
>
> It really seems to me that I need to turn the static arrays into pointers
> and alloc them during init(). Then maybe we can use this macro to pass
> env.trigger_count as a size.
>
>
> I'll give it a go. If it works I'll add an extra patch converting stuff
> first before introducing trigger_count.
>
>
> Cheers,
> Daniel
>
>
>
>
>
>> VMSTATE_END_OF_LIST()
>> }
>> };
>>
>> Orthogonal, we'd need to replace these target_ulong at some point
>> (likely bumping the version number again).
>
On 6/10/2026 6:04 AM, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
>
> On 9/6/26 23:39, Daniel Henrique Barboza wrote:
>> All CPUs have the same amount of triggers: 2 triggers per hart, set via
>> RV_MAX_TRIGGERS. This is not enough anymore: we'll have at least one
>> future CPU that will demand more triggers per hart when implementing the
>> RISC-V Server Ref Platform, requiring at least 11 triggers per hart.
>>
>> Parametrize the trigger amount using a new 'trigger_count' env property.
>> The default amount is kept at 2. Set the new maximum to an educated
>> guess of 128 triggers per hart (which is less than 2^XLEN theoretical
>> maximum by the spec). All relevant callers (csr.c and debug.c) will now
>> use env->trigger_count instead of RV_MAX_TRIGGERS.
>>
>> We're also adding two ways of setting the property:
>>
>> - a '.trigger_count' CPU definition flag, allowing CPUs to set a custom
>> amount inside the CPU def in DEFINE_RISCV_CPU();
>> - a new 'trigger-count' user property. The user property has a higher
>> priority than an existing '.trigger_count' CPU def setting.
>>
>> Assuming a hypotetical case where a CPU 'X' is defined with
>> '.trigger_count = 8':
>>
>> - -cpu X,trigger-count=30 => trigger_count set to 30
>> - -cpu X (...) => trigger_count set to 8
>>
>> For a CPU that doesn't set '.trigger_count':
>>
>> - -cpu rv64,trigger-count=30 => trigger_count set to 30
>> - -cpu rv64 (...) => trigger_count set to 2
>>
>> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
>> ---
>> target/riscv/cpu.c | 11 +++++++++++
>> target/riscv/cpu.h | 23 ++++++++++++++++++++++-
>> target/riscv/csr.c | 2 +-
>> target/riscv/debug.c | 16 ++++++++--------
>> target/riscv/machine.c | 4 ++--
>> target/riscv/tcg/tcg-cpu.c | 7 +++++++
>> 6 files changed, 51 insertions(+), 12 deletions(-)
>
>
>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> index 6e70b145a5..a667c06b17 100644
>> --- a/target/riscv/machine.c
>> +++ b/target/riscv/machine.c
>> @@ -240,8 +240,8 @@ 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,
>
> Since you bump the migration version, we can also change the array size
> to only serialize the number of trigger used:
>
> @@ -246,9 +246,9 @@ static const VMStateDescription vmstate_debug = {
> .post_load = debug_post_load,
> .fields = (const VMStateField[]) {
> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
> - VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
> - VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
> - VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
> + VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
> + VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, env.trigger_count),
> + VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, env.trigger_count),
Good stuff. I searched a bit yesterday to see if there was a way of putting a
variable in the array size of this macro and figured this wasn't possible.
Didn't find any example after searching for like 2 minutes, then convinced
myself that the vmstate array should have a static size.
I'll send a v2.
> VMSTATE_END_OF_LIST()
> }
> };
>
> Orthogonal, we'd need to replace these target_ulong at some point
> (likely bumping the version number again).
I suppose we should do that in a separated patch that handles all ulongs
from all vmstate objects of this file. Then we can do a mass bump on
everything.
Thanks,
Daniel
On 10/6/26 11:37, Daniel Henrique Barboza wrote:
>
>
> On 6/10/2026 6:04 AM, Philippe Mathieu-Daudé wrote:
>> Hi Daniel,
>>
>> On 9/6/26 23:39, Daniel Henrique Barboza wrote:
>>> All CPUs have the same amount of triggers: 2 triggers per hart, set via
>>> RV_MAX_TRIGGERS. This is not enough anymore: we'll have at least one
>>> future CPU that will demand more triggers per hart when implementing the
>>> RISC-V Server Ref Platform, requiring at least 11 triggers per hart.
>>>
>>> Parametrize the trigger amount using a new 'trigger_count' env property.
>>> The default amount is kept at 2. Set the new maximum to an educated
>>> guess of 128 triggers per hart (which is less than 2^XLEN theoretical
>>> maximum by the spec). All relevant callers (csr.c and debug.c) will now
>>> use env->trigger_count instead of RV_MAX_TRIGGERS.
>>>
>>> We're also adding two ways of setting the property:
>>>
>>> - a '.trigger_count' CPU definition flag, allowing CPUs to set a custom
>>> amount inside the CPU def in DEFINE_RISCV_CPU();
>>> - a new 'trigger-count' user property. The user property has a higher
>>> priority than an existing '.trigger_count' CPU def setting.
>>>
>>> Assuming a hypotetical case where a CPU 'X' is defined with
>>> '.trigger_count = 8':
>>>
>>> - -cpu X,trigger-count=30 => trigger_count set to 30
>>> - -cpu X (...) => trigger_count set to 8
>>>
>>> For a CPU that doesn't set '.trigger_count':
>>>
>>> - -cpu rv64,trigger-count=30 => trigger_count set to 30
>>> - -cpu rv64 (...) => trigger_count set to 2
>>>
>>> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
>>> ---
>>> target/riscv/cpu.c | 11 +++++++++++
>>> target/riscv/cpu.h | 23 ++++++++++++++++++++++-
>>> target/riscv/csr.c | 2 +-
>>> target/riscv/debug.c | 16 ++++++++--------
>>> target/riscv/machine.c | 4 ++--
>>> target/riscv/tcg/tcg-cpu.c | 7 +++++++
>>> 6 files changed, 51 insertions(+), 12 deletions(-)
>>
>>
>>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>>> index 6e70b145a5..a667c06b17 100644
>>> --- a/target/riscv/machine.c
>>> +++ b/target/riscv/machine.c
>>> @@ -240,8 +240,8 @@ 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,
>>
>> Since you bump the migration version, we can also change the array size
>> to only serialize the number of trigger used:
>>
>> @@ -246,9 +246,9 @@ static const VMStateDescription vmstate_debug = {
>> .post_load = debug_post_load,
>> .fields = (const VMStateField[]) {
>> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
>> - VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
>> - VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
>> - VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
>> + VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
>> + VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, env.trigger_count),
>> + VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, env.trigger_count),
>
> Good stuff. I searched a bit yesterday to see if there was a way of
> putting a
> variable in the array size of this macro and figured this wasn't possible.
> Didn't find any example after searching for like 2 minutes, then convinced
> myself that the vmstate array should have a static size.
Same happened to me, unfortunately there is no documentation at all.
>
> I'll send a v2.
>
>
>
>> VMSTATE_END_OF_LIST()
>> }
>> };
>>
>> Orthogonal, we'd need to replace these target_ulong at some point
>> (likely bumping the version number again).
>
> I suppose we should do that in a separated patch that handles all ulongs
> from all vmstate objects of this file. Then we can do a mass bump on
> everything.
Sure :)
>
>
> Thanks,
> Daniel
>
>
On 6/10/2026 7:54 AM, Philippe Mathieu-Daudé wrote:
> On 10/6/26 11:37, Daniel Henrique Barboza wrote:
>>
>>
>> On 6/10/2026 6:04 AM, Philippe Mathieu-Daudé wrote:
>>> Hi Daniel,
>>>
>>> On 9/6/26 23:39, Daniel Henrique Barboza wrote:
>>>> All CPUs have the same amount of triggers: 2 triggers per hart, set via
>>>> RV_MAX_TRIGGERS. This is not enough anymore: we'll have at least one
>>>> future CPU that will demand more triggers per hart when implementing the
>>>> RISC-V Server Ref Platform, requiring at least 11 triggers per hart.
>>>>
>>>> Parametrize the trigger amount using a new 'trigger_count' env property.
>>>> The default amount is kept at 2. Set the new maximum to an educated
>>>> guess of 128 triggers per hart (which is less than 2^XLEN theoretical
>>>> maximum by the spec). All relevant callers (csr.c and debug.c) will now
>>>> use env->trigger_count instead of RV_MAX_TRIGGERS.
>>>>
>>>> We're also adding two ways of setting the property:
>>>>
>>>> - a '.trigger_count' CPU definition flag, allowing CPUs to set a custom
>>>> amount inside the CPU def in DEFINE_RISCV_CPU();
>>>> - a new 'trigger-count' user property. The user property has a higher
>>>> priority than an existing '.trigger_count' CPU def setting.
>>>>
>>>> Assuming a hypotetical case where a CPU 'X' is defined with
>>>> '.trigger_count = 8':
>>>>
>>>> - -cpu X,trigger-count=30 => trigger_count set to 30
>>>> - -cpu X (...) => trigger_count set to 8
>>>>
>>>> For a CPU that doesn't set '.trigger_count':
>>>>
>>>> - -cpu rv64,trigger-count=30 => trigger_count set to 30
>>>> - -cpu rv64 (...) => trigger_count set to 2
>>>>
>>>> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
>>>> ---
>>>> target/riscv/cpu.c | 11 +++++++++++
>>>> target/riscv/cpu.h | 23 ++++++++++++++++++++++-
>>>> target/riscv/csr.c | 2 +-
>>>> target/riscv/debug.c | 16 ++++++++--------
>>>> target/riscv/machine.c | 4 ++--
>>>> target/riscv/tcg/tcg-cpu.c | 7 +++++++
>>>> 6 files changed, 51 insertions(+), 12 deletions(-)
>>>
>>>
>>>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>>>> index 6e70b145a5..a667c06b17 100644
>>>> --- a/target/riscv/machine.c
>>>> +++ b/target/riscv/machine.c
>>>> @@ -240,8 +240,8 @@ 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,
>>>
>>> Since you bump the migration version, we can also change the array size
>>> to only serialize the number of trigger used:
>>>
>>> @@ -246,9 +246,9 @@ static const VMStateDescription vmstate_debug = {
>>> .post_load = debug_post_load,
>>> .fields = (const VMStateField[]) {
>>> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
>>> - VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
>>> - VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
>>> - VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
>>> + VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, env.trigger_count),
>>> + VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, env.trigger_count),
>>> + VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, env.trigger_count),
>>
>> Good stuff. I searched a bit yesterday to see if there was a way of putting a
>> variable in the array size of this macro and figured this wasn't possible.
>> Didn't find any example after searching for like 2 minutes, then convinced
>> myself that the vmstate array should have a static size.
>
> Same happened to me, unfortunately there is no documentation at all.
>
>>
>> I'll send a v2.
>>
>>
>>
>>> VMSTATE_END_OF_LIST()
>>> }
>>> };
>>>
>>> Orthogonal, we'd need to replace these target_ulong at some point
>>> (likely bumping the version number again).
>>
>> I suppose we should do that in a separated patch that handles all ulongs
>> from all vmstate objects of this file. Then we can do a mass bump on
>> everything.
>
> Sure :)
Btw, on Alistair's branch, this change was already made. This code is now:
.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_END_OF_LIST()
This was done by a patch from Anton Johansson in that branch:
"target/riscv: Fix size of trigger data
mcontext is at most 14 bits in size with the H extension, fix to 16
bits. trigger_cur indexes into tdata*[RV_MAX_TRIGGERS] which holds 2
elements, fix to 8 bits.
This patch also adds a migration entry for mcontext which is used in
tandem with other debug data that is already migrated.
Note, the cpu/debug VMSTATE version is bumped, breaking migration from
older versions."
Cheers,
Daniel
>
>>
>>
>> Thanks,
>> Daniel
>>
>>
>
On 10/6/26 12:59, Daniel Henrique Barboza wrote:
> On 6/10/2026 7:54 AM, Philippe Mathieu-Daudé wrote:
>> On 10/6/26 11:37, Daniel Henrique Barboza wrote:
>>> On 6/10/2026 6:04 AM, Philippe Mathieu-Daudé wrote:
>>>> Orthogonal, we'd need to replace these target_ulong at some point
>>>> (likely bumping the version number again).
>>>
>>> I suppose we should do that in a separated patch that handles all ulongs
>>> from all vmstate objects of this file. Then we can do a mass bump on
>>> everything.
>>
>> Sure :)
>
> Btw, on Alistair's branch, this change was already made. This code is now:
>
> .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_END_OF_LIST()
>
>
> This was done by a patch from Anton Johansson in that branch:
>
> "target/riscv: Fix size of trigger data
>
> mcontext is at most 14 bits in size with the H extension, fix to 16
> bits. trigger_cur indexes into tdata*[RV_MAX_TRIGGERS] which holds 2
> elements, fix to 8 bits.
>
> This patch also adds a migration entry for mcontext which is used in
> tandem with other debug data that is already migrated.
> Note, the cpu/debug VMSTATE version is bumped, breaking migration from
> older versions."
Ah great! Too many series to track, I forgot about this recent change.
© 2016 - 2026 Red Hat, Inc.