Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-20150320-1' into staging

usb: bugfix collection.

# gpg: Signature made Fri Mar 20 07:51:19 2015 GMT using RSA key ID D3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>"
# gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>"
# gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>"

* remotes/kraxel/tags/pull-usb-20150320-1:
  ehci: fix segfault when hot-unplugging ehci controller
  ohci: fix resource cleanup leak
  uhci: fix segfault when hot-unplugging uhci controller
  hw/usb: Include USB files only if necessary
  usb/dev-storage: Avoid qerror_report_err() outside QMP handlers
  usb/dev-storage: Fix QMP device_add missing encryption key failure
  monitor usb: Inline monitor_read_bdrv_key_start()'s first part
  monitor: Plug memory leak in monitor_read_bdrv_key_start()
  monitor: Drop dead QMP check from monitor_read_password()
  uhci: Convert to realize
  ohci: Complete conversion to realize
  usb: Improve companion configuration error messages
  usb: Propagate errors through usb_register_companion()

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

9 files changed, 131 insertions, 86 deletions

diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 0ccd477577..7443e386b3 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -1,6 +1,6 @@
 # usb subsystem core
-common-obj-y += core.o combined-packet.o bus.o desc.o desc-msos.o
-common-obj-y += libhw.o
+common-obj-y += core.o combined-packet.o bus.o libhw.o
+common-obj-$(CONFIG_USB) += desc.o desc-msos.o
 
 # usb host adapters
 common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o
@@ -11,8 +11,8 @@ common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o
 common-obj-$(CONFIG_USB_MUSB) += hcd-musb.o
 
 # emulated usb devices
-common-obj-y += dev-hub.o
-common-obj-y += dev-hid.o
+common-obj-$(CONFIG_USB) += dev-hub.o
+common-obj-$(CONFIG_USB) += dev-hid.o
 common-obj-$(CONFIG_USB_TABLET_WACOM) += dev-wacom.o
 common-obj-$(CONFIG_USB_STORAGE_BOT)  += dev-storage.o
 common-obj-$(CONFIG_USB_STORAGE_UAS)  += dev-uas.o
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 91fc3e20d9..375167573d 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -360,9 +360,10 @@ void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
     bus->nfree++;
 }
 
-int usb_register_companion(const char *masterbus, USBPort *ports[],
-                           uint32_t portcount, uint32_t firstport,
-                           void *opaque, USBPortOps *ops, int speedmask)
+void usb_register_companion(const char *masterbus, USBPort *ports[],
+                            uint32_t portcount, uint32_t firstport,
+                            void *opaque, USBPortOps *ops, int speedmask,
+                            Error **errp)
 {
     USBBus *bus;
     int i;
@@ -373,22 +374,22 @@ int usb_register_companion(const char *masterbus, USBPort *ports[],
         }
     }
 
-    if (!bus || !bus->ops->register_companion) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "masterbus",
-                      "an USB masterbus");
-        if (bus) {
-            error_printf_unless_qmp(
-                "USB bus '%s' does not allow companion controllers\n",
-                masterbus);
-        }
-        return -1;
+    if (!bus) {
+        error_setg(errp, "USB bus '%s' not found", masterbus);
+        return;
+    }
+    if (!bus->ops->register_companion) {
+        error_setg(errp, "Can't use USB bus '%s' as masterbus,"
+                   " it doesn't support companion controllers",
+                   masterbus);
+        return;
     }
 
     for (i = 0; i < portcount; i++) {
         usb_fill_port(ports[i], opaque, i, ops, speedmask);
     }
 
-    return bus->ops->register_companion(bus, ports, portcount, firstport);
+    bus->ops->register_companion(bus, ports, portcount, firstport, errp);
 }
 
 void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr)
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index dacefd71a5..ae8d40dc77 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -559,8 +559,7 @@ static void usb_msd_password_cb(void *opaque, int err)
     }
 
     if (local_err) {
-        qerror_report_err(local_err);
-        error_free(local_err);
+        error_report_err(local_err);
         qdev_unplug(&s->dev.qdev, NULL);
     }
 }
@@ -610,6 +609,23 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
         return;
     }
 
+    bdrv_add_key(blk_bs(blk), NULL, &err);
+    if (err) {
+        if (monitor_cur_is_qmp()) {
+            error_propagate(errp, err);
+            return;
+        }
+        error_free(err);
+        err = NULL;
+        if (cur_mon) {
+            monitor_read_bdrv_key_start(cur_mon, blk_bs(blk),
+                                        usb_msd_password_cb, s);
+            s->dev.auto_attach = 0;
+        } else {
+            autostart = 0;
+        }
+    }
+
     blkconf_serial(&s->conf, &dev->serial);
     blkconf_blocksizes(&s->conf);
 
@@ -638,16 +654,6 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
     }
     usb_msd_handle_reset(dev);
     s->scsi_dev = scsi_dev;
-
-    if (bdrv_key_required(blk_bs(blk))) {
-        if (cur_mon) {
-            monitor_read_bdrv_key_start(cur_mon, blk_bs(blk),
-                                        usb_msd_password_cb, s);
-            s->dev.auto_attach = 0;
-        } else {
-            autostart = 0;
-        }
-    }
 }
 
 static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 4c80707f85..7afa5f9d67 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -101,6 +101,15 @@ static void usb_ehci_pci_exit(PCIDevice *dev)
     }
 }
 
+static void usb_ehci_pci_reset(DeviceState *dev)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(dev);
+    EHCIPCIState *i = PCI_EHCI(pci_dev);
+    EHCIState *s = &i->ehci;
+
+    ehci_reset(s);
+}
+
 static void usb_ehci_pci_write_config(PCIDevice *dev, uint32_t addr,
                                       uint32_t val, int l)
 {
@@ -143,6 +152,7 @@ static void ehci_class_init(ObjectClass *klass, void *data)
     k->config_write = usb_ehci_pci_write_config;
     dc->vmsd = &vmstate_ehci_pci;
     dc->props = ehci_pci_properties;
+    dc->reset = usb_ehci_pci_reset;
 }
 
 static const TypeInfo ehci_pci_type_info = {
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 19ed2c26aa..cd1cc142ab 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -42,6 +42,15 @@ static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
     sysbus_init_irq(d, &s->irq);
 }
 
+static void usb_ehci_sysbus_reset(DeviceState *dev)
+{
+    SysBusDevice *d = SYS_BUS_DEVICE(dev);
+    EHCISysBusState *i = SYS_BUS_EHCI(d);
+    EHCIState *s = &i->ehci;
+
+    ehci_reset(s);
+}
+
 static void ehci_sysbus_init(Object *obj)
 {
     SysBusDevice *d = SYS_BUS_DEVICE(obj);
@@ -70,6 +79,7 @@ static void ehci_sysbus_class_init(ObjectClass *klass, void *data)
     dc->realize = usb_ehci_sysbus_realize;
     dc->vmsd = &vmstate_ehci_sysbus;
     dc->props = ehci_sysbus_properties;
+    dc->reset = usb_ehci_sysbus_reset;
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index ccf54b6e09..d4d754765b 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -769,30 +769,26 @@ static void ehci_wakeup(USBPort *port)
     qemu_bh_schedule(s->async_bh);
 }
 
-static int ehci_register_companion(USBBus *bus, USBPort *ports[],
-                                   uint32_t portcount, uint32_t firstport)
+static void ehci_register_companion(USBBus *bus, USBPort *ports[],
+                                    uint32_t portcount, uint32_t firstport,
+                                    Error **errp)
 {
     EHCIState *s = container_of(bus, EHCIState, bus);
     uint32_t i;
 
     if (firstport + portcount > NB_PORTS) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "firstport",
-                      "firstport on masterbus");
-        error_printf_unless_qmp(
-            "firstport value of %u makes companion take ports %u - %u, which "
-            "is outside of the valid range of 0 - %u\n", firstport, firstport,
-            firstport + portcount - 1, NB_PORTS - 1);
-        return -1;
+        error_setg(errp, "firstport must be between 0 and %u",
+                   NB_PORTS - portcount);
+        return;
     }
 
     for (i = 0; i < portcount; i++) {
         if (s->companion_ports[firstport + i]) {
-            qerror_report(QERR_INVALID_PARAMETER_VALUE, "masterbus",
-                          "an USB masterbus");
-            error_printf_unless_qmp(
-                "port %u on masterbus %s already has a companion assigned\n",
-                firstport + i, bus->qbus.name);
-            return -1;
+            error_setg(errp, "firstport %u asks for ports %u-%u,"
+                       " but port %u has a companion assigned already",
+                       firstport, firstport, firstport + portcount - 1,
+                       firstport + i);
+            return;
         }
     }
 
@@ -806,8 +802,6 @@ static int ehci_register_companion(USBBus *bus, USBPort *ports[],
 
     s->companion_count++;
     s->caps[0x05] = (s->companion_count << 4) | portcount;
-
-    return 0;
 }
 
 static void ehci_wakeup_endpoint(USBBus *bus, USBEndpoint *ep,
@@ -845,7 +839,7 @@ static USBDevice *ehci_find_device(EHCIState *ehci, uint8_t addr)
 }
 
 /* 4.1 host controller initialization */
-static void ehci_reset(void *opaque)
+void ehci_reset(void *opaque)
 {
     EHCIState *s = opaque;
     int i;
@@ -2471,7 +2465,6 @@ void usb_ehci_realize(EHCIState *s, DeviceState *dev, Error **errp)
     s->async_bh = qemu_bh_new(ehci_frame_timer, s);
     s->device = dev;
 
-    qemu_register_reset(ehci_reset, s);
     s->vmstate = qemu_add_vm_change_state_handler(usb_ehci_vm_state_change, s);
 }
 
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 2bc259c9b4..87b240f70a 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -325,6 +325,7 @@ extern const VMStateDescription vmstate_ehci;
 void usb_ehci_init(EHCIState *s, DeviceState *dev);
 void usb_ehci_realize(EHCIState *s, DeviceState *dev, Error **errp);
 void usb_ehci_unrealize(EHCIState *s, DeviceState *dev, Error **errp);
+void ehci_reset(void *opaque);
 
 #define TYPE_PCI_EHCI "pci-ehci-usb"
 #define PCI_EHCI(obj) OBJECT_CHECK(EHCIPCIState, (obj), TYPE_PCI_EHCI)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index a0d478e63e..1a22c9c0cb 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1827,11 +1827,12 @@ static USBPortOps ohci_port_ops = {
 static USBBusOps ohci_bus_ops = {
 };
 
-static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
-                         int num_ports, dma_addr_t localmem_base,
-                         char *masterbus, uint32_t firstport,
-                         AddressSpace *as)
+static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
+                          int num_ports, dma_addr_t localmem_base,
+                          char *masterbus, uint32_t firstport,
+                          AddressSpace *as, Error **errp)
 {
+    Error *err = NULL;
     int i;
 
     ohci->as = as;
@@ -1857,10 +1858,13 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
         for(i = 0; i < num_ports; i++) {
             ports[i] = &ohci->rhport[i].port;
         }
-        if (usb_register_companion(masterbus, ports, num_ports,
-                firstport, ohci, &ohci_port_ops,
-                USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL) != 0) {
-            return -1;
+        usb_register_companion(masterbus, ports, num_ports,
+                               firstport, ohci, &ohci_port_ops,
+                               USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL,
+                               &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
         }
     } else {
         usb_bus_new(&ohci->bus, sizeof(ohci->bus), &ohci_bus_ops, dev);
@@ -1879,9 +1883,6 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
     usb_packet_init(&ohci->usb_packet);
 
     ohci->async_td = 0;
-    qemu_register_reset(ohci_reset, ohci);
-
-    return 0;
 }
 
 #define TYPE_PCI_OHCI "pci-ohci"
@@ -1914,22 +1915,24 @@ static void ohci_die(OHCIState *ohci)
                  PCI_STATUS_DETECTED_PARITY);
 }
 
-static int usb_ohci_initfn_pci(PCIDevice *dev)
+static void usb_ohci_realize_pci(PCIDevice *dev, Error **errp)
 {
+    Error *err = NULL;
     OHCIPCIState *ohci = PCI_OHCI(dev);
 
     dev->config[PCI_CLASS_PROG] = 0x10; /* OHCI */
     dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
 
-    if (usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
-                      ohci->masterbus, ohci->firstport,
-                      pci_get_address_space(dev)) != 0) {
-        return -1;
+    usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
+                  ohci->masterbus, ohci->firstport,
+                  pci_get_address_space(dev), &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
     }
-    ohci->state.irq = pci_allocate_irq(dev);
 
+    ohci->state.irq = pci_allocate_irq(dev);
     pci_register_bar(dev, 0, 0, &ohci->state.mem);
-    return 0;
 }
 
 static void usb_ohci_exit(PCIDevice *dev)
@@ -1951,6 +1954,15 @@ static void usb_ohci_exit(PCIDevice *dev)
     }
 }
 
+static void usb_ohci_reset_pci(DeviceState *d)
+{
+    PCIDevice *dev = PCI_DEVICE(d);
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+    OHCIState *s = &ohci->state;
+
+    ohci_reset(s);
+}
+
 #define TYPE_SYSBUS_OHCI "sysbus-ohci"
 #define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
 
@@ -1971,11 +1983,19 @@ static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 
     /* Cannot fail as we pass NULL for masterbus */
     usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
-                  &address_space_memory);
+                  &address_space_memory, &error_abort);
     sysbus_init_irq(sbd, &s->ohci.irq);
     sysbus_init_mmio(sbd, &s->ohci.mem);
 }
 
+static void usb_ohci_reset_sysbus(DeviceState *dev)
+{
+    OHCISysBusState *s = SYSBUS_OHCI(dev);
+    OHCIState *ohci = &s->ohci;
+
+    ohci_reset(ohci);
+}
+
 static Property ohci_pci_properties[] = {
     DEFINE_PROP_STRING("masterbus", OHCIPCIState, masterbus),
     DEFINE_PROP_UINT32("num-ports", OHCIPCIState, num_ports, 3),
@@ -2087,7 +2107,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = usb_ohci_initfn_pci;
+    k->realize = usb_ohci_realize_pci;
     k->exit = usb_ohci_exit;
     k->vendor_id = PCI_VENDOR_ID_APPLE;
     k->device_id = PCI_DEVICE_ID_APPLE_IPID_USB;
@@ -2097,6 +2117,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
     dc->props = ohci_pci_properties;
     dc->hotpluggable = false;
     dc->vmsd = &vmstate_ohci;
+    dc->reset = usb_ohci_reset_pci;
 }
 
 static const TypeInfo ohci_pci_info = {
@@ -2120,6 +2141,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
     dc->desc = "OHCI USB Controller";
     dc->props = ohci_sysbus_properties;
+    dc->reset = usb_ohci_reset_sysbus;
 }
 
 static const TypeInfo ohci_sysbus_info = {
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index f903de7072..327f26da70 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -66,7 +66,7 @@ struct UHCIInfo {
     uint16_t   device_id;
     uint8_t    revision;
     uint8_t    irq_pin;
-    int        (*initfn)(PCIDevice *dev);
+    void       (*realize)(PCIDevice *dev, Error **errp);
     bool       unplug;
 };
 
@@ -348,9 +348,10 @@ static void uhci_update_irq(UHCIState *s)
     pci_set_irq(&s->dev, level);
 }
 
-static void uhci_reset(void *opaque)
+static void uhci_reset(DeviceState *dev)
 {
-    UHCIState *s = opaque;
+    PCIDevice *d = PCI_DEVICE(dev);
+    UHCIState *s = DO_UPCAST(UHCIState, dev, d);
     uint8_t *pci_conf;
     int i;
     UHCIPort *port;
@@ -454,11 +455,11 @@ static void uhci_port_write(void *opaque, hwaddr addr,
                 port = &s->ports[i];
                 usb_device_reset(port->port.dev);
             }
-            uhci_reset(s);
+            uhci_reset(DEVICE(s));
             return;
         }
         if (val & UHCI_CMD_HCRESET) {
-            uhci_reset(s);
+            uhci_reset(DEVICE(s));
             return;
         }
         s->cmd = val;
@@ -1190,8 +1191,9 @@ static USBPortOps uhci_port_ops = {
 static USBBusOps uhci_bus_ops = {
 };
 
-static int usb_uhci_common_initfn(PCIDevice *dev)
+static void usb_uhci_common_realize(PCIDevice *dev, Error **errp)
 {
+    Error *err = NULL;
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
     UHCIPCIDeviceClass *u = container_of(pc, UHCIPCIDeviceClass, parent_class);
     UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
@@ -1209,10 +1211,13 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
         for(i = 0; i < NB_PORTS; i++) {
             ports[i] = &s->ports[i].port;
         }
-        if (usb_register_companion(s->masterbus, ports, NB_PORTS,
-                s->firstport, s, &uhci_port_ops,
-                USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL) != 0) {
-            return -1;
+        usb_register_companion(s->masterbus, ports, NB_PORTS,
+                               s->firstport, s, &uhci_port_ops,
+                               USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL,
+                               &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
         }
     } else {
         usb_bus_new(&s->bus, sizeof(s->bus), &uhci_bus_ops, DEVICE(dev));
@@ -1226,19 +1231,15 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
     s->num_ports_vmstate = NB_PORTS;
     QTAILQ_INIT(&s->queues);
 
-    qemu_register_reset(uhci_reset, s);
-
     memory_region_init_io(&s->io_bar, OBJECT(s), &uhci_ioport_ops, s,
                           "uhci", 0x20);
 
     /* Use region 4 for consistency with real hardware.  BSD guests seem
        to rely on this.  */
     pci_register_bar(&s->dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &s->io_bar);
-
-    return 0;
 }
 
-static int usb_uhci_vt82c686b_initfn(PCIDevice *dev)
+static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
 {
     UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
     uint8_t *pci_conf = s->dev.config;
@@ -1250,7 +1251,7 @@ static int usb_uhci_vt82c686b_initfn(PCIDevice *dev)
     /* USB legacy support  */
     pci_set_long(pci_conf + 0xc0,0x00002000);
 
-    return usb_uhci_common_initfn(dev);
+    usb_uhci_common_realize(dev, errp);
 }
 
 static void usb_uhci_exit(PCIDevice *dev)
@@ -1296,13 +1297,14 @@ static void uhci_class_init(ObjectClass *klass, void *data)
     UHCIPCIDeviceClass *u = container_of(k, UHCIPCIDeviceClass, parent_class);
     UHCIInfo *info = data;
 
-    k->init = info->initfn ? info->initfn : usb_uhci_common_initfn;
+    k->realize = info->realize ? info->realize : usb_uhci_common_realize;
     k->exit = info->unplug ? usb_uhci_exit : NULL;
     k->vendor_id = info->vendor_id;
     k->device_id = info->device_id;
     k->revision  = info->revision;
     k->class_id  = PCI_CLASS_SERIAL_USB;
     dc->vmsd = &vmstate_uhci;
+    dc->reset = uhci_reset;
     if (!info->unplug) {
         /* uhci controllers in companion setups can't be hotplugged */
         dc->hotpluggable = false;
@@ -1335,7 +1337,7 @@ static UHCIInfo uhci_info[] = {
         .device_id = PCI_DEVICE_ID_VIA_UHCI,
         .revision  = 0x01,
         .irq_pin   = 3,
-        .initfn    = usb_uhci_vt82c686b_initfn,
+        .realize   = usb_uhci_vt82c686b_realize,
         .unplug    = true,
     },{
         .name      = "ich9-usb-uhci1", /* 00:1d.0 */