The CPUState::halted field is always re-initialized in
cpu_common_reset_hold(), itself called by cpu_reset().
No need to have targets manually initializing it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/misc/mips_cpc.c        | 1 -
 hw/ppc/e500.c             | 1 -
 target/arm/arm-powerctl.c | 2 --
 target/hppa/cpu.c         | 1 -
 4 files changed, 5 deletions(-)

diff --git a/hw/misc/mips_cpc.c b/hw/misc/mips_cpc.c
index 772b8c0017d..4ec8226c416 100644
--- a/hw/misc/mips_cpc.c
+++ b/hw/misc/mips_cpc.c
@@ -38,7 +38,6 @@ static void mips_cpu_reset_async_work(CPUState *cs, run_on_cpu_data data)
     MIPSCPCState *cpc = (MIPSCPCState *) data.host_ptr;
 
     cpu_reset(cs);
-    cs->halted = 0;
     cpc->vp_running |= 1ULL << cs->cpu_index;
 }
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 4551157c011..8b90b5b2448 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -785,7 +785,6 @@ static void ppce500_cpu_reset(void *opaque)
     cpu_reset(cs);
 
     /* Set initial guest state. */
-    cs->halted = 0;
     env->gpr[1] = (16 * MiB) - 8;
     env->gpr[3] = bi->dt_base;
     env->gpr[4] = 0;
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index 20c70c7d6bb..8e948171c7c 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -67,7 +67,6 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
     /* Initialize the cpu we are turning on */
     cpu_reset(target_cpu_state);
     arm_emulate_firmware_reset(target_cpu_state, info->target_el);
-    target_cpu_state->halted = 0;
 
     /* We check if the started CPU is now at the correct level */
     assert(info->target_el == arm_current_el(&target_cpu->env));
@@ -194,7 +193,6 @@ static void arm_set_cpu_on_and_reset_async_work(CPUState *target_cpu_state,
 
     /* Initialize the cpu we are turning on */
     cpu_reset(target_cpu_state);
-    target_cpu_state->halted = 0;
 
     /* Finally set the power status */
     assert(bql_locked());
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 9b355bfe902..b4092037888 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -203,7 +203,6 @@ static void hppa_cpu_reset_hold(Object *obj, ResetType type)
     if (scc->parent_phases.hold) {
         scc->parent_phases.hold(obj, type);
     }
-    cs->halted = 0;
     cpu_set_pc(cs, 0xf0000004);
 
     memset(env, 0, offsetof(CPUHPPAState, end_reset_fields));
-- 
2.47.1

On 12/30/24 07:39, Philippe Mathieu-Daudé wrote:
> The CPUState::halted field is always re-initialized in
> cpu_common_reset_hold(), itself called by cpu_reset().
> No need to have targets manually initializing it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/misc/mips_cpc.c        | 1 -
>   hw/ppc/e500.c             | 1 -
>   target/arm/arm-powerctl.c | 2 --
>   target/hppa/cpu.c         | 1 -
>   4 files changed, 5 deletions(-)

This is a behavior change if cpu->start_powered_off.

I think it's likely that "start powered off' should not apply to warm reset, and that 
otherwise this change is ok.  But I think there's that nit to address first.


r~

Am 30. Dezember 2024 15:39:29 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>The CPUState::halted field is always re-initialized in
>cpu_common_reset_hold(), itself called by cpu_reset().
>No need to have targets manually initializing it.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/misc/mips_cpc.c        | 1 -
> hw/ppc/e500.c             | 1 -
> target/arm/arm-powerctl.c | 2 --
> target/hppa/cpu.c         | 1 -
> 4 files changed, 5 deletions(-)
>
>diff --git a/hw/misc/mips_cpc.c b/hw/misc/mips_cpc.c
>index 772b8c0017d..4ec8226c416 100644
>--- a/hw/misc/mips_cpc.c
>+++ b/hw/misc/mips_cpc.c
>@@ -38,7 +38,6 @@ static void mips_cpu_reset_async_work(CPUState *cs, run_on_cpu_data data)
>     MIPSCPCState *cpc = (MIPSCPCState *) data.host_ptr;
> 
>     cpu_reset(cs);
>-    cs->halted = 0;
>     cpc->vp_running |= 1ULL << cs->cpu_index;
> }
> 
>diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>index 4551157c011..8b90b5b2448 100644
>--- a/hw/ppc/e500.c
>+++ b/hw/ppc/e500.c
>@@ -785,7 +785,6 @@ static void ppce500_cpu_reset(void *opaque)
>     cpu_reset(cs);
> 
>     /* Set initial guest state. */
>-    cs->halted = 0;
>     env->gpr[1] = (16 * MiB) - 8;
>     env->gpr[3] = bi->dt_base;
>     env->gpr[4] = 0;

For e500:

Acked-by: Bernhard Beschow <shentey@gmail.com>

>diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
>index 20c70c7d6bb..8e948171c7c 100644
>--- a/target/arm/arm-powerctl.c
>+++ b/target/arm/arm-powerctl.c
>@@ -67,7 +67,6 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
>     /* Initialize the cpu we are turning on */
>     cpu_reset(target_cpu_state);
>     arm_emulate_firmware_reset(target_cpu_state, info->target_el);
>-    target_cpu_state->halted = 0;
> 
>     /* We check if the started CPU is now at the correct level */
>     assert(info->target_el == arm_current_el(&target_cpu->env));
>@@ -194,7 +193,6 @@ static void arm_set_cpu_on_and_reset_async_work(CPUState *target_cpu_state,
> 
>     /* Initialize the cpu we are turning on */
>     cpu_reset(target_cpu_state);
>-    target_cpu_state->halted = 0;
> 
>     /* Finally set the power status */
>     assert(bql_locked());
>diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
>index 9b355bfe902..b4092037888 100644
>--- a/target/hppa/cpu.c
>+++ b/target/hppa/cpu.c
>@@ -203,7 +203,6 @@ static void hppa_cpu_reset_hold(Object *obj, ResetType type)
>     if (scc->parent_phases.hold) {
>         scc->parent_phases.hold(obj, type);
>     }
>-    cs->halted = 0;
>     cpu_set_pc(cs, 0xf0000004);
> 
>     memset(env, 0, offsetof(CPUHPPAState, end_reset_fields));