[PATCH 1/2] hw/core: Dump cpu_reset in the reset.exit phase

Richard Henderson posted 2 patches 1 month ago
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>
[PATCH 1/2] hw/core: Dump cpu_reset in the reset.exit phase
Posted by Richard Henderson 1 month ago
During reset.hold, the cpu is in an inconsistent state,
where the leaf class has not had a chance to initialize
state at all.

This is visible as a SIGSEGV in "qemu-system-sparc64 -d cpu_reset".

Move the dump to the exit phase, where all initialization
is certain to be complete.

Reported-by: Henk van der Laak <henk@laaksoft.nl>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/core/cpu-common.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 39e674aca2..26321be785 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -119,11 +119,6 @@ static void cpu_common_reset_hold(Object *obj, ResetType type)
 {
     CPUState *cpu = CPU(obj);
 
-    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
-        qemu_log("CPU Reset (CPU %d)\n", cpu->cpu_index);
-        log_cpu_state(cpu, cpu->cc->reset_dump_flags);
-    }
-
     cpu->interrupt_request = 0;
     cpu->halted = cpu->start_powered_off;
     cpu->mem_io_pc = 0;
@@ -137,6 +132,16 @@ static void cpu_common_reset_hold(Object *obj, ResetType type)
     cpu_exec_reset_hold(cpu);
 }
 
+static void cpu_common_reset_exit(Object *obj, ResetType type)
+{
+    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
+        CPUState *cpu = CPU(obj);
+
+        qemu_log("CPU Reset (CPU %d)\n", cpu->cpu_index);
+        log_cpu_state(cpu, cpu->cc->reset_dump_flags);
+    }
+}
+
 ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
 {
     ObjectClass *oc;
@@ -380,6 +385,7 @@ static void cpu_common_class_init(ObjectClass *klass, const void *data)
     dc->realize = cpu_common_realizefn;
     dc->unrealize = cpu_common_unrealizefn;
     rc->phases.hold = cpu_common_reset_hold;
+    rc->phases.exit = cpu_common_reset_exit;
     cpu_class_init_props(dc);
     /*
      * Reason: CPUs still need special care by board code: wiring up
-- 
2.43.0
Re: [PATCH 1/2] hw/core: Dump cpu_reset in the reset.exit phase
Posted by Peter Maydell 1 month ago
On Wed, 27 Aug 2025 at 06:39, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> During reset.hold, the cpu is in an inconsistent state,
> where the leaf class has not had a chance to initialize
> state at all.
>
> This is visible as a SIGSEGV in "qemu-system-sparc64 -d cpu_reset".
>
> Move the dump to the exit phase, where all initialization
> is certain to be complete.
>
> Reported-by: Henk van der Laak <henk@laaksoft.nl>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/core/cpu-common.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index 39e674aca2..26321be785 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -119,11 +119,6 @@ static void cpu_common_reset_hold(Object *obj, ResetType type)
>  {
>      CPUState *cpu = CPU(obj);
>
> -    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
> -        qemu_log("CPU Reset (CPU %d)\n", cpu->cpu_index);
> -        log_cpu_state(cpu, cpu->cc->reset_dump_flags);
> -    }
> -
>      cpu->interrupt_request = 0;
>      cpu->halted = cpu->start_powered_off;
>      cpu->mem_io_pc = 0;
> @@ -137,6 +132,16 @@ static void cpu_common_reset_hold(Object *obj, ResetType type)
>      cpu_exec_reset_hold(cpu);
>  }
>
> +static void cpu_common_reset_exit(Object *obj, ResetType type)
> +{
> +    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
> +        CPUState *cpu = CPU(obj);
> +
> +        qemu_log("CPU Reset (CPU %d)\n", cpu->cpu_index);
> +        log_cpu_state(cpu, cpu->cc->reset_dump_flags);
> +    }
> +}
> +
>  ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>  {
>      ObjectClass *oc;
> @@ -380,6 +385,7 @@ static void cpu_common_class_init(ObjectClass *klass, const void *data)
>      dc->realize = cpu_common_realizefn;
>      dc->unrealize = cpu_common_unrealizefn;
>      rc->phases.hold = cpu_common_reset_hold;
> +    rc->phases.exit = cpu_common_reset_exit;
>      cpu_class_init_props(dc);
>      /*
>       * Reason: CPUs still need special care by board code: wiring up

If we ever have CPUs that actually update their state in
the reset exit phase (e.g. if we manage to complete the refactoring
that would let us implement M-profile "load starting PC and SP
from memory" in reset-exit after rom blob loading rather than
having a hack to do it in reset-hold), this won't capture that.
But it's clearly better than trying to do it in the common
reset-hold method...

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Re: [PATCH 1/2] hw/core: Dump cpu_reset in the reset.exit phase
Posted by Philippe Mathieu-Daudé 1 month ago
On 28/8/25 17:24, Peter Maydell wrote:
> On Wed, 27 Aug 2025 at 06:39, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> During reset.hold, the cpu is in an inconsistent state,
>> where the leaf class has not had a chance to initialize
>> state at all.
>>
>> This is visible as a SIGSEGV in "qemu-system-sparc64 -d cpu_reset".
>>
>> Move the dump to the exit phase, where all initialization
>> is certain to be complete.
>>
>> Reported-by: Henk van der Laak <henk@laaksoft.nl>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   hw/core/cpu-common.c | 16 +++++++++++-----
>>   1 file changed, 11 insertions(+), 5 deletions(-)


>> @@ -380,6 +385,7 @@ static void cpu_common_class_init(ObjectClass *klass, const void *data)
>>       dc->realize = cpu_common_realizefn;
>>       dc->unrealize = cpu_common_unrealizefn;
>>       rc->phases.hold = cpu_common_reset_hold;
>> +    rc->phases.exit = cpu_common_reset_exit;
>>       cpu_class_init_props(dc);
>>       /*
>>        * Reason: CPUs still need special care by board code: wiring up
> 
> If we ever have CPUs that actually update their state in
> the reset exit phase (e.g. if we manage to complete the refactoring
> that would let us implement M-profile "load starting PC and SP
> from memory" in reset-exit after rom blob loading rather than
> having a hack to do it in reset-hold), this won't capture that.
> But it's clearly better than trying to do it in the common
> reset-hold method...
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>