On Tue, 20 Feb 2024 at 19:06, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > On 20/2/24 17:06, Peter Maydell wrote: > > Move the reset of the sysbus (and thus all devices and buses anywhere > > on the qbus tree) from qemu_register_reset() to qemu_register_resettable(). > > > > This is a behaviour change: because qemu_register_resettable() is > > aware of three-phase reset, this now means that: > > * 'enter' phase reset methods of devices and buses are called > > before any legacy reset callbacks registered with qemu_register_reset() > > * 'exit' phase reset methods of devices and buses are called > > after any legacy qemu_register_reset() callbacks > > > > Put another way, a qemu_register_reset() callback is now correctly > > ordered in the 'hold' phase along with any other 'hold' phase methods. > > > > The motivation for doing this is that we will now be able to resolve > > some reset-ordering issues using the three-phase mechanism, because > > the 'exit' phase is always after the 'hold' phase, even when the > > 'hold' phase function was registered with qemu_register_reset(). > > > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > --- > > I believe that given we don't make much use of enter/exit phases > > currently that this is unlikely to cause unexpected regressions due > > to an accidental reset-order dependency that is no longer satisfied, > > but it's always possible... > > --- > > hw/core/machine.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c > > index fb5afdcae4c..9ac5d5389a6 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -1577,14 +1577,13 @@ void qdev_machine_creation_done(void) > > /* TODO: once all bus devices are qdevified, this should be done > > * when bus is created by qdev.c */ > > /* > > - * TODO: If we had a main 'reset container' that the whole system > > - * lived in, we could reset that using the multi-phase reset > > - * APIs. For the moment, we just reset the sysbus, which will cause > > + * This is where we arrange for the sysbus to be reset when the > > + * whole simulation is reset. In turn, resetting the sysbus will cause > > * all devices hanging off it (and all their child buses, recursively) > > * to be reset. Note that this will *not* reset any Device objects > > * which are not attached to some part of the qbus tree! > > */ > > - qemu_register_reset(resettable_cold_reset_fn, sysbus_get_default()); > > Interestingly after this patch TYPE_S390_IPL is the last device > using resettable_cold_reset_fn(). Per commit cd45c506c8e: > > /* > * Because this Device is not on any bus in the qbus tree (it is > * not a sysbus device and it's not on some other bus like a PCI > * bus) it will not be automatically reset by the 'reset the > * sysbus' hook registered by vl.c like most devices. So we must > * manually register a reset hook for it. > * TODO: there should be a better way to do this. > */
Mmm, we could now have that s390 code call qemu_register_resettable(OBJECT(dev)). Though the "better way" remark still applies, because ideally we shouldn't be doing reset only via the qbus tree. thanks -- PMM
