[PATCH QEMU HCI-8.0 v5 05/13] usb-host: drain in-flight URBs across CPR #VSTOR-137800

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

URBs submitted by the old QEMU must not outlive it across the
switchover: the kernel would complete them into buffers of an address
space which is gone after exec, and the new libusb context would reap
URB pointers it never submitted.

So, in .pre_save() for the CPR modes, cancel and reap every pending
transfer.  One that already completed in the kernel is delivered
normally - its data is real and the guest must see it.  One still in
flight is suppressed: its packet stays ASYNC and its TD stays on the
transfer ring, so the target re-executes it from the start of the TD
when usb_xhci_post_load() kicks the endpoints.  Completing a transfer
can make xHCI queue the next TD, so keep cancelling until the request
list stays empty.

If the device won't release its URBs within the bounded wait, error
the still in-flight packets out -> the resumed guest retries them --
leak the requests, since libusb still owns their transfers, and abort
the migration.

Isochronous URBs are not on the request list but on their own rings,
and freeing them while in-flight would leave them queued on the
preserved FD.  Cancel them so they are unlinked and reaped: an iso URB
in flight when the guest stops may never complete on its own.  The
target reconstructs the iso stream from the guest's transfer ring.

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/usb/host-libusb.c | 158 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 135 insertions(+), 23 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 9b70d6deef9..ebad04ec305 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -48,6 +48,7 @@
 #include "qapi/error.h"
 #include "migration/blocker.h"
 #include "migration/cpr.h"
+#include "migration/misc.h"
 #include "migration/vmstate.h"
 #include "monitor/monitor.h"
 #include "qemu/error-report.h"
@@ -403,7 +404,10 @@ static void usb_host_req_free(USBHostRequest *r)
 #ifdef CONFIG_WIN32
     request_count--;
 #endif
-    QTAILQ_REMOVE(&r->host->requests, r, next);
+    /* Skip if a timed-out reap already unlinked it (avoid a double remove) */
+    if (QTAILQ_IN_USE(r, next)) {
+        QTAILQ_REMOVE(&r->host->requests, r, next);
+    }
     libusb_free_transfer(r->xfer);
     g_free(r->buffer);
     g_free(r);
@@ -430,6 +434,9 @@ static void LIBUSB_CALL usb_host_req_complete_ctrl(struct libusb_transfer *xfer)
     if (r->p == NULL) {
         goto out; /* request was canceled */
     }
+    if (xfer->status == LIBUSB_TRANSFER_CANCELLED) {
+        goto out; /* CPR drain: the target will replay this packet */
+    }
 
     r->p->status = status_map[xfer->status];
     r->p->actual_length = xfer->actual_length;
@@ -480,6 +487,9 @@ static void LIBUSB_CALL usb_host_req_complete_data(struct libusb_transfer *xfer)
     if (r->p == NULL) {
         goto out; /* request was canceled */
     }
+    if (xfer->status == LIBUSB_TRANSFER_CANCELLED) {
+        goto out; /* CPR drain: the target will replay this packet */
+    }
 
     r->p->status = status_map[xfer->status];
     if (r->in && xfer->actual_length) {
@@ -500,22 +510,29 @@ out:
     }
 }
 
-static void usb_host_req_abort(USBHostRequest *r)
+/* Fail r's in-flight packet with an error status and detach it from r. */
+static void usb_host_req_complete_err(USBHostRequest *r, int status)
 {
-    USBHostDevice  *s = r->host;
-    bool inflight = (r->p && r->p->state == USB_PACKET_ASYNC);
+    USBHostDevice *s = r->host;
 
-    if (inflight) {
-        r->p->status = USB_RET_NODEV;
-        trace_usb_host_req_complete(s->bus_num, s->addr, r->p,
-                                    r->p->status, r->p->actual_length);
-        if (r->p->ep->nr == 0) {
-            usb_generic_async_ctrl_complete(USB_DEVICE(s), r->p);
-        } else {
-            usb_packet_complete(USB_DEVICE(s), r->p);
-        }
-        r->p = NULL;
+    if (!r->p || r->p->state != USB_PACKET_ASYNC) {
+        return;
+    }
+    r->p->status = status;
+    trace_usb_host_req_complete(s->bus_num, s->addr, r->p,
+                                r->p->status, r->p->actual_length);
+    if (r->p->ep->nr == 0) {
+        usb_generic_async_ctrl_complete(USB_DEVICE(s), r->p);
+    } else {
+        usb_packet_complete(USB_DEVICE(s), r->p);
+    }
+    r->p = NULL;
+}
 
+static void usb_host_req_abort(USBHostRequest *r)
+{
+    if (r->p && r->p->state == USB_PACKET_ASYNC) {
+        usb_host_req_complete_err(r, USB_RET_NODEV);
         libusb_cancel_transfer(r->xfer);
     }
 }
@@ -1096,9 +1113,18 @@ fail:
     return -1;
 }
 
-static void usb_host_reap_xfers(USBHostDevice *s)
+/* libusb still owns cancelled-but-unreaped transfers: unlink, don't free. */
+static void usb_host_drop_requests(USBHostDevice *s)
 {
     USBHostRequest *r, *rtmp;
+
+    QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
+        QTAILQ_REMOVE(&s->requests, r, next);
+    }
+}
+
+static void usb_host_reap_xfers(USBHostDevice *s)
+{
     int limit = 100;
 
     while (QTAILQ_FIRST(&s->requests) != NULL) {
@@ -1108,15 +1134,10 @@ static void usb_host_reap_xfers(USBHostDevice *s)
         libusb_handle_events_timeout(ctx, &tv);
         if (--limit == 0) {
             /*
-             * Don't wait forever for libusb calling the complete
-             * callback (which will unlink and free the request).
-             *
-             * Leaking memory here, to make sure libusb will not
-             * access memory which we have released already.
+             * Don't wait forever for libusb to call the complete callback
+             * (which unlinks and frees the request).
              */
-            QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
-                QTAILQ_REMOVE(&s->requests, r, next);
-            }
+            usb_host_drop_requests(s);
             return;
         }
     }
@@ -1132,6 +1153,81 @@ static void usb_host_abort_xfers(USBHostDevice *s)
     usb_host_reap_xfers(s);
 }
 
+static bool usb_host_iso_inflight(USBHostDevice *s)
+{
+    USBHostIsoRing *ring;
+
+    QTAILQ_FOREACH(ring, &s->isorings, next) {
+        if (!QTAILQ_EMPTY(&ring->inflight)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+/*
+ * Quiesce the device for a CPR switchover.  Cancel and reap all URBs,
+ * so that no URB submitted by this process can outlive it.
+ */
+static int usb_host_cpr_drain_xfers(USBHostDevice *s)
+{
+    USBHostRequest *r, *rtmp;
+    USBHostIsoRing *ring;
+    USBHostIsoXfer *xfer;
+    int limit = 100;
+
+    /*
+     * Cancel and reap every URB.  Completing a reaped URB can make xHCI queue
+     * the next TD, so re-cancel whatever reappears until the queue stays empty.
+     */
+    while (!QTAILQ_EMPTY(&s->requests)) {
+        struct timeval tv = { .tv_usec = 2500 };
+
+        QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
+            libusb_cancel_transfer(r->xfer);
+        }
+        libusb_handle_events_timeout(ctx, &tv);
+        if (--limit == 0) {
+            /*
+             * The device won't release these URBs.  Error the packets out so
+             * the resumed guest retries them, leak the requests (libusb still
+             * owns their transfers) and abort the migration.
+             */
+            QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
+                usb_host_req_complete_err(r, USB_RET_IOERROR);
+            }
+            usb_host_drop_requests(s);
+            return -1;
+        }
+    }
+
+    /*
+     * Iso URBs are on the rings, not s->requests.  Cancel them: one in
+     * flight when the guest stops may never complete on its own, and none
+     * may be left on the preserved fd.  The target replays from the ring.
+     */
+    QTAILQ_FOREACH(ring, &s->isorings, next) {
+        QTAILQ_FOREACH(xfer, &ring->inflight, next) {
+            libusb_cancel_transfer(xfer->xfer);
+        }
+    }
+
+    /*
+     * Cap the reap at 2x a full ring:
+     * iso_urb_count URBs x iso_urb_frames packets each
+     */
+    limit = 2 * s->iso_urb_count * s->iso_urb_frames;
+    while (usb_host_iso_inflight(s)) {
+        struct timeval tv = { .tv_usec = 1000 };
+        libusb_handle_events_timeout(ctx, &tv);
+        if (limit-- == 0) {
+            return -1;
+        }
+    }
+    usb_host_iso_free_all(s);
+    return 0;
+}
+
 static int usb_host_close(USBHostDevice *s)
 {
     USBDevice *udev = USB_DEVICE(s);
@@ -1807,10 +1903,26 @@ static int usb_host_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static int usb_host_pre_save(void *opaque)
+{
+    USBHostDevice *s = opaque;
+    MigMode mode = migrate_mode();
+
+    /* URBs submitted by this process must not outlive CPR migration */
+    if ((mode == MIG_MODE_CPR_EXEC || mode == MIG_MODE_CPR_TRANSFER) &&
+        s->dh) {
+        if (usb_host_cpr_drain_xfers(s) < 0) {
+            return -1;
+        }
+    }
+    return 0;
+}
+
 static const VMStateDescription vmstate_usb_host = {
     .name = "usb-host",
     .version_id = 1,
     .minimum_version_id = 1,
+    .pre_save = usb_host_pre_save,
     .post_load = usb_host_post_load,
     .fields = (const VMStateField[]) {
         VMSTATE_USB_DEVICE(parent_obj, USBHostDevice),

-- 
2.47.1