[PATCH] vfio/migration: Don't reset a device on a declined stop transition

[Pankaj Kumar <pshit@nvidia.com>]

QEMU resets a VFIO device whenever a migration state transition it requests
fails. This is done by vfio_migration_set_state_or_reset(), which passes
VFIO_DEVICE_STATE_ERROR as the recover state, forcing an immediate
VFIO_DEVICE_RESET.

However, a device may legitimately decline a state transition, e.g. a vGPU
that refuses to stop while a profiling/debug session is active in the guest.
Such a device is still healthy and running the guest, so resetting it destroys
a perfectly good VM.

Instead of resetting on a declined stop, recover the device to its current
state. The current device state is always a valid, already-reached state, so
recovering to it is effectively a no-op transition that cannot itself fail and
trigger a reset:

 - vfio_vmstate_change() and vfio_vmstate_change_prepare() recover to the
   current device state for the stop direction. The resume direction keeps
   ERROR as the recover state, since a device that cannot resume is genuinely
   broken.

 - vfio_save_complete_precopy() recovers to the current device state rather
   than STOP. If it runs after an earlier stop was declined (the device still
   in a running state), the stop-copy attempt is then rolled back instead of
   resetting the device.

This lets vendor drivers decline a stop as a matter of policy without losing
the guest, and lets such drivers drop their own workarounds that swallowed the
spurious reset.

Signed-off-by: Pankaj Kumar <pshit@nvidia.com>
Reviewed-by: Avihai Horon <avihaih@nvidia.com>
---
 hw/vfio/migration.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 73c49d8c24..4a98afa7c2 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -771,9 +771,16 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
 
     trace_vfio_save_complete_precopy_start(vbasedev->name);
 
-    /* We reach here with device state STOP or STOP_COPY only */
+    /*
+     * We normally reach here with the device in STOP or STOP_COPY. However, if
+     * an earlier stop transition was declined by the device, it may still be in
+     * a running state. Recover to the current device state rather than STOP, so
+     * a device that declines the stop-copy transition is rolled back to its
+     * current state instead of being reset out from under the guest.
+     */
     ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
-                                   VFIO_DEVICE_STATE_STOP, &local_err);
+                                   vbasedev->migration->device_state,
+                                   &local_err);
     if (ret) {
         error_report_err(local_err);
         return ret;
@@ -993,6 +1000,7 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
     VFIODevice *vbasedev = opaque;
     VFIOMigration *migration = vbasedev->migration;
     enum vfio_device_mig_state new_state;
+    enum vfio_device_mig_state recover_state;
     Error *local_err = NULL;
     int ret;
 
@@ -1009,7 +1017,15 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
         vfio_final_precopy_reinit_check(vbasedev);
     }
 
-    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
+    /*
+     * If the device declines a stop-direction transition, roll it back to its
+     * current state instead of resetting it out from under the guest. On resume
+     * (running), a device that cannot transition is genuinely broken, so reset
+     * it by using ERROR as the recover state.
+     */
+    recover_state = running ? VFIO_DEVICE_STATE_ERROR : migration->device_state;
+    ret = vfio_migration_set_state(vbasedev, new_state, recover_state,
+                                   &local_err);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
@@ -1026,21 +1042,34 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
 static void vfio_vmstate_change(void *opaque, bool running, RunState state)
 {
     VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
     enum vfio_device_mig_state new_state;
+    enum vfio_device_mig_state recover_state;
     Error *local_err = NULL;
     int ret;
 
     if (running) {
         new_state = VFIO_DEVICE_STATE_RUNNING;
+        /*
+         * If the device fails to resume, it is genuinely broken, so reset it
+         * by using ERROR as the recover state.
+         */
+        recover_state = VFIO_DEVICE_STATE_ERROR;
     } else {
         new_state =
             (vfio_device_state_is_precopy(vbasedev) &&
              (state == RUN_STATE_FINISH_MIGRATE || state == RUN_STATE_PAUSED)) ?
                 VFIO_DEVICE_STATE_STOP_COPY :
                 VFIO_DEVICE_STATE_STOP;
+        /*
+         * A device may decline to stop (e.g. it is busy). Roll it back to its
+         * current state rather than resetting it out from under the guest.
+         */
+        recover_state = migration->device_state;
     }
 
-    ret = vfio_migration_set_state_or_reset(vbasedev, new_state, &local_err);
+    ret = vfio_migration_set_state(vbasedev, new_state, recover_state,
+                                   &local_err);
     if (ret) {
         /*
          * Migration should be aborted in this case, but vm_state_notify()
-- 
2.25.1