Re: [PATCH 3/3] misc: ppc/spapr: replace qemu_system_reset_request() calls with watchdog_perform_action()

[Peter Maydell]

On Fri, 16 Feb 2024 at 13:56, Abhiram Tilak <atp.exp@gmail.com> wrote:
>
> A few watchdog devices use qemu_system_reset_request(). This is not ideal 
> since
> behaviour of watchdog-expiry can't be changed by QMP using `watchdog_action`.
> As stated in BiteSizedTasks wiki page, instead of using 
> qemu_system_reset_request()
> to reset when a watchdog timer expires, let watchdog_perform_action() decide
> what to do.
>
> I am unsure about the changes in `spapr_watchdog.c` in patch 3, it would be 
> great
> if any of the maintainers review it.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2124
> Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
> ---
>  hw/watchdog/spapr_watchdog.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/watchdog/spapr_watchdog.c b/hw/watchdog/spapr_watchdog.c
> index 2bb1d3c532..9751b19506 100644
> --- a/hw/watchdog/spapr_watchdog.c
> +++ b/hw/watchdog/spapr_watchdog.c
> @@ -18,6 +18,7 @@
>  #include "target/ppc/cpu.h"
>  #include "migration/vmstate.h"
>  #include "trace.h"
> +#include "sysemu/watchdog.h"
>
>  #include "hw/ppc/spapr.h"
>
> @@ -114,7 +115,7 @@ static void watchdog_expired(void *pw)
>          qemu_system_vmstop_request(RUN_STATE_SHUTDOWN);
>          break;
>      case PSERIES_WDTF_ACTION_HARD_RESTART:
> -        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> +        watchdog_perform_action();
>          break;
>      case PSERIES_WDTF_ACTION_DUMP_RESTART:
>          CPU_FOREACH(cs) {

This one is more complicated, because the spapr watchdog
has multiple possible behaviours which the guest can ask for.

We had a discussion on the mailing list about this a little while back:
https://lore.kernel.org/qemu-devel/CAFEAcA_KjSgt-oC=d2m6WAdqoRsUcs1W_ji7Ng2fgVjxAWLZEw@mail.gmail.com/

The conclusion was that the watchdog-behaviour QAPI API
needs to be enhanced to be able to handle this kind of
"the guest picks an action" watchdog, so that the user can
either override the guest's choice, or request that the
behaviour be what the guest wants it to be.

thanks
-- PMM