[Qemu-devel] [PATCH 22/54] usb-redir: Move to core packet id and queue handling

[Gerd Hoffmann]

From: Hans de Goede <address@hidden>

Signed-off-by: Hans de Goede <address@hidden>
Signed-off-by: Gerd Hoffmann <address@hidden>
---
 hw/usb/redirect.c |  226 ++++++++++++++++++++++-------------------------------
 1 files changed, 92 insertions(+), 134 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 6593d50..fd1f8cc 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -43,7 +43,7 @@
 #define EP2I(ep_address) (((ep_address & 0x80) >> 3) | (ep_address & 0x0f))
 #define I2EP(i) (((i & 0x10) << 3) | (i & 0x0f))
 
-typedef struct AsyncURB AsyncURB;
+typedef struct Cancelled Cancelled;
 typedef struct USBRedirDevice USBRedirDevice;
 
 /* Struct to hold buffered packets (iso or int input packets) */
@@ -86,8 +86,7 @@ struct USBRedirDevice {
     int64_t next_attach_time;
     struct usbredirparser *parser;
     struct endp_data endpoint[MAX_ENDPOINTS];
-    uint32_t packet_id;
-    QTAILQ_HEAD(, AsyncURB) asyncq;
+    QTAILQ_HEAD(, Cancelled) cancelled;
     /* Data for device filtering */
     struct usb_redir_device_connect_header device_info;
     struct usb_redir_interface_info_header interface_info;
@@ -95,10 +94,9 @@ struct USBRedirDevice {
     int filter_rules_count;
 };
 
-struct AsyncURB {
-    USBPacket *packet;
-    uint32_t packet_id;
-    QTAILQ_ENTRY(AsyncURB)next;
+struct Cancelled {
+    uint64_t id;
+    QTAILQ_ENTRY(Cancelled)next;
 };
 
 static void usbredir_hello(void *priv, struct usb_redir_hello_header *h);
@@ -238,57 +236,58 @@ static int usbredir_write(void *priv, uint8_t *data, int 
count)
 }
 
 /*
- * Async and buffered packets helpers
+ * Cancelled and buffered packets helpers
  */
 
-static AsyncURB *async_alloc(USBRedirDevice *dev, USBPacket *p)
+static void usbredir_cancel_packet(USBDevice *udev, USBPacket *p)
 {
-    AsyncURB *aurb = (AsyncURB *) g_malloc0(sizeof(AsyncURB));
-    aurb->packet = p;
-    aurb->packet_id = dev->packet_id;
-    QTAILQ_INSERT_TAIL(&dev->asyncq, aurb, next);
-    dev->packet_id++;
+    USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
+    Cancelled *c;
 
-    return aurb;
-}
+    DPRINTF("cancel packet id %"PRIu64"\n", p->id);
 
-static void async_free(USBRedirDevice *dev, AsyncURB *aurb)
-{
-    QTAILQ_REMOVE(&dev->asyncq, aurb, next);
-    g_free(aurb);
+    c = g_malloc0(sizeof(Cancelled));
+    c->id = p->id;
+    QTAILQ_INSERT_TAIL(&dev->cancelled, c, next);
+
+    usbredirparser_send_cancel_data_packet(dev->parser, p->id);
+    usbredirparser_do_write(dev->parser);
 }
 
-static AsyncURB *async_find(USBRedirDevice *dev, uint32_t packet_id)
+static int usbredir_is_cancelled(USBRedirDevice *dev, uint64_t id)
 {
-    AsyncURB *aurb;
+    Cancelled *c;
+
+    if (!dev->dev.attached) {
+        return 1; /* Treat everything as cancelled after a disconnect */
+    }
 
-    QTAILQ_FOREACH(aurb, &dev->asyncq, next) {
-        if (aurb->packet_id == packet_id) {
-            return aurb;
+    QTAILQ_FOREACH(c, &dev->cancelled, next) {
+        if (c->id == id) {
+            QTAILQ_REMOVE(&dev->cancelled, c, next);
+            g_free(c);
+            return 1;
         }
     }
-    DPRINTF("could not find async urb for packet_id %u\n", packet_id);
-    return NULL;
+    return 0;
 }
 
-static void usbredir_cancel_packet(USBDevice *udev, USBPacket *p)
+static USBPacket *usbredir_find_packet_by_id(USBRedirDevice *dev,
+    uint8_t ep, uint64_t id)
 {
-    USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
-    AsyncURB *aurb;
-
-    QTAILQ_FOREACH(aurb, &dev->asyncq, next) {
-        if (p != aurb->packet) {
-            continue;
-        }
+    USBPacket *p;
 
-        DPRINTF("async cancel id %u\n", aurb->packet_id);
-        usbredirparser_send_cancel_data_packet(dev->parser, aurb->packet_id);
-        usbredirparser_do_write(dev->parser);
+    if (usbredir_is_cancelled(dev, id)) {
+        return NULL;
+    }
 
-        /* Mark it as dead */
-        aurb->packet = NULL;
-        break;
+    p = usb_ep_find_packet_by_id(&dev->dev,
+                            (ep & USB_DIR_IN) ? USB_TOKEN_IN : USB_TOKEN_OUT,
+                            ep & 0x0f, id);
+    if (p == NULL) {
+        ERROR("could not find packet with id %"PRIu64"\n", id);
     }
+    return p;
 }
 
 static void bufp_alloc(USBRedirDevice *dev,
@@ -484,24 +483,22 @@ static void usbredir_stop_iso_stream(USBRedirDevice *dev, 
uint8_t ep)
 static int usbredir_handle_bulk_data(USBRedirDevice *dev, USBPacket *p,
                                       uint8_t ep)
 {
-    AsyncURB *aurb = async_alloc(dev, p);
     struct usb_redir_bulk_packet_header bulk_packet;
 
-    DPRINTF("bulk-out ep %02X len %zd id %u\n", ep,
-            p->iov.size, aurb->packet_id);
+    DPRINTF("bulk-out ep %02X len %zd id %"PRIu64"\n", ep, p->iov.size, p->id);
 
     bulk_packet.endpoint  = ep;
     bulk_packet.length    = p->iov.size;
     bulk_packet.stream_id = 0;
 
     if (ep & USB_DIR_IN) {
-        usbredirparser_send_bulk_packet(dev->parser, aurb->packet_id,
+        usbredirparser_send_bulk_packet(dev->parser, p->id,
                                         &bulk_packet, NULL, 0);
     } else {
         uint8_t buf[p->iov.size];
         usb_packet_copy(p, buf, p->iov.size);
         usbredir_log_data(dev, "bulk data out:", buf, p->iov.size);
-        usbredirparser_send_bulk_packet(dev->parser, aurb->packet_id,
+        usbredirparser_send_bulk_packet(dev->parser, p->id,
                                         &bulk_packet, buf, p->iov.size);
     }
     usbredirparser_do_write(dev->parser);
@@ -564,19 +561,18 @@ static int usbredir_handle_interrupt_data(USBRedirDevice 
*dev,
         return len;
     } else {
         /* Output interrupt endpoint, normal async operation */
-        AsyncURB *aurb = async_alloc(dev, p);
         struct usb_redir_interrupt_packet_header interrupt_packet;
         uint8_t buf[p->iov.size];
 
-        DPRINTF("interrupt-out ep %02X len %zd id %u\n", ep, p->iov.size,
-                aurb->packet_id);
+        DPRINTF("interrupt-out ep %02X len %zd id %"PRIu64"\n", ep,
+                p->iov.size, p->id);
 
         interrupt_packet.endpoint  = ep;
         interrupt_packet.length    = p->iov.size;
 
         usb_packet_copy(p, buf, p->iov.size);
         usbredir_log_data(dev, "interrupt data out:", buf, p->iov.size);
-        usbredirparser_send_interrupt_packet(dev->parser, aurb->packet_id,
+        usbredirparser_send_interrupt_packet(dev->parser, p->id,
                                         &interrupt_packet, buf, p->iov.size);
         usbredirparser_do_write(dev->parser);
         return USB_RET_ASYNC;
@@ -630,10 +626,9 @@ static int usbredir_set_config(USBRedirDevice *dev, 
USBPacket *p,
                                 int config)
 {
     struct usb_redir_set_configuration_header set_config;
-    AsyncURB *aurb = async_alloc(dev, p);
     int i;
 
-    DPRINTF("set config %d id %u\n", config, aurb->packet_id);
+    DPRINTF("set config %d id %"PRIu64"\n", config, p->id);
 
     for (i = 0; i < MAX_ENDPOINTS; i++) {
         switch (dev->endpoint[i].type) {
@@ -650,19 +645,16 @@ static int usbredir_set_config(USBRedirDevice *dev, 
USBPacket *p,
     }
 
     set_config.configuration = config;
-    usbredirparser_send_set_configuration(dev->parser, aurb->packet_id,
-                                          &set_config);
+    usbredirparser_send_set_configuration(dev->parser, p->id, &set_config);
     usbredirparser_do_write(dev->parser);
     return USB_RET_ASYNC;
 }
 
 static int usbredir_get_config(USBRedirDevice *dev, USBPacket *p)
 {
-    AsyncURB *aurb = async_alloc(dev, p);
-
-    DPRINTF("get config id %u\n", aurb->packet_id);
+    DPRINTF("get config id %"PRIu64"\n", p->id);
 
-    usbredirparser_send_get_configuration(dev->parser, aurb->packet_id);
+    usbredirparser_send_get_configuration(dev->parser, p->id);
     usbredirparser_do_write(dev->parser);
     return USB_RET_ASYNC;
 }
@@ -671,11 +663,9 @@ static int usbredir_set_interface(USBRedirDevice *dev, 
USBPacket *p,
                                    int interface, int alt)
 {
     struct usb_redir_set_alt_setting_header set_alt;
-    AsyncURB *aurb = async_alloc(dev, p);
     int i;
 
-    DPRINTF("set interface %d alt %d id %u\n", interface, alt,
-            aurb->packet_id);
+    DPRINTF("set interface %d alt %d id %"PRIu64"\n", interface, alt, p->id);
 
     for (i = 0; i < MAX_ENDPOINTS; i++) {
         if (dev->endpoint[i].interface == interface) {
@@ -695,8 +685,7 @@ static int usbredir_set_interface(USBRedirDevice *dev, 
USBPacket *p,
 
     set_alt.interface = interface;
     set_alt.alt = alt;
-    usbredirparser_send_set_alt_setting(dev->parser, aurb->packet_id,
-                                        &set_alt);
+    usbredirparser_send_set_alt_setting(dev->parser, p->id, &set_alt);
     usbredirparser_do_write(dev->parser);
     return USB_RET_ASYNC;
 }
@@ -705,13 +694,11 @@ static int usbredir_get_interface(USBRedirDevice *dev, 
USBPacket *p,
                                    int interface)
 {
     struct usb_redir_get_alt_setting_header get_alt;
-    AsyncURB *aurb = async_alloc(dev, p);
 
-    DPRINTF("get interface %d id %u\n", interface, aurb->packet_id);
+    DPRINTF("get interface %d id %"PRIu64"\n", interface, p->id);
 
     get_alt.interface = interface;
-    usbredirparser_send_get_alt_setting(dev->parser, aurb->packet_id,
-                                        &get_alt);
+    usbredirparser_send_get_alt_setting(dev->parser, p->id, &get_alt);
     usbredirparser_do_write(dev->parser);
     return USB_RET_ASYNC;
 }
@@ -721,7 +708,6 @@ static int usbredir_handle_control(USBDevice *udev, 
USBPacket *p,
 {
     USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
     struct usb_redir_control_packet_header control_packet;
-    AsyncURB *aurb;
 
     /* Special cases for certain standard device requests */
     switch (request) {
@@ -739,13 +725,10 @@ static int usbredir_handle_control(USBDevice *udev, 
USBPacket *p,
         return usbredir_get_interface(dev, p, index);
     }
 
-    /* "Normal" ctrl requests */
-    aurb = async_alloc(dev, p);
-
-    /* Note request is (bRequestType << 8) | bRequest */
-    DPRINTF("ctrl-out type 0x%x req 0x%x val 0x%x index %d len %d id %u\n",
-            request >> 8, request & 0xff, value, index, length,
-            aurb->packet_id);
+    /* Normal ctrl requests, note request is (bRequestType << 8) | bRequest */
+    DPRINTF(
+        "ctrl-out type 0x%x req 0x%x val 0x%x index %d len %d id %"PRIu64"\n",
+        request >> 8, request & 0xff, value, index, length, p->id);
 
     control_packet.request     = request & 0xFF;
     control_packet.requesttype = request >> 8;
@@ -755,11 +738,11 @@ static int usbredir_handle_control(USBDevice *udev, 
USBPacket *p,
     control_packet.length      = length;
 
     if (control_packet.requesttype & USB_DIR_IN) {
-        usbredirparser_send_control_packet(dev->parser, aurb->packet_id,
+        usbredirparser_send_control_packet(dev->parser, p->id,
                                            &control_packet, NULL, 0);
     } else {
         usbredir_log_data(dev, "ctrl data out:", data, length);
-        usbredirparser_send_control_packet(dev->parser, aurb->packet_id,
+        usbredirparser_send_control_packet(dev->parser, p->id,
                                            &control_packet, data, length);
     }
     usbredirparser_do_write(dev->parser);
@@ -913,7 +896,7 @@ static int usbredir_initfn(USBDevice *udev)
     dev->chardev_close_bh = qemu_bh_new(usbredir_chardev_close_bh, dev);
     dev->attach_timer = qemu_new_timer_ms(vm_clock, usbredir_do_attach, dev);
 
-    QTAILQ_INIT(&dev->asyncq);
+    QTAILQ_INIT(&dev->cancelled);
     for (i = 0; i < MAX_ENDPOINTS; i++) {
         QTAILQ_INIT(&dev->endpoint[i].bufpq);
     }
@@ -932,11 +915,12 @@ static int usbredir_initfn(USBDevice *udev)
 
 static void usbredir_cleanup_device_queues(USBRedirDevice *dev)
 {
-    AsyncURB *aurb, *next_aurb;
+    Cancelled *c, *next_c;
     int i;
 
-    QTAILQ_FOREACH_SAFE(aurb, &dev->asyncq, next, next_aurb) {
-        async_free(dev, aurb);
+    QTAILQ_FOREACH_SAFE(c, &dev->cancelled, next, next_c) {
+        QTAILQ_REMOVE(&dev->cancelled, c, next);
+        g_free(c);
     }
     for (i = 0; i < MAX_ENDPOINTS; i++) {
         usbredir_free_bufpq(dev, I2EP(i));
@@ -1202,33 +1186,28 @@ static void usbredir_configuration_status(void *priv, 
uint32_t id,
     struct usb_redir_configuration_status_header *config_status)
 {
     USBRedirDevice *dev = priv;
-    AsyncURB *aurb;
+    USBPacket *p;
     int len = 0;
 
     DPRINTF("set config status %d config %d id %u\n", config_status->status,
             config_status->configuration, id);
 
-    aurb = async_find(dev, id);
-    if (!aurb) {
-        return;
-    }
-    if (aurb->packet) {
+    p = usbredir_find_packet_by_id(dev, 0, id);
+    if (p) {
         if (dev->dev.setup_buf[0] & USB_DIR_IN) {
             dev->dev.data_buf[0] = config_status->configuration;
             len = 1;
         }
-        aurb->packet->result =
-            usbredir_handle_status(dev, config_status->status, len);
-        usb_generic_async_ctrl_complete(&dev->dev, aurb->packet);
+        p->result = usbredir_handle_status(dev, config_status->status, len);
+        usb_generic_async_ctrl_complete(&dev->dev, p);
     }
-    async_free(dev, aurb);
 }
 
 static void usbredir_alt_setting_status(void *priv, uint32_t id,
     struct usb_redir_alt_setting_status_header *alt_setting_status)
 {
     USBRedirDevice *dev = priv;
-    AsyncURB *aurb;
+    USBPacket *p;
     int len = 0;
 
     DPRINTF("alt status %d intf %d alt %d id: %u\n",
@@ -1236,20 +1215,16 @@ static void usbredir_alt_setting_status(void *priv, 
uint32_t id,
             alt_setting_status->interface,
             alt_setting_status->alt, id);
 
-    aurb = async_find(dev, id);
-    if (!aurb) {
-        return;
-    }
-    if (aurb->packet) {
+    p = usbredir_find_packet_by_id(dev, 0, id);
+    if (p) {
         if (dev->dev.setup_buf[0] & USB_DIR_IN) {
             dev->dev.data_buf[0] = alt_setting_status->alt;
             len = 1;
         }
-        aurb->packet->result =
+        p->result =
             usbredir_handle_status(dev, alt_setting_status->status, len);
-        usb_generic_async_ctrl_complete(&dev->dev, aurb->packet);
+        usb_generic_async_ctrl_complete(&dev->dev, p);
     }
-    async_free(dev, aurb);
 }
 
 static void usbredir_iso_stream_status(void *priv, uint32_t id,
@@ -1304,19 +1279,14 @@ static void usbredir_control_packet(void *priv, 
uint32_t id,
     uint8_t *data, int data_len)
 {
     USBRedirDevice *dev = priv;
+    USBPacket *p;
     int len = control_packet->length;
-    AsyncURB *aurb;
 
     DPRINTF("ctrl-in status %d len %d id %u\n", control_packet->status,
             len, id);
 
-    aurb = async_find(dev, id);
-    if (!aurb) {
-        free(data);
-        return;
-    }
-
-    if (aurb->packet) {
+    p = usbredir_find_packet_by_id(dev, 0, id);
+    if (p) {
         len = usbredir_handle_status(dev, control_packet->status, len);
         if (len > 0) {
             usbredir_log_data(dev, "ctrl data in:", data, data_len);
@@ -1328,10 +1298,9 @@ static void usbredir_control_packet(void *priv, uint32_t 
id,
                 len = USB_RET_STALL;
             }
         }
-        aurb->packet->result = len;
-        usb_generic_async_ctrl_complete(&dev->dev, aurb->packet);
+        p->result = len;
+        usb_generic_async_ctrl_complete(&dev->dev, p);
     }
-    async_free(dev, aurb);
     free(data);
 }
 
@@ -1342,33 +1311,27 @@ static void usbredir_bulk_packet(void *priv, uint32_t 
id,
     USBRedirDevice *dev = priv;
     uint8_t ep = bulk_packet->endpoint;
     int len = bulk_packet->length;
-    AsyncURB *aurb;
+    USBPacket *p;
 
     DPRINTF("bulk-in status %d ep %02X len %d id %u\n", bulk_packet->status,
             ep, len, id);
 
-    aurb = async_find(dev, id);
-    if (!aurb) {
-        free(data);
-        return;
-    }
-
-    if (aurb->packet) {
+    p = usbredir_find_packet_by_id(dev, ep, id);
+    if (p) {
         len = usbredir_handle_status(dev, bulk_packet->status, len);
         if (len > 0) {
             usbredir_log_data(dev, "bulk data in:", data, data_len);
-            if (data_len <= aurb->packet->iov.size) {
-                usb_packet_copy(aurb->packet, data, data_len);
+            if (data_len <= p->iov.size) {
+                usb_packet_copy(p, data, data_len);
             } else {
                 ERROR("bulk buffer too small (%d > %zd)\n", data_len,
-                      aurb->packet->iov.size);
+                      p->iov.size);
                 len = USB_RET_STALL;
             }
         }
-        aurb->packet->result = len;
-        usb_packet_complete(&dev->dev, aurb->packet);
+        p->result = len;
+        usb_packet_complete(&dev->dev, p);
     }
-    async_free(dev, aurb);
     free(data);
 }
 
@@ -1426,17 +1389,12 @@ static void usbredir_interrupt_packet(void *priv, 
uint32_t id,
     } else {
         int len = interrupt_packet->length;
 
-        AsyncURB *aurb = async_find(dev, id);
-        if (!aurb) {
-            return;
-        }
-
-        if (aurb->packet) {
-            aurb->packet->result = usbredir_handle_status(dev,
+        USBPacket *p = usbredir_find_packet_by_id(dev, ep, id);
+        if (p) {
+            p->result = usbredir_handle_status(dev,
                                                interrupt_packet->status, len);
-            usb_packet_complete(&dev->dev, aurb->packet);
+            usb_packet_complete(&dev->dev, p);
         }
-        async_free(dev, aurb);
     }
 }
 
-- 
1.7.1