We tend to apply cpu_reset inconsistently throughout our various
models which leads to unintended ordering dependencies. This got in
the way in my last plugins series:

  https://patchew.org/QEMU/20251219190849.238323-1-alex.bennee@linaro.org/

where I needed to shuffle things around to ensure that gdb register
creation was done after dependant peripherals had created their cpu
interfaces.

Regardless of that we do have a proper reset interface now and most
architectures have moved to it. This series attempts to clean-up the
remaining cases with proper qemu_register_reset() calls so reset is
called when we intend to.

Alex.

Alex Bennée (12):
  target/sh4: drop cpu_reset from realizefn
  target/m68k: introduce env->reset_pc
  hw/m68k: register a nextcube_cpu_reset handler
  hw/m68k: register a mcf5208evb_cpu_reset handler
  hw/m68k: register a an5206_cpu_reset handler
  hw/m68k: just use reset_pc for virt platform
  target/m68k: drop cpu_reset on realizefn
  hw/mips: defer finalising gcr_base until reset time
  hw/mips: drop cpu_reset in mips_cpu_realizefn
  target/tricore: move cpu_reset from tricore_cpu_realizefn
  target/arm: remove extraneous cpu_reset from realizefn
  include/hw: expand cpu_reset function docs

 include/hw/core/cpu.h |  3 +++
 target/m68k/cpu.h     |  1 +
 hw/m68k/an5206.c      | 24 +++++++++++++++++-------
 hw/m68k/mcf5208.c     | 25 +++++++++++++++++++------
 hw/m68k/next-cube.c   | 23 +++++++++++++++++------
 hw/m68k/virt.c        | 24 +++++++-----------------
 hw/mips/cps.c         | 22 +++++++++++++---------
 hw/misc/mips_cmgcr.c  |  1 -
 target/arm/cpu.c      |  1 -
 target/m68k/cpu.c     |  1 -
 target/mips/cpu.c     |  1 -
 target/sh4/cpu.c      |  1 -
 target/tricore/cpu.c  |  9 ++++++++-
 13 files changed, 85 insertions(+), 51 deletions(-)

-- 
2.47.3

On 8/1/26 15:34, Alex Bennée wrote:
> We tend to apply cpu_reset inconsistently throughout our various
> models which leads to unintended ordering dependencies. This got in
> the way in my last plugins series:
> 
>    https://patchew.org/QEMU/20251219190849.238323-1-alex.bennee@linaro.org/
> 
> where I needed to shuffle things around to ensure that gdb register
> creation was done after dependant peripherals had created their cpu
> interfaces.
> 
> Regardless of that we do have a proper reset interface now and most
> architectures have moved to it. This series attempts to clean-up the
> remaining cases with proper qemu_register_reset() calls so reset is
> called when we intend to.

Last time I was blocked at this comment:
https://lore.kernel.org/qemu-devel/20231128170008.57ddb03e@imammedo.users.ipa.redhat.com/

> Alex Bennée (12):
>    target/sh4: drop cpu_reset from realizefn
>    target/m68k: introduce env->reset_pc
>    hw/m68k: register a nextcube_cpu_reset handler
>    hw/m68k: register a mcf5208evb_cpu_reset handler
>    hw/m68k: register a an5206_cpu_reset handler
>    hw/m68k: just use reset_pc for virt platform
>    target/m68k: drop cpu_reset on realizefn
>    hw/mips: defer finalising gcr_base until reset time
>    hw/mips: drop cpu_reset in mips_cpu_realizefn
>    target/tricore: move cpu_reset from tricore_cpu_realizefn
>    target/arm: remove extraneous cpu_reset from realizefn
>    include/hw: expand cpu_reset function docs

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

> On 8/1/26 15:34, Alex Bennée wrote:
>> We tend to apply cpu_reset inconsistently throughout our various
>> models which leads to unintended ordering dependencies. This got in
>> the way in my last plugins series:
>>    https://patchew.org/QEMU/20251219190849.238323-1-alex.bennee@linaro.org/
>> where I needed to shuffle things around to ensure that gdb register
>> creation was done after dependant peripherals had created their cpu
>> interfaces.
>> Regardless of that we do have a proper reset interface now and most
>> architectures have moved to it. This series attempts to clean-up the
>> remaining cases with proper qemu_register_reset() calls so reset is
>> called when we intend to.
>
> Last time I was blocked at this comment:
> https://lore.kernel.org/qemu-devel/20231128170008.57ddb03e@imammedo.users.ipa.redhat.com/

From that:

 --cpu_reset()  <- brings cpu to known (after reset|poweron) state
   cpu_common_realizefn()
       if (dev->hotplugged) {                                                       
           cpu_synchronize_post_init(cpu);                                          
           cpu_resume(cpu);                                                         
       }
 ++cpu_reset()  <-- looks to be too late, we already told hypervisor to run undefined CPU, didn't we?

I would posit that the hotplug path is different as we online the CPU as
soon as its ready. Maybe that is just special cased as:

       if (dev->hotplugged) {
           cpu_reset(cpu);
           cpu_synchronize_post_init(cpu);                                          
           cpu_resume(cpu);                                                         
       }

Unless hotplug should also honour the reset tree in which case that
logic could be moved:

  void cpu_reset(CPUState *cpu)
  {
      DeviceState *dev = DEVICE(cpu);

      if (!dev->hotplugged) {
          device_cold_reset(DEVICE(cpu));
      } else {
          /* hotplugging implies we should know how to setup */
          cpu_synchronize_post_init(cpu);    
      }
      trace_cpu_reset(cpu->cpu_index);

  #ifdef CONFIG_TCG
      /*
       * Because vCPUs get "started" before the machine is ready we often
       * can't provide all the information plugins need during
       * cpu_common_initfn. However the vCPU will be reset a few times
       * before we eventually set things going giving plugins an
       * opportunity to update things.
       */
      qemu_plugin_vcpu_reset_hook(cpu);
  #endif
  }

Do we have test cases for hotplugging CPUs?

>
>> Alex Bennée (12):
>>    target/sh4: drop cpu_reset from realizefn
>>    target/m68k: introduce env->reset_pc
>>    hw/m68k: register a nextcube_cpu_reset handler
>>    hw/m68k: register a mcf5208evb_cpu_reset handler
>>    hw/m68k: register a an5206_cpu_reset handler
>>    hw/m68k: just use reset_pc for virt platform
>>    target/m68k: drop cpu_reset on realizefn
>>    hw/mips: defer finalising gcr_base until reset time
>>    hw/mips: drop cpu_reset in mips_cpu_realizefn
>>    target/tricore: move cpu_reset from tricore_cpu_realizefn
>>    target/arm: remove extraneous cpu_reset from realizefn
>>    include/hw: expand cpu_reset function docs

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On Thu, 8 Jan 2026 at 16:34, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > On 8/1/26 15:34, Alex Bennée wrote:
> >> We tend to apply cpu_reset inconsistently throughout our various
> >> models which leads to unintended ordering dependencies. This got in
> >> the way in my last plugins series:
> >>    https://patchew.org/QEMU/20251219190849.238323-1-alex.bennee@linaro.org/
> >> where I needed to shuffle things around to ensure that gdb register
> >> creation was done after dependant peripherals had created their cpu
> >> interfaces.
> >> Regardless of that we do have a proper reset interface now and most
> >> architectures have moved to it. This series attempts to clean-up the
> >> remaining cases with proper qemu_register_reset() calls so reset is
> >> called when we intend to.
> >
> > Last time I was blocked at this comment:
> > https://lore.kernel.org/qemu-devel/20231128170008.57ddb03e@imammedo.users.ipa.redhat.com/
>
> From that:
>
>  --cpu_reset()  <- brings cpu to known (after reset|poweron) state
>    cpu_common_realizefn()
>        if (dev->hotplugged) {
>            cpu_synchronize_post_init(cpu);
>            cpu_resume(cpu);
>        }
>  ++cpu_reset()  <-- looks to be too late, we already told hypervisor to run undefined CPU, didn't we?
>
> I would posit that the hotplug path is different as we online the CPU as
> soon as its ready. Maybe that is just special cased as:
>
>        if (dev->hotplugged) {
>            cpu_reset(cpu);
>            cpu_synchronize_post_init(cpu);
>            cpu_resume(cpu);
>        }
>
> Unless hotplug should also honour the reset tree in which case that
> logic could be moved:
>
>   void cpu_reset(CPUState *cpu)
>   {
>       DeviceState *dev = DEVICE(cpu);
>
>       if (!dev->hotplugged) {
>           device_cold_reset(DEVICE(cpu));
>       } else {
>           /* hotplugging implies we should know how to setup */
>           cpu_synchronize_post_init(cpu);
>       }
>       trace_cpu_reset(cpu->cpu_index);
>
>   #ifdef CONFIG_TCG
>       /*
>        * Because vCPUs get "started" before the machine is ready we often
>        * can't provide all the information plugins need during
>        * cpu_common_initfn. However the vCPU will be reset a few times
>        * before we eventually set things going giving plugins an
>        * opportunity to update things.
>        */
>       qemu_plugin_vcpu_reset_hook(cpu);
>   #endif
>   }

You don't want to special case anything in the cpu_reset()
function, because it is valid (indeed, encouraged :-)) to
reset CPU objects in ways that don't pass through it.
For instance, qemu_register_resettable(OBJECT(cpu)) is one way.

The problem with cpu_reset() is that it is not 3-phase aware,
so (like everything that calls device_cold_reset()) it will
call all 3 reset phases for the CPU at once, even in the middle
of doing a full-system 3-phase reset. It's fine for the
special case of "we are resetting nothing else except the CPU",
but there's a lot of places where we aren't using it like that.

thanks
-- PMM