On 27/5/2026 3:58 PM, Philippe Mathieu-Daudé wrote:
Hi,
On 27/5/26 05:41, Alistair wrote:
On Wed, 27 May 2026, at 1:03 PM, [email protected] wrote:
From: Portia Stephens <[email protected]>
Custom vendor CSR implementations may require custom state information.
This adds a custom_arch-state struct to the CPUArchState. It also adds a
RISCVCPUDef function pointer for handling allocation and initialization
of the custom_arch_state as well as a reset function pointer for setting
the custom_arch_state fields to known values on reset.
Signed-off-by: Portia Stephens <[email protected]>
---
target/riscv/cpu.c | 15 +++++++++++++++
target/riscv/cpu.h | 7 +++++++
2 files changed, 22 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 862834b480..ca557d27b5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -819,6 +819,10 @@ static void riscv_cpu_reset_hold(Object *obj,
ResetType type)
if (kvm_enabled()) {
kvm_riscv_reset_vcpu(cpu);
}
+
+ if (mcc->def->custom_arch_state_reset) {
+ mcc->def->custom_arch_state_reset(env);
ResetType shouldn't be discarded IMO.
Are you referring to Alistair's comment about why do we need reset? Yes
it is necessary to set the registers to a known state after a reset. A
finalize function is probably also needed since init will allocate the
struct.
+ }
#endif
}
@@ -1167,6 +1171,9 @@ static void riscv_cpu_init(Object *obj)
if (mcc->def->custom_csrs) {
riscv_register_custom_csrs(cpu, mcc->def->custom_csrs);
}
+ if (mcc->def->custom_arch_state_init) {
+ mcc->def->custom_arch_state_init(&cpu->env);
+ }
#endif
accel_cpu_instance_init(CPU(obj));
@@ -2706,6 +2713,14 @@ static void
riscv_cpu_class_base_init(ObjectClass *c, const void *data)
assert(!mcc->def->custom_csrs);
mcc->def->custom_csrs = def->custom_csrs;
}
+ if (def->custom_arch_state_init) {
+ assert(!mcc->def->custom_arch_state_init);
+ mcc->def->custom_arch_state_init = def-
>custom_arch_state_init;
+ }
+ if (def->custom_arch_state_reset) {
+ assert(!mcc->def->custom_arch_state_reset);
+ mcc->def->custom_arch_state_reset = def-
>custom_arch_state_reset;
+ }
}
if (!object_class_is_abstract(c)) {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7d79c7a5a7..8132c65012 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -213,6 +213,9 @@ typedef struct PMUFixedCtrState {
uint64_t counter_virt_prev[2];
} PMUFixedCtrState;
+typedef void (*riscv_csr_custom_init_fn)(CPURISCVState *env);
+typedef void (*riscv_csr_custom_reset_fn)(CPURISCVState *env);
+
struct CPUArchState {
target_ulong gpr[32];
target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
@@ -509,6 +512,8 @@ struct CPUArchState {
uint64_t rnmip;
uint64_t rnmi_irqvec;
uint64_t rnmi_excpvec;
+
+ const void *custom_arch_state;
};
/*
@@ -561,6 +566,8 @@ typedef struct RISCVCPUDef {
RISCVCPUConfig cfg;
bool bare;
const RISCVCSR *custom_csrs;
+ riscv_csr_custom_init_fn custom_arch_state_init;
+ riscv_csr_custom_reset_fn custom_arch_state_reset;
Why do we need an init and a reset? I assume init is to allocate the
stuct, but that should probably be commented here.
How do you expect these to be set? Do you have an example?
I don't see the point of upstreaming this patch without concrete
example. So far it is maintenance burden without any value.
Okay.
Alistair
} RISCVCPUDef;
/**
--
2.43.0
