[PULL 0/4] Usb 20200317 patches

[PULL 0/4] Usb 20200317 patches

[Gerd Hoffmann]

The following changes since commit 61c265f0660ee476985808c8aa7915617c44fd53:

  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200313a' into staging (2020-03-13 10:33:04 +0000)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/usb-20200317-pull-request

for you to fetch changes up to 647ee9877272d4359659e2595262db0e062c8ffc:

  usb-serial: Fix timeout closing the device (2020-03-17 09:05:34 +0100)

----------------------------------------------------------------
usb: bugfixes for usb-serial @ xhci.

----------------------------------------------------------------

Jason Andryuk (4):
  usb-serial: Move USB_TOKEN_IN into a helper function
  usb-serial: chunk data to wMaxPacketSize
  usb-serial: Increase receive buffer to 496
  usb-serial: Fix timeout closing the device

 hw/usb/dev-serial.c | 95 ++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 36 deletions(-)

-- 
2.18.2

[PULL 1/4] usb-serial: Move USB_TOKEN_IN into a helper function

[Gerd Hoffmann]

From: Jason Andryuk <jandryuk@gmail.com>

We'll be adding a loop, so move the code into a helper function.  breaks
are replaced with returns.  While making this change, add braces to
single line if statements to comply with coding style and keep
checkpatch happy.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Message-id: 20200316174610.115820-2-jandryuk@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c | 80 ++++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index daac75b7aec2..d807ce577110 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -358,13 +358,56 @@ static void usb_serial_handle_control(USBDevice *dev, USBPacket *p,
     }
 }
 
+static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
+{
+    int first_len, len;
+    uint8_t header[2];
+
+    first_len = RECV_BUF - s->recv_ptr;
+    len = p->iov.size;
+    if (len <= 2) {
+        p->status = USB_RET_NAK;
+        return;
+    }
+    header[0] = usb_get_modem_lines(s) | 1;
+    /* We do not have the uart details */
+    /* handle serial break */
+    if (s->event_trigger && s->event_trigger & FTDI_BI) {
+        s->event_trigger &= ~FTDI_BI;
+        header[1] = FTDI_BI;
+        usb_packet_copy(p, header, 2);
+        return;
+    } else {
+        header[1] = 0;
+    }
+    len -= 2;
+    if (len > s->recv_used) {
+        len = s->recv_used;
+    }
+    if (!len) {
+        p->status = USB_RET_NAK;
+        return;
+    }
+    if (first_len > len) {
+        first_len = len;
+    }
+    usb_packet_copy(p, header, 2);
+    usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
+    if (len > first_len) {
+        usb_packet_copy(p, s->recv_buf, len - first_len);
+    }
+    s->recv_used -= len;
+    s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
+
+    return;
+}
+
 static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
 {
     USBSerialState *s = (USBSerialState *)dev;
     uint8_t devep = p->ep->nr;
     struct iovec *iov;
-    uint8_t header[2];
-    int i, first_len, len;
+    int i;
 
     switch (p->pid) {
     case USB_TOKEN_OUT:
@@ -382,38 +425,7 @@ static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
     case USB_TOKEN_IN:
         if (devep != 1)
             goto fail;
-        first_len = RECV_BUF - s->recv_ptr;
-        len = p->iov.size;
-        if (len <= 2) {
-            p->status = USB_RET_NAK;
-            break;
-        }
-        header[0] = usb_get_modem_lines(s) | 1;
-        /* We do not have the uart details */
-        /* handle serial break */
-        if (s->event_trigger && s->event_trigger & FTDI_BI) {
-            s->event_trigger &= ~FTDI_BI;
-            header[1] = FTDI_BI;
-            usb_packet_copy(p, header, 2);
-            break;
-        } else {
-            header[1] = 0;
-        }
-        len -= 2;
-        if (len > s->recv_used)
-            len = s->recv_used;
-        if (!len) {
-            p->status = USB_RET_NAK;
-            break;
-        }
-        if (first_len > len)
-            first_len = len;
-        usb_packet_copy(p, header, 2);
-        usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
-        if (len > first_len)
-            usb_packet_copy(p, s->recv_buf, len - first_len);
-        s->recv_used -= len;
-        s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
+        usb_serial_token_in(s, p);
         break;
 
     default:
-- 
2.18.2

[PULL 2/4] usb-serial: chunk data to wMaxPacketSize

[Gerd Hoffmann]

From: Jason Andryuk <jandryuk@gmail.com>

usb-serial has issues with xHCI controllers where data is lost in the
VM.  Inspecting the URBs in the guest, EHCI starts every 64 byte boundary
(wMaxPacketSize) with a header.  EHCI hands packets into
usb_serial_token_in() with size 64, so these cannot cross the 64 byte
boundary.  The xHCI controller has packets of 512 bytes and the usb-serial
will just write through the 64 byte boundary.  In the guest, this means
data bytes are interpreted as header, so data bytes don't make it out
the serial interface.

Re-work usb_serial_token_in to chunk data into 64 byte units - 2 byte
header and 62 bytes data.  The Linux driver reads wMaxPacketSize to find
the chunk size, so we match that.

Real hardware was observed to pass in 512 byte URBs (496 bytes data +
8 * 2 byte headers).  Since usb-serial only buffers 384 bytes of data,
usb-serial will pass in 6 64 byte blocks and 1 12 byte partial block for
462 bytes max.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Message-id: 20200316174610.115820-3-jandryuk@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c | 47 ++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index d807ce577110..ec091b6a366c 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -360,15 +360,16 @@ static void usb_serial_handle_control(USBDevice *dev, USBPacket *p,
 
 static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
 {
-    int first_len, len;
+    const int max_packet_size = desc_iface0.eps[0].wMaxPacketSize;
+    int packet_len;
     uint8_t header[2];
 
-    first_len = RECV_BUF - s->recv_ptr;
-    len = p->iov.size;
-    if (len <= 2) {
+    packet_len = p->iov.size;
+    if (packet_len <= 2) {
         p->status = USB_RET_NAK;
         return;
     }
+
     header[0] = usb_get_modem_lines(s) | 1;
     /* We do not have the uart details */
     /* handle serial break */
@@ -380,24 +381,34 @@ static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
     } else {
         header[1] = 0;
     }
-    len -= 2;
-    if (len > s->recv_used) {
-        len = s->recv_used;
-    }
-    if (!len) {
+
+    if (!s->recv_used) {
         p->status = USB_RET_NAK;
         return;
     }
-    if (first_len > len) {
-        first_len = len;
+
+    while (s->recv_used && packet_len > 2) {
+        int first_len, len;
+
+        len = MIN(packet_len, max_packet_size);
+        len -= 2;
+        if (len > s->recv_used) {
+            len = s->recv_used;
+        }
+
+        first_len = RECV_BUF - s->recv_ptr;
+        if (first_len > len) {
+            first_len = len;
+        }
+        usb_packet_copy(p, header, 2);
+        usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
+        if (len > first_len) {
+            usb_packet_copy(p, s->recv_buf, len - first_len);
+        }
+        s->recv_used -= len;
+        s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
+        packet_len -= len + 2;
     }
-    usb_packet_copy(p, header, 2);
-    usb_packet_copy(p, s->recv_buf + s->recv_ptr, first_len);
-    if (len > first_len) {
-        usb_packet_copy(p, s->recv_buf, len - first_len);
-    }
-    s->recv_used -= len;
-    s->recv_ptr = (s->recv_ptr + len) % RECV_BUF;
 
     return;
 }
-- 
2.18.2

[PULL 3/4] usb-serial: Increase receive buffer to 496

[Gerd Hoffmann]

From: Jason Andryuk <jandryuk@gmail.com>

A FTDI USB adapter on an xHCI controller can send 512 byte USB packets.
These are 8 * ( 2 bytes header + 62 bytes data).  A 384 byte receive
buffer is insufficient to fill a 512 byte packet, so bump the receive
size to 496 ( 512 - 2 * 8 ).

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-id: 20200316174610.115820-4-jandryuk@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index ec091b6a366c..77b964588b5f 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -29,7 +29,7 @@ do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while (0)
 #define DPRINTF(fmt, ...) do {} while(0)
 #endif
 
-#define RECV_BUF 384
+#define RECV_BUF (512 - (2 * 8))
 
 /* Commands */
 #define FTDI_RESET		0
-- 
2.18.2

[PULL 4/4] usb-serial: Fix timeout closing the device

[Gerd Hoffmann]

From: Jason Andryuk <jandryuk@gmail.com>

Linux guests wait ~30 seconds when closing the emulated /dev/ttyUSB0.
During that time, the kernel driver is sending many control URBs
requesting GetModemStat (5).  Real hardware returns a status with
FTDI_THRE (Transmitter Holding Register) and FTDI_TEMT (Transmitter
Empty) set.  QEMU leaves them clear, and it seems Linux is waiting for
FTDI_TEMT to be set to indicate the tx queue is empty before closing.

Set the bits when responding to a GetModemStat query and avoid the
shutdown delay.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-id: 20200316174610.115820-5-jandryuk@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 77b964588b5f..d2c03681b7f0 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -332,7 +332,7 @@ static void usb_serial_handle_control(USBDevice *dev, USBPacket *p,
         break;
     case DeviceInVendor | FTDI_GET_MDM_ST:
         data[0] = usb_get_modem_lines(s) | 1;
-        data[1] = 0;
+        data[1] = FTDI_THRE | FTDI_TEMT;
         p->actual_length = 2;
         break;
     case DeviceOutVendor | FTDI_SET_EVENT_CHR:
-- 
2.18.2

Re: [PULL 0/4] Usb 20200317 patches

[Peter Maydell]

On Tue, 17 Mar 2020 at 09:52, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 61c265f0660ee476985808c8aa7915617c44fd53:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200313a' into staging (2020-03-13 10:33:04 +0000)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/usb-20200317-pull-request
>
> for you to fetch changes up to 647ee9877272d4359659e2595262db0e062c8ffc:
>
>   usb-serial: Fix timeout closing the device (2020-03-17 09:05:34 +0100)
>
> ----------------------------------------------------------------
> usb: bugfixes for usb-serial @ xhci.
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM