[Qemu-devel] [PULL 00/10] usb patch queue

[Qemu-devel] [PULL 00/10] usb patch queue

[Gerd Hoffmann]

  Hi,

Here comes the usb patch queue.  Nothing major, just a bunch of little
bugfixes, some of them lingering in the queue _way_ too long already.

please pull,
  Gerd

The following changes since commit 162cbbd1736de2bca43fdefa7e98c54a361ee60d:

  Merge remote-tracking branch 'luiz/queue/qmp' into staging (2013-04-02 14:07:35 -0500)

are available in the git repository at:


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

for you to fetch changes up to aa1c9e971e80d25b92908dce3dec7c38b49480ea:

  usb-tablet: Don't claim wakeup capability for USB-2 version (2013-04-03 11:39:43 +0200)

----------------------------------------------------------------
Gerd Hoffmann (7):
      xhci: fix numintrs sanity checks
      xhci: remove leftover debug printf
      xhci: remove unimplemented printfs
      xhci: zap unused name field
      usb-hub: limit chain length
      usb-hub: report status changes only once
      usb: update docs for bus name change

Hans de Goede (3):
      usb-redir: Fix crash on migration with no client connected
      usb-redir: Add flow control support
      usb-tablet: Don't claim wakeup capability for USB-2 version

 docs/usb2.txt     |   20 ++++++++++----------
 hw/usb.h          |    1 +
 hw/usb/bus.c      |    2 ++
 hw/usb/dev-hid.c  |    2 +-
 hw/usb/dev-hub.c  |   12 +++++++++++-
 hw/usb/hcd-xhci.c |   28 ++++++++++++----------------
 hw/usb/redirect.c |   37 ++++++++++++++++++++++++++++++++++++-
 trace-events      |    1 +
 8 files changed, 74 insertions(+), 29 deletions(-)

[Qemu-devel] [PATCH 01/10] usb-redir: Fix crash on migration with no client connected

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

If no client is connected on the src side, then we won't receive a
parser during migrate, in this case usbredir_post_load() should be a nop,
rather then to try to derefefence the NULL dev->parser pointer.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/redirect.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index d02a7b9..cf66df1 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1973,6 +1973,10 @@ static int usbredir_post_load(void *priv, int version_id)
 {
     USBRedirDevice *dev = priv;
 
+    if (dev->parser == NULL) {
+        return 0;
+    }
+
     switch (dev->device_info.speed) {
     case usb_redir_speed_low:
         dev->dev.speed = USB_SPEED_LOW;
-- 
1.7.9.7

[Qemu-devel] [PATCH 02/10] usb-redir: Add flow control support

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/redirect.c |   33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index cf66df1..0ddb081 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -104,6 +104,8 @@ struct USBRedirDevice {
     /* Data passed from chardev the fd_read cb to the usbredirparser read cb */
     const uint8_t *read_buf;
     int read_buf_size;
+    /* Active chardev-watch-tag */
+    guint watch;
     /* For async handling of close */
     QEMUBH *chardev_close_bh;
     /* To delay the usb attach in case of quick chardev close + open */
@@ -254,9 +256,21 @@ static int usbredir_read(void *priv, uint8_t *data, int count)
     return count;
 }
 
+static gboolean usbredir_write_unblocked(GIOChannel *chan, GIOCondition cond,
+                                         void *opaque)
+{
+    USBRedirDevice *dev = opaque;
+
+    dev->watch = 0;
+    usbredirparser_do_write(dev->parser);
+
+    return FALSE;
+}
+
 static int usbredir_write(void *priv, uint8_t *data, int count)
 {
     USBRedirDevice *dev = priv;
+    int r;
 
     if (!dev->cs->be_open) {
         return 0;
@@ -267,7 +281,17 @@ static int usbredir_write(void *priv, uint8_t *data, int count)
         return 0;
     }
 
-    return qemu_chr_fe_write(dev->cs, data, count);
+    r = qemu_chr_fe_write(dev->cs, data, count);
+    if (r < count) {
+        if (!dev->watch) {
+            dev->watch = qemu_chr_fe_add_watch(dev->cs, G_IO_OUT,
+                                               usbredir_write_unblocked, dev);
+        }
+        if (r < 0) {
+            r = 0;
+        }
+    }
+    return r;
 }
 
 /*
@@ -1085,6 +1109,10 @@ static void usbredir_chardev_close_bh(void *opaque)
         usbredirparser_destroy(dev->parser);
         dev->parser = NULL;
     }
+    if (dev->watch) {
+        g_source_remove(dev->watch);
+        dev->watch = 0;
+    }
 }
 
 static void usbredir_create_parser(USBRedirDevice *dev)
@@ -1317,6 +1345,9 @@ static void usbredir_handle_destroy(USBDevice *udev)
     if (dev->parser) {
         usbredirparser_destroy(dev->parser);
     }
+    if (dev->watch) {
+        g_source_remove(dev->watch);
+    }
 
     free(dev->filter_rules);
 }
-- 
1.7.9.7

[Qemu-devel] [PATCH 03/10] xhci: fix numintrs sanity checks

[Gerd Hoffmann]

Make sure numintrs is a power of two, msi requires this.

https://bugzilla.redhat.com/show_bug.cgi?id=918035

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 5aa342b..9d06c51 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3290,6 +3290,9 @@ static int usb_xhci_initfn(struct PCIDevice *dev)
     if (xhci->numintrs > MAXINTRS) {
         xhci->numintrs = MAXINTRS;
     }
+    while (xhci->numintrs & (xhci->numintrs - 1)) {   /* ! power of 2 */
+        xhci->numintrs++;
+    }
     if (xhci->numintrs < 1) {
         xhci->numintrs = 1;
     }
-- 
1.7.9.7

[Qemu-devel] [PATCH 04/10] xhci: remove leftover debug printf

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 9d06c51..382ac88 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1172,8 +1172,6 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
     uint32_t ctx[5];
     uint32_t ctx2[2];
 
-    fprintf(stderr, "%s: epid %d, state %d\n",
-            __func__, epctx->epid, state);
     xhci_dma_read_u32s(xhci, epctx->pctx, ctx, sizeof(ctx));
     ctx[0] &= ~EP_STATE_MASK;
     ctx[0] |= state;
-- 
1.7.9.7

[Qemu-devel] [PATCH 05/10] xhci: remove unimplemented printfs

[Gerd Hoffmann]

Replace them with a tracepoint, so they don't spam stderr by default.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c |   22 +++++++++-------------
 trace-events      |    1 +
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 382ac88..5ea1fe7 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2566,7 +2566,7 @@ static void xhci_process_commands(XHCIState *xhci)
         }
         break;
         default:
-            fprintf(stderr, "xhci: unimplemented command %d\n", type);
+            trace_usb_xhci_unimplemented("command", type);
             event.ccode = CC_TRB_ERROR;
             break;
         }
@@ -2765,7 +2765,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, unsigned size)
         ret = 0x00000000; /* reserved */
         break;
     default:
-        fprintf(stderr, "xhci_cap_read: reg %d unimplemented\n", (int)reg);
+        trace_usb_xhci_unimplemented("cap read", reg);
         ret = 0;
     }
 
@@ -2788,8 +2788,7 @@ static uint64_t xhci_port_read(void *ptr, hwaddr reg, unsigned size)
         break;
     case 0x0c: /* reserved */
     default:
-        fprintf(stderr, "xhci_port_read (port %d): reg 0x%x unimplemented\n",
-                port->portnr, (uint32_t)reg);
+        trace_usb_xhci_unimplemented("port read", reg);
         ret = 0;
     }
 
@@ -2829,8 +2828,7 @@ static void xhci_port_write(void *ptr, hwaddr reg,
     case 0x04: /* PORTPMSC */
     case 0x08: /* PORTLI */
     default:
-        fprintf(stderr, "xhci_port_write (port %d): reg 0x%x unimplemented\n",
-                port->portnr, (uint32_t)reg);
+        trace_usb_xhci_unimplemented("port write", reg);
     }
 }
 
@@ -2868,7 +2866,7 @@ static uint64_t xhci_oper_read(void *ptr, hwaddr reg, unsigned size)
         ret = xhci->config;
         break;
     default:
-        fprintf(stderr, "xhci_oper_read: reg 0x%x unimplemented\n", (int)reg);
+        trace_usb_xhci_unimplemented("oper read", reg);
         ret = 0;
     }
 
@@ -2933,7 +2931,7 @@ static void xhci_oper_write(void *ptr, hwaddr reg,
         xhci->config = val & 0xff;
         break;
     default:
-        fprintf(stderr, "xhci_oper_write: reg 0x%x unimplemented\n", (int)reg);
+        trace_usb_xhci_unimplemented("oper write", reg);
     }
 }
 
@@ -2949,8 +2947,7 @@ static uint64_t xhci_runtime_read(void *ptr, hwaddr reg,
             ret = xhci_mfindex_get(xhci) & 0x3fff;
             break;
         default:
-            fprintf(stderr, "xhci_runtime_read: reg 0x%x unimplemented\n",
-                    (int)reg);
+            trace_usb_xhci_unimplemented("runtime read", reg);
             break;
         }
     } else {
@@ -2994,7 +2991,7 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
     trace_usb_xhci_runtime_write(reg, val);
 
     if (reg < 0x20) {
-        fprintf(stderr, "%s: reg 0x%x unimplemented\n", __func__, (int)reg);
+        trace_usb_xhci_unimplemented("runtime write", reg);
         return;
     }
 
@@ -3036,8 +3033,7 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
         xhci_events_update(xhci, v);
         break;
     default:
-        fprintf(stderr, "xhci_oper_write: reg 0x%x unimplemented\n",
-                (int)reg);
+        trace_usb_xhci_unimplemented("oper write", reg);
     }
 }
 
diff --git a/trace-events b/trace-events
index 7f34112..412f7e4 100644
--- a/trace-events
+++ b/trace-events
@@ -380,6 +380,7 @@ usb_xhci_xfer_nak(void *xfer) "%p"
 usb_xhci_xfer_retry(void *xfer) "%p"
 usb_xhci_xfer_success(void *xfer, uint32_t bytes) "%p: len %d"
 usb_xhci_xfer_error(void *xfer, uint32_t ret) "%p: ret %d"
+usb_xhci_unimplemented(const char *item, int nr) "%s (0x%x)"
 
 # hw/usb/desc.c
 usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d"
-- 
1.7.9.7

[Qemu-devel] [PATCH 06/10] xhci: zap unused name field

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 5ea1fe7..efd4b0d 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -452,7 +452,6 @@ struct XHCIState {
     MemoryRegion mem_oper;
     MemoryRegion mem_runtime;
     MemoryRegion mem_doorbell;
-    const char *name;
     unsigned int devaddr;
 
     /* properties */
-- 
1.7.9.7

[Qemu-devel] [PATCH 07/10] usb-hub: limit chain length

[Gerd Hoffmann]

USB supports up to 5 hubs chained.
Catch attempts to chain more.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.h         |    1 +
 hw/usb/bus.c     |    2 ++
 hw/usb/dev-hub.c |    6 ++++++
 3 files changed, 9 insertions(+)

diff --git a/hw/usb.h b/hw/usb.h
index 1b10684..4d9d05e 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -337,6 +337,7 @@ typedef struct USBPortOps {
 struct USBPort {
     USBDevice *dev;
     int speedmask;
+    int hubcount;
     char path[16];
     USBPortOps *ops;
     void *opaque;
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index e58cd9a..b10c290 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -341,8 +341,10 @@ void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr)
     if (upstream) {
         snprintf(downstream->path, sizeof(downstream->path), "%s.%d",
                  upstream->path, portnr);
+        downstream->hubcount = upstream->hubcount + 1;
     } else {
         snprintf(downstream->path, sizeof(downstream->path), "%d", portnr);
+        downstream->hubcount = 0;
     }
 }
 
diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 504c98c..a5f092b 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -25,6 +25,7 @@
 #include "trace.h"
 #include "hw/usb.h"
 #include "hw/usb/desc.h"
+#include "qemu/error-report.h"
 
 #define NUM_PORTS 8
 
@@ -514,6 +515,11 @@ static int usb_hub_initfn(USBDevice *dev)
     USBHubPort *port;
     int i;
 
+    if (dev->port->hubcount == 5) {
+        error_report("usb hub chain too deep");
+        return -1;
+    }
+
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
-- 
1.7.9.7

[Qemu-devel] [PATCH 08/10] usb-hub: report status changes only once

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-hub.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index a5f092b..0b71abd 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -33,6 +33,7 @@ typedef struct USBHubPort {
     USBPort port;
     uint16_t wPortStatus;
     uint16_t wPortChange;
+    uint16_t wPortChange_reported;
 } USBHubPort;
 
 typedef struct USBHubState {
@@ -467,8 +468,11 @@ static void usb_hub_handle_data(USBDevice *dev, USBPacket *p)
             status = 0;
             for(i = 0; i < NUM_PORTS; i++) {
                 port = &s->ports[i];
-                if (port->wPortChange)
+                if (port->wPortChange &&
+                    port->wPortChange_reported != port->wPortChange) {
                     status |= (1 << (i + 1));
+                }
+                port->wPortChange_reported = port->wPortChange;
             }
             if (status != 0) {
                 for(i = 0; i < n; i++) {
-- 
1.7.9.7

[Qemu-devel] [PATCH 09/10] usb: update docs for bus name change

[Gerd Hoffmann]

At some point the default usb bus name changed from 'usb.0' to
'usb-bus.0' (probably as part of the qom conversion).  Update
the usb documentation accordingly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/usb2.txt |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/docs/usb2.txt b/docs/usb2.txt
index 43dacde..c7a445a 100644
--- a/docs/usb2.txt
+++ b/docs/usb2.txt
@@ -11,7 +11,7 @@ one USB 2.0 bus driven by the EHCI controller.  Devices must be
 attached to the correct controller manually.
 
 The '-usb' switch will make qemu create the UHCI controller as part of
-the PIIX3 chipset.  The USB 1.1 bus will carry the name "usb.0".
+the PIIX3 chipset.  The USB 1.1 bus will carry the name "usb-bus.0".
 
 You can use the standard -device switch to add a EHCI controller to
 your virtual machine.  It is strongly recommended to specify an ID for
@@ -27,7 +27,7 @@ a complete example:
         -drive if=none,id=usbstick,file=/path/to/image   \
         -usb                                             \
         -device usb-ehci,id=ehci                         \
-        -device usb-tablet,bus=usb.0                     \
+        -device usb-tablet,bus=usb-bus.0                 \
         -device usb-storage,bus=ehci.0,drive=usbstick
 
 This attaches a usb tablet to the UHCI adapter and a usb mass storage
@@ -88,22 +88,22 @@ ports (1-4), the emulated (1.1) USB hub has eight ports.
 
 Plugging a tablet into UHCI port 1 works like this:
 
-        -device usb-tablet,bus=usb.0,port=1
+        -device usb-tablet,bus=usb-bus.0,port=1
 
 Plugging a hub into UHCI port 2 works like this:
 
-        -device usb-hub,bus=usb.0,port=2
+        -device usb-hub,bus=usb-bus.0,port=2
 
 Plugging a virtual usb stick into port 4 of the hub just plugged works
 this way:
 
-        -device usb-storage,bus=usb.0,port=2.4,drive=...
+        -device usb-storage,bus=usb-bus.0,port=2.4,drive=...
 
 You can do basically the same in the monitor using the device_add
 command.  If you want to unplug devices too you should specify some
 unique id which you can use to refer to the device ...
 
-        (qemu) device_add usb-tablet,bus=usb.0,port=1,id=my-tablet
+        (qemu) device_add usb-tablet,bus=usb-bus.0,port=1,id=my-tablet
         (qemu) device_del my-tablet
 
 ... when unplugging it with device_del.
@@ -148,10 +148,10 @@ using for testing is bus 1 + port 1 for 2.0 devices and bus 3 + port 1
 for 1.1 devices.  Passing through any device plugged into that port
 and also assign them to the correct bus can be done this way:
 
-    qemu -M pc ${otheroptions}                           \
-        -usb                                             \
-        -device usb-ehci,id=ehci                         \
-        -device usb-host,bus=usb.0,hostbus=3,hostport=1  \
+    qemu -M pc ${otheroptions}                               \
+        -usb                                                 \
+        -device usb-ehci,id=ehci                             \
+        -device usb-host,bus=usb-bus.0,hostbus=3,hostport=1  \
         -device usb-host,bus=ehci.0,hostbus=1,hostport=1
 
 enjoy,
-- 
1.7.9.7

[Qemu-devel] [PATCH 10/10] usb-tablet: Don't claim wakeup capability for USB-2 version

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

Our ehci code does not implement wakeup support, so claiming support for
it with usb-tablet in USB-2 mode causes all tablet events to get lost.

http://bugzilla.redhat.com/show_bug.cgi?id=929068

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-hid.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 9701048..317b474 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -236,7 +236,7 @@ static const USBDescDevice desc_device_tablet2 = {
             .bNumInterfaces        = 1,
             .bConfigurationValue   = 1,
             .iConfiguration        = STR_CONFIG_TABLET,
-            .bmAttributes          = 0xa0,
+            .bmAttributes          = 0x80,
             .bMaxPower             = 50,
             .nif = 1,
             .ifs = &desc_iface_tablet2,
-- 
1.7.9.7

[Qemu-devel] [PULL 00/10] usb patch queue

[Gerd Hoffmann]

  Hi,

Here comes the next usb patch queue pull, featuring a bunch of mtp
bugfixes and ohci live migration support.

please pull,
  Gerd

The following changes since commit fdaad4715ae9e998fd0595bedfb16fdaf0c68ccc:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140501' into staging (2014-05-02 11:32:00 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-usb-6

for you to fetch changes up to 8ebb87635790e7033e19e9a26e8c5eef22560bcc:

  usb: mtp: reply INCOMPLETE_TRANSFER on read errors (2014-05-05 12:58:02 +0200)

----------------------------------------------------------------
ohci live migration.
mtp bugfixes.

----------------------------------------------------------------
Alexey Kardashevskiy (1):
      usb-ohci: Add vmstate descriptor

Gerd Hoffmann (9):
      usb: mtp: replace debug printfs with trace points
      usb: mtp: fix usb_mtp_add_u64
      usb: mtp: fix version (is decimal not bcd)
      usb: mtp: fix serial (must be exact 32 chars)
      usb: mtp: fix error path memory leak
      usb: mtp: avoid empty description string
      usb: mtp: drop data-out hexdump
      usb: mtp: fix possible buffer overflow
      usb: mtp: reply INCOMPLETE_TRANSFER on read errors

 hw/usb/dev-mtp.c  |  35 ++++++++++------
 hw/usb/hcd-ohci.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 trace-events      |   3 ++
 3 files changed, 137 insertions(+), 20 deletions(-)

Re: [Qemu-devel] [PULL 00/10] usb patch queue

[Peter Maydell]

On 5 May 2014 12:12, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here comes the next usb patch queue pull, featuring a bunch of mtp
> bugfixes and ohci live migration support.
>
> please pull,
>   Gerd
>
> The following changes since commit fdaad4715ae9e998fd0595bedfb16fdaf0c68ccc:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140501' into staging (2014-05-02 11:32:00 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-usb-6
>
> for you to fetch changes up to 8ebb87635790e7033e19e9a26e8c5eef22560bcc:
>
>   usb: mtp: reply INCOMPLETE_TRANSFER on read errors (2014-05-05 12:58:02 +0200)
>
> ----------------------------------------------------------------
> ohci live migration.
> mtp bugfixes.
>

Applied, thanks.

-- PMM