[v2] target/riscv: Add support for a custom CPU arch state

[PATCH v2] target/riscv: Add support for a custom CPU arch state

Posted by stephensportia@gmail.com 3 days, 13 hours ago

From: Portia Stephens <portias@oss.tenstorrent.com>

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 <portias@oss.tenstorrent.com>
---
 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);
+    }
 #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;
 } RISCVCPUDef;
 
 /**
-- 
2.43.0

Re: [PATCH v2] target/riscv: Add support for a custom CPU arch state

Posted by Alistair 3 days, 13 hours ago

On Wed, 27 May 2026, at 1:03 PM, stephensportia@gmail.com wrote:
> From: Portia Stephens <portias@oss.tenstorrent.com>
> 
> 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 <portias@oss.tenstorrent.com>
> ---
> 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);
> +    }
> #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?

Alistair

> } RISCVCPUDef;
>  
> /**
> -- 
> 2.43.0
> 
> 
>

Re: [PATCH v2] target/riscv: Add support for a custom CPU arch state

Posted by Philippe Mathieu-Daudé 3 days, 10 hours ago

Hi,

On 27/5/26 05:41, Alistair wrote:
> On Wed, 27 May 2026, at 1:03 PM, stephensportia@gmail.com wrote:
>> From: Portia Stephens <portias@oss.tenstorrent.com>
>>
>> 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 <portias@oss.tenstorrent.com>
>> ---
>> 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.

>> +    }
>> #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.

> 
> Alistair
> 
>> } RISCVCPUDef;
>>   
>> /**
>> -- 
>> 2.43.0
>>
>>
>>
>

Re: [PATCH v2] target/riscv: Add support for a custom CPU arch state

Posted by Portia Stephens 2 days, 21 hours ago


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, stephensportia@gmail.com wrote:
>>> From: Portia Stephens <portias@oss.tenstorrent.com>
>>>
>>> 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 <portias@oss.tenstorrent.com>
>>> ---
>>> 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
>>>
>>>
>>>
>>
>

Re: [PATCH v2] target/riscv: Add support for a custom CPU arch state

Posted by Philippe Mathieu-Daudé 2 days, 20 hours ago

On 27/5/26 21:41, Portia Stephens wrote:
> 
> 
> 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, stephensportia@gmail.com wrote:
>>>> From: Portia Stephens <portias@oss.tenstorrent.com>
>>>>
>>>> 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 <portias@oss.tenstorrent.com>
>>>> ---
>>>> 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.

No; you introduce the RISCVCPUDef::custom_arch_state_init() handler
but discard the ResetType argument. Implementations could use it.

> 
>>
>>>> +    }
>>>> #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
>>>>
>>>>
>>>>
>>>
>>
>