Re: [PATCH 09/10] hw/core/machine: Use qemu_register_resettable for sysbus reset

[Philippe Mathieu-Daudé]

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.maydell@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.
     */

> +    qemu_register_resettable(OBJECT(sysbus_get_default()));
>   
>       notifier_list_notify(&machine_init_done_notifiers, NULL);
>