From 4806ec9b2c57ff42a91d5419ac1137fffd1c9fb5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:08 +0100 Subject: [PATCH 1/8] usb: usb_create() can't fail, drop useless error handling Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 4 ---- hw/usb/dev-bluetooth.c | 6 +----- hw/usb/dev-network.c | 3 --- hw/usb/dev-serial.c | 3 --- hw/usb/dev-storage.c | 3 --- 5 files changed, 1 insertion(+), 18 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 986b2d8da8..eeb6872324 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -320,10 +320,6 @@ USBDevice *usb_create_simple(USBBus *bus, const char *name) USBDevice *dev = usb_create(bus, name); int rc; - if (!dev) { - error_report("Failed to create USB device '%s'", name); - return NULL; - } rc = qdev_init(&dev->qdev); if (rc < 0) { error_report("Failed to initialize USB device '%s'", name); diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c index 390d475c16..8b3b31694b 100644 --- a/hw/usb/dev-bluetooth.c +++ b/hw/usb/dev-bluetooth.c @@ -530,14 +530,10 @@ static USBDevice *usb_bt_init(USBBus *bus, const char *cmdline) } else { hci = bt_new_hci(qemu_find_bt_vlan(0)); } - if (!hci) return NULL; + dev = usb_create(bus, name); - if (!dev) { - error_report("Failed to create USB device '%s'", name); - return NULL; - } s = DO_UPCAST(struct USBBtState, dev, dev); s->hci = hci; if (qdev_init(&dev->qdev) < 0) { diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c index 5b95d5c382..620fc116cf 100644 --- a/hw/usb/dev-network.c +++ b/hw/usb/dev-network.c @@ -1405,9 +1405,6 @@ static USBDevice *usb_net_init(USBBus *bus, const char *cmdline) } dev = usb_create(bus, "usb-net"); - if (!dev) { - return NULL; - } qdev_set_nic_properties(&dev->qdev, &nd_table[idx]); qdev_init_nofail(&dev->qdev); return dev; diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c index 1cee450259..f347c05b54 100644 --- a/hw/usb/dev-serial.c +++ b/hw/usb/dev-serial.c @@ -544,9 +544,6 @@ static USBDevice *usb_serial_init(USBBus *bus, const char *filename) return NULL; dev = usb_create(bus, "usb-serial"); - if (!dev) { - return NULL; - } qdev_prop_set_chr(&dev->qdev, "chardev", cdrv); if (vendorid) qdev_prop_set_uint16(&dev->qdev, "vendorid", vendorid); diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 4539733e42..5f222754ba 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -706,9 +706,6 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename) /* create guest device */ dev = usb_create(bus, "usb-storage"); - if (!dev) { - return NULL; - } if (qdev_prop_set_drive(&dev->qdev, "drive", blk_by_legacy_dinfo(dinfo)) < 0) { object_unparent(OBJECT(dev)); From 3bc36a401e0f33e63a4d2c58b646ddf78efb567c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:09 +0100 Subject: [PATCH 2/8] usb: Improve -usbdevice error reporting a bit Most LegacyUSBFactory usbdevice_init() methods realize with qdev_init_nofail(), even though their caller usbdevice_create() can handle failure. Okay if it really can't fail (I didn't check), but somewhat brittle. usb_msd_init() and usb_bt_init() call qdev_init(). The latter additionally reports an error when qdev_init() fails. Realization failure produces multiple error reports: a specific one from qdev_init(), and generic ones from usb_bt_init(), usb_create_simple(), usbdevice_create() and usb_parse(). Remove realization from the usbdevice_init() methods. Realize in usbdevice_create(), and produce exactly one error message there. You still get another one from usb_parse(). Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 22 +++++++++++++++++++--- hw/usb/dev-bluetooth.c | 5 ----- hw/usb/dev-network.c | 1 - hw/usb/dev-serial.c | 4 ---- hw/usb/dev-storage.c | 3 --- hw/usb/host-legacy.c | 1 - 6 files changed, 19 insertions(+), 17 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index eeb6872324..3f69fe1b23 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -651,10 +651,12 @@ USBDevice *usbdevice_create(const char *cmdline) { USBBus *bus = usb_bus_find(-1 /* any */); LegacyUSBFactory *f = NULL; + Error *err = NULL; GSList *i; char driver[32]; const char *params; int len; + USBDevice *dev; params = strchr(cmdline,':'); if (params) { @@ -689,14 +691,28 @@ USBDevice *usbdevice_create(const char *cmdline) return NULL; } - if (!f->usbdevice_init) { + if (f->usbdevice_init) { + dev = f->usbdevice_init(bus, params); + } else { if (*params) { error_report("usbdevice %s accepts no params", driver); return NULL; } - return usb_create_simple(bus, f->name); + dev = usb_create(bus, f->name); } - return f->usbdevice_init(bus, params); + if (!dev) { + error_report("Failed to create USB device '%s'", f->name); + return NULL; + } + object_property_set_bool(OBJECT(dev), true, "realized", &err); + if (err) { + error_report("Failed to initialize USB device '%s': %s", + f->name, error_get_pretty(err)); + error_free(err); + object_unparent(OBJECT(dev)); + return NULL; + } + return dev; } static void usb_device_class_init(ObjectClass *klass, void *data) diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c index 8b3b31694b..9bf673057a 100644 --- a/hw/usb/dev-bluetooth.c +++ b/hw/usb/dev-bluetooth.c @@ -536,11 +536,6 @@ static USBDevice *usb_bt_init(USBBus *bus, const char *cmdline) dev = usb_create(bus, name); s = DO_UPCAST(struct USBBtState, dev, dev); s->hci = hci; - if (qdev_init(&dev->qdev) < 0) { - error_report("Failed to initialize USB device '%s'", name); - return NULL; - } - return dev; } diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c index 620fc116cf..7131abdb21 100644 --- a/hw/usb/dev-network.c +++ b/hw/usb/dev-network.c @@ -1406,7 +1406,6 @@ static USBDevice *usb_net_init(USBBus *bus, const char *cmdline) dev = usb_create(bus, "usb-net"); qdev_set_nic_properties(&dev->qdev, &nd_table[idx]); - qdev_init_nofail(&dev->qdev); return dev; } diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c index f347c05b54..67c2072ce7 100644 --- a/hw/usb/dev-serial.c +++ b/hw/usb/dev-serial.c @@ -549,8 +549,6 @@ static USBDevice *usb_serial_init(USBBus *bus, const char *filename) qdev_prop_set_uint16(&dev->qdev, "vendorid", vendorid); if (productid) qdev_prop_set_uint16(&dev->qdev, "productid", productid); - qdev_init_nofail(&dev->qdev); - return dev; } @@ -565,8 +563,6 @@ static USBDevice *usb_braille_init(USBBus *bus, const char *unused) dev = usb_create(bus, "usb-braille"); qdev_prop_set_chr(&dev->qdev, "chardev", cdrv); - qdev_init_nofail(&dev->qdev); - return dev; } diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 5f222754ba..af2e1b915d 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -711,9 +711,6 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename) object_unparent(OBJECT(dev)); return NULL; } - if (qdev_init(&dev->qdev) < 0) - return NULL; - return dev; } diff --git a/hw/usb/host-legacy.c b/hw/usb/host-legacy.c index 3cc9c4282c..422ed9a65f 100644 --- a/hw/usb/host-legacy.c +++ b/hw/usb/host-legacy.c @@ -128,7 +128,6 @@ USBDevice *usb_host_device_open(USBBus *bus, const char *devname) qdev_prop_set_uint32(&dev->qdev, "hostaddr", filter.addr); qdev_prop_set_uint32(&dev->qdev, "vendorid", filter.vendor_id); qdev_prop_set_uint32(&dev->qdev, "productid", filter.product_id); - qdev_init_nofail(&dev->qdev); return dev; fail: From 06f22eb78f3eb557c667f5d0a46099e43a2aeb0d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:10 +0100 Subject: [PATCH 3/8] usb: Do not prefix error_setg() messages with "Error: " Because it produces beauties like (qemu) usb_add mouse Failed to initialize USB device 'usb-mouse': Error: tried to attach usb device QEMU USB Mouse to a bus with no free ports Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 3f69fe1b23..3e85afeff5 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -412,7 +412,7 @@ void usb_claim_port(USBDevice *dev, Error **errp) } } if (port == NULL) { - error_setg(errp, "Error: usb port %s (bus %s) not found (in use?)", + error_setg(errp, "usb port %s (bus %s) not found (in use?)", dev->port_path, bus->qbus.name); return; } @@ -422,7 +422,7 @@ void usb_claim_port(USBDevice *dev, Error **errp) usb_create_simple(bus, "usb-hub"); } if (bus->nfree == 0) { - error_setg(errp, "Error: tried to attach usb device %s to a bus " + error_setg(errp, "tried to attach usb device %s to a bus " "with no free ports", dev->product_desc); return; } From bd8b92d5c8387c2c94f06665514c05000169fafd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:11 +0100 Subject: [PATCH 4/8] usb: Suppress bogus error when automatic usb-hub creation fails USBDevice's realize method usb_qdev_realize() automatically creates a usb-hub when only one port is left. Creating devices in realize methods is questionable, but works. If usb-hub creation fails, an error is reported to stderr, but the failure is otherwise ignored. We then create the actual device using the last port, which may well succeed. Example: $ qemu -nodefaults -S -display none -machine usb=on -monitor stdio QEMU 2.2.50 monitor - type 'help' for more information (qemu) device_add usb-mouse [Repeat 36 times] (qemu) info usb Device 0.0, Port 1, Speed 12 Mb/s, Product QEMU USB Mouse Device 0.0, Port 2, Speed 12 Mb/s, Product QEMU USB Hub Device 0.0, Port 2.1, Speed 12 Mb/s, Product QEMU USB Mouse [More mice and hubs omitted...] Device 0.0, Port 2.8.8.8.8.7, Speed 12 Mb/s, Product QEMU USB Mouse (qemu) device_add usb-mouse usb hub chain too deep Failed to initialize USB device 'usb-hub' (qemu) info usb [...] Device 0.0, Port 2.8.8.8.8.7, Speed 12 Mb/s, Product QEMU USB Mouse Device 0.0, Port 2.8.8.8.8.8, Speed 12 Mb/s, Product QEMU USB Mouse Despite the "Failed" message, the command actually succeeded. In QMP, it's worse. When adding the 37th mouse via QMP, the command fails with {"error": {"class": "GenericError", "desc": "usb hub chain too deep"}} Additionally, "Failed to initialize USB device 'usb-hub'" is reported on stderr. Despite the command failure, the device was created. This is wrong. Fix by avoiding qdev_init() for usb-hub creation, so we can ignore errors cleanly. Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 3e85afeff5..5abfac0532 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -315,15 +315,36 @@ USBDevice *usb_create(USBBus *bus, const char *name) return USB_DEVICE(dev); } +static USBDevice *usb_try_create_simple(USBBus *bus, const char *name, + Error **errp) +{ + Error *err = NULL; + USBDevice *dev; + + dev = USB_DEVICE(qdev_try_create(&bus->qbus, name)); + if (!dev) { + error_setg(errp, "Failed to create USB device '%s'", name); + return NULL; + } + object_property_set_bool(OBJECT(dev), true, "realized", &err); + if (err) { + error_setg(errp, "Failed to initialize USB device '%s': %s", + name, error_get_pretty(err)); + error_free(err); + object_unparent(OBJECT(dev)); + return NULL; + } + return dev; +} + USBDevice *usb_create_simple(USBBus *bus, const char *name) { - USBDevice *dev = usb_create(bus, name); - int rc; + Error *err = NULL; + USBDevice *dev = usb_try_create_simple(bus, name, &err); - rc = qdev_init(&dev->qdev); - if (rc < 0) { - error_report("Failed to initialize USB device '%s'", name); - return NULL; + if (!dev) { + error_report("%s", error_get_pretty(err)); + error_free(err); } return dev; } @@ -419,7 +440,7 @@ void usb_claim_port(USBDevice *dev, Error **errp) } else { if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)), "usb-hub") != 0) { /* Create a new hub and chain it on */ - usb_create_simple(bus, "usb-hub"); + usb_try_create_simple(bus, "usb-hub", NULL); } if (bus->nfree == 0) { error_setg(errp, "tried to attach usb device %s to a bus " From 599655c91f3a55ac75007e851deca09010787bd7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:12 +0100 Subject: [PATCH 5/8] usb: Change usb_create_simple() to abort on failure Instead of returning null pointer. Matches pci_create_simple(), isa_create_simple(), sysbus_create_simple(). It's unused since the previous commit, but I'll put it to use again shortly. Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 5abfac0532..d83a93887f 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -339,14 +339,7 @@ static USBDevice *usb_try_create_simple(USBBus *bus, const char *name, USBDevice *usb_create_simple(USBBus *bus, const char *name) { - Error *err = NULL; - USBDevice *dev = usb_try_create_simple(bus, name, &err); - - if (!dev) { - error_report("%s", error_get_pretty(err)); - error_free(err); - } - return dev; + return usb_try_create_simple(bus, name, &error_abort); } static void usb_fill_port(USBPort *port, void *opaque, int index, From 456dcd8ab45a83dd51cca357cd9aa6ad337dc20d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:13 +0100 Subject: [PATCH 6/8] r2d: Don't use legacy -usbdevice support for setting up board It's tempting, because usbdevice_create() is so simple to use. But there's a lot of unwanted complexity behind the simple interface. Switch to usb_create_simple(). Cc: Magnus Damm Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/sh4/r2d.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c index 12f44d28f0..d1d0847ba2 100644 --- a/hw/sh4/r2d.c +++ b/hw/sh4/r2d.c @@ -301,7 +301,7 @@ static void r2d_init(MachineState *machine) "rtl8139", i==0 ? "2" : NULL); /* USB keyboard */ - usbdevice_create("keyboard"); + usb_create_simple(usb_bus_find(-1), "usb-kbd"); /* Todo: register on board registers */ memset(&boot_params, 0, sizeof(boot_params)); From c86580b889a0c22deba3afd4672472f23a1249d0 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:14 +0100 Subject: [PATCH 7/8] PPC: Don't use legacy -usbdevice support for setting up board It's tempting, because usbdevice_create() is so simple to use. But there's a lot of unwanted complexity behind the simple interface. Switch to usb_create_simple(). Cc: Alexander Graf Cc: qemu-ppc@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Alexander Graf Signed-off-by: Gerd Hoffmann --- hw/ppc/mac_newworld.c | 7 +++++-- hw/ppc/spapr.c | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index c3770121e2..624b4ab50b 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -420,11 +420,14 @@ static void ppc_core99_init(MachineState *machine) if (machine->usb) { pci_create_simple(pci_bus, -1, "pci-ohci"); + /* U3 needs to use USB for input because Linux doesn't support via-cuda on PPC64 */ if (machine_arch == ARCH_MAC99_U3) { - usbdevice_create("keyboard"); - usbdevice_create("mouse"); + USBBus *usb_bus = usb_bus_find(-1); + + usb_create_simple(usb_bus, "usb-kbd"); + usb_create_simple(usb_bus, "usb-mouse"); } } diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 812d03054d..a82a0f99b3 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1533,9 +1533,12 @@ static void ppc_spapr_init(MachineState *machine) if (machine->usb) { pci_create_simple(phb->bus, -1, "pci-ohci"); + if (spapr->has_graphics) { - usbdevice_create("keyboard"); - usbdevice_create("mouse"); + USBBus *usb_bus = usb_bus_find(-1); + + usb_create_simple(usb_bus, "usb-kbd"); + usb_create_simple(usb_bus, "usb-mouse"); } } From c3cf77cb63b71618224129df41f114488e0f74e4 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 18 Feb 2015 16:01:01 +1100 Subject: [PATCH 8/8] Make sysbus EHCI devices ARM only by default A number of ARM embedded boards include EHCI USB host controllers which appear as directly mapped devices, rather than sitting on a PCI bus. At present code to emulate such devices is included whenever EHCI support is included. This patch adjusts teh config options to only include them in builds targetting ARM by default. Signed-off-by: David Gibson Signed-off-by: Gerd Hoffmann --- default-configs/arm-softmmu.mak | 1 + hw/usb/Makefile.objs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 7671ee278a..b00c2e150e 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -33,6 +33,7 @@ CONFIG_PFLASH_CFI01=y CONFIG_PFLASH_CFI02=y CONFIG_MICRODRIVE=y CONFIG_USB_MUSB=y +CONFIG_USB_EHCI_SYSBUS=y CONFIG_ARM11MPCORE=y CONFIG_A9MPCORE=y diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs index 3fe4dff3bd..0ccd477577 100644 --- a/hw/usb/Makefile.objs +++ b/hw/usb/Makefile.objs @@ -5,7 +5,8 @@ common-obj-y += libhw.o # usb host adapters common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o common-obj-$(CONFIG_USB_OHCI) += hcd-ohci.o -common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o hcd-ehci-pci.o hcd-ehci-sysbus.o +common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o hcd-ehci-pci.o +common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci-sysbus.o common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o common-obj-$(CONFIG_USB_MUSB) += hcd-musb.o