| Message ID | 20200312125524.7812-3-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb-serial: xHCI and timeout fixes | expand |
Jason Andryuk, le jeu. 12 mars 2020 08:55:21 -0400, a ecrit: > 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> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> > --- > hw/usb/dev-serial.c | 43 +++++++++++++++++++++++++++---------------- > 1 file changed, 27 insertions(+), 16 deletions(-) > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > index 71fa786bd8..96b6c34202 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,21 +381,31 @@ 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; > - 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; > + > + 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; > + } > > return; > } > -- > 2.24.1 >
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c index 71fa786bd8..96b6c34202 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,21 +381,31 @@ 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; - 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; + + 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; + } return; }
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> --- hw/usb/dev-serial.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-)