Patchwork ehci: uhci-ehci co-existence (handling v1.1 and v2 devices)

login
register
mail settings
Submitter David S. Ahern
Date July 7, 2010, 4:58 p.m.
Message ID <1278521907-13582-1-git-send-email-daahern@cisco.com>
Download mbox | patch
Permalink /patch/58129/
State New
Headers show

Comments

David S. Ahern - July 7, 2010, 4:58 p.m.
Per the EHCI specification a USB 2.0 host controller is comprised of one high-speed controller and 0 to N USB 1.1 companion host controllers (UHCI or OHCI) for low- and full-speed devices. Port routing and control logic determines which HC owns a particular port. See Sections 1.2 and 4.2 of the EHCI specification.

http://www.intel.com/technology/usb/download/ehci-r10.pdf

In essence a USB 2.0 bus has N ports. Those N ports can be controlled by either the companion controller or the ehci controller. The ports default to the companion controller. At boot if the OS has an EHCI host driver it can take control of the ports by default and when a low/full speed device is connected switch the port to a companion controller. After looking into this for the past 6+ weeks, the port routing and control logic gets rather complex to implement in qemu.

To keep the implementation simple I propose keeping the UCHI/OHCI and EHCI buses implemented independently -- using the 0 option for the number of companion host controllers.

When USB devices are created they are assigned to a specific bus:

                .-------------------.
                |  device creation  |
                '-------------------'
                  /                \
    --------------------      --------------------
   |  UHCI controller   |    |  EHCI controller   |
    --------------------      --------------------
             |                         |
    --------------------      --------------------
   | Low/Full speed dev |    | High speed devices |
    --------------------      --------------------

qemu's emulated devices already know which USB version they are compatible with, so no need to probe for it. Host based devices can default to ehci (or uhci if preferred) and then use the speed information obtained from the scan to determine if the device should be attached to the uhci bus instead.

The changes for this design are fairly small and contained. Furthermore, it extends easily to USB 3.0 in the future.

Summary of the changes:
1. add a version based API in usb-bus.c for registering buses and creating devices

2. change the USB device models to attach to a bus based on version - v1 or v2.

3. If a bus is not specified (e.g., usb_create_simple path) default to v1 (for compatibility with existing code).

4. For PC's create a UCHI and EHCI controller in pc_piix.c

5. For host based devices default to ehci and use the speed information to determine if the device should be attached to the uhci bus.

The attached patch is a first cut at this path and has worked fairly well in limited testing. Take this as a proof of concept; I'll clean it up if the overall architecture is acceptable.

Example:
/tmp/qemu.latest/bin/qemu-system-x86_64 -m 1024 -smp 1 -drive file=/images/fc12/disk-1.img,cache=none,media=disk -monitor unix:/tmp/fc12-x86_64,server,nowait -enable-kvm -usb -usbdevice disk:/images/fc12/usb.img -usbdevice net:vlan=2 -net user,vlan=2 -usbdevice serial::telnet::7777,server,nowait -usbdevice host:2.16

(qemu) info usb
  Device 0.2, Speed 12 Mb/s, Product QEMU USB Network Interface
  Device 0.3, Speed 12 Mb/s, Product QEMU USB Hub
  Device 0.4, Speed 12 Mb/s, Product QEMU USB Serial
  Device 1.2, Speed 480 Mb/s, Product QEMU USB MSD
  Device 1.3, Speed 480 Mb/s, Product DT 101 II

Signed-off-by: David Ahern <daahern@cisco.com>
---
 hw/pc_piix.c    |    7 +----
 hw/usb-bus.c    |   70 +++++++++++++++++++++++++++++++++++++++++-------------
 hw/usb-ehci.c   |    6 ++++-
 hw/usb-msd.c    |    2 +-
 hw/usb-net.c    |    6 ++--
 hw/usb-ohci.c   |    2 +-
 hw/usb-serial.c |    4 +-
 hw/usb-uhci.c   |    2 +-
 hw/usb.h        |    8 ++++--
 usb-bsd.c       |    2 +-
 usb-linux.c     |   22 +++++++++++------
 11 files changed, 88 insertions(+), 43 deletions(-)
David S. Ahern - July 8, 2010, 1:46 p.m.
On 07/08/10 01:49, Jan Kiszka wrote:
> David Ahern wrote:
>> Per the EHCI specification a USB 2.0 host controller is comprised of one high-speed controller and 0 to N USB 1.1 companion host controllers (UHCI or OHCI) for low- and full-speed devices. Port routing and control logic determines which HC owns a particular port. See Sections 1.2 and 4.2 of the EHCI specification.
>>
>> http://www.intel.com/technology/usb/download/ehci-r10.pdf
>>
>> In essence a USB 2.0 bus has N ports. Those N ports can be controlled by either the companion controller or the ehci controller. The ports default to the companion controller. At boot if the OS has an EHCI host driver it can take control of the ports by default and when a low/full speed device is connected switch the port to a companion controller. After looking into this for the past 6+ weeks, the port routing and control logic gets rather complex to implement in qemu.
>>
>> To keep the implementation simple I propose keeping the UCHI/OHCI and EHCI buses implemented independently -- using the 0 option for the number of companion host controllers.
>>
>> When USB devices are created they are assigned to a specific bus:
>>
>>                 .-------------------.
>>                 |  device creation  |
>>                 '-------------------'
>>                   /                \
>>     --------------------      --------------------
>>    |  UHCI controller   |    |  EHCI controller   |
>>     --------------------      --------------------
>>              |                         |
>>     --------------------      --------------------
>>    | Low/Full speed dev |    | High speed devices |
>>     --------------------      --------------------
>>
>> qemu's emulated devices already know which USB version they are compatible with, so no need to probe for it. Host based devices can default to ehci (or uhci if preferred) and then use the speed information obtained from the scan to determine if the device should be attached to the uhci bus instead.
> 
> What about the case the guest does not use EHCI (or later on XHCI)? Can
> we avoid attaching device of higher speed to those buses then? And
> migrate them over once the guest disables EHCI (XHCI), e.g. by unloading
> the related module? Otherwise, those devices will be unusable for the
> guest IIUC.
> 
> Jan
> 

There really is no way of knowing if the guest loads or unloads the
kernel module. For example, recent seabios images configure and enable
the ehci controller -- and leave it enabled on the hand off to the guest
OS. Also, the guest driver enables and disables the periodic and
asynchronous lists as needed. Given that I'm not sure there is a way to
know 100% that the guest does not support ehci. Then there is the
complexity of moving devices between the USB buses as the driver is
loaded and unloaded.

As an alternative, what about enhancing the -usb option to note the
maximum version to enable: e.g., -usb v2[,v3]? -usb alone defaults to
uhci/ohci for compatibility with current design. Then ehci can be
enabled by the user. Enabling v2 means v1 is also enabled. Similarly
when v3 comes along -usb v3 means uhci/ohci, ehci and xhci are all enabled.

David
Jan Kiszka - July 8, 2010, 2:42 p.m.
David S. Ahern wrote:
> 
> On 07/08/10 01:49, Jan Kiszka wrote:
>> David Ahern wrote:
>>> Per the EHCI specification a USB 2.0 host controller is comprised of one high-speed controller and 0 to N USB 1.1 companion host controllers (UHCI or OHCI) for low- and full-speed devices. Port routing and control logic determines which HC owns a particular port. See Sections 1.2 and 4.2 of the EHCI specification.
>>>
>>> http://www.intel.com/technology/usb/download/ehci-r10.pdf
>>>
>>> In essence a USB 2.0 bus has N ports. Those N ports can be controlled by either the companion controller or the ehci controller. The ports default to the companion controller. At boot if the OS has an EHCI host driver it can take control of the ports by default and when a low/full speed device is connected switch the port to a companion controller. After looking into this for the past 6+ weeks, the port routing and control logic gets rather complex to implement in qemu.
>>>
>>> To keep the implementation simple I propose keeping the UCHI/OHCI and EHCI buses implemented independently -- using the 0 option for the number of companion host controllers.
>>>
>>> When USB devices are created they are assigned to a specific bus:
>>>
>>>                 .-------------------.
>>>                 |  device creation  |
>>>                 '-------------------'
>>>                   /                \
>>>     --------------------      --------------------
>>>    |  UHCI controller   |    |  EHCI controller   |
>>>     --------------------      --------------------
>>>              |                         |
>>>     --------------------      --------------------
>>>    | Low/Full speed dev |    | High speed devices |
>>>     --------------------      --------------------
>>>
>>> qemu's emulated devices already know which USB version they are compatible with, so no need to probe for it. Host based devices can default to ehci (or uhci if preferred) and then use the speed information obtained from the scan to determine if the device should be attached to the uhci bus instead.
>> What about the case the guest does not use EHCI (or later on XHCI)? Can
>> we avoid attaching device of higher speed to those buses then? And
>> migrate them over once the guest disables EHCI (XHCI), e.g. by unloading
>> the related module? Otherwise, those devices will be unusable for the
>> guest IIUC.
>>
>> Jan
>>
> 
> There really is no way of knowing if the guest loads or unloads the
> kernel module. For example, recent seabios images configure and enable
> the ehci controller -- and leave it enabled on the hand off to the guest
> OS.

We only need to behave like real HW does. If Seabios leaves the
controllers in an improper state, then it's a Seabios bug that can be
fixed independently.

> Also, the guest driver enables and disables the periodic and
> asynchronous lists as needed. Given that I'm not sure there is a way to
> know 100% that the guest does not support ehci.

According to quick glance at the spec, the logic to route a device to
the companion controller is !EHCI-configured || !port.EHCI-owned. So
detection should be a non-issue...

> Then there is the
> complexity of moving devices between the USB buses as the driver is
> loaded and unloaded.

...but I guess this is the actual problem. What makes moving devices
between buses so complex in QEMU?

> 
> As an alternative, what about enhancing the -usb option to note the
> maximum version to enable: e.g., -usb v2[,v3]? -usb alone defaults to
> uhci/ohci for compatibility with current design. Then ehci can be
> enabled by the user. Enabling v2 means v1 is also enabled. Similarly
> when v3 comes along -usb v3 means uhci/ohci, ehci and xhci are all enabled.

I think we either go for your current proposal as an intermediate
solution and fix the routing later on, or we find a way to do it
correctly on first run.

Jan
David S. Ahern - July 8, 2010, 3:33 p.m.
On 07/08/10 08:42, Jan Kiszka wrote:
> David S. Ahern wrote:
>>
>> On 07/08/10 01:49, Jan Kiszka wrote:
>>> David Ahern wrote:
>>>> Per the EHCI specification a USB 2.0 host controller is comprised of one high-speed controller and 0 to N USB 1.1 companion host controllers (UHCI or OHCI) for low- and full-speed devices. Port routing and control logic determines which HC owns a particular port. See Sections 1.2 and 4.2 of the EHCI specification.
>>>>
>>>> http://www.intel.com/technology/usb/download/ehci-r10.pdf
>>>>
>>>> In essence a USB 2.0 bus has N ports. Those N ports can be controlled by either the companion controller or the ehci controller. The ports default to the companion controller. At boot if the OS has an EHCI host driver it can take control of the ports by default and when a low/full speed device is connected switch the port to a companion controller. After looking into this for the past 6+ weeks, the port routing and control logic gets rather complex to implement in qemu.
>>>>
>>>> To keep the implementation simple I propose keeping the UCHI/OHCI and EHCI buses implemented independently -- using the 0 option for the number of companion host controllers.
>>>>
>>>> When USB devices are created they are assigned to a specific bus:
>>>>
>>>>                 .-------------------.
>>>>                 |  device creation  |
>>>>                 '-------------------'
>>>>                   /                \
>>>>     --------------------      --------------------
>>>>    |  UHCI controller   |    |  EHCI controller   |
>>>>     --------------------      --------------------
>>>>              |                         |
>>>>     --------------------      --------------------
>>>>    | Low/Full speed dev |    | High speed devices |
>>>>     --------------------      --------------------
>>>>
>>>> qemu's emulated devices already know which USB version they are compatible with, so no need to probe for it. Host based devices can default to ehci (or uhci if preferred) and then use the speed information obtained from the scan to determine if the device should be attached to the uhci bus instead.
>>> What about the case the guest does not use EHCI (or later on XHCI)? Can
>>> we avoid attaching device of higher speed to those buses then? And
>>> migrate them over once the guest disables EHCI (XHCI), e.g. by unloading
>>> the related module? Otherwise, those devices will be unusable for the
>>> guest IIUC.
>>>
>>> Jan
>>>
>>
>> There really is no way of knowing if the guest loads or unloads the
>> kernel module. For example, recent seabios images configure and enable
>> the ehci controller -- and leave it enabled on the hand off to the guest
>> OS.
> 
> We only need to behave like real HW does. If Seabios leaves the
> controllers in an improper state, then it's a Seabios bug that can be
> fixed independently.
> 
>> Also, the guest driver enables and disables the periodic and
>> asynchronous lists as needed. Given that I'm not sure there is a way to
>> know 100% that the guest does not support ehci.
> 
> According to quick glance at the spec, the logic to route a device to
> the companion controller is !EHCI-configured || !port.EHCI-owned. So
> detection should be a non-issue...

Per Section 2.2.3 if there are no companion controllers then the port
ownership handoff is not supported. The configured flag you mentioned
(Section 2.3.8) is not applicable if port routing is not supported. Port
routing as defined in the EHCI spec is the complexity part that I think
should be avoided.

> 
>> Then there is the
>> complexity of moving devices between the USB buses as the driver is
>> loaded and unloaded.
> 
> ...but I guess this is the actual problem. What makes moving devices
> between buses so complex in QEMU?

From a terminology perspective my reference to moving devices between
buses is separate from the ehci port routing which is way more complex
(a very subtle point).

In my first cut at this I was using the following to switch buses and it
works fine:

+void usb_device_migrate(USBDevice *dev, USBBus *bus)
+{
+    BusState *bus_state = &bus->qbus;
+
+    /* remove from current */
+    usb_device_detach(dev);
+
+    dev->qdev.parent_bus = bus_state;
+
+    /* add to given one */
+    if (bus->nfree == 1) {
+        usb_create_simple(bus, "usb-hub");
+    }
+    do_attach(dev);
+    return;
+}


> 
>>
>> As an alternative, what about enhancing the -usb option to note the
>> maximum version to enable: e.g., -usb v2[,v3]? -usb alone defaults to
>> uhci/ohci for compatibility with current design. Then ehci can be
>> enabled by the user. Enabling v2 means v1 is also enabled. Similarly
>> when v3 comes along -usb v3 means uhci/ohci, ehci and xhci are all enabled.
> 
> I think we either go for your current proposal as an intermediate
> solution and fix the routing later on, or we find a way to do it
> correctly on first run.

Moving devices from ehci to uhci and vice versa can be implemented later
if there is agreement that a simplified ehci model (ie, no companion
controllers) is acceptable.

David

> 
> Jan
>
Jan Kiszka - July 8, 2010, 4:35 p.m.
David S. Ahern wrote:
>>> As an alternative, what about enhancing the -usb option to note the
>>> maximum version to enable: e.g., -usb v2[,v3]? -usb alone defaults to
>>> uhci/ohci for compatibility with current design. Then ehci can be
>>> enabled by the user. Enabling v2 means v1 is also enabled. Similarly
>>> when v3 comes along -usb v3 means uhci/ohci, ehci and xhci are all enabled.
>> I think we either go for your current proposal as an intermediate
>> solution and fix the routing later on, or we find a way to do it
>> correctly on first run.
> 
> Moving devices from ehci to uhci and vice versa can be implemented later
> if there is agreement that a simplified ehci model (ie, no companion
> controllers) is acceptable.

Now I see clearer.

Zero-companion EHCIs are allowed by the spec, so I see no reason to not
support this option as well - and specifically make it the first step
towards full EHCI emulation. Adding companion support with port routing
to EHCI is indeed an exercise that can be done later on.

Regarding how to configure things: We only need a way to explicitly
specify the controller bus (or some to-be-implemented hub bus) that an
USB device shall be attached to. And that could become a qdev property.

We can still apply some smart selection logic to the legacy interfaces
as you suggested. But for anything special, there will be -device /
device_add with full control over the setup.

Jan

Patch

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index dffcbe1..934e336 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -153,11 +153,8 @@  static void pc_init1(ram_addr_t ram_size,
                  idebus[0], idebus[1], floppy_controller, rtc_state);
 
     if (pci_enabled && usb_enabled) {
-#if 0
         usb_uhci_piix3_init(pci_bus, piix3_devfn + 2);
-#else
-        usb_ehci_init(pci_bus, piix3_devfn + 2);
-#endif
+        usb_ehci_init(pci_bus, piix3_devfn + 3);
     }
 
     if (pci_enabled && acpi_enabled) {
@@ -167,7 +164,7 @@  static void pc_init1(ram_addr_t ram_size,
         cmos_s3 = qemu_allocate_irqs(pc_cmos_set_s3_resume, rtc_state, 1);
         smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1);
         /* TODO: Populate SPD eeprom data.  */
-        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
+        smbus = piix4_pm_init(pci_bus, piix3_devfn + 4, 0xb100,
                               isa_reserve_irq(9), *cmos_s3, *smi_irq,
                               kvm_enabled());
         for (i = 0; i < 8; i++) {
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index b692503..8d02169 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -14,29 +14,52 @@  static struct BusInfo usb_bus_info = {
 static int next_usb_bus = 0;
 static QTAILQ_HEAD(, USBBus) busses = QTAILQ_HEAD_INITIALIZER(busses);
 
-void usb_bus_new(USBBus *bus, DeviceState *host)
+void usb_bus_new(USBBus *bus, int version, DeviceState *host)
 {
     qbus_create_inplace(&bus->qbus, &usb_bus_info, host, NULL);
     bus->busnr = next_usb_bus++;
     bus->qbus.allow_hotplug = 1; /* Yes, we can */
+    bus->version = version;
     QTAILQ_INIT(&bus->free);
     QTAILQ_INIT(&bus->used);
     QTAILQ_INSERT_TAIL(&busses, bus, next);
 }
 
-USBBus *usb_bus_find(int busnr)
+static USBBus *usb_bus_find(int busnr)
 {
     USBBus *bus;
 
-    if (-1 == busnr)
-        return QTAILQ_FIRST(&busses);
+    if (-1 == busnr) {
+        /* no bus number given -- default to USB 1.1 */
+        bus = usb_bus_find_version(1);
+        if (!bus)
+            bus = QTAILQ_FIRST(&busses);
+
+    } else {
+        QTAILQ_FOREACH(bus, &busses, next) {
+            if (bus->busnr == busnr) {
+                break;
+            }
+        }
+    }
+    return bus;
+}
+
+/* device creation should be using this one */
+USBBus *usb_bus_find_version(int version)
+{
+    USBBus *bus, *match = NULL;
+
     QTAILQ_FOREACH(bus, &busses, next) {
-        if (bus->busnr == busnr)
-            return bus;
+        if (bus->version == version) {
+            match = bus;
+            break;
+        }
     }
-    return NULL;
+    return match;
 }
 
+
 static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
 {
     USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
@@ -80,20 +103,14 @@  void usb_qdev_register_many(USBDeviceInfo *info)
     }
 }
 
-USBDevice *usb_create(USBBus *bus, const char *name)
+static USBDevice *usb_create(USBBus *bus, const char *name)
 {
     DeviceState *dev;
 
-#if 1
-    /* temporary stopgap until all usb is properly qdev-ified */
     if (!bus) {
-        bus = usb_bus_find(-1);
-        if (!bus)
-            return NULL;
-        fprintf(stderr, "%s: no bus specified, using \"%s\" for \"%s\"\n",
-                __FUNCTION__, bus->qbus.name, name);
+        fprintf(stderr, "usb_create: no bus specified for %s\n", name);
+        return NULL;
     }
-#endif
 
     dev = qdev_create(&bus->qbus, name);
     return DO_UPCAST(USBDevice, qdev, dev);
@@ -101,7 +118,12 @@  USBDevice *usb_create(USBBus *bus, const char *name)
 
 USBDevice *usb_create_simple(USBBus *bus, const char *name)
 {
-    USBDevice *dev = usb_create(bus, name);
+    USBDevice *dev;
+   
+    if (!bus)
+        bus = usb_bus_find_version(1);
+
+    dev = usb_create(bus, name);
     if (!dev) {
         hw_error("Failed to create USB device '%s'\n", name);
     }
@@ -109,6 +131,20 @@  USBDevice *usb_create_simple(USBBus *bus, const char *name)
     return dev;
 }
 
+/* create USB device attached to USB 1.1 controller */
+USBDevice *usb_create_v1(const char *name)
+{
+    USBBus *bus = usb_bus_find_version(1);
+    return usb_create(bus, name);
+}
+
+/* create USB device attached to USB 2.0 controller */
+USBDevice *usb_create_v2(const char *name)
+{
+    USBBus *bus = usb_bus_find_version(2);
+    return usb_create(bus, name);
+}
+
 void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
                        usb_attachfn attach)
 {
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index ab9a23e..c6acd7e 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -778,20 +778,24 @@  static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
         break;
 
     case PERIODICLISTBASE:
+#if 0
         if (s->usbcmd & USBCMD_PSE) {
             fprintf(stderr, "Guest OS should not be setting the periodic"
                    " list base register while periodic schedule is enabled\n");
             return;
         }
+#endif
         DPRINTF("ehci_mem_writel: %s set to 0x%08X\n", str, val);
         break;
 
     case ASYNCLISTADDR:
+#if 0
         if (s->usbcmd & USBCMD_ASE) {
             fprintf(stderr, "Guest OS should not be setting the async list"
                    " address register while async schedule is enabled\n");
             return;
         }
+#endif
         DPRINTF("ehci_mem_writel: A-LIST ADDR set to 0x%08X\n", val);
         break;
     }
@@ -2044,7 +2048,7 @@  static int usb_ehci_initfn(PCIDevice *dev)
 
     // TODO come up with a better port allocation scheme
     // added ehci->bus, need to find ehci->DeviceState
-    usb_bus_new(&s->bus, &s->dev.qdev);
+    usb_bus_new(&s->bus, 2, &s->dev.qdev);
     for(i = 0; i < NB_PORTS; i++) {
         usb_register_port(&s->bus, &s->ports[i], s, i, ehci_attach);
         s->ports[i].dev = 0;
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 09a6a33..cf2485a 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -606,7 +606,7 @@  static USBDevice *usb_msd_init(const char *filename)
     }
 
     /* create guest device */
-    dev = usb_create(NULL /* FIXME */, "usb-storage");
+    dev = usb_create_v2("usb-storage");
     if (!dev) {
         return NULL;
     }
diff --git a/hw/usb-net.c b/hw/usb-net.c
index a43bd17..994d24a 100644
--- a/hw/usb-net.c
+++ b/hw/usb-net.c
@@ -94,11 +94,11 @@  enum usbstring_idx {
 static const uint8_t qemu_net_dev_descriptor[] = {
     0x12,			/*  u8 bLength; */
     USB_DT_DEVICE,		/*  u8 bDescriptorType; Device */
-    0x00, 0x02,			/*  u16 bcdUSB; v2.0 */
+    0x00, 0x01,			/*  u16 bcdUSB; v1.0 */
     USB_CLASS_COMM,		/*  u8  bDeviceClass; */
     0x00,			/*  u8  bDeviceSubClass; */
     0x00,			/*  u8  bDeviceProtocol; [ low/full only ] */
-    0x40,			/*  u8  bMaxPacketSize0 */
+    0x08,			/*  u8  bMaxPacketSize0 */
     RNDIS_VENDOR_NUM & 0xff, RNDIS_VENDOR_NUM >> 8,	/*  u16 idVendor; */
     RNDIS_PRODUCT_NUM & 0xff, RNDIS_PRODUCT_NUM >> 8,	/*  u16 idProduct; */
     0x00, 0x00,			/*  u16 bcdDevice */
@@ -1484,7 +1484,7 @@  static USBDevice *usb_net_init(const char *cmdline)
         return NULL;
     }
 
-    dev = usb_create(NULL /* FIXME */, "usb-net");
+    dev = usb_create_v1("usb-net");
     if (!dev) {
         return NULL;
     }
diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index c60fd8d..952777b 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -1702,7 +1702,7 @@  static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
 
     ohci->name = dev->info->name;
 
-    usb_bus_new(&ohci->bus, dev);
+    usb_bus_new(&ohci->bus, 1, dev);
     ohci->num_ports = num_ports;
     for (i = 0; i < num_ports; i++) {
         usb_register_port(&ohci->bus, &ohci->rhport[i].port, ohci, i, ohci_attach);
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index c19580f..f1c47b9 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -599,7 +599,7 @@  static USBDevice *usb_serial_init(const char *filename)
     if (!cdrv)
         return NULL;
 
-    dev = usb_create(NULL /* FIXME */, "usb-serial");
+    dev = usb_create_v1("usb-serial");
     if (!dev) {
         return NULL;
     }
@@ -622,7 +622,7 @@  static USBDevice *usb_braille_init(const char *unused)
     if (!cdrv)
         return NULL;
 
-    dev = usb_create(NULL /* FIXME */, "usb-braille");
+    dev = usb_create_v1("usb-braille");
     qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
     qdev_init_nofail(&dev->qdev);
 
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 4cdb55e..908685e 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -1114,7 +1114,7 @@  static int usb_uhci_common_initfn(UHCIState *s)
     pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
     pci_conf[0x60] = 0x10; // release number
 
-    usb_bus_new(&s->bus, &s->dev.qdev);
+    usb_bus_new(&s->bus, 1, &s->dev.qdev);
     for(i = 0; i < NB_PORTS; i++) {
         usb_register_port(&s->bus, &s->ports[i].port, s, i, uhci_attach);
     }
diff --git a/hw/usb.h b/hw/usb.h
index 00d2802..65dcfb8 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -299,16 +299,18 @@  struct USBBus {
     int busnr;
     int nfree;
     int nused;
+    int version;
     QTAILQ_HEAD(, USBPort) free;
     QTAILQ_HEAD(, USBPort) used;
     QTAILQ_ENTRY(USBBus) next;
 };
 
-void usb_bus_new(USBBus *bus, DeviceState *host);
-USBBus *usb_bus_find(int busnr);
+void usb_bus_new(USBBus *bus, int version, DeviceState *host);
+USBBus *usb_bus_find_version(int version);
+USBDevice *usb_create_v1(const char *name);
+USBDevice *usb_create_v2(const char *name);
 void usb_qdev_register(USBDeviceInfo *info);
 void usb_qdev_register_many(USBDeviceInfo *info);
-USBDevice *usb_create(USBBus *bus, const char *name);
 USBDevice *usb_create_simple(USBBus *bus, const char *name);
 USBDevice *usbdevice_create(const char *cmdline);
 void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
diff --git a/usb-bsd.c b/usb-bsd.c
index 48567a3..b2c3777 100644
--- a/usb-bsd.c
+++ b/usb-bsd.c
@@ -361,7 +361,7 @@  USBDevice *usb_host_device_open(const char *devname)
             goto fail;
         }
 
-        d = usb_create(NULL /* FIXME */, "usb-host");
+        d = usb_create_v1("usb-host");
         dev = DO_UPCAST(USBHostDevice, dev, d);
 
         if (dev_info.udi_speed == 1)
diff --git a/usb-linux.c b/usb-linux.c
index 4790973..1f6b4d3 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -910,7 +910,7 @@  static int usb_linux_update_endp_table(USBHostDevice *s)
 }
 
 static int usb_host_open(USBHostDevice *dev, int bus_num,
-                         int addr, const char *prod_name)
+                         int addr, const char *prod_name, int speed)
 {
     int fd = -1, ret;
     struct usbdevfs_connectinfo ci;
@@ -973,17 +973,22 @@  static int usb_host_open(USBHostDevice *dev, int bus_num,
         goto fail;
     }
 
-    printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
+    printf("husb: grabbed usb device %d.%d, %s speed\n",
+           bus_num, addr, speed == USB_SPEED_HIGH ? "high":"low/full");
 
     ret = usb_linux_update_endp_table(dev);
     if (ret) {
         goto fail;
     }
 
-    if (ci.slow) {
-        dev->dev.speed = USB_SPEED_LOW;
-    } else {
-        dev->dev.speed = USB_SPEED_HIGH;
+    dev->dev.speed = speed;
+    if (speed != USB_SPEED_HIGH) {
+        USBBus *bus = usb_bus_find_version(1);
+        if (bus) {
+            /* TO-DO: need qdev api for this */
+            BusState *bus_state = &bus->qbus;
+            dev->dev.qdev.parent_bus = bus_state;
+        }
     }
 
     if (!prod_name || prod_name[0] == '\0') {
@@ -1078,7 +1083,8 @@  USBDevice *usb_host_device_open(const char *devname)
     USBDevice *dev;
     char *p;
 
-    dev = usb_create(NULL /* FIXME */, "usb-host");
+    /* default host devices to ehci */
+    dev = usb_create_v2("usb-host");
 
     if (strstr(devname, "auto:")) {
         if (parse_filter(devname, &filter) < 0) {
@@ -1495,7 +1501,7 @@  static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
         }
         DPRINTF("husb: auto open: bus_num %d addr %d\n", bus_num, addr);
 
-        usb_host_open(s, bus_num, addr, product_name);
+        usb_host_open(s, bus_num, addr, product_name, speed);
     }
 
     return 0;