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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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());
> +    qemu_register_resettable(OBJECT(sysbus_get_default()));

Correct, so:

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

But I'd rather move that call to main_system_bus_create() in
hw/core/sysbus.c, as it doesn't seem to be related to the
machine creation phases anymore. Maybe clearer to do in a
separate patch although.

>       notifier_list_notify(&machine_init_done_notifiers, NULL);
>