[Qemu-devel] USB: automatically choose bus to connect to based on speed

[Qemu-devel] USB: automatically choose bus to connect to based on speed

[Hans de Goede]

This patch series makes the usb subsystem automatically attach a usb
device to the right bus, based on the device's and the bus' speed, unless
a bus is explicitly specified by the user. This makes the uhci controller
more or less act as a companion controller to the ehci controller (if enabled),
and makes it easier for end users to redirect usb devices (as they don't
need to think about which bus they need to redirect to).

Regards,

Hans

[Qemu-devel] [PATCH 1/4] usb: Add a speedmask to devices

[Hans de Goede]

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.
---
 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 5b6878b..9e7d6d2 100644
--- a/hw/usb-ccid.c
+++ b/hw/usb-ccid.c
@@ -1283,6 +1283,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 57e8d6b..4f58ae8 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 097d789..5623f34 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -169,7 +169,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 9bab6e3..11829a2 100644
--- a/usb-bsd.c
+++ b/usb-bsd.c
@@ -368,8 +368,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 fe21da1..71e5f4d 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1141,6 +1141,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.5.1

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

[Hans de Goede]

---
 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 2ae2678..1817d5c 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)) {
+        fprintf(stderr, "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.5.1

[Qemu-devel] [PATCH 3/4] usb-bus: Automatically select right usb bus based on device and bus speed

[Hans de Goede]

If the bus was not explictly specified by the user, automatically select the
right usb bus based on device and bus speed.
---
 hw/qdev.c    |    2 +-
 hw/usb-bus.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/usb.h     |    1 +
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 9519f5d..f032412 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -260,11 +260,11 @@ DeviceState *qdev_device_add(QemuOpts *opts)
         qdev_free(qdev);
         return NULL;
     }
+    qdev->opts = opts;
     if (qdev_init(qdev) < 0) {
         qerror_report(QERR_DEVICE_INIT_FAILED, driver);
         return NULL;
     }
-    qdev->opts = opts;
     return qdev;
 }
 
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 1817d5c..00b2df8 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -72,6 +72,9 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
     pstrcpy(dev->product_desc, sizeof(dev->product_desc), info->product_desc);
     dev->info = info;
     dev->auto_attach = 1;
+    if (qdev->opts && qemu_opt_get(qdev->opts, "bus")) {
+        dev->bus_specified = 1;
+    }
     QLIST_INIT(&dev->strings);
     rc = dev->info->init(dev);
     if (rc == 0 && dev->auto_attach)
@@ -114,6 +117,8 @@ void usb_qdev_register_many(USBDeviceInfo *info)
 USBDevice *usb_create(USBBus *bus, const char *name)
 {
     DeviceState *dev;
+    USBDevice *usbdev;
+    int bus_specified = 1;
 
 #if 1
     /* temporary stopgap until all usb is properly qdev-ified */
@@ -123,11 +128,15 @@ USBDevice *usb_create(USBBus *bus, const char *name)
             return NULL;
         fprintf(stderr, "%s: no bus specified, using \"%s\" for \"%s\"\n",
                 __FUNCTION__, bus->qbus.name, name);
+        bus_specified = 0;
     }
 #endif
 
     dev = qdev_create(&bus->qbus, name);
-    return DO_UPCAST(USBDevice, qdev, dev);
+    usbdev = DO_UPCAST(USBDevice, qdev, dev);
+    usbdev->bus_specified = bus_specified;
+
+    return usbdev;
 }
 
 USBDevice *usb_create_simple(USBBus *bus, const char *name)
@@ -171,6 +180,35 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
     bus->nfree--;
 }
 
+static USBBus *find_bus_by_speed(int speedmask)
+{
+    USBBus *bus;
+    USBPort *port;
+
+    QTAILQ_FOREACH(bus, &busses, next) {
+        port = QTAILQ_FIRST(&bus->free);
+        if (port && (port->speedmask & speedmask)) {
+            return bus;
+        }
+    }
+    return NULL;
+}
+
+static USBBus *find_best_bus_by_speed(USBDevice *dev)
+{
+    USBBus *bus;
+
+    /* If the device supports high speed, first try to find a highspeed port */
+    if (dev->speedmask & USB_SPEED_MASK_HIGH) {
+        bus = find_bus_by_speed(USB_SPEED_MASK_HIGH);
+        if (bus) {
+            return bus;
+        }
+    }
+
+    return find_bus_by_speed(dev->speedmask);
+}
+
 static int do_attach(USBDevice *dev)
 {
     USBBus *bus = usb_bus_from_device(dev);
@@ -181,6 +219,19 @@ static int do_attach(USBDevice *dev)
                 dev->product_desc);
         return -1;
     }
+
+    /* If the bus was not explicitly specified, select one by speed */
+    if (!dev->bus_specified) {
+        USBBus *newbus = find_best_bus_by_speed(dev);
+        if (newbus && newbus != bus) {
+            /* A bit tricky, but safe since we're not attached */
+            QLIST_REMOVE(&dev->qdev, sibling);
+            dev->qdev.parent_bus = &newbus->qbus;
+            QLIST_INSERT_HEAD(&newbus->qbus.children, &dev->qdev, sibling);
+            bus = newbus;
+        }
+    }
+
     if (bus->nfree == 0) {
         fprintf(stderr, "Error: tried to attach usb device %s to a bus with no free ports\n",
                 dev->product_desc);
diff --git a/hw/usb.h b/hw/usb.h
index 5623f34..807f9e3 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -177,6 +177,7 @@ struct USBDevice {
     char product_desc[32];
     int auto_attach;
     int attached;
+    int bus_specified;
 
     int32_t state;
     uint8_t setup_buf[8];
-- 
1.7.5.1

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

[Hans de Goede]

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.
---
 usb-linux.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 71e5f4d..187e6ae 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1062,6 +1062,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)
 {
@@ -1142,6 +1178,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.5.1

Re: [Qemu-devel] USB: automatically choose bus to connect to based on speed

[Paolo Bonzini]

On 05/31/2011 11:39 AM, Hans de Goede wrote:
> This patch series makes the usb subsystem automatically attach a usb
> device to the right bus, based on the device's and the bus' speed, unless
> a bus is explicitly specified by the user. This makes the uhci controller
> more or less act as a companion controller to the ehci controller (if enabled),
> and makes it easier for end users to redirect usb devices (as they don't
> need to think about which bus they need to redirect to).

Shouldn't the correct order for the patches be 1, 4, 2, 3?

Paolo

Re: [Qemu-devel] USB: automatically choose bus to connect to based on speed

[Hans de Goede]

Hi,

On 05/31/2011 11:52 AM, Paolo Bonzini wrote:
> On 05/31/2011 11:39 AM, Hans de Goede wrote:
>> This patch series makes the usb subsystem automatically attach a usb
>> device to the right bus, based on the device's and the bus' speed, unless
>> a bus is explicitly specified by the user. This makes the uhci controller
>> more or less act as a companion controller to the ehci controller (if enabled),
>> and makes it easier for end users to redirect usb devices (as they don't
>> need to think about which bus they need to redirect to).
>
> Shouldn't the correct order for the patches be 1, 4, 2, 3?

That is not the order in which they were written :)  But I see your point.

Regards,

Hans