[Qemu-devel] [PATCH 0/5] usb: make host adapters hot-pluggable

[Qemu-devel] [PATCH 0/5] usb: make host adapters hot-pluggable

[Gerd Hoffmann]

  Hi,

This series the usb host adapters hot-pluggable.  Problem is that we
can NOT hotplug host adapters which are part of a companion setup.  The
way this is tackled now is to split the usb host adapter variants into
two groups:

  * All ich9-* variants (both uhci and ehci) support companion controller
    and are NOT hot-pluggable.
  * All other variants do NOT support companion controller setups and
    can be hotplugged.

[ ohci is to be done, we have only one type there, so we have to create
  a new one and put the existing into one of the two groups,  /me wants
  collect feedback on the approach though. ]

Comments?  Other suggestions?

cheers,
  Gerd

Gerd Hoffmann (4):
  usb: tag xhci as hotpluggable
  usb: tag standalone uhci as hotpluggable
  usb: tag standalone ehci as hotpluggable
  [wip] usb: tag ohci as hotpluggable

Gonglei (1):
  tests: add usb hcds hotplugging qtest

 hw/usb/hcd-ehci-pci.c        | 12 ++++++-
 hw/usb/hcd-ehci.c            |  8 +++--
 hw/usb/hcd-ehci.h            |  1 +
 hw/usb/hcd-ohci.c            |  1 -
 hw/usb/hcd-uhci.c            | 16 +++++++--
 hw/usb/hcd-xhci.c            |  1 -
 tests/Makefile               |  2 ++
 tests/usb-hcd-hotplug-test.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 119 insertions(+), 8 deletions(-)
 create mode 100644 tests/usb-hcd-hotplug-test.c

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/5] usb: tag xhci as hotpluggable

[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 73ced1f..c556367 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3874,7 +3874,6 @@ static void xhci_class_init(ObjectClass *klass, void *data)
     dc->vmsd    = &vmstate_xhci;
     dc->props   = xhci_properties;
     dc->reset   = xhci_reset;
-    dc->hotpluggable   = false;
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
     k->init         = usb_xhci_initfn;
     k->exit         = usb_xhci_exit;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/5] usb: tag standalone uhci as hotpluggable

[Gerd Hoffmann]

uhci hostadapters in companion setups can't be hotplugged.  So leave
hotplug disabled for all ich9 variants (which are already tagged with
unplug = true in the info struct).  For the other variants we'll enable
hotplug and remove the companion setup properties.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 3b3ebcd..5b88f30 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1279,13 +1279,18 @@ static void usb_uhci_exit(PCIDevice *dev)
     }
 }
 
-static Property uhci_properties[] = {
+static Property uhci_properties_companion[] = {
     DEFINE_PROP_STRING("masterbus", UHCIState, masterbus),
     DEFINE_PROP_UINT32("firstport", UHCIState, firstport, 0),
     DEFINE_PROP_UINT32("bandwidth", UHCIState, frame_bandwidth, 1280),
     DEFINE_PROP_UINT32("maxframes", UHCIState, maxframes, 128),
     DEFINE_PROP_END_OF_LIST(),
 };
+static Property uhci_properties_standalone[] = {
+    DEFINE_PROP_UINT32("bandwidth", UHCIState, frame_bandwidth, 1280),
+    DEFINE_PROP_UINT32("maxframes", UHCIState, maxframes, 128),
+    DEFINE_PROP_END_OF_LIST(),
+};
 
 static void uhci_class_init(ObjectClass *klass, void *data)
 {
@@ -1300,9 +1305,14 @@ static void uhci_class_init(ObjectClass *klass, void *data)
     k->device_id = info->device_id;
     k->revision  = info->revision;
     k->class_id  = PCI_CLASS_SERIAL_USB;
-    dc->hotpluggable = false;
     dc->vmsd = &vmstate_uhci;
-    dc->props = uhci_properties;
+    if (!info->unplug) {
+        /* uhci controllers in companion setups can't be hotplugged */
+        dc->hotpluggable = false;
+        dc->props = uhci_properties_companion;
+    } else {
+        dc->props = uhci_properties_standalone;
+    }
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
     u->info = *info;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/5] usb: tag standalone ehci as hotpluggable

[Gerd Hoffmann]

Add a flag to EHCIPCIInfo saying whenever the controller supports
companions or not.  Make sure we only allow registering companions for
ehci versions supporting that.  Enable pci hotplug for the ehci
variants not supporting companions.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci-pci.c | 12 +++++++++++-
 hw/usb/hcd-ehci.c     |  8 ++++++--
 hw/usb/hcd-ehci.h     |  1 +
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 289ca3b..490f2b6 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -23,6 +23,7 @@ typedef struct EHCIPCIInfo {
     uint16_t vendor_id;
     uint16_t device_id;
     uint8_t  revision;
+    bool companion;
 } EHCIPCIInfo;
 
 static int usb_ehci_pci_initfn(PCIDevice *dev)
@@ -71,6 +72,7 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
 
 static void usb_ehci_pci_init(Object *obj)
 {
+    DeviceClass *dc = OBJECT_GET_CLASS(DeviceClass, obj, TYPE_DEVICE);
     EHCIPCIState *i = PCI_EHCI(obj);
     EHCIState *s = &i->ehci;
 
@@ -81,6 +83,10 @@ static void usb_ehci_pci_init(Object *obj)
     s->portscbase = 0x44;
     s->portnr = NB_PORTS;
 
+    if (!dc->hotpluggable) {
+        s->companion_enable = true;
+    }
+
     usb_ehci_init(s, DEVICE(obj));
 }
 
@@ -137,7 +143,6 @@ static void ehci_class_init(ObjectClass *klass, void *data)
     k->exit = usb_ehci_pci_exit;
     k->class_id = PCI_CLASS_SERIAL_USB;
     k->config_write = usb_ehci_pci_write_config;
-    dc->hotpluggable = false;
     dc->vmsd = &vmstate_ehci_pci;
     dc->props = ehci_pci_properties;
 }
@@ -161,6 +166,9 @@ static void ehci_data_class_init(ObjectClass *klass, void *data)
     k->device_id = i->device_id;
     k->revision = i->revision;
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
+    if (i->companion) {
+        dc->hotpluggable = false;
+    }
 }
 
 static struct EHCIPCIInfo ehci_pci_info[] = {
@@ -174,11 +182,13 @@ static struct EHCIPCIInfo ehci_pci_info[] = {
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_EHCI1,
         .revision  = 0x03,
+        .companion = true,
     },{
         .name      = "ich9-usb-ehci2", /* 00:1a.7 */
         .vendor_id = PCI_VENDOR_ID_INTEL,
         .device_id = PCI_DEVICE_ID_INTEL_82801I_EHCI2,
         .revision  = 0x03,
+        .companion = true,
     }
 };
 
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index bacb7ce..1cc0fc1 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2347,10 +2347,13 @@ static USBPortOps ehci_port_ops = {
     .complete = ehci_async_complete_packet,
 };
 
-static USBBusOps ehci_bus_ops = {
+static USBBusOps ehci_bus_ops_companion = {
     .register_companion = ehci_register_companion,
     .wakeup_endpoint = ehci_wakeup_endpoint,
 };
+static USBBusOps ehci_bus_ops_standalone = {
+    .wakeup_endpoint = ehci_wakeup_endpoint,
+};
 
 static void usb_ehci_pre_save(void *opaque)
 {
@@ -2456,7 +2459,8 @@ void usb_ehci_realize(EHCIState *s, DeviceState *dev, Error **errp)
         return;
     }
 
-    usb_bus_new(&s->bus, sizeof(s->bus), &ehci_bus_ops, dev);
+    usb_bus_new(&s->bus, sizeof(s->bus), s->companion_enable ?
+                &ehci_bus_ops_companion : &ehci_bus_ops_standalone, dev);
     for (i = 0; i < s->portnr; i++) {
         usb_register_port(&s->bus, &s->ports[i], s, i, &ehci_port_ops,
                           USB_SPEED_MASK_HIGH);
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 4858b7e..2bc259c 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -262,6 +262,7 @@ struct EHCIState {
     MemoryRegion mem_opreg;
     MemoryRegion mem_ports;
     int companion_count;
+    bool companion_enable;
     uint16_t capsbase;
     uint16_t opregbase;
     uint16_t portscbase;
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/5] [wip] usb: tag ohci as hotpluggable

[Gerd Hoffmann]

FIXME: turn off hotplug for companions

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

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 83bec34..8113b5a 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -2116,7 +2116,6 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
     dc->desc = "Apple USB Controller";
     dc->props = ohci_pci_properties;
-    dc->hotpluggable = false;
     dc->vmsd = &vmstate_ohci;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/5] tests: add usb hcds hotplugging qtest

[Gerd Hoffmann]

From: Gonglei <arei.gonglei@huawei.com>

Because of we now support usb hcds hotplugging, and
collect all hcds hotplugging tests into one file
for code sharing.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/Makefile               |  2 ++
 tests/usb-hcd-hotplug-test.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)
 create mode 100644 tests/usb-hcd-hotplug-test.c

diff --git a/tests/Makefile b/tests/Makefile
index d5db97b..7c4d0b7 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -165,6 +165,7 @@ gcov-files-i386-y += hw/usb/dev-hid.c
 gcov-files-i386-y += hw/usb/dev-storage.c
 check-qtest-i386-y += tests/usb-hcd-xhci-test$(EXESUF)
 gcov-files-i386-y += hw/usb/hcd-xhci.c
+check-qtest-i386-y += tests/usb-hcd-hotplug-test$(EXESUF)
 check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
@@ -345,6 +346,7 @@ tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o
 tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o
 tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-pc-obj-y)
 tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o
+tests/usb-hcd-hotplug-test$(EXESUF): tests/usb-hcd-hotplug-test.o
 tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
diff --git a/tests/usb-hcd-hotplug-test.c b/tests/usb-hcd-hotplug-test.c
new file mode 100644
index 0000000..8287b18
--- /dev/null
+++ b/tests/usb-hcd-hotplug-test.c
@@ -0,0 +1,86 @@
+/*
+ * QTest testcase for usb host adapters hotplug/unplug
+ *
+ * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO.,LTD.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include <string.h>
+#include "libqtest.h"
+#include "qemu/osdep.h"
+
+
+static void usb_hcd_hotplug(const char *hcd, const char *id)
+{
+    QDict *response;
+    char *bus;
+
+    qtest_start("");
+
+    /* hotplug an usb host adapter */
+    response = qmp("{ 'execute': 'device_add',"
+                   " 'arguments': { 'driver': %s,"
+                   " 'id': %s } }", hcd, id);
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    /* hotplug an usb-tablet to the usb host adapter, bus=$id.0 */
+    bus = g_strdup_printf("%s.0", id);
+    response = qmp("{ 'execute': 'device_add',"
+                   "  'arguments': { 'driver': 'usb-tablet',"
+                   "  'bus': %s } }", bus);
+    g_free(bus);
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    /* delete the usb host adapter */
+    response = qmp("{ 'execute': 'device_del',"
+                   " 'arguments': {"
+                   " 'id': %s } }", id);
+    g_assert(response);
+    g_assert(!qdict_haskey(response, "error"));
+    QDECREF(response);
+
+    qtest_end();
+}
+
+static void test_ohci_hotplug(void)
+{
+    usb_hcd_hotplug("pci-ohci", "ohci");
+}
+
+static void test_uhci_hotplug(void)
+{
+    usb_hcd_hotplug("piix3-usb-uhci", "uhci");
+}
+
+static void test_ehci_hotplug(void)
+{
+    usb_hcd_hotplug("usb-ehci", "ehci");
+}
+
+static void test_xhci_hotplug(void)
+{
+    usb_hcd_hotplug("nec-usb-xhci", "xhci");
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("/usb/hcd/pci/ohci-hotplug", test_ohci_hotplug);
+    qtest_add_func("/usb/hcd/pci/uhci-hotplug", test_uhci_hotplug);
+    qtest_add_func("/usb/hcd/pci/ehci-hotplug", test_ehci_hotplug);
+    qtest_add_func("/usb/hcd/pci/xhci-hotplug", test_xhci_hotplug);
+
+    ret = g_test_run();
+
+    return ret;
+}
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/5] usb: make host adapters hot-pluggable

[Gonglei (Arei)]

Hi,

> Subject: [Qemu-devel] [PATCH 0/5] usb: make host adapters hot-pluggable
> 
>   Hi,
> 
> This series the usb host adapters hot-pluggable.  Problem is that we
> can NOT hotplug host adapters which are part of a companion setup.  The
> way this is tackled now is to split the usb host adapter variants into
> two groups:
> 
>   * All ich9-* variants (both uhci and ehci) support companion controller
>     and are NOT hot-pluggable.
>   * All other variants do NOT support companion controller setups and
>     can be hotplugged.
> 
> [ ohci is to be done, we have only one type there, so we have to create
>   a new one and put the existing into one of the two groups,  /me wants
>   collect feedback on the approach though. ]
> 
> Comments?  Other suggestions?
> 

I agree with you. 

Because we must set 'dc->hotpluggable = false or true' at
$hci_class_init() to assure the host adapters whether supporting hotplugging or not.
And we can only pass parameters from Typeinfo struct's class_data property to
$hci_class_init() at present.

Best regards,
-Gonglei