Add the CPU reset method, which resets all CPU registers and the TLB to
zero. Then the CPU will switch to 32-bit mode (PSW_W bit is not set) and
start execution at address 0xf0000004.
Although we currently want to zero out all values in the CPUHPPAState
struct, add the end_reset_fields marker in case the state structs gets
extended with other variables later on which should not be reset.

Signed-off-by: Helge Deller <deller@gmx.de>

V3:
- Call reset function from hppa_machine_reset() instead

V2:
- Add end_reset_fields marker
- call reset function in hppa_cpu_initfn()

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index a31dc32a9f..05fd43ce9c 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -655,12 +655,12 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
     for (i = 0; i < smp_cpus; i++) {
         CPUState *cs = CPU(cpu[i]);
 
+        /* reset CPU */
+        resettable_reset(OBJECT(cs), RESET_TYPE_COLD);
+
         cpu_set_pc(cs, firmware_entry);
         cpu[i]->env.psw = PSW_Q;
         cpu[i]->env.gr[5] = CPU_HPA + i * 0x1000;
-
-        cs->exception_index = -1;
-        cs->halted = 0;
     }
 
     /* already initialized by machine_hppa_init()? */
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index c38439c180..b908cf65c6 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -235,15 +235,39 @@ static const TCGCPUOps hppa_tcg_ops = {
 #endif /* !CONFIG_USER_ONLY */
 };
 
+static void hppa_cpu_reset_hold(Object *obj, ResetType type)
+{
+    HPPACPU *cpu = HPPA_CPU(obj);
+    HPPACPUClass *scc = HPPA_CPU_GET_CLASS(cpu);
+    CPUHPPAState *env = &cpu->env;
+    CPUState *cs = CPU(cpu);
+
+    if (scc->parent_phases.hold) {
+        scc->parent_phases.hold(obj, type);
+    }
+
+    memset(env, 0, offsetof(CPUHPPAState, end_reset_fields));
+    cpu_set_pc(cs, 0xf0000004);
+    cpu_hppa_put_psw(env, hppa_is_pa20(env) ? PSW_W : 0);
+    cpu_hppa_loaded_fr0(env);
+
+    cs->exception_index = -1;
+    cs->halted = 0;
+}
+
 static void hppa_cpu_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
     CPUClass *cc = CPU_CLASS(oc);
     HPPACPUClass *acc = HPPA_CPU_CLASS(oc);
+    ResettableClass *rc = RESETTABLE_CLASS(oc);
 
     device_class_set_parent_realize(dc, hppa_cpu_realizefn,
                                     &acc->parent_realize);
 
+    resettable_class_set_parent_phases(rc, NULL, hppa_cpu_reset_hold, NULL,
+                                       &acc->parent_phases);
+
     cc->class_by_name = hppa_cpu_class_by_name;
     cc->has_work = hppa_cpu_has_work;
     cc->mmu_index = hppa_cpu_mmu_index;
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index e45ba50a59..32a674a8b8 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -263,6 +263,9 @@ typedef struct CPUArchState {
     IntervalTreeRoot tlb_root;
 
     HPPATLBEntry tlb[HPPA_TLB_ENTRIES];
+
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
 } CPUHPPAState;
 
 /**
@@ -281,6 +284,7 @@ struct ArchCPU {
 /**
  * HPPACPUClass:
  * @parent_realize: The parent class' realize handler.
+ * @parent_phases: The parent class' reset phase handlers.
  *
  * An HPPA CPU model.
  */
@@ -288,6 +292,7 @@ struct HPPACPUClass {
     CPUClass parent_class;
 
     DeviceRealize parent_realize;
+    ResettablePhases parent_phases;
 };
 
 #include "exec/cpu-all.h"

On 12/27/24 15:17, Helge Deller wrote:
> Add the CPU reset method, which resets all CPU registers and the TLB to
> zero. Then the CPU will switch to 32-bit mode (PSW_W bit is not set) and
> start execution at address 0xf0000004.
> Although we currently want to zero out all values in the CPUHPPAState
> struct, add the end_reset_fields marker in case the state structs gets
> extended with other variables later on which should not be reset.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> V3:
> - Call reset function from hppa_machine_reset() instead
> 
> V2:
> - Add end_reset_fields marker
> - call reset function in hppa_cpu_initfn()
...
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index c38439c180..b908cf65c6 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -235,15 +235,39 @@ static const TCGCPUOps hppa_tcg_ops = {
>   #endif /* !CONFIG_USER_ONLY */
>   };
>   
> +static void hppa_cpu_reset_hold(Object *obj, ResetType type)
> +{
> +    HPPACPU *cpu = HPPA_CPU(obj);
> +    HPPACPUClass *scc = HPPA_CPU_GET_CLASS(cpu);
> +    CPUHPPAState *env = &cpu->env;
> +    CPUState *cs = CPU(cpu);
> +
> +    if (scc->parent_phases.hold) {
> +        scc->parent_phases.hold(obj, type);
> +    }
> +
> +    memset(env, 0, offsetof(CPUHPPAState, end_reset_fields));
> +    cpu_set_pc(cs, 0xf0000004);
> +    cpu_hppa_put_psw(env, hppa_is_pa20(env) ? PSW_W : 0);
> +    cpu_hppa_loaded_fr0(env);
> +
> +    cs->exception_index = -1;
> +    cs->halted = 0;
> +}

There's also a set of exception_index in hppa_cpu_initfn.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

On 12/29/24 02:58, Richard Henderson wrote:
> On 12/27/24 15:17, Helge Deller wrote:
>> Add the CPU reset method, which resets all CPU registers and the TLB to
>> zero. Then the CPU will switch to 32-bit mode (PSW_W bit is not set) and
>> start execution at address 0xf0000004.
>> Although we currently want to zero out all values in the CPUHPPAState
>> struct, add the end_reset_fields marker in case the state structs gets
>> extended with other variables later on which should not be reset.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> V3:
>> - Call reset function from hppa_machine_reset() instead
>>
>> V2:
>> - Add end_reset_fields marker
>> - call reset function in hppa_cpu_initfn()
> ...
>> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
>> index c38439c180..b908cf65c6 100644
>> --- a/target/hppa/cpu.c
>> +++ b/target/hppa/cpu.c
>> @@ -235,15 +235,39 @@ static const TCGCPUOps hppa_tcg_ops = {
>>   #endif /* !CONFIG_USER_ONLY */
>>   };
>> +static void hppa_cpu_reset_hold(Object *obj, ResetType type)
>> +{
>> +    HPPACPU *cpu = HPPA_CPU(obj);
>> +    HPPACPUClass *scc = HPPA_CPU_GET_CLASS(cpu);
>> +    CPUHPPAState *env = &cpu->env;
>> +    CPUState *cs = CPU(cpu);
>> +
>> +    if (scc->parent_phases.hold) {
>> +        scc->parent_phases.hold(obj, type);
>> +    }
>> +
>> +    memset(env, 0, offsetof(CPUHPPAState, end_reset_fields));
>> +    cpu_set_pc(cs, 0xf0000004);
>> +    cpu_hppa_put_psw(env, hppa_is_pa20(env) ? PSW_W : 0);
>> +    cpu_hppa_loaded_fr0(env);
>> +
>> +    cs->exception_index = -1;
>> +    cs->halted = 0;
>> +}
>
> There's also a set of exception_index in hppa_cpu_initfn.

I don't quite understand...
Do you suggest I should remove the initialization of exception_index here,
or drop the one in hppa_cpu_initfn() ?

> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!
Helge

On 12/28/24 18:44, Helge Deller wrote:
> On 12/29/24 02:58, Richard Henderson wrote:
>> On 12/27/24 15:17, Helge Deller wrote:
>>> Add the CPU reset method, which resets all CPU registers and the TLB to
>>> zero. Then the CPU will switch to 32-bit mode (PSW_W bit is not set) and
>>> start execution at address 0xf0000004.
>>> Although we currently want to zero out all values in the CPUHPPAState
>>> struct, add the end_reset_fields marker in case the state structs gets
>>> extended with other variables later on which should not be reset.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>
>>> V3:
>>> - Call reset function from hppa_machine_reset() instead
>>>
>>> V2:
>>> - Add end_reset_fields marker
>>> - call reset function in hppa_cpu_initfn()
>> ...
>>> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
>>> index c38439c180..b908cf65c6 100644
>>> --- a/target/hppa/cpu.c
>>> +++ b/target/hppa/cpu.c
>>> @@ -235,15 +235,39 @@ static const TCGCPUOps hppa_tcg_ops = {
>>>   #endif /* !CONFIG_USER_ONLY */
>>>   };
>>> +static void hppa_cpu_reset_hold(Object *obj, ResetType type)
>>> +{
>>> +    HPPACPU *cpu = HPPA_CPU(obj);
>>> +    HPPACPUClass *scc = HPPA_CPU_GET_CLASS(cpu);
>>> +    CPUHPPAState *env = &cpu->env;
>>> +    CPUState *cs = CPU(cpu);
>>> +
>>> +    if (scc->parent_phases.hold) {
>>> +        scc->parent_phases.hold(obj, type);
>>> +    }
>>> +
>>> +    memset(env, 0, offsetof(CPUHPPAState, end_reset_fields));
>>> +    cpu_set_pc(cs, 0xf0000004);
>>> +    cpu_hppa_put_psw(env, hppa_is_pa20(env) ? PSW_W : 0);
>>> +    cpu_hppa_loaded_fr0(env);
>>> +
>>> +    cs->exception_index = -1;
>>> +    cs->halted = 0;
>>> +}
>>
>> There's also a set of exception_index in hppa_cpu_initfn.
> 
> I don't quite understand...
> Do you suggest I should remove the initialization of exception_index here,
> or drop the one in hppa_cpu_initfn() ?
I think the one in initfn is redundant.

r~