[Qemu-devel] [USB fixes for 1.3] Fix some interrupt eps not properly nak-ing

[Qemu-devel] [USB fixes for 1.3] Fix some interrupt eps not properly nak-ing

[Hans de Goede]

As will become clear in my next patchset, I've been reviewing all usb-devices
in qemu with interrupt endpoints to add wakeup calls to all of them, during
this I found that the bt and smartcard-reader devices don't properly return
NAK from interrupt endpoints when they've no data, these patches fix this.

Regards,

Hans

[Qemu-devel] [PATCH 1/2] usb-bt: Return NAK instead of STALL when interrupt ep has no data

[Hans de Goede]

I noticed this while making all devices with interrupt endpoints properly
do wakeup. While at it also add wakeup support.

Note that I've not tested this, but returning STALL for an interrupt ep
which has no data is cleary the wrong thing to do.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/dev-bluetooth.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
index bfb96bf..39984f5 100644
--- a/hw/usb/dev-bluetooth.c
+++ b/hw/usb/dev-bluetooth.c
@@ -27,6 +27,7 @@
 struct USBBtState {
     USBDevice dev;
     struct HCIInfo *hci;
+    USBEndpoint *intr;
 
     int config;
 
@@ -290,10 +291,7 @@ static inline void usb_bt_fifo_dequeue(struct usb_hci_in_fifo_s *fifo,
 {
     int len;
 
-    if (likely(!fifo->len)) {
-        p->status = USB_RET_STALL;
-        return;
-    }
+    assert(fifo->len != 0);
 
     len = MIN(p->iov.size, fifo->fifo[fifo->start].len);
     usb_packet_copy(p, fifo->fifo[fifo->start].data, len);
@@ -422,14 +420,26 @@ static void usb_bt_handle_data(USBDevice *dev, USBPacket *p)
     case USB_TOKEN_IN:
         switch (p->ep->nr) {
         case USB_EVT_EP:
+            if (s->evt.len == 0) {
+                p->status = USB_RET_NAK;
+                break;
+            }
             usb_bt_fifo_dequeue(&s->evt, p);
             break;
 
         case USB_ACL_EP:
+            if (s->evt.len == 0) {
+                p->status = USB_RET_STALL;
+                break;
+            }
             usb_bt_fifo_dequeue(&s->acl, p);
             break;
 
         case USB_SCO_EP:
+            if (s->evt.len == 0) {
+                p->status = USB_RET_STALL;
+                break;
+            }
             usb_bt_fifo_dequeue(&s->sco, p);
             break;
 
@@ -467,6 +477,9 @@ static void usb_bt_out_hci_packet_event(void *opaque,
 {
     struct USBBtState *s = (struct USBBtState *) opaque;
 
+    if (s->evt.len == 0) {
+        usb_wakeup(s->intr);
+    }
     usb_bt_fifo_enqueue(&s->evt, data, len);
 }
 
@@ -489,8 +502,12 @@ static void usb_bt_handle_destroy(USBDevice *dev)
 
 static int usb_bt_initfn(USBDevice *dev)
 {
+    struct USBBtState *s = DO_UPCAST(struct USBBtState, dev, dev);
+
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
+    s->intr = usb_ep_get(dev, USB_TOKEN_IN, USB_EVT_EP);
+
     return 0;
 }
 
-- 
1.8.0

[Qemu-devel] [PATCH 2/2] usb-smartcard-reader: Properly NAK interrupt eps when we've no events

[Hans de Goede]

When we've no data to return from the interrupt endpoint, return NAK rather
then a 0 length packet.

CC: Alon Levy <alevy@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/dev-smartcard-reader.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 190fcd6..de955b7 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1002,6 +1002,8 @@ static void ccid_handle_data(USBDevice *dev, USBPacket *p)
                         "handle_data: int_in: notify_slot_change %X, "
                         "requested len %zd\n",
                         s->bmSlotICCState, p->iov.size);
+            } else {
+                p->status = USB_RET_NAK;
             }
             break;
         default:
-- 
1.8.0