[PATCH 0/2] hppa CPU reset and speedup

[PATCH 0/2] hppa CPU reset and speedup

[deller]

From: Helge Deller <deller@gmx.de>

Add CPU reset function and speed up runtime and translataion.

Helge Deller (2):
  target/hppa: Add CPU reset method
  target/hppa: Speed up hppa_is_pa20()

 hw/hppa/machine.c |  6 +++---
 target/hppa/cpu.c | 27 +++++++++++++++++++++++++--
 target/hppa/cpu.h |  9 ++++++++-
 3 files changed, 36 insertions(+), 6 deletions(-)

-- 
2.47.0

[PATCH 1/2] target/hppa: Add CPU reset method

[deller]

From: Helge Deller <deller@gmx.de>

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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

V4:
- Drop initialization of exception_index in hppa_cpu_initfn()

V3:
- Call reset function from hppa_machine_reset() instead

V2:
- Add end_reset_fields marker
- call reset function in hppa_cpu_initfn()
---
 hw/hppa/machine.c |  6 +++---
 target/hppa/cpu.c | 26 ++++++++++++++++++++++++--
 target/hppa/cpu.h |  5 +++++
 3 files changed, 32 insertions(+), 5 deletions(-)

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..cb1b5191a4 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -194,11 +194,9 @@ static void hppa_cpu_realizefn(DeviceState *dev, Error **errp)
 
 static void hppa_cpu_initfn(Object *obj)
 {
-    CPUState *cs = CPU(obj);
     HPPACPU *cpu = HPPA_CPU(obj);
     CPUHPPAState *env = &cpu->env;
 
-    cs->exception_index = -1;
     cpu_hppa_loaded_fr0(env);
     cpu_hppa_put_psw(env, PSW_W);
 }
@@ -235,15 +233,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"
-- 
2.47.0

[PATCH 2/2] target/hppa: Speed up hppa_is_pa20()

[deller]

From: Helge Deller <deller@gmx.de>

Although the hppa_is_pa20() helper is costly due to string comparisons
in object_dynamic_cast(), it is called quite often during memory lookups
and at each start of a block of instruction translations.
Speed hppa_is_pa20() up by calling object_dynamic_cast() only once at
CPU creation and store the result in the is_pa20 of struct CPUArchState.

Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

v3:
- use !!object_dynamic_cast()
- typo fix in commit message
  (feedback by Philippe Mathieu-Daudé)

v2:
- moved init to hppa_cpu_initfn() and is_pa20 to end of CPUArchState struct
  (feedback by Richard Henderson)
---
 target/hppa/cpu.c | 1 +
 target/hppa/cpu.h | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index cb1b5191a4..64a762b410 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -197,6 +197,7 @@ static void hppa_cpu_initfn(Object *obj)
     HPPACPU *cpu = HPPA_CPU(obj);
     CPUHPPAState *env = &cpu->env;
 
+    env->is_pa20 = !!object_dynamic_cast(obj, TYPE_HPPA64_CPU);
     cpu_hppa_loaded_fr0(env);
     cpu_hppa_put_psw(env, PSW_W);
 }
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 32a674a8b8..288ce6d98a 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -266,6 +266,8 @@ typedef struct CPUArchState {
 
     /* Fields up to this point are cleared by a CPU reset */
     struct {} end_reset_fields;
+
+    bool is_pa20;
 } CPUHPPAState;
 
 /**
@@ -299,7 +301,7 @@ struct HPPACPUClass {
 
 static inline bool hppa_is_pa20(CPUHPPAState *env)
 {
-    return object_dynamic_cast(OBJECT(env_cpu(env)), TYPE_HPPA64_CPU) != NULL;
+    return env->is_pa20;
 }
 
 static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
-- 
2.47.0

Re: [PATCH 1/2] target/hppa: Add CPU reset method

[Richard Henderson]

On 12/29/24 15:41, deller@kernel.org wrote:
> From: Helge Deller<deller@gmx.de>
> 
> 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>
> Reviewed-by: Richard Henderson<richard.henderson@linaro.org>
> 
> V4:
> - Drop initialization of exception_index in hppa_cpu_initfn()
> 
> V3:
> - Call reset function from hppa_machine_reset() instead
> 
> V2:
> - Add end_reset_fields marker
> - call reset function in hppa_cpu_initfn()
> ---
>   hw/hppa/machine.c |  6 +++---
>   target/hppa/cpu.c | 26 ++++++++++++++++++++++++--
>   target/hppa/cpu.h |  5 +++++
>   3 files changed, 32 insertions(+), 5 deletions(-)

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

r~

Re: [PATCH 2/2] target/hppa: Speed up hppa_is_pa20()

[Richard Henderson]

On 12/29/24 15:41, deller@kernel.org wrote:
> From: Helge Deller<deller@gmx.de>
> 
> Although the hppa_is_pa20() helper is costly due to string comparisons
> in object_dynamic_cast(), it is called quite often during memory lookups
> and at each start of a block of instruction translations.
> Speed hppa_is_pa20() up by calling object_dynamic_cast() only once at
> CPU creation and store the result in the is_pa20 of struct CPUArchState.
> 
> Signed-off-by: Helge Deller<deller@gmx.de>
> Reviewed-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> 
> v3:
> - use !!object_dynamic_cast()
> - typo fix in commit message
>    (feedback by Philippe Mathieu-Daudé)
> 
> v2:
> - moved init to hppa_cpu_initfn() and is_pa20 to end of CPUArchState struct
>    (feedback by Richard Henderson)
> ---
>   target/hppa/cpu.c | 1 +
>   target/hppa/cpu.h | 4 +++-
>   2 files changed, 4 insertions(+), 1 deletion(-)

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

r~

Re: [PATCH 1/2] target/hppa: Add CPU reset method

[Philippe Mathieu-Daudé]

On 30/12/24 00:41, deller@kernel.org wrote:
> From: Helge Deller <deller@gmx.de>
> 
> 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> V4:
> - Drop initialization of exception_index in hppa_cpu_initfn()
> 
> V3:
> - Call reset function from hppa_machine_reset() instead
> 
> V2:
> - Add end_reset_fields marker
> - call reset function in hppa_cpu_initfn()
> ---
>   hw/hppa/machine.c |  6 +++---
>   target/hppa/cpu.c | 26 ++++++++++++++++++++++++--
>   target/hppa/cpu.h |  5 +++++
>   3 files changed, 32 insertions(+), 5 deletions(-)


> +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);

This ends calling cpu_common_reset_hold() ...

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

... which already sets these values. Why is that required?

> +}