[Qemu-devel] [PULL] usb bugfix patch queue

[Qemu-devel] [PULL] usb bugfix patch queue

[Gerd Hoffmann]

  Hi,

Here comes a collection of bugfixes for usb, check the individual
patches for details.

All four patches should be charry-picked into stable once they are
merged into master.

please pull,
  Gerd

The following changes since commit f3c6a169a39d188e98c17a0a0ebfa7f85e5aafdd:

  Merge remote-tracking branch 'qemu-kvm/memory/page_desc' into staging (2012-01-03 14:39:05 -0600)

are available in the git repository at:

  git://git.kraxel.org/qemu usb.33

Andriy Gapon (1):
      usb-ohci: td.cbp incorrectly updated near page end

Cao,Bing Bu (1):
      Fix parse of usb device description with multiple configurations

Gerd Hoffmann (2):
      usb-storage: cancel I/O on reset
      usb-host: properly release port on unplug & exit

 hw/usb-msd.c  |   12 ++++++++++++
 hw/usb-ohci.c |    6 +++---
 usb-linux.c   |   47 +++++++++++++++++++++++++++++++----------------
 3 files changed, 46 insertions(+), 19 deletions(-)

[Qemu-devel] [PATCH 1/4] Fix parse of usb device description with multiple configurations

[Gerd Hoffmann]

From: Cao,Bing Bu <mars@linux.vnet.ibm.com>

Changed From V1:
Use DPRINTF instead of fprintf,because it is not an error.

When testing ipod on QEMU by He Jie Xu<xuhj@linux.vnet.ibm.com>,qemu made a assertion.
We found that the ipod with 2 configurations,and the usb-linux did not parse the descriptor correctly.
The descr_len returned is the total length of the all configurations,not one configuration.
The older version will through the other configurations instead of skip,continue parsing the descriptor of interfaces/endpoints in other configurations,then went wrong.

This patch will put the configuration descriptor parse in loop outside and dispel the other configurations not requested.

Signed-off-by: Cao,Bing Bu <mars@linux.vnet.ibm.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 usb-linux.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index ab4c693..ed14bb1 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1141,15 +1141,18 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
     length = s->descr_len - 18;
     i = 0;
 
-    if (descriptors[i + 1] != USB_DT_CONFIG ||
-        descriptors[i + 5] != s->configuration) {
-        fprintf(stderr, "invalid descriptor data - configuration %d\n",
-                s->configuration);
-        return 1;
-    }
-    i += descriptors[i];
-
     while (i < length) {
+        if (descriptors[i + 1] != USB_DT_CONFIG) {
+            fprintf(stderr, "invalid descriptor data\n");
+            return 1;
+        } else if (descriptors[i + 5] != s->configuration) {
+            DPRINTF("not requested configuration %d\n", s->configuration);
+            i += (descriptors[i + 3] << 8) + descriptors[i + 2];
+            continue;
+        }
+
+        i += descriptors[i];
+
         if (descriptors[i + 1] != USB_DT_INTERFACE ||
             (descriptors[i + 1] == USB_DT_INTERFACE &&
              descriptors[i + 4] == 0)) {
-- 
1.7.1

[Qemu-devel] [PATCH 2/4] usb-storage: cancel I/O on reset

[Gerd Hoffmann]

When resetting the usb-storage device we'll have to carefully cancel
and clear any requests which might be in flight, otherwise we'll confuse
the state machine.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-msd.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 4c06950..3147131 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -278,6 +278,18 @@ static void usb_msd_handle_reset(USBDevice *dev)
     MSDState *s = (MSDState *)dev;
 
     DPRINTF("Reset\n");
+    if (s->req) {
+        scsi_req_cancel(s->req);
+    }
+    assert(s->req == NULL);
+
+    if (s->packet) {
+        USBPacket *p = s->packet;
+        s->packet = NULL;
+        p->result = USB_RET_STALL;
+        usb_packet_complete(dev, p);
+    }
+
     s->mode = USB_MSDM_CBW;
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 3/4] usb-host: properly release port on unplug & exit

[Gerd Hoffmann]

Factor out port release into a separate function.  Call release function
in exit notifier too.  Add explicit call the USBDEVFS_RELEASE_PORT
ioctl, just closing the hub file handle seems not to be enougth.  Make
sure we release the port before resetting the device, otherwise host
drivers will not re-attach.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 usb-linux.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index ed14bb1..749ce71 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -116,6 +116,7 @@ typedef struct USBHostDevice {
     USBDevice dev;
     int       fd;
     int       hub_fd;
+    int       hub_port;
 
     uint8_t   descr[8192];
     int       descr_len;
@@ -434,7 +435,7 @@ static int usb_host_claim_port(USBHostDevice *s)
 {
 #ifdef USBDEVFS_CLAIM_PORT
     char *h, hub_name[64], line[1024];
-    int hub_addr, portnr, ret;
+    int hub_addr, ret;
 
     snprintf(hub_name, sizeof(hub_name), "%d-%s",
              s->match.bus_num, s->match.port);
@@ -442,13 +443,13 @@ static int usb_host_claim_port(USBHostDevice *s)
     /* try strip off last ".$portnr" to get hub */
     h = strrchr(hub_name, '.');
     if (h != NULL) {
-        portnr = atoi(h+1);
+        s->hub_port = atoi(h+1);
         *h = '\0';
     } else {
         /* no dot in there -> it is the root hub */
         snprintf(hub_name, sizeof(hub_name), "usb%d",
                  s->match.bus_num);
-        portnr = atoi(s->match.port);
+        s->hub_port = atoi(s->match.port);
     }
 
     if (!usb_host_read_file(line, sizeof(line), "devnum",
@@ -469,20 +470,32 @@ static int usb_host_claim_port(USBHostDevice *s)
         return -1;
     }
 
-    ret = ioctl(s->hub_fd, USBDEVFS_CLAIM_PORT, &portnr);
+    ret = ioctl(s->hub_fd, USBDEVFS_CLAIM_PORT, &s->hub_port);
     if (ret < 0) {
         close(s->hub_fd);
         s->hub_fd = -1;
         return -1;
     }
 
-    trace_usb_host_claim_port(s->match.bus_num, hub_addr, portnr);
+    trace_usb_host_claim_port(s->match.bus_num, hub_addr, s->hub_port);
     return 0;
 #else
     return -1;
 #endif
 }
 
+static void usb_host_release_port(USBHostDevice *s)
+{
+    if (s->hub_fd == -1) {
+        return;
+    }
+#ifdef USBDEVFS_RELEASE_PORT
+    ioctl(s->hub_fd, USBDEVFS_RELEASE_PORT, &s->hub_port);
+#endif
+    close(s->hub_fd);
+    s->hub_fd = -1;
+}
+
 static int usb_host_disconnect_ifaces(USBHostDevice *dev, int nb_interfaces)
 {
     /* earlier Linux 2.4 do not support that */
@@ -635,10 +648,8 @@ static void usb_host_handle_destroy(USBDevice *dev)
 {
     USBHostDevice *s = (USBHostDevice *)dev;
 
+    usb_host_release_port(s);
     usb_host_close(s);
-    if (s->hub_fd != -1) {
-        close(s->hub_fd);
-    }
     QTAILQ_REMOVE(&hostdevs, s, next);
     qemu_remove_exit_notifier(&s->exit);
 }
@@ -1402,6 +1413,7 @@ static void usb_host_exit_notifier(struct Notifier *n, void *data)
 {
     USBHostDevice *s = container_of(n, USBHostDevice, exit);
 
+    usb_host_release_port(s);
     if (s->fd != -1) {
         usb_host_do_reset(s);;
     }
-- 
1.7.1

[Qemu-devel] [PATCH 4/4] usb-ohci: td.cbp incorrectly updated near page end

[Gerd Hoffmann]

From: Andriy Gapon <avg@FreeBSD.org>

The current code that updates the cbp value after a transfer looks like this:
td.cbp += ret;
if ((td.cbp & 0xfff) + ret > 0xfff) {
	<handle page overflow>
because the 'ret' value is effectively added twice the check may fire too early
when the overflow hasn't happened yet.

Below is one of the possible changes that correct the behavior:

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ohci.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index e68be70..81488c4 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -1025,10 +1025,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
         if (ret == len) {
             td.cbp = 0;
         } else {
-            td.cbp += ret;
             if ((td.cbp & 0xfff) + ret > 0xfff) {
-                td.cbp &= 0xfff;
-                td.cbp |= td.be & ~0xfff;
+                td.cbp = (td.be & ~0xfff) + ((td.cbp + ret) & 0xfff);
+            } else {
+                td.cbp += ret;
             }
         }
         td.flags |= OHCI_TD_T1;
-- 
1.7.1

Re: [Qemu-devel] [PULL] usb bugfix patch queue

[Anthony Liguori]

On 01/06/2012 06:50 AM, Gerd Hoffmann wrote:
>    Hi,
>
> Here comes a collection of bugfixes for usb, check the individual
> patches for details.
>
> All four patches should be charry-picked into stable once they are
> merged into master.

Pulled.  Thanks.

Regards,

Anthony Liguori

>
> please pull,
>    Gerd
>
> The following changes since commit f3c6a169a39d188e98c17a0a0ebfa7f85e5aafdd:
>
>    Merge remote-tracking branch 'qemu-kvm/memory/page_desc' into staging (2012-01-03 14:39:05 -0600)
>
> are available in the git repository at:
>
>    git://git.kraxel.org/qemu usb.33
>
> Andriy Gapon (1):
>        usb-ohci: td.cbp incorrectly updated near page end
>
> Cao,Bing Bu (1):
>        Fix parse of usb device description with multiple configurations
>
> Gerd Hoffmann (2):
>        usb-storage: cancel I/O on reset
>        usb-host: properly release port on unplug&  exit
>
>   hw/usb-msd.c  |   12 ++++++++++++
>   hw/usb-ohci.c |    6 +++---
>   usb-linux.c   |   47 +++++++++++++++++++++++++++++++----------------
>   3 files changed, 46 insertions(+), 19 deletions(-)
>
>