On Thu, 8 Jan 2026 at 16:34, Alex Bennée <[email protected]> wrote: > > Philippe Mathieu-Daudé <[email protected]> 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/[email protected]/ > >> 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/[email protected]/ > > 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
