[Qemu-devel] [PULL] usb patch queue

[Qemu-devel] [PULL] usb patch queue

[Gerd Hoffmann]

  Hi,

Here comes the USB patch queue.  Nothing major, just a bunch of little
fixes and improvements.

please pull,
  Gerd

The following changes since commit 48e2faf222cbf4abab7c8e4b3f44229ec98eae7f:

  net: Warn about "-net nic" options which were ignored (2011-06-22 07:18:39 -0500)

are available in the git repository at:
  git://git.kraxel.org/qemu usb.17

Gerd Hoffmann (6):
      usb-linux: add get_endp()
      usb-linux: make iso urb count contigurable
      usb-linux: track inflight iso urb count
      ehci: add freq + maxframes properties
      ehci: switch to nanoseconds
      usb: ignore USB_DT_DEBUG

Hans de Goede (5):
      usb-bus: Don't allow attaching a device to a bus with no free ports
      usb: Proper error propagation for usb_device_attach errors
      usb: Add a speedmask to devices
      usb-linux: allow "compatible" high speed devices to connect at fullspeed
      usb-bus: Don't allow speed mismatch while attaching devices

Markus Armbruster (1):
      usb-storage: Turn drive serial into a qdev property usb-storage.serial

Peter Maydell (1):
      hw/usb-ohci.c: Fix handling of remote wakeup corner cases

 hw/usb-bus.c  |   31 ++++++++++-----
 hw/usb-ccid.c |    1 +
 hw/usb-desc.c |   14 ++++++
 hw/usb-ehci.c |   43 +++++++++++---------
 hw/usb-msd.c  |   19 ++++++--
 hw/usb-ohci.c |   17 ++++++-
 hw/usb.h      |    4 ++
 usb-bsd.c     |    2 +
 usb-linux.c   |  124 +++++++++++++++++++++++++++++++++++++++++++++------------
 9 files changed, 191 insertions(+), 64 deletions(-)

[Qemu-devel] [PATCH 01/13] usb-linux: add get_endp()

[Gerd Hoffmann]

Add a helper function to get the endpoint data structure
and put it into use.

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

diff --git a/usb-linux.c b/usb-linux.c
index 5d2ec5c..3c6156a 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -142,74 +142,79 @@ static void usb_host_auto_check(void *unused);
 static int usb_host_read_file(char *line, size_t line_size,
                             const char *device_file, const char *device_name);
 
+static struct endp_data *get_endp(USBHostDevice *s, int ep)
+{
+    return s->endp_table + ep - 1;
+}
+
 static int is_isoc(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].type == USBDEVFS_URB_TYPE_ISO;
+    return get_endp(s, ep)->type == USBDEVFS_URB_TYPE_ISO;
 }
 
 static int is_valid(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].type != INVALID_EP_TYPE;
+    return get_endp(s, ep)->type != INVALID_EP_TYPE;
 }
 
 static int is_halted(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].halted;
+    return get_endp(s, ep)->halted;
 }
 
 static void clear_halt(USBHostDevice *s, int ep)
 {
-    s->endp_table[ep - 1].halted = 0;
+    get_endp(s, ep)->halted = 0;
 }
 
 static void set_halt(USBHostDevice *s, int ep)
 {
-    s->endp_table[ep - 1].halted = 1;
+    get_endp(s, ep)->halted = 1;
 }
 
 static int is_iso_started(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].iso_started;
+    return get_endp(s, ep)->iso_started;
 }
 
 static void clear_iso_started(USBHostDevice *s, int ep)
 {
-    s->endp_table[ep - 1].iso_started = 0;
+    get_endp(s, ep)->iso_started = 0;
 }
 
 static void set_iso_started(USBHostDevice *s, int ep)
 {
-    s->endp_table[ep - 1].iso_started = 1;
+    get_endp(s, ep)->iso_started = 1;
 }
 
 static void set_iso_urb(USBHostDevice *s, int ep, AsyncURB *iso_urb)
 {
-    s->endp_table[ep - 1].iso_urb = iso_urb;
+    get_endp(s, ep)->iso_urb = iso_urb;
 }
 
 static AsyncURB *get_iso_urb(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].iso_urb;
+    return get_endp(s, ep)->iso_urb;
 }
 
 static void set_iso_urb_idx(USBHostDevice *s, int ep, int i)
 {
-    s->endp_table[ep - 1].iso_urb_idx = i;
+    get_endp(s, ep)->iso_urb_idx = i;
 }
 
 static int get_iso_urb_idx(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].iso_urb_idx;
+    return get_endp(s, ep)->iso_urb_idx;
 }
 
 static void set_iso_buffer_used(USBHostDevice *s, int ep, int i)
 {
-    s->endp_table[ep - 1].iso_buffer_used = i;
+    get_endp(s, ep)->iso_buffer_used = i;
 }
 
 static int get_iso_buffer_used(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].iso_buffer_used;
+    return get_endp(s, ep)->iso_buffer_used;
 }
 
 static void set_max_packet_size(USBHostDevice *s, int ep, uint8_t *descriptor)
@@ -223,14 +228,12 @@ static void set_max_packet_size(USBHostDevice *s, int ep, uint8_t *descriptor)
     case 2:  microframes = 3; break;
     default: microframes = 1; break;
     }
-    DPRINTF("husb: max packet size: 0x%x -> %d x %d\n",
-            raw, microframes, size);
-    s->endp_table[ep - 1].max_packet_size = size * microframes;
+    get_endp(s, ep)->max_packet_size = size * microframes;
 }
 
 static int get_max_packet_size(USBHostDevice *s, int ep)
 {
-    return s->endp_table[ep - 1].max_packet_size;
+    return get_endp(s, ep)->max_packet_size;
 }
 
 /*
-- 
1.7.1

[Qemu-devel] [PATCH 02/13] usb-linux: make iso urb count contigurable

[Gerd Hoffmann]

Add a qdev property for the number of iso urbs which
usb-linux keeps in flight, so it can be configured at
runtime.  Make it default to four (old hardcoded value
used to be three).

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

diff --git a/usb-linux.c b/usb-linux.c
index 3c6156a..f65dcb7 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -85,7 +85,6 @@ static int usb_fs_type;
 
 /* endpoint association data */
 #define ISO_FRAME_DESC_PER_URB 32
-#define ISO_URB_COUNT 3
 #define INVALID_EP_TYPE 255
 
 /* devio.c limits single requests to 16k */
@@ -120,6 +119,7 @@ typedef struct USBHostDevice {
     int       configuration;
     int       ninterfaces;
     int       closing;
+    uint32_t  iso_urb_count;
     Notifier  exit;
 
     struct endp_data endp_table[MAX_ENDPOINTS];
@@ -505,8 +505,8 @@ static AsyncURB *usb_host_alloc_iso(USBHostDevice *s, uint8_t ep, int in)
     AsyncURB *aurb;
     int i, j, len = get_max_packet_size(s, ep);
 
-    aurb = qemu_mallocz(ISO_URB_COUNT * sizeof(*aurb));
-    for (i = 0; i < ISO_URB_COUNT; i++) {
+    aurb = qemu_mallocz(s->iso_urb_count * sizeof(*aurb));
+    for (i = 0; i < s->iso_urb_count; i++) {
         aurb[i].urb.endpoint      = ep;
         aurb[i].urb.buffer_length = ISO_FRAME_DESC_PER_URB * len;
         aurb[i].urb.buffer        = qemu_malloc(aurb[i].urb.buffer_length);
@@ -536,7 +536,7 @@ static void usb_host_stop_n_free_iso(USBHostDevice *s, uint8_t ep)
         return;
     }
 
-    for (i = 0; i < ISO_URB_COUNT; i++) {
+    for (i = 0; i < s->iso_urb_count; i++) {
         /* in flight? */
         if (aurb[i].iso_frame_idx == -1) {
             ret = ioctl(s->fd, USBDEVFS_DISCARDURB, &aurb[i]);
@@ -554,7 +554,7 @@ static void usb_host_stop_n_free_iso(USBHostDevice *s, uint8_t ep)
         async_complete(s);
     }
 
-    for (i = 0; i < ISO_URB_COUNT; i++) {
+    for (i = 0; i < s->iso_urb_count; i++) {
         qemu_free(aurb[i].urb.buffer);
     }
 
@@ -639,7 +639,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p, int in)
         }
         aurb[i].iso_frame_idx++;
         if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
-            i = (i + 1) % ISO_URB_COUNT;
+            i = (i + 1) % s->iso_urb_count;
             set_iso_urb_idx(s, p->devep, i);
         }
     } else {
@@ -652,7 +652,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p, int in)
 
     if (is_iso_started(s, p->devep)) {
         /* (Re)-submit all fully consumed / filled urbs */
-        for (i = 0; i < ISO_URB_COUNT; i++) {
+        for (i = 0; i < s->iso_urb_count; i++) {
             if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
                 ret = ioctl(s->fd, USBDEVFS_SUBMITURB, &aurb[i]);
                 if (ret < 0) {
@@ -1233,6 +1233,7 @@ static struct USBDeviceInfo usb_host_dev_info = {
         DEFINE_PROP_STRING("hostport", USBHostDevice, match.port),
         DEFINE_PROP_HEX32("vendorid",  USBHostDevice, match.vendor_id,  0),
         DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0),
+        DEFINE_PROP_UINT32("isobufs",  USBHostDevice, iso_urb_count,    4),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
-- 
1.7.1

[Qemu-devel] [PATCH 03/13] usb-linux: track inflight iso urb count

[Gerd Hoffmann]

Track the number of iso urbs which are currently in flight.
Log a message in case the count goes down to zero.  Also
warn in case many urbs are returned at the same time.

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

diff --git a/usb-linux.c b/usb-linux.c
index f65dcb7..42baafe 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -100,6 +100,7 @@ struct endp_data {
     int iso_urb_idx;
     int iso_buffer_used;
     int max_packet_size;
+    int inflight;
 };
 
 struct USBAutoFilter {
@@ -184,7 +185,19 @@ static void clear_iso_started(USBHostDevice *s, int ep)
 
 static void set_iso_started(USBHostDevice *s, int ep)
 {
-    get_endp(s, ep)->iso_started = 1;
+    struct endp_data *e = get_endp(s, ep);
+    if (!e->iso_started) {
+        e->iso_started = 1;
+        e->inflight = 0;
+    }
+}
+
+static int change_iso_inflight(USBHostDevice *s, int ep, int value)
+{
+    struct endp_data *e = get_endp(s, ep);
+
+    e->inflight += value;
+    return e->inflight;
 }
 
 static void set_iso_urb(USBHostDevice *s, int ep, AsyncURB *iso_urb)
@@ -282,6 +295,7 @@ static void async_complete(void *opaque)
 {
     USBHostDevice *s = opaque;
     AsyncURB *aurb;
+    int urbs = 0;
 
     while (1) {
         USBPacket *p;
@@ -289,6 +303,9 @@ static void async_complete(void *opaque)
         int r = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb);
         if (r < 0) {
             if (errno == EAGAIN) {
+                if (urbs > 2) {
+                    fprintf(stderr, "husb: %d iso urbs finished at once\n", urbs);
+                }
                 return;
             }
             if (errno == ENODEV && !s->closing) {
@@ -306,10 +323,16 @@ static void async_complete(void *opaque)
         /* If this is a buffered iso urb mark it as complete and don't do
            anything else (it is handled further in usb_host_handle_iso_data) */
         if (aurb->iso_frame_idx == -1) {
+            int inflight;
             if (aurb->urb.status == -EPIPE) {
                 set_halt(s, aurb->urb.endpoint & 0xf);
             }
             aurb->iso_frame_idx = 0;
+            urbs++;
+            inflight = change_iso_inflight(s, aurb->urb.endpoint & 0xf, -1);
+            if (inflight == 0 && is_iso_started(s, aurb->urb.endpoint & 0xf)) {
+                fprintf(stderr, "husb: out of buffers for iso stream\n");
+            }
             continue;
         }
 
@@ -670,6 +693,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p, int in)
                     break;
                 }
                 aurb[i].iso_frame_idx = -1;
+                change_iso_inflight(s, p->devep, +1);
             }
         }
     }
-- 
1.7.1

[Qemu-devel] [PATCH 04/13] ehci: add freq + maxframes properties

[Gerd Hoffmann]

Add properties for the wakeup rate and the max number of frames ehci
will process at once.

The wakeup rate defaults to 1000 which equals the usb frame rate.  This
can be reduced to make qemu wake up less often when ehci is active.

In case the wakeup rate is reduced or the ehci timer is delayed due to
latency issues elsewhere in qemu ehci will process multiple frames at
once.  The maxframes property specifies the upper limit for this.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index e33e546..fa9792e 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -373,6 +373,11 @@ struct EHCIState {
     target_phys_addr_t mem_base;
     int mem;
     int num_ports;
+
+    /* properties */
+    uint32_t freq;
+    uint32_t maxframes;
+
     /*
      *  EHCI spec version 1.0 Section 2.3
      *  Host Controller Operational Registers
@@ -2048,7 +2053,7 @@ static void ehci_frame_timer(void *opaque)
 
 
     t_now = qemu_get_clock_ns(vm_clock);
-    expire_time = t_now + (get_ticks_per_sec() / FRAME_TIMER_FREQ);
+    expire_time = t_now + (get_ticks_per_sec() / ehci->freq);
     if (expire_time == t_now) {
         expire_time++;
     }
@@ -2073,7 +2078,7 @@ static void ehci_frame_timer(void *opaque)
             ehci->sofv &= 0x000003ff;
         }
 
-        if (frames - i > 10) {
+        if (frames - i > ehci->maxframes) {
             skipped_frames++;
         } else {
             ehci_advance_periodic_state(ehci);
@@ -2146,6 +2151,11 @@ static PCIDeviceInfo ehci_info = {
     .device_id    = PCI_DEVICE_ID_INTEL_82801D,
     .revision     = 0x10,
     .class_id     = PCI_CLASS_SERIAL_USB,
+    .qdev.props   = (Property[]) {
+        DEFINE_PROP_UINT32("freq",      EHCIState, freq, FRAME_TIMER_FREQ),
+        DEFINE_PROP_UINT32("maxframes", EHCIState, maxframes, 128),
+        DEFINE_PROP_END_OF_LIST(),
+    },
 };
 
 static int usb_ehci_initfn(PCIDevice *dev)
-- 
1.7.1

[Qemu-devel] [PATCH 05/13] ehci: switch to nanoseconds

[Gerd Hoffmann]

Make ehci use nanoseconds everywhere.
Simplifies time calculations.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index fa9792e..91fb7de 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -130,7 +130,7 @@
 #define PORTSC_CONNECT       (1 << 0)     // Current Connect Status
 
 #define FRAME_TIMER_FREQ 1000
-#define FRAME_TIMER_USEC (1000000 / FRAME_TIMER_FREQ)
+#define FRAME_TIMER_NS   (1000000000 / FRAME_TIMER_FREQ)
 
 #define NB_MAXINTRATE    8        // Max rate at which controller issues ints
 #define NB_PORTS         4        // Number of downstream ports
@@ -348,7 +348,8 @@ struct EHCIQueue {
     EHCIState *ehci;
     QTAILQ_ENTRY(EHCIQueue) next;
     bool async_schedule;
-    uint32_t seen, ts;
+    uint32_t seen;
+    uint64_t ts;
 
     /* cached data from guest - needs to be flushed
      * when guest removes an entry (doorbell, handshake sequence)
@@ -418,12 +419,11 @@ struct EHCIState {
     uint8_t ibuffer[BUFF_SIZE];
     int isoch_pause;
 
-    uint32_t last_run_usec;
-    uint32_t frame_end_usec;
+    uint64_t last_run_ns;
 };
 
 #define SET_LAST_RUN_CLOCK(s) \
-    (s)->last_run_usec = qemu_get_clock_ns(vm_clock) / 1000;
+    (s)->last_run_ns = qemu_get_clock_ns(vm_clock);
 
 /* nifty macros from Arnon's EHCI version  */
 #define get_field(data, field) \
@@ -690,10 +690,10 @@ static void ehci_queues_rip_unused(EHCIState *ehci)
     QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
         if (q->seen) {
             q->seen = 0;
-            q->ts = ehci->last_run_usec;
+            q->ts = ehci->last_run_ns;
             continue;
         }
-        if (ehci->last_run_usec < q->ts + 250000) {
+        if (ehci->last_run_ns < q->ts + 250000000) {
             /* allow 0.25 sec idle */
             continue;
         }
@@ -2045,23 +2045,16 @@ static void ehci_frame_timer(void *opaque)
 {
     EHCIState *ehci = opaque;
     int64_t expire_time, t_now;
-    int usec_elapsed;
+    uint64_t ns_elapsed;
     int frames;
-    int usec_now;
     int i;
     int skipped_frames = 0;
 
-
     t_now = qemu_get_clock_ns(vm_clock);
     expire_time = t_now + (get_ticks_per_sec() / ehci->freq);
-    if (expire_time == t_now) {
-        expire_time++;
-    }
 
-    usec_now = t_now / 1000;
-    usec_elapsed = usec_now - ehci->last_run_usec;
-    frames = usec_elapsed / FRAME_TIMER_USEC;
-    ehci->frame_end_usec = usec_now + FRAME_TIMER_USEC - 10;
+    ns_elapsed = t_now - ehci->last_run_ns;
+    frames = ns_elapsed / FRAME_TIMER_NS;
 
     for (i = 0; i < frames; i++) {
         if ( !(ehci->usbsts & USBSTS_HALT)) {
@@ -2084,7 +2077,7 @@ static void ehci_frame_timer(void *opaque)
             ehci_advance_periodic_state(ehci);
         }
 
-        ehci->last_run_usec += FRAME_TIMER_USEC;
+        ehci->last_run_ns += FRAME_TIMER_NS;
     }
 
 #if 0
-- 
1.7.1

[Qemu-devel] [PATCH 06/13] usb-bus: Don't allow attaching a device to a bus with no free ports

[Gerd Hoffmann]

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

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

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 480956d..0a49921 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -181,6 +181,11 @@ static void do_attach(USBDevice *dev)
                 dev->product_desc);
         return;
     }
+    if (bus->nfree == 0) {
+        fprintf(stderr, "Warning: tried to attach usb device %s to a bus with no free ports\n",
+                dev->product_desc);
+        return;
+    }
     if (dev->port_path) {
         QTAILQ_FOREACH(port, &bus->free, next) {
             if (strcmp(port->path, dev->port_path) == 0) {
-- 
1.7.1

[Qemu-devel] [PATCH 07/13] usb: Proper error propagation for usb_device_attach errors

[Gerd Hoffmann]

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

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-bus.c |   25 +++++++++++++------------
 hw/usb-msd.c |    5 +++--
 usb-linux.c  |    6 +++++-
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 0a49921..fc72018 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -75,7 +75,7 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
     QLIST_INIT(&dev->strings);
     rc = dev->info->init(dev);
     if (rc == 0 && dev->auto_attach)
-        usb_device_attach(dev);
+        rc = usb_device_attach(dev);
     return rc;
 }
 
@@ -121,7 +121,7 @@ USBDevice *usb_create(USBBus *bus, const char *name)
         bus = usb_bus_find(-1);
         if (!bus)
             return NULL;
-        fprintf(stderr, "%s: no bus specified, using \"%s\" for \"%s\"\n",
+        error_report("%s: no bus specified, using \"%s\" for \"%s\"\n",
                 __FUNCTION__, bus->qbus.name, name);
     }
 #endif
@@ -171,20 +171,20 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
     bus->nfree--;
 }
 
-static void do_attach(USBDevice *dev)
+static int do_attach(USBDevice *dev)
 {
     USBBus *bus = usb_bus_from_device(dev);
     USBPort *port;
 
     if (dev->attached) {
-        fprintf(stderr, "Warning: tried to attach usb device %s twice\n",
+        error_report("Error: tried to attach usb device %s twice\n",
                 dev->product_desc);
-        return;
+        return -1;
     }
     if (bus->nfree == 0) {
-        fprintf(stderr, "Warning: tried to attach usb device %s to a bus with no free ports\n",
+        error_report("Error: tried to attach usb device %s to a bus with no free ports\n",
                 dev->product_desc);
-        return;
+        return -1;
     }
     if (dev->port_path) {
         QTAILQ_FOREACH(port, &bus->free, next) {
@@ -193,9 +193,9 @@ static void do_attach(USBDevice *dev)
             }
         }
         if (port == NULL) {
-            fprintf(stderr, "Warning: usb port %s (bus %s) not found\n",
+            error_report("Error: usb port %s (bus %s) not found\n",
                     dev->port_path, bus->qbus.name);
-            return;
+            return -1;
         }
     } else {
         port = QTAILQ_FIRST(&bus->free);
@@ -209,6 +209,8 @@ static void do_attach(USBDevice *dev)
 
     QTAILQ_INSERT_TAIL(&bus->used, port, next);
     bus->nused++;
+
+    return 0;
 }
 
 int usb_device_attach(USBDevice *dev)
@@ -220,8 +222,7 @@ int usb_device_attach(USBDevice *dev)
            (unless a physical port location is specified). */
         usb_create_simple(bus, "usb-hub");
     }
-    do_attach(dev);
-    return 0;
+    return do_attach(dev);
 }
 
 int usb_device_detach(USBDevice *dev)
@@ -230,7 +231,7 @@ int usb_device_detach(USBDevice *dev)
     USBPort *port;
 
     if (!dev->attached) {
-        fprintf(stderr, "Warning: tried to detach unattached usb device %s\n",
+        error_report("Error: tried to detach unattached usb device %s\n",
                 dev->product_desc);
         return -1;
     }
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index c59797b..a75948b 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -497,8 +497,9 @@ static void usb_msd_password_cb(void *opaque, int err)
     MSDState *s = opaque;
 
     if (!err)
-        usb_device_attach(&s->dev);
-    else
+        err = usb_device_attach(&s->dev);
+
+    if (err)
         qdev_unplug(&s->dev.qdev);
 }
 
diff --git a/usb-linux.c b/usb-linux.c
index 42baafe..9b6f2be 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1178,10 +1178,14 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
                 prod_name);
     }
 
+    ret = usb_device_attach(&dev->dev);
+    if (ret) {
+        goto fail;
+    }
+
     /* USB devio uses 'write' flag to check for async completions */
     qemu_set_fd_handler(dev->fd, NULL, async_complete, dev);
 
-    usb_device_attach(&dev->dev);
     return 0;
 
 fail:
-- 
1.7.1

[Qemu-devel] [PATCH 08/13] usb: Add a speedmask to devices

[Gerd Hoffmann]

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

This is used to indicate at which speed[s] the device can operate,
so that this can be checked to match the ports capabilities when it gets
attached to a bus.

Note that currently all usb1 emulated device claim to be fullspeed, this
seems to not cause any problems, but still seems wrong, because with real
hardware keyboards, mice and tablets usually are lo-speed, so reporting these
as fullspeed devices seems wrong.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ccid.c |    1 +
 hw/usb-desc.c |   10 ++++++++++
 hw/usb.h      |    3 +++
 usb-bsd.c     |    2 ++
 usb-linux.c   |    1 +
 5 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c
index 59c6431..30bb4d6 100644
--- a/hw/usb-ccid.c
+++ b/hw/usb-ccid.c
@@ -1271,6 +1271,7 @@ static int ccid_initfn(USBDevice *dev)
     s->migration_target_ip = 0;
     s->migration_target_port = 0;
     s->dev.speed = USB_SPEED_FULL;
+    s->dev.speedmask = USB_SPEED_MASK_FULL;
     s->notify_slot_change = false;
     s->powered = true;
     s->pending_answers_num = 0;
diff --git a/hw/usb-desc.c b/hw/usb-desc.c
index e4a4680..0b9d351 100644
--- a/hw/usb-desc.c
+++ b/hw/usb-desc.c
@@ -242,7 +242,17 @@ static void usb_desc_setdefaults(USBDevice *dev)
 
 void usb_desc_init(USBDevice *dev)
 {
+    const USBDesc *desc = dev->info->usb_desc;
+
+    assert(desc != NULL);
     dev->speed = USB_SPEED_FULL;
+    dev->speedmask = 0;
+    if (desc->full) {
+        dev->speedmask |= USB_SPEED_MASK_FULL;
+    }
+    if (desc->high) {
+        dev->speedmask |= USB_SPEED_MASK_HIGH;
+    }
     usb_desc_setdefaults(dev);
 }
 
diff --git a/hw/usb.h b/hw/usb.h
index 06ce058..a6b311f 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -168,7 +168,10 @@ struct USBDevice {
     char *port_path;
     void *opaque;
 
+    /* Actual connected speed */
     int speed;
+    /* Supported speeds, not in info because it may be variable (hostdevs) */
+    int speedmask;
     uint8_t addr;
     char product_desc[32];
     int auto_attach;
diff --git a/usb-bsd.c b/usb-bsd.c
index c1bcc4a..3b97eb4 100644
--- a/usb-bsd.c
+++ b/usb-bsd.c
@@ -367,8 +367,10 @@ USBDevice *usb_host_device_open(const char *devname)
 
     if (dev_info.udi_speed == 1) {
         dev->dev.speed = USB_SPEED_LOW - 1;
+        dev->dev.speedmask = USB_SPEED_MASK_LOW;
     } else {
         dev->dev.speed = USB_SPEED_FULL - 1;
+        dev->dev.speedmask = USB_SPEED_MASK_FULL;
     }
 
     if (strncmp(dev_info.udi_product, "product", 7) != 0) {
diff --git a/usb-linux.c b/usb-linux.c
index 9b6f2be..4d22c9c 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1167,6 +1167,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
         }
     }
     dev->dev.speed = speed;
+    dev->dev.speedmask = (1 << speed);
 
     printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
 
-- 
1.7.1

[Qemu-devel] [PATCH 09/13] usb-linux: allow "compatible" high speed devices to connect at fullspeed

[Gerd Hoffmann]

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

Some usb2 highspeed devices, like usb-msd devices, work fine when redirected
to a usb1 virtual controller. Allow this to avoid the new speedhecks causing
regressions for users who do not enable the new experimental ehci code.

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

diff --git a/usb-linux.c b/usb-linux.c
index 4d22c9c..1a2deb3 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1088,6 +1088,42 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
     return 0;
 }
 
+/*
+ * Check if we can safely redirect a usb2 device to a usb1 virtual controller,
+ * this function assumes this is safe, if:
+ * 1) There are no isoc endpoints
+ * 2) There are no interrupt endpoints with a max_packet_size > 64
+ * Note bulk endpoints with a max_packet_size > 64 in theory also are not
+ * usb1 compatible, but in practice this seems to work fine.
+ */
+static int usb_linux_full_speed_compat(USBHostDevice *dev)
+{
+    int i, packet_size;
+
+    /*
+     * usb_linux_update_endp_table only registers info about ep in the current
+     * interface altsettings, so we need to parse the descriptors again.
+     */
+    for (i = 0; (i + 5) < dev->descr_len; i += dev->descr[i]) {
+        if (dev->descr[i + 1] == USB_DT_ENDPOINT) {
+            switch (dev->descr[i + 3] & 0x3) {
+            case 0x00: /* CONTROL */
+                break;
+            case 0x01: /* ISO */
+                return 0;
+            case 0x02: /* BULK */
+                break;
+            case 0x03: /* INTERRUPT */
+                packet_size = dev->descr[i + 4] + (dev->descr[i + 5] << 8);
+                if (packet_size > 64)
+                    return 0;
+                break;
+            }
+        }
+    }
+    return 1;
+}
+
 static int usb_host_open(USBHostDevice *dev, int bus_num,
                         int addr, char *port, const char *prod_name, int speed)
 {
@@ -1168,6 +1204,9 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
     }
     dev->dev.speed = speed;
     dev->dev.speedmask = (1 << speed);
+    if (dev->dev.speed == USB_SPEED_HIGH && usb_linux_full_speed_compat(dev)) {
+        dev->dev.speedmask |= USB_SPEED_MASK_FULL;
+    }
 
     printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
 
-- 
1.7.1

[Qemu-devel] [PATCH 10/13] usb-bus: Don't allow speed mismatch while attaching devices

[Gerd Hoffmann]

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

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

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index fc72018..2abce12 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -200,6 +200,11 @@ static int do_attach(USBDevice *dev)
     } else {
         port = QTAILQ_FIRST(&bus->free);
     }
+    if (!(port->speedmask & dev->speedmask)) {
+        error_report("Warning: speed mismatch trying to attach usb device %s to bus %s\n",
+                dev->product_desc, bus->qbus.name);
+        return -1;
+    }
 
     dev->attached++;
     QTAILQ_REMOVE(&bus->free, port, next);
-- 
1.7.1

[Qemu-devel] [PATCH 11/13] hw/usb-ohci.c: Fix handling of remote wakeup corner cases

[Gerd Hoffmann]

From: Peter Maydell <peter.maydell@linaro.org>

Correct a number of minor errors in the OHCI wakeup implementation:
 * when the port is suspended but the controller is not, raise RHSC
 * when the controller is suspended but the port is not, raise RD
 * when the controller is suspended, move it to resume state

These fix some edge cases where a USB device might not successfully get
the attention of the guest OS if it tried to do so at the wrong time.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ohci.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index 5d2ae01..1c29b9f 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -373,14 +373,25 @@ static void ohci_wakeup(USBDevice *dev)
     OHCIState *s = container_of(bus, OHCIState, bus);
     int portnum = dev->port->index;
     OHCIPort *port = &s->rhport[portnum];
+    uint32_t intr = 0;
     if (port->ctrl & OHCI_PORT_PSS) {
         DPRINTF("usb-ohci: port %d: wakeup\n", portnum);
         port->ctrl |= OHCI_PORT_PSSC;
         port->ctrl &= ~OHCI_PORT_PSS;
-        if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
-            ohci_set_interrupt(s, OHCI_INTR_RD);
-        }
+        intr = OHCI_INTR_RHSC;
+    }
+    /* Note that the controller can be suspended even if this port is not */
+    if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
+        DPRINTF("usb-ohci: remote-wakeup: SUSPEND->RESUME\n");
+        /* This is the one state transition the controller can do by itself */
+        s->ctl &= ~OHCI_CTL_HCFS;
+        s->ctl |= OHCI_USB_RESUME;
+        /* In suspend mode only ResumeDetected is possible, not RHSC:
+         * see the OHCI spec 5.1.2.3.
+         */
+        intr = OHCI_INTR_RD;
     }
+    ohci_set_interrupt(s, intr);
 }
 
 /* Reset the controller */
-- 
1.7.1

[Qemu-devel] [PATCH 12/13] usb-storage: Turn drive serial into a qdev property usb-storage.serial

[Gerd Hoffmann]

From: Markus Armbruster <armbru@redhat.com>

It needs to be a qdev property, because it belongs to the drive's
guest part.  Precedence: commit a0fef654 and 6ced55a5.

Bonus: info qtree now shows the serial number.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-msd.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index a75948b..86582cc 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -51,6 +51,7 @@ typedef struct {
     SCSIRequest *req;
     SCSIBus bus;
     BlockConf conf;
+    char *serial;
     SCSIDevice *scsi_dev;
     uint32_t removable;
     int result;
@@ -532,9 +533,15 @@ static int usb_msd_initfn(USBDevice *dev)
     bdrv_detach(bs, &s->dev.qdev);
     s->conf.bs = NULL;
 
-    dinfo = drive_get_by_blockdev(bs);
-    if (dinfo && dinfo->serial) {
-        usb_desc_set_string(dev, STR_SERIALNUMBER, dinfo->serial);
+    if (!s->serial) {
+        /* try to fall back to value set with legacy -drive serial=... */
+        dinfo = drive_get_by_blockdev(bs);
+        if (*dinfo->serial) {
+            s->serial = strdup(dinfo->serial);
+        }
+    }
+    if (s->serial) {
+        usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial);
     }
 
     usb_desc_init(dev);
@@ -633,6 +640,7 @@ static struct USBDeviceInfo msd_info = {
     .usbdevice_init = usb_msd_init,
     .qdev.props     = (Property[]) {
         DEFINE_BLOCK_PROPERTIES(MSDState, conf),
+        DEFINE_PROP_STRING("serial", MSDState, serial),
         DEFINE_PROP_BIT("removable", MSDState, removable, 0, false),
         DEFINE_PROP_END_OF_LIST(),
     },
-- 
1.7.1

[Qemu-devel] [PATCH 13/13] usb: ignore USB_DT_DEBUG

[Gerd Hoffmann]

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

diff --git a/hw/usb-desc.c b/hw/usb-desc.c
index 0b9d351..bc6858f 100644
--- a/hw/usb-desc.c
+++ b/hw/usb-desc.c
@@ -385,6 +385,10 @@ int usb_desc_get_descriptor(USBDevice *dev, int value, uint8_t *dest, size_t len
         trace_usb_desc_other_speed_config(dev->addr, index, len, ret);
         break;
 
+    case USB_DT_DEBUG:
+        /* ignore silently */
+        break;
+
     default:
         fprintf(stderr, "%s: %d unknown type %d (len %zd)\n", __FUNCTION__,
                 dev->addr, type, len);
diff --git a/hw/usb.h b/hw/usb.h
index a6b311f..076e2ff 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -130,6 +130,7 @@
 #define USB_DT_ENDPOINT			0x05
 #define USB_DT_DEVICE_QUALIFIER         0x06
 #define USB_DT_OTHER_SPEED_CONFIG       0x07
+#define USB_DT_DEBUG                    0x0A
 #define USB_DT_INTERFACE_ASSOC          0x0B
 
 #define USB_ENDPOINT_XFER_CONTROL	0
-- 
1.7.1

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

[Hans de Goede]

Hi,

Entire series looks good to me, including my own patches ;)

Ack series.

Regards,

Hans


On 06/24/2011 12:59 PM, Gerd Hoffmann wrote:
>    Hi,
>
> Here comes the USB patch queue.  Nothing major, just a bunch of little
> fixes and improvements.
>
> please pull,
>    Gerd
>
> The following changes since commit 48e2faf222cbf4abab7c8e4b3f44229ec98eae7f:
>
>    net: Warn about "-net nic" options which were ignored (2011-06-22 07:18:39 -0500)
>
> are available in the git repository at:
>    git://git.kraxel.org/qemu usb.17
>
> Gerd Hoffmann (6):
>        usb-linux: add get_endp()
>        usb-linux: make iso urb count contigurable
>        usb-linux: track inflight iso urb count
>        ehci: add freq + maxframes properties
>        ehci: switch to nanoseconds
>        usb: ignore USB_DT_DEBUG
>
> Hans de Goede (5):
>        usb-bus: Don't allow attaching a device to a bus with no free ports
>        usb: Proper error propagation for usb_device_attach errors
>        usb: Add a speedmask to devices
>        usb-linux: allow "compatible" high speed devices to connect at fullspeed
>        usb-bus: Don't allow speed mismatch while attaching devices
>
> Markus Armbruster (1):
>        usb-storage: Turn drive serial into a qdev property usb-storage.serial
>
> Peter Maydell (1):
>        hw/usb-ohci.c: Fix handling of remote wakeup corner cases
>
>   hw/usb-bus.c  |   31 ++++++++++-----
>   hw/usb-ccid.c |    1 +
>   hw/usb-desc.c |   14 ++++++
>   hw/usb-ehci.c |   43 +++++++++++---------
>   hw/usb-msd.c  |   19 ++++++--
>   hw/usb-ohci.c |   17 ++++++-
>   hw/usb.h      |    4 ++
>   usb-bsd.c     |    2 +
>   usb-linux.c   |  124 +++++++++++++++++++++++++++++++++++++++++++++------------
>   9 files changed, 191 insertions(+), 64 deletions(-)
>

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

[Anthony Liguori]

On 06/24/2011 05:59 AM, Gerd Hoffmann wrote:
>    Hi,
>
> Here comes the USB patch queue.  Nothing major, just a bunch of little
> fixes and improvements.
>
> please pull,

Pulled.  Thanks.

Regards,

Anthony Liguori

>    Gerd
>
> The following changes since commit 48e2faf222cbf4abab7c8e4b3f44229ec98eae7f:
>
>    net: Warn about "-net nic" options which were ignored (2011-06-22 07:18:39 -0500)
>
> are available in the git repository at:
>    git://git.kraxel.org/qemu usb.17
>
> Gerd Hoffmann (6):
>        usb-linux: add get_endp()
>        usb-linux: make iso urb count contigurable
>        usb-linux: track inflight iso urb count
>        ehci: add freq + maxframes properties
>        ehci: switch to nanoseconds
>        usb: ignore USB_DT_DEBUG
>
> Hans de Goede (5):
>        usb-bus: Don't allow attaching a device to a bus with no free ports
>        usb: Proper error propagation for usb_device_attach errors
>        usb: Add a speedmask to devices
>        usb-linux: allow "compatible" high speed devices to connect at fullspeed
>        usb-bus: Don't allow speed mismatch while attaching devices
>
> Markus Armbruster (1):
>        usb-storage: Turn drive serial into a qdev property usb-storage.serial
>
> Peter Maydell (1):
>        hw/usb-ohci.c: Fix handling of remote wakeup corner cases
>
>   hw/usb-bus.c  |   31 ++++++++++-----
>   hw/usb-ccid.c |    1 +
>   hw/usb-desc.c |   14 ++++++
>   hw/usb-ehci.c |   43 +++++++++++---------
>   hw/usb-msd.c  |   19 ++++++--
>   hw/usb-ohci.c |   17 ++++++-
>   hw/usb.h      |    4 ++
>   usb-bsd.c     |    2 +
>   usb-linux.c   |  124 +++++++++++++++++++++++++++++++++++++++++++++------------
>   9 files changed, 191 insertions(+), 64 deletions(-)
>
>