qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci.
@ 2017-02-06 11:28 Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 1/9] hw/usb/dev-hid: Improve guest compatibility of usb-tablet Gerd Hoffmann
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi,

Here is the usb patch queue, with a collection of xhci bugfixes,
also some other usb fixes in various places.

please pull,
  Gerd

The following changes since commit a951316b8a5c3c63254f20a826afeed940dd4cba:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2017-02-03 14:41:49 +0000)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-usb-20170206-1

for you to fetch changes up to 7da76e12cc5cc902dda4c168d8d608fd4e61cbc5:

  xhci: fix event queue IRQ handling (2017-02-06 12:12:26 +0100)

----------------------------------------------------------------
usb: various bugfixes, mostly xhci.

----------------------------------------------------------------
Gerd Hoffmann (7):
      usb/uas: more verbose error message
      usb: accept usb3 control requests
      xhci: only free completed transfers
      xhci: rename xhci_complete_packet to xhci_try_complete_packet
      xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer
      xhci: guard xhci_kick_epctx against recursive calls
      xhci: fix event queue IRQ handling

Phil Dennis-Jordan (1):
      hw/usb/dev-hid: Improve guest compatibility of usb-tablet

Prasad J Pandit (1):
      usb: ccid: check ccid apdu length

 hw/usb/desc.c                 |  7 +++++++
 hw/usb/dev-hid.c              |  6 +++---
 hw/usb/dev-smartcard-reader.c |  2 +-
 hw/usb/dev-uas.c              |  3 ++-
 hw/usb/hcd-xhci.c             | 44 ++++++++++++++++++++++++++++---------------
 include/hw/usb.h              |  2 ++
 6 files changed, 44 insertions(+), 20 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 1/9] hw/usb/dev-hid: Improve guest compatibility of usb-tablet
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 2/9] usb/uas: more verbose error message Gerd Hoffmann
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Phil Dennis-Jordan, Gerd Hoffmann

From: Phil Dennis-Jordan <phil@philjordan.eu>

 1. Set bInterfaceProtocol to 0x00 for usb-tablet. This should be
    non-zero for boot protocol devices only, which the usb-tablet is not.
 2. Set the usb-tablet's usage to "mouse" in the report descriptor.

The boot protocol of 0x02 specifically confused OS X/macOS' HID driver
stack, causing it to generate additional bogus HID events with relative
motion in addition to the tablet's absolute coordinate events.

Absolute pointing devices with HID Report Descriptor usage of 0x01
(pointing) are treated by the macOS HID driver as analog sticks, and
absolute coordinates are not directly translated to absolute mouse
cursor positions. Changing it to 0x02 (mouse) fixes the problem, and
does not have any adverse effect in other operating systems and
windowing systems. (VMWare does the same thing.)

Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
Message-id: 1485365075-32702-1-git-send-email-phil@philjordan.eu
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-hid.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 24d05f7..dda0bf0 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -144,7 +144,7 @@ static const USBDescIface desc_iface_tablet = {
     .bInterfaceNumber              = 0,
     .bNumEndpoints                 = 1,
     .bInterfaceClass               = USB_CLASS_HID,
-    .bInterfaceProtocol            = 0x02,
+    .bInterfaceProtocol            = 0x00,
     .ndesc                         = 1,
     .descs = (USBDescOther[]) {
         {
@@ -174,7 +174,7 @@ static const USBDescIface desc_iface_tablet2 = {
     .bInterfaceNumber              = 0,
     .bNumEndpoints                 = 1,
     .bInterfaceClass               = USB_CLASS_HID,
-    .bInterfaceProtocol            = 0x02,
+    .bInterfaceProtocol            = 0x00,
     .ndesc                         = 1,
     .descs = (USBDescOther[]) {
         {
@@ -487,7 +487,7 @@ static const uint8_t qemu_mouse_hid_report_descriptor[] = {
 
 static const uint8_t qemu_tablet_hid_report_descriptor[] = {
     0x05, 0x01,		/* Usage Page (Generic Desktop) */
-    0x09, 0x01,		/* Usage (Pointer) */
+    0x09, 0x02,		/* Usage (Mouse) */
     0xa1, 0x01,		/* Collection (Application) */
     0x09, 0x01,		/*   Usage (Pointer) */
     0xa1, 0x00,		/*   Collection (Physical) */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 2/9] usb/uas: more verbose error message
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 1/9] hw/usb/dev-hid: Improve guest compatibility of usb-tablet Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 3/9] usb: accept usb3 control requests Gerd Hoffmann
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Print some more details in case we get a unknown
control request, to ease trouble-shooting.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485870727-21956-1-git-send-email-kraxel@redhat.com
---
 hw/usb/dev-uas.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 3a8ff18..da2fb70 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -653,7 +653,8 @@ static void usb_uas_handle_control(USBDevice *dev, USBPacket *p,
     if (ret >= 0) {
         return;
     }
-    error_report("%s: unhandled control request", __func__);
+    error_report("%s: unhandled control request (req 0x%x, val 0x%x, idx 0x%x",
+                 __func__, request, value, index);
     p->status = USB_RET_STALL;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 3/9] usb: accept usb3 control requests
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 1/9] hw/usb/dev-hid: Improve guest compatibility of usb-tablet Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 2/9] usb/uas: more verbose error message Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 4/9] xhci: only free completed transfers Gerd Hoffmann
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Windows 10 reportedly sends these, so accept them in case
the device in question is a superspeed (usb3) device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485870727-21956-2-git-send-email-kraxel@redhat.com
---
 hw/usb/desc.c    | 7 +++++++
 include/hw/usb.h | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 7828e52..c36bf30 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -774,6 +774,13 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
         trace_usb_set_device_feature(dev->addr, value, ret);
         break;
 
+    case DeviceOutRequest | USB_REQ_SET_SEL:
+    case DeviceOutRequest | USB_REQ_SET_ISOCH_DELAY:
+        if (dev->speed == USB_SPEED_SUPER) {
+            ret = 0;
+        }
+        break;
+
     case InterfaceRequest | USB_REQ_GET_INTERFACE:
         if (index < 0 || index >= dev->ninterfaces) {
             break;
diff --git a/include/hw/usb.h b/include/hw/usb.h
index 43838c9..c42b29c 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -135,6 +135,8 @@
 #define USB_REQ_GET_INTERFACE		0x0A
 #define USB_REQ_SET_INTERFACE		0x0B
 #define USB_REQ_SYNCH_FRAME		0x0C
+#define USB_REQ_SET_SEL                 0x30
+#define USB_REQ_SET_ISOCH_DELAY         0x31
 
 #define USB_DEVICE_SELF_POWERED		0
 #define USB_DEVICE_REMOTE_WAKEUP	1
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 4/9] xhci: only free completed transfers
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2017-02-06 11:28 ` [Qemu-devel] [PULL 3/9] usb: accept usb3 control requests Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 5/9] xhci: rename xhci_complete_packet to xhci_try_complete_packet Gerd Hoffmann
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, 1653384

Most callsites check already, one was missed.

Cc: 1653384@bugs.launchpad.net
Fixes: 94b037f2a451b3dc855f9f2c346e5049a361bd55
Reported-by: Fabian Lesniak <fabian@lesniak-it.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485790607-31399-2-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index f810678..6a1d3dc 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2198,7 +2198,9 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
             xhci_complete_packet(xfer);
         }
         assert(!xfer->running_retry);
-        xhci_ep_free_xfer(epctx->retry);
+        if (xfer->complete) {
+            xhci_ep_free_xfer(epctx->retry);
+        }
         epctx->retry = NULL;
     }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 5/9] xhci: rename xhci_complete_packet to xhci_try_complete_packet
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2017-02-06 11:28 ` [Qemu-devel] [PULL 4/9] xhci: only free completed transfers Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 6/9] xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer Gerd Hoffmann
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Make clear that this isn't guaranteed to actually complete the transfer,
the usb packet can still be in flight after calling that function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485790607-31399-3-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 6a1d3dc..7e863d3 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1897,7 +1897,7 @@ static int xhci_setup_packet(XHCITransfer *xfer)
     return 0;
 }
 
-static int xhci_complete_packet(XHCITransfer *xfer)
+static int xhci_try_complete_packet(XHCITransfer *xfer)
 {
     if (xfer->packet.status == USB_RET_ASYNC) {
         trace_usb_xhci_xfer_async(xfer);
@@ -2002,7 +2002,7 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
 
     usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
 
-    xhci_complete_packet(xfer);
+    xhci_try_complete_packet(xfer);
     if (!xfer->running_async && !xfer->running_retry) {
         xhci_kick_epctx(xfer->epctx, 0);
     }
@@ -2106,7 +2106,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx
     }
     usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
 
-    xhci_complete_packet(xfer);
+    xhci_try_complete_packet(xfer);
     if (!xfer->running_async && !xfer->running_retry) {
         xhci_kick_epctx(xfer->epctx, xfer->streamid);
     }
@@ -2185,7 +2185,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
             }
             usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
             assert(xfer->packet.status != USB_RET_NAK);
-            xhci_complete_packet(xfer);
+            xhci_try_complete_packet(xfer);
         } else {
             /* retry nak'ed transfer */
             if (xhci_setup_packet(xfer) < 0) {
@@ -2195,7 +2195,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
             if (xfer->packet.status == USB_RET_NAK) {
                 return;
             }
-            xhci_complete_packet(xfer);
+            xhci_try_complete_packet(xfer);
         }
         assert(!xfer->running_retry);
         if (xfer->complete) {
@@ -3492,7 +3492,7 @@ static void xhci_complete(USBPort *port, USBPacket *packet)
         xhci_ep_nuke_one_xfer(xfer, 0);
         return;
     }
-    xhci_complete_packet(xfer);
+    xhci_try_complete_packet(xfer);
     xhci_kick_epctx(xfer->epctx, xfer->streamid);
     if (xfer->complete) {
         xhci_ep_free_xfer(xfer);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 6/9] xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2017-02-06 11:28 ` [Qemu-devel] [PULL 5/9] xhci: rename xhci_complete_packet to xhci_try_complete_packet Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 7/9] xhci: guard xhci_kick_epctx against recursive calls Gerd Hoffmann
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, 1653384

xhci_submit and xhci_fire_ctl_transfer are is called from
xhci_kick_epctx processing loop only, so there is no need to call
xhci_kick_epctx make sure processing continues.  Also eecursive calls
into xhci_kick_epctx can cause trouble.

Drop the xhci_kick_epctx calls.

Cc: 1653384@bugs.launchpad.net
Fixes: 94b037f2a451b3dc855f9f2c346e5049a361bd55
Reported-by: Fabian Lesniak <fabian@lesniak-it.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485790607-31399-4-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 7e863d3..f89d8da 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2001,11 +2001,7 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
     xfer->packet.parameter = trb_setup->parameter;
 
     usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
-
     xhci_try_complete_packet(xfer);
-    if (!xfer->running_async && !xfer->running_retry) {
-        xhci_kick_epctx(xfer->epctx, 0);
-    }
     return 0;
 }
 
@@ -2105,11 +2101,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx
         return -1;
     }
     usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
-
     xhci_try_complete_packet(xfer);
-    if (!xfer->running_async && !xfer->running_retry) {
-        xhci_kick_epctx(xfer->epctx, xfer->streamid);
-    }
     return 0;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 7/9] xhci: guard xhci_kick_epctx against recursive calls
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2017-02-06 11:28 ` [Qemu-devel] [PULL 6/9] xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 8/9] usb: ccid: check ccid apdu length Gerd Hoffmann
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, 1653384

Track xhci_kick_epctx processing being active in a variable.  Check the
variable before calling xhci_kick_epctx from xhci_kick_ep.  Add an
assert to make sure we don't call recursively into xhci_kick_epctx.

Cc: 1653384@bugs.launchpad.net
Fixes: 94b037f2a451b3dc855f9f2c346e5049a361bd55
Reported-by: Fabian Lesniak <fabian@lesniak-it.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1486035372-3621-1-git-send-email-kraxel@redhat.com
Message-id: 1485790607-31399-5-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index f89d8da..1878dad 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -390,6 +390,7 @@ struct XHCIEPContext {
     dma_addr_t pctx;
     unsigned int max_psize;
     uint32_t state;
+    uint32_t kick_active;
 
     /* streams */
     unsigned int max_pstreams;
@@ -2131,6 +2132,9 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
         return;
     }
 
+    if (epctx->kick_active) {
+        return;
+    }
     xhci_kick_epctx(epctx, streamid);
 }
 
@@ -2146,6 +2150,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
     int i;
 
     trace_usb_xhci_ep_kick(epctx->slotid, epctx->epid, streamid);
+    assert(!epctx->kick_active);
 
     /* If the device has been detached, but the guest has not noticed this
        yet the 2 above checks will succeed, but we must NOT continue */
@@ -2217,6 +2222,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
     }
     assert(ring->dequeue != 0);
 
+    epctx->kick_active++;
     while (1) {
         length = xhci_ring_chain_length(xhci, ring);
         if (length <= 0) {
@@ -2253,6 +2259,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
             break;
         }
     }
+    epctx->kick_active--;
 
     ep = xhci_epid_to_usbep(epctx);
     if (ep) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 8/9] usb: ccid: check ccid apdu length
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2017-02-06 11:28 ` [Qemu-devel] [PULL 7/9] xhci: guard xhci_kick_epctx against recursive calls Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:28 ` [Qemu-devel] [PULL 9/9] xhci: fix event queue IRQ handling Gerd Hoffmann
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Prasad J Pandit, Gerd Hoffmann

From: Prasad J Pandit <pjp@fedoraproject.org>

CCID device emulator uses Application Protocol Data Units(APDU)
to exchange command and responses to and from the host.
The length in these units couldn't be greater than 65536. Add
check to ensure the same. It'd also avoid potential integer
overflow in emulated_apdu_from_guest.

Reported-by: Li Qiang <liqiang6-s@360.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-id: 20170202192228.10847-1-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-smartcard-reader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 89e11b6..1325ea1 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -967,7 +967,7 @@ static void ccid_on_apdu_from_guest(USBCCIDState *s, CCID_XferBlock *recv)
     DPRINTF(s, 1, "%s: seq %d, len %d\n", __func__,
                 recv->hdr.bSeq, len);
     ccid_add_pending_answer(s, (CCID_Header *)recv);
-    if (s->card) {
+    if (s->card && len <= BULK_OUT_DATA_SIZE) {
         ccid_card_apdu_from_guest(s->card, recv->abData, len);
     } else {
         DPRINTF(s, D_WARN, "warning: discarded apdu\n");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 9/9] xhci: fix event queue IRQ handling
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (7 preceding siblings ...)
  2017-02-06 11:28 ` [Qemu-devel] [PULL 8/9] usb: ccid: check ccid apdu length Gerd Hoffmann
@ 2017-02-06 11:28 ` Gerd Hoffmann
  2017-02-06 11:42 ` [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci no-reply
  2017-02-06 12:29 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2017-02-06 11:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, M.Cerveny, 1373228

The qemu xhci emulation doesn't handle the ERDP_EHB flag correctly.

When the host adapter queues a new event the ERDP_EHB flag is set.  The
flag is cleared (via w1c) by the guest when it updates the ERDP (event
ring dequeue pointer) register to notify the host adapter which events
it has fetched.

An IRQ must be raised in case the ERDP_EHB flag flips from clear to set.
If the flag is set already (which implies there are events queued up
which are not yet processed by the guest) xhci must *not* raise a IRQ.

Qemu got that wrong and raised an IRQ on every event, thereby generating
spurious interrupts in case we've queued events faster than the guest
processed them.  This patch fixes that.

With that change in place we also have to check ERDP updates, to see
whenever the guest has fetched all queued events.  In case there are
still pending events set ERDP_EHB and raise an IRQ again, to make sure
the events don't linger unseen forever.

The linux kernel driver and the microsoft windows driver (shipped with
win8+) can deal with the spurious interrupts without problems.  The
renesas windows driver (v2.1.39) which can be used on older windows
versions is quite upset though.  It does spurious ERDP updates now and
then (not every time, seems we must hit a race window for this to
happen), which in turn makes the qemu xhci emulation think the event
ring is full.  Things go south from here ...

tl;dr: This is the "fix xhci on win7" patch.

Cc: M.Cerveny@computer.org
Cc: 1373228@bugs.launchpad.net
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1486104705-13761-1-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 1878dad..54b3901 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -789,11 +789,15 @@ static void xhci_msix_update(XHCIState *xhci, int v)
 static void xhci_intr_raise(XHCIState *xhci, int v)
 {
     PCIDevice *pci_dev = PCI_DEVICE(xhci);
+    bool pending = (xhci->intr[v].erdp_low & ERDP_EHB);
 
     xhci->intr[v].erdp_low |= ERDP_EHB;
     xhci->intr[v].iman |= IMAN_IP;
     xhci->usbsts |= USBSTS_EINT;
 
+    if (pending) {
+        return;
+    }
     if (!(xhci->intr[v].iman & IMAN_IE)) {
         return;
     }
@@ -3352,6 +3356,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
             intr->erdp_low &= ~ERDP_EHB;
         }
         intr->erdp_low = (val & ~ERDP_EHB) | (intr->erdp_low & ERDP_EHB);
+        if (val & ERDP_EHB) {
+            dma_addr_t erdp = xhci_addr64(intr->erdp_low, intr->erdp_high);
+            unsigned int dp_idx = (erdp - intr->er_start) / TRB_SIZE;
+            if (erdp >= intr->er_start &&
+                erdp < (intr->er_start + TRB_SIZE * intr->er_size) &&
+                dp_idx != intr->er_ep_idx) {
+                xhci_intr_raise(xhci, v);
+            }
+        }
         break;
     case 0x1c: /* ERDP high */
         intr->erdp_high = val;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci.
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (8 preceding siblings ...)
  2017-02-06 11:28 ` [Qemu-devel] [PULL 9/9] xhci: fix event queue IRQ handling Gerd Hoffmann
@ 2017-02-06 11:42 ` no-reply
  2017-02-06 12:29 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: no-reply @ 2017-02-06 11:42 UTC (permalink / raw)
  To: kraxel; +Cc: famz, qemu-devel

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci.
Message-id: 1486380501-13431-1-git-send-email-kraxel@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1486380501-13431-1-git-send-email-kraxel@redhat.com -> patchew/1486380501-13431-1-git-send-email-kraxel@redhat.com
Switched to a new branch 'test'
4dbd0d1 xhci: fix event queue IRQ handling
c2e151b usb: ccid: check ccid apdu length
d2e396c xhci: guard xhci_kick_epctx against recursive calls
8f04397 xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer
1361fc9 xhci: rename xhci_complete_packet to xhci_try_complete_packet
93cd5d4 xhci: only free completed transfers
81fac17 usb: accept usb3 control requests
36fec08 usb/uas: more verbose error message
ecf8fb4 hw/usb/dev-hid: Improve guest compatibility of usb-tablet

=== OUTPUT BEGIN ===
Checking PATCH 1/9: hw/usb/dev-hid: Improve guest compatibility of usb-tablet...
ERROR: code indent should never use tabs
#53: FILE: hw/usb/dev-hid.c:490:
+    0x09, 0x02,^I^I/* Usage (Mouse) */$

total: 1 errors, 0 warnings, 24 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/9: usb/uas: more verbose error message...
Checking PATCH 3/9: usb: accept usb3 control requests...
Checking PATCH 4/9: xhci: only free completed transfers...
Checking PATCH 5/9: xhci: rename xhci_complete_packet to xhci_try_complete_packet...
Checking PATCH 6/9: xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer...
Checking PATCH 7/9: xhci: guard xhci_kick_epctx against recursive calls...
Checking PATCH 8/9: usb: ccid: check ccid apdu length...
Checking PATCH 9/9: xhci: fix event queue IRQ handling...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci.
  2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
                   ` (9 preceding siblings ...)
  2017-02-06 11:42 ` [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci no-reply
@ 2017-02-06 12:29 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2017-02-06 12:29 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: QEMU Developers

On 6 February 2017 at 11:28, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here is the usb patch queue, with a collection of xhci bugfixes,
> also some other usb fixes in various places.
>
> please pull,
>   Gerd
>
> The following changes since commit a951316b8a5c3c63254f20a826afeed940dd4cba:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2017-02-03 14:41:49 +0000)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-usb-20170206-1
>
> for you to fetch changes up to 7da76e12cc5cc902dda4c168d8d608fd4e61cbc5:
>
>   xhci: fix event queue IRQ handling (2017-02-06 12:12:26 +0100)
>
> ----------------------------------------------------------------
> usb: various bugfixes, mostly xhci.
>
> ----------------------------------------------------------------
> Gerd Hoffmann (7):
>       usb/uas: more verbose error message
>       usb: accept usb3 control requests
>       xhci: only free completed transfers
>       xhci: rename xhci_complete_packet to xhci_try_complete_packet
>       xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer
>       xhci: guard xhci_kick_epctx against recursive calls
>       xhci: fix event queue IRQ handling
>
> Phil Dennis-Jordan (1):
>       hw/usb/dev-hid: Improve guest compatibility of usb-tablet
>
> Prasad J Pandit (1):
>       usb: ccid: check ccid apdu length

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-02-06 12:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-06 11:28 [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 1/9] hw/usb/dev-hid: Improve guest compatibility of usb-tablet Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 2/9] usb/uas: more verbose error message Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 3/9] usb: accept usb3 control requests Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 4/9] xhci: only free completed transfers Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 5/9] xhci: rename xhci_complete_packet to xhci_try_complete_packet Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 6/9] xhci: don't kick in xhci_submit and xhci_fire_ctl_transfer Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 7/9] xhci: guard xhci_kick_epctx against recursive calls Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 8/9] usb: ccid: check ccid apdu length Gerd Hoffmann
2017-02-06 11:28 ` [Qemu-devel] [PULL 9/9] xhci: fix event queue IRQ handling Gerd Hoffmann
2017-02-06 11:42 ` [Qemu-devel] [PULL 0/9] usb: various bugfixes, mostly xhci no-reply
2017-02-06 12:29 ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).