[Qemu-devel] [PATCH 0/8] qdev: allow embedded bus structs

[Qemu-devel] [PATCH 0/8] qdev: allow embedded bus structs

[Gerd Hoffmann]

  Hi,

This patch series allows to have qdev bus structs embedded into another
struct.  Typically that would be the state struct of the parent device.
The first patch provides the qdev infrastructure, the other ones which
over some bus implementations.

New in v2:
  Patch #2 which adds a qbus_free() function and winds it up correctly.

cheers,
  Gerd

[Qemu-devel] [PATCH 1/8] allow qdev busses allocations be inplace

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   15 ++++++++++++---
 hw/qdev.h |    3 +++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 43b1beb..43372c1 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -502,13 +502,12 @@ static BusState *qbus_find(const char *path)
     }
 }
 
-BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name)
+void qbus_create_inplace(BusState *bus, BusInfo *info,
+                         DeviceState *parent, const char *name)
 {
-    BusState *bus;
     char *buf;
     int i,len;
 
-    bus = qemu_mallocz(info->size);
     bus->info = info;
     bus->parent = parent;
 
@@ -537,6 +536,16 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name)
         QLIST_INSERT_HEAD(&parent->child_bus, bus, sibling);
         parent->num_child_bus++;
     }
+
+}
+
+BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name)
+{
+    BusState *bus;
+
+    bus = qemu_mallocz(info->size);
+    bus->qdev_allocated = 1;
+    qbus_create_inplace(bus, info, parent, name);
     return bus;
 }
 
diff --git a/hw/qdev.h b/hw/qdev.h
index 623ded5..ccc45b9 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -47,6 +47,7 @@ struct BusState {
     DeviceState *parent;
     BusInfo *info;
     const char *name;
+    int qdev_allocated;
     QLIST_HEAD(, DeviceState) children;
     QLIST_ENTRY(BusState) sibling;
 };
@@ -144,6 +145,8 @@ BusState *qdev_get_parent_bus(DeviceState *dev);
 
 /*** BUS API. ***/
 
+void qbus_create_inplace(BusState *bus, BusInfo *info,
+                         DeviceState *parent, const char *name);
 BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name);
 
 #define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/8] qdev: device free fixups.

[Gerd Hoffmann]

Two bug fixes:
 * When freeing a device we unregister stuff unconditionally,
   even in case we didn't register in the first place because
   the ->init() callback failed.
 * When freeing a device with child busses attached we don't
   zap the child bus (and the devices attached to it).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   35 +++++++++++++++++++++++++++++++----
 hw/qdev.h |    7 +++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 43372c1..4931da1 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -102,6 +102,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
     qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
     qdev_prop_set_compat(dev);
     QLIST_INSERT_HEAD(&bus->children, dev, sibling);
+    dev->state = DEV_STATE_CREATED;
     return dev;
 }
 
@@ -216,6 +217,7 @@ int qdev_init(DeviceState *dev)
 {
     int rc;
 
+    assert(dev->state == DEV_STATE_CREATED);
     rc = dev->info->init(dev, dev->info);
     if (rc < 0)
         return rc;
@@ -223,18 +225,27 @@ int qdev_init(DeviceState *dev)
         qemu_register_reset(dev->info->reset, dev);
     if (dev->info->vmsd)
         vmstate_register(-1, dev->info->vmsd, dev);
+    dev->state = DEV_STATE_INITIALIZED;
     return 0;
 }
 
 /* Unlink device from bus and free the structure.  */
 void qdev_free(DeviceState *dev)
 {
+    BusState *bus;
+
+    if (dev->state == DEV_STATE_INITIALIZED) {
+        while (dev->num_child_bus) {
+            bus = QLIST_FIRST(&dev->child_bus);
+            qbus_free(bus);
+        }
 #if 0 /* FIXME: need sane vmstate_unregister function */
-    if (dev->info->vmsd)
-        vmstate_unregister(dev->info->vmsd, dev);
+        if (dev->info->vmsd)
+            vmstate_unregister(dev->info->vmsd, dev);
 #endif
-    if (dev->info->reset)
-        qemu_unregister_reset(dev->info->reset, dev);
+        if (dev->info->reset)
+            qemu_unregister_reset(dev->info->reset, dev);
+    }
     QLIST_REMOVE(dev, sibling);
     qemu_free(dev);
 }
@@ -549,6 +560,22 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name)
     return bus;
 }
 
+void qbus_free(BusState *bus)
+{
+    DeviceState *dev;
+
+    while ((dev = QLIST_FIRST(&bus->children)) != NULL) {
+        qdev_free(dev);
+    }
+    if (bus->parent) {
+        QLIST_REMOVE(bus, sibling);
+        bus->parent->num_child_bus--;
+    }
+    if (bus->qdev_allocated) {
+        qemu_free(bus);
+    }
+}
+
 #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__)
 static void qbus_print(Monitor *mon, BusState *bus, int indent);
 
diff --git a/hw/qdev.h b/hw/qdev.h
index ccc45b9..c036aff 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -19,10 +19,16 @@ typedef struct BusState BusState;
 
 typedef struct BusInfo BusInfo;
 
+enum DevState {
+    DEV_STATE_CREATED = 1,
+    DEV_STATE_INITIALIZED,
+};
+
 /* This structure should not be accessed directly.  We declare it here
    so that it can be embedded in individual device state structures.  */
 struct DeviceState {
     const char *id;
+    enum DevState state;
     DeviceInfo *info;
     BusState *parent_bus;
     int num_gpio_out;
@@ -148,6 +154,7 @@ BusState *qdev_get_parent_bus(DeviceState *dev);
 void qbus_create_inplace(BusState *bus, BusInfo *info,
                          DeviceState *parent, const char *name);
 BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name);
+void qbus_free(BusState *bus);
 
 #define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/8] switch scsi bus to inplace allocation.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/esp.c        |   10 +++++-----
 hw/lsi53c895a.c |   12 ++++++------
 hw/scsi-bus.c   |    9 +++------
 hw/scsi-disk.h  |    4 ++--
 hw/usb-msd.c    |    6 +++---
 5 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index b5ddfb2..951770d 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -63,7 +63,7 @@ struct ESPState {
     uint8_t ti_buf[TI_BUFSZ];
     uint32_t sense;
     uint32_t dma;
-    SCSIBus *bus;
+    SCSIBus bus;
     SCSIDevice *current_dev;
     uint8_t cmdbuf[TI_BUFSZ];
     uint32_t cmdlen;
@@ -191,7 +191,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
         s->async_len = 0;
     }
 
-    if (target >= ESP_MAX_DEVS || !s->bus->devs[target]) {
+    if (target >= ESP_MAX_DEVS || !s->bus.devs[target]) {
         // No such drive
         s->rregs[ESP_RSTAT] = 0;
         s->rregs[ESP_RINTR] = INTR_DC;
@@ -199,7 +199,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
         esp_raise_irq(s);
         return 0;
     }
-    s->current_dev = s->bus->devs[target];
+    s->current_dev = s->bus.devs[target];
     return dmalen;
 }
 
@@ -692,8 +692,8 @@ static int esp_init1(SysBusDevice *dev)
 
     qdev_init_gpio_in(&dev->qdev, parent_esp_reset, 1);
 
-    s->bus = scsi_bus_new(&dev->qdev, 0, ESP_MAX_DEVS, esp_command_complete);
-    scsi_bus_legacy_handle_cmdline(s->bus);
+    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, esp_command_complete);
+    scsi_bus_legacy_handle_cmdline(&s->bus);
     return 0;
 }
 
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 62bdca8..7c45391 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -193,7 +193,7 @@ typedef struct {
      * 2 if processing DMA from lsi_execute_script.
      * 3 if a DMA operation is in progress.  */
     int waiting;
-    SCSIBus *bus;
+    SCSIBus bus;
     SCSIDevice *current_dev;
     int current_lun;
     /* The tag is a combination of the device ID and the SCSI tag.  */
@@ -585,7 +585,7 @@ static void lsi_reselect(LSIState *s, uint32_t tag)
     id = (tag >> 8) & 0xf;
     s->ssid = id | 0x80;
     DPRINTF("Reselected target %d\n", id);
-    s->current_dev = s->bus->devs[id];
+    s->current_dev = s->bus.devs[id];
     s->current_tag = tag;
     s->scntl1 |= LSI_SCNTL1_CON;
     lsi_set_phase(s, PHASE_MI);
@@ -1041,7 +1041,7 @@ again:
                 }
                 s->sstat0 |= LSI_SSTAT0_WOA;
                 s->scntl1 &= ~LSI_SCNTL1_IARB;
-                if (id >= LSI_MAX_DEVS || !s->bus->devs[id]) {
+                if (id >= LSI_MAX_DEVS || !s->bus.devs[id]) {
                     DPRINTF("Selected absent target %d\n", id);
                     lsi_script_scsi_interrupt(s, 0, LSI_SIST1_STO);
                     lsi_disconnect(s);
@@ -1052,7 +1052,7 @@ again:
                 /* ??? Linux drivers compain when this is set.  Maybe
                    it only applies in low-level mode (unimplemented).
                 lsi_script_scsi_interrupt(s, LSI_SIST0_CMP, 0); */
-                s->current_dev = s->bus->devs[id];
+                s->current_dev = s->bus.devs[id];
                 s->current_tag = id << 8;
                 s->scntl1 |= LSI_SCNTL1_CON;
                 if (insn & (1 << 3)) {
@@ -2178,8 +2178,8 @@ static int lsi_scsi_init(PCIDevice *dev)
 
     lsi_soft_reset(s);
 
-    s->bus = scsi_bus_new(&dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
-    scsi_bus_legacy_handle_cmdline(s->bus);
+    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
+    scsi_bus_legacy_handle_cmdline(&s->bus);
     register_savevm("lsiscsi", -1, 0, lsi_scsi_save, lsi_scsi_load, s);
     return 0;
 }
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 16afa05..881e363 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -15,17 +15,14 @@ static struct BusInfo scsi_bus_info = {
 static int next_scsi_bus;
 
 /* Create a scsi bus, and attach devices to it.  */
-SCSIBus *scsi_bus_new(DeviceState *host, int tcq, int ndev,
-                      scsi_completionfn complete)
+void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
+                  scsi_completionfn complete)
 {
-    SCSIBus *bus;
-
-    bus = FROM_QBUS(SCSIBus, qbus_create(&scsi_bus_info, host, NULL));
+    qbus_create_inplace(&bus->qbus, &scsi_bus_info, host, NULL);
     bus->busnr = next_scsi_bus++;
     bus->tcq = tcq;
     bus->ndev = ndev;
     bus->complete = complete;
-    return bus;
 }
 
 static int scsi_qdev_init(DeviceState *qdev, DeviceInfo *base)
diff --git a/hw/scsi-disk.h b/hw/scsi-disk.h
index febde44..b6b6c12 100644
--- a/hw/scsi-disk.h
+++ b/hw/scsi-disk.h
@@ -52,8 +52,8 @@ struct SCSIBus {
     SCSIDevice *devs[8];
 };
 
-SCSIBus *scsi_bus_new(DeviceState *host, int tcq, int ndev,
-                      scsi_completionfn complete);
+void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
+                  scsi_completionfn complete);
 void scsi_qdev_register(SCSIDeviceInfo *info);
 
 static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index aa0ce6a..6b9c8a5 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -45,7 +45,7 @@ typedef struct {
     uint32_t data_len;
     uint32_t residue;
     uint32_t tag;
-    SCSIBus *bus;
+    SCSIBus bus;
     DriveInfo *dinfo;
     SCSIDevice *scsi_dev;
     int result;
@@ -527,8 +527,8 @@ static int usb_msd_initfn(USBDevice *dev)
     }
 
     s->dev.speed = USB_SPEED_FULL;
-    s->bus = scsi_bus_new(&s->dev.qdev, 0, 1, usb_msd_command_complete);
-    s->scsi_dev = scsi_bus_legacy_add_drive(s->bus, s->dinfo, 0);
+    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
+    s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, s->dinfo, 0);
     usb_msd_handle_reset(dev);
     return 0;
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/8] switch usb bus to inplace allocation.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-bus.c  |    7 ++-----
 hw/usb-musb.c |    6 +++---
 hw/usb-ohci.c |    6 +++---
 hw/usb-uhci.c |    6 +++---
 hw/usb.h      |    2 +-
 5 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 03933f1..2cac1e8 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -14,16 +14,13 @@ static struct BusInfo usb_bus_info = {
 static int next_usb_bus = 0;
 static QTAILQ_HEAD(, USBBus) busses = QTAILQ_HEAD_INITIALIZER(busses);
 
-USBBus *usb_bus_new(DeviceState *host)
+void usb_bus_new(USBBus *bus, DeviceState *host)
 {
-    USBBus *bus;
-
-    bus = FROM_QBUS(USBBus, qbus_create(&usb_bus_info, host, NULL));
+    qbus_create_inplace(&bus->qbus, &usb_bus_info, host, NULL);
     bus->busnr = next_usb_bus++;
     QTAILQ_INIT(&bus->free);
     QTAILQ_INIT(&bus->used);
     QTAILQ_INSERT_TAIL(&busses, bus, next);
-    return bus;
 }
 
 USBBus *usb_bus_find(int busnr)
diff --git a/hw/usb-musb.c b/hw/usb-musb.c
index 9eb0d63..09ec5a1 100644
--- a/hw/usb-musb.c
+++ b/hw/usb-musb.c
@@ -281,7 +281,7 @@ typedef struct {
 
 struct MUSBState {
     qemu_irq *irqs;
-    USBBus *bus;
+    USBBus bus;
     USBPort port;
 
     int idx;
@@ -331,8 +331,8 @@ struct MUSBState {
         s->ep[i].epnum = i;
     }
 
-    s->bus = usb_bus_new(NULL /* FIXME */);
-    usb_register_port(s->bus, &s->port, s, 0, musb_attach);
+    usb_bus_new(&s->bus, NULL /* FIXME */);
+    usb_register_port(&s->bus, &s->port, s, 0, musb_attach);
 
     return s;
 }
diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index 6e428c4..48ccd49 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -65,7 +65,7 @@ enum ohci_type {
 };
 
 typedef struct {
-    USBBus *bus;
+    USBBus bus;
     qemu_irq irq;
     enum ohci_type type;
     int mem;
@@ -1690,10 +1690,10 @@ static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
     ohci->irq = irq;
     ohci->type = type;
 
-    ohci->bus = usb_bus_new(dev);
+    usb_bus_new(&ohci->bus, 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);
+        usb_register_port(&ohci->bus, &ohci->rhport[i].port, ohci, i, ohci_attach);
     }
 
     ohci->async_td = 0;
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 6807413..a3ed9b2 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -122,7 +122,7 @@ typedef struct UHCIPort {
 
 typedef struct UHCIState {
     PCIDevice dev;
-    USBBus *bus;
+    USBBus bus;
     uint16_t cmd; /* cmd register */
     uint16_t status;
     uint16_t intr; /* interrupt enable register */
@@ -1083,9 +1083,9 @@ static int usb_uhci_common_initfn(UHCIState *s)
     pci_conf[0x3d] = 4; // interrupt pin 3
     pci_conf[0x60] = 0x10; // release number
 
-    s->bus = usb_bus_new(&s->dev.qdev);
+    usb_bus_new(&s->bus, &s->dev.qdev);
     for(i = 0; i < NB_PORTS; i++) {
-        usb_register_port(s->bus, &s->ports[i].port, s, i, uhci_attach);
+        usb_register_port(&s->bus, &s->ports[i].port, s, i, uhci_attach);
     }
     s->frame_timer = qemu_new_timer(vm_clock, uhci_frame_timer, s);
 
diff --git a/hw/usb.h b/hw/usb.h
index 7c5cf83..467cddb 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -303,7 +303,7 @@ struct USBBus {
     QTAILQ_ENTRY(USBBus) next;
 };
 
-USBBus *usb_bus_new(DeviceState *host);
+void usb_bus_new(USBBus *bus, DeviceState *host);
 USBBus *usb_bus_find(int busnr);
 void usb_qdev_register(USBDeviceInfo *info);
 void usb_qdev_register_many(USBDeviceInfo *info);
-- 
1.6.2.5

[Qemu-devel] [PATCH 5/8] switch ide bus to inplace allocation.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/internal.h |    2 +-
 hw/ide/isa.c      |   24 ++++++++++++------------
 hw/ide/pci.c      |   42 +++++++++++++++++++++---------------------
 hw/ide/qdev.c     |    7 ++-----
 4 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 9df759d..029bf80 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -551,7 +551,7 @@ void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
 
 /* hw/ide/qdev.c */
-IDEBus *ide_bus_new(DeviceState *dev);
+void ide_bus_new(IDEBus *idebus, DeviceState *dev);
 IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
 
 #endif /* HW_IDE_INTERNAL_H */
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index d2fe0c0..3205f40 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -37,7 +37,7 @@
 
 typedef struct ISAIDEState {
     ISADevice dev;
-    IDEBus    *bus;
+    IDEBus    bus;
     uint32_t  iobase;
     uint32_t  iobase2;
     uint32_t  isairq;
@@ -48,18 +48,18 @@ static void isa_ide_save(QEMUFile* f, void *opaque)
 {
     ISAIDEState *s = opaque;
 
-    idebus_save(f, s->bus);
-    ide_save(f, &s->bus->ifs[0]);
-    ide_save(f, &s->bus->ifs[1]);
+    idebus_save(f, &s->bus);
+    ide_save(f, &s->bus.ifs[0]);
+    ide_save(f, &s->bus.ifs[1]);
 }
 
 static int isa_ide_load(QEMUFile* f, void *opaque, int version_id)
 {
     ISAIDEState *s = opaque;
 
-    idebus_load(f, s->bus, version_id);
-    ide_load(f, &s->bus->ifs[0], version_id);
-    ide_load(f, &s->bus->ifs[1], version_id);
+    idebus_load(f, &s->bus, version_id);
+    ide_load(f, &s->bus.ifs[0], version_id);
+    ide_load(f, &s->bus.ifs[1], version_id);
     return 0;
 }
 
@@ -67,10 +67,10 @@ static int isa_ide_initfn(ISADevice *dev)
 {
     ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
 
-    s->bus = ide_bus_new(&s->dev.qdev);
-    ide_init_ioport(s->bus, s->iobase, s->iobase2);
+    ide_bus_new(&s->bus, &s->dev.qdev);
+    ide_init_ioport(&s->bus, s->iobase, s->iobase2);
     isa_init_irq(dev, &s->irq, s->isairq);
-    ide_init2(s->bus, NULL, NULL, s->irq);
+    ide_init2(&s->bus, NULL, NULL, s->irq);
     register_savevm("isa-ide", 0, 3, isa_ide_save, isa_ide_load, s);
     return 0;
 };
@@ -90,9 +90,9 @@ int isa_ide_init(int iobase, int iobase2, int isairq,
 
     s = DO_UPCAST(ISAIDEState, dev, dev);
     if (hd0)
-        ide_create_drive(s->bus, 0, hd0);
+        ide_create_drive(&s->bus, 0, hd0);
     if (hd1)
-        ide_create_drive(s->bus, 1, hd1);
+        ide_create_drive(&s->bus, 1, hd1);
     return 0;
 }
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 89ecd44..bddf541 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -51,7 +51,7 @@
 
 typedef struct PCIIDEState {
     PCIDevice dev;
-    IDEBus *bus[2];
+    IDEBus bus[2];
     BMDMAState bmdma[2];
     int type; /* see IDE_TYPE_xxx */
     uint32_t secondary;
@@ -66,7 +66,7 @@ static void ide_map(PCIDevice *pci_dev, int region_num,
     IDEBus *bus;
 
     if (region_num <= 3) {
-        bus = d->bus[(region_num >> 1)];
+        bus = &d->bus[(region_num >> 1)];
         if (region_num & 1) {
             register_ioport_read(addr + 2, 1, 1, ide_status_read, bus);
             register_ioport_write(addr + 2, 1, 1, ide_cmd_write, bus);
@@ -252,9 +252,9 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num,
 
     for(i = 0;i < 2; i++) {
         BMDMAState *bm = &d->bmdma[i];
-        d->bus[i]->bmdma = bm;
+        d->bus[i].bmdma = bm;
         bm->pci_dev = DO_UPCAST(PCIIDEState, dev, pci_dev);
-        bm->bus = d->bus[i];
+        bm->bus = d->bus+i;
         qemu_add_vm_change_state_handler(ide_dma_restart_cb, bm);
 
         register_ioport_write(addr, 1, 1, bmdma_cmd_writeb, bm);
@@ -294,13 +294,13 @@ static void pci_ide_save(QEMUFile* f, void *opaque)
 
     /* per IDE interface data */
     for(i = 0; i < 2; i++) {
-        idebus_save(f, d->bus[i]);
+        idebus_save(f, d->bus+i);
     }
 
     /* per IDE drive data */
     for(i = 0; i < 2; i++) {
-        ide_save(f, &d->bus[i]->ifs[0]);
-        ide_save(f, &d->bus[i]->ifs[1]);
+        ide_save(f, &d->bus[i].ifs[0]);
+        ide_save(f, &d->bus[i].ifs[1]);
     }
 }
 
@@ -330,13 +330,13 @@ static int pci_ide_load(QEMUFile* f, void *opaque, int version_id)
 
     /* per IDE interface data */
     for(i = 0; i < 2; i++) {
-        idebus_load(f, d->bus[i], version_id);
+        idebus_load(f, d->bus+i, version_id);
     }
 
     /* per IDE drive data */
     for(i = 0; i < 2; i++) {
-        ide_load(f, &d->bus[i]->ifs[0], version_id);
-        ide_load(f, &d->bus[i]->ifs[1], version_id);
+        ide_load(f, &d->bus[i].ifs[0], version_id);
+        ide_load(f, &d->bus[i].ifs[1], version_id);
     }
     return 0;
 }
@@ -351,7 +351,7 @@ static void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table)
     for (i = 0; i < 4; i++) {
         if (hd_table[i] == NULL)
             continue;
-        ide_create_drive(d->bus[bus[i]], unit[i], hd_table[i]);
+        ide_create_drive(d->bus+bus[i], unit[i], hd_table[i]);
     }
 }
 
@@ -427,10 +427,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
     pci_conf[0x3d] = 0x01; // interrupt on pin 1
 
     irq = qemu_allocate_irqs(cmd646_set_irq, d, 2);
-    d->bus[0] = ide_bus_new(&d->dev.qdev);
-    d->bus[1] = ide_bus_new(&d->dev.qdev);
-    ide_init2(d->bus[0], NULL, NULL, irq[0]);
-    ide_init2(d->bus[1], NULL, NULL, irq[1]);
+    ide_bus_new(&d->bus[0], &d->dev.qdev);
+    ide_bus_new(&d->bus[1], &d->dev.qdev);
+    ide_init2(&d->bus[0], NULL, NULL, irq[0]);
+    ide_init2(&d->bus[1], NULL, NULL, irq[1]);
 
     register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
     qemu_register_reset(cmd646_reset, d);
@@ -482,13 +482,13 @@ static int pci_piix_ide_initfn(PCIIDEState *d)
 
     register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
 
-    d->bus[0] = ide_bus_new(&d->dev.qdev);
-    d->bus[1] = ide_bus_new(&d->dev.qdev);
-    ide_init_ioport(d->bus[0], 0x1f0, 0x3f6);
-    ide_init_ioport(d->bus[1], 0x170, 0x376);
+    ide_bus_new(&d->bus[0], &d->dev.qdev);
+    ide_bus_new(&d->bus[1], &d->dev.qdev);
+    ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
+    ide_init_ioport(&d->bus[1], 0x170, 0x376);
 
-    ide_init2(d->bus[0], NULL, NULL, isa_reserve_irq(14));
-    ide_init2(d->bus[1], NULL, NULL, isa_reserve_irq(15));
+    ide_init2(&d->bus[0], NULL, NULL, isa_reserve_irq(14));
+    ide_init2(&d->bus[1], NULL, NULL, isa_reserve_irq(15));
     return 0;
 }
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index d467e3a..c562bc6 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -29,12 +29,9 @@ static struct BusInfo ide_bus_info = {
     .size  = sizeof(IDEBus),
 };
 
-IDEBus *ide_bus_new(DeviceState *dev)
+void ide_bus_new(IDEBus *idebus, DeviceState *dev)
 {
-    IDEBus *idebus;
-
-    idebus = FROM_QBUS(IDEBus, qbus_create(&ide_bus_info, dev, NULL));
-    return idebus;
+    qbus_create_inplace(&idebus->qbus, &ide_bus_info, dev, NULL);
 }
 
 static int ide_qdev_init(DeviceState *qdev, DeviceInfo *base)
-- 
1.6.2.5

[Qemu-devel] [PATCH 6/8] support inplace allocation for pci bus, split irq init.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci.c |   43 +++++++++++++++++++++++++++++++++----------
 hw/pci.h |    5 +++++
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 64d70ed..ad508a5 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -98,24 +98,47 @@ static void pci_bus_reset(void *opaque)
     }
 }
 
-PCIBus *pci_register_bus(DeviceState *parent, const char *name,
-                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-                         void *irq_opaque, int devfn_min, int nirq)
+void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
+                         const char *name, int devfn_min)
 {
-    PCIBus *bus;
     static int nbus = 0;
 
-    bus = FROM_QBUS(PCIBus, qbus_create(&pci_bus_info, parent, name));
-    bus->set_irq = set_irq;
-    bus->map_irq = map_irq;
-    bus->irq_opaque = irq_opaque;
+    qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
     bus->devfn_min = devfn_min;
-    bus->nirq = nirq;
-    bus->irq_count = qemu_mallocz(nirq * sizeof(bus->irq_count[0]));
     bus->next = first_bus;
     first_bus = bus;
     vmstate_register(nbus++, &vmstate_pcibus, bus);
     qemu_register_reset(pci_bus_reset, bus);
+}
+
+PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min)
+{
+    PCIBus *bus;
+
+    bus = qemu_mallocz(sizeof(*bus));
+    bus->qbus.qdev_allocated = 1;
+    pci_bus_new_inplace(bus, parent, name, devfn_min);
+    return bus;
+}
+
+void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+                  void *irq_opaque, int nirq)
+{
+    bus->set_irq = set_irq;
+    bus->map_irq = map_irq;
+    bus->irq_opaque = irq_opaque;
+    bus->nirq = nirq;
+    bus->irq_count = qemu_mallocz(nirq * sizeof(bus->irq_count[0]));
+}
+
+PCIBus *pci_register_bus(DeviceState *parent, const char *name,
+                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+                         void *irq_opaque, int devfn_min, int nirq)
+{
+    PCIBus *bus;
+
+    bus = pci_bus_new(parent, name, devfn_min);
+    pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
     return bus;
 }
 
diff --git a/hw/pci.h b/hw/pci.h
index e7bf33a..33970fe 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -242,6 +242,11 @@ int pci_device_load(PCIDevice *s, QEMUFile *f);
 
 typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
+void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
+                         const char *name, int devfn_min);
+PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min);
+void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+                  void *irq_opaque, int nirq);
 PCIBus *pci_register_bus(DeviceState *parent, const char *name,
                          pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          void *irq_opaque, int devfn_min, int nirq);
-- 
1.6.2.5

[Qemu-devel] [PATCH 7/8] convert pci bridge to qdev

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci.c |   66 +++++++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index ad508a5..608792a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -142,18 +142,16 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
     return bus;
 }
 
-static PCIBus *pci_register_secondary_bus(PCIDevice *dev,
-                                          pci_map_irq_fn map_irq,
-                                          const char *name)
+static void pci_register_secondary_bus(PCIBus *bus,
+                                       PCIDevice *dev,
+                                       pci_map_irq_fn map_irq,
+                                       const char *name)
 {
-    PCIBus *bus;
-
-    bus = FROM_QBUS(PCIBus, qbus_create(&pci_bus_info, &dev->qdev, name));
+    qbus_create_inplace(&bus->qbus, &pci_bus_info, &dev->qdev, name);
     bus->map_irq = map_irq;
     bus->parent_dev = dev;
     bus->next = dev->bus->next;
     dev->bus->next = bus;
-    return bus;
 }
 
 int pci_bus_num(PCIBus *s)
@@ -858,7 +856,9 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
 
 typedef struct {
     PCIDevice dev;
-    PCIBus *bus;
+    PCIBus bus;
+    uint32_t vid;
+    uint32_t did;
 } PCIBridge;
 
 static void pci_bridge_write_config(PCIDevice *d,
@@ -867,7 +867,7 @@ static void pci_bridge_write_config(PCIDevice *d,
     PCIBridge *s = (PCIBridge *)d;
 
     pci_default_write_config(d, address, val, len);
-    s->bus->bus_num = d->config[PCI_SECONDARY_BUS];
+    s->bus.bus_num = d->config[PCI_SECONDARY_BUS];
 }
 
 PCIBus *pci_find_bus(int bus_num)
@@ -890,15 +890,12 @@ PCIDevice *pci_find_device(int bus_num, int slot, int function)
     return bus->devices[PCI_DEVFN(slot, function)];
 }
 
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
-                        pci_map_irq_fn map_irq, const char *name)
+static int pci_bridge_initfn(PCIDevice *dev)
 {
-    PCIBridge *s;
-    s = (PCIBridge *)pci_register_device(bus, name, sizeof(PCIBridge),
-                                         devfn, NULL, pci_bridge_write_config);
+    PCIBridge *s = DO_UPCAST(PCIBridge, dev, dev);
 
-    pci_config_set_vendor_id(s->dev.config, vid);
-    pci_config_set_device_id(s->dev.config, did);
+    pci_config_set_vendor_id(s->dev.config, s->vid);
+    pci_config_set_device_id(s->dev.config, s->did);
 
     s->dev.config[0x04] = 0x06; // command = bus master, pci mem
     s->dev.config[0x05] = 0x00;
@@ -911,9 +908,23 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
     s->dev.config[PCI_HEADER_TYPE] =
         PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
     s->dev.config[0x1E] = 0xa0; // secondary status
+    return 0;
+}
 
-    s->bus = pci_register_secondary_bus(&s->dev, map_irq, name);
-    return s->bus;
+PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
+                        pci_map_irq_fn map_irq, const char *name)
+{
+    PCIDevice *dev;
+    PCIBridge *s;
+
+    dev = pci_create_noinit(bus, devfn, "pci-bridge");
+    qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
+    qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
+    qdev_init(&dev->qdev);
+
+    s = DO_UPCAST(PCIBridge, dev, dev);
+    pci_register_secondary_bus(&s->bus, &s->dev, map_irq, name);
+    return &s->bus;
 }
 
 static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
@@ -1074,3 +1085,22 @@ static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
                        r->addr, r->addr + r->size - 1);
     }
 }
+
+static PCIDeviceInfo bridge_info = {
+    .qdev.name    = "pci-bridge",
+    .qdev.size    = sizeof(PCIBridge),
+    .init         = pci_bridge_initfn,
+    .config_write = pci_bridge_write_config,
+    .qdev.props   = (Property[]) {
+        DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
+        DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
+        DEFINE_PROP_END_OF_LIST(),
+    }
+};
+
+static void pci_register_devices(void)
+{
+    pci_qdev_register(&bridge_info);
+}
+
+device_init(pci_register_devices)
-- 
1.6.2.5

[Qemu-devel] [PATCH 8/8] piix_pci: kill PIIX3IrqState

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/piix_pci.c |   37 +++++++++++++++++--------------------
 1 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index edd6df0..0be3b02 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -36,18 +36,14 @@ typedef PCIHostState I440FXState;
 typedef struct PIIX3State {
     PCIDevice dev;
     int pci_irq_levels[4];
-} PIIX3State;
-
-typedef struct PIIX3IrqState {
-    PIIX3State *piix3;
     qemu_irq *pic;
-} PIIX3IrqState;
+} PIIX3State;
 
 struct PCII440FXState {
     PCIDevice dev;
     target_phys_addr_t isa_page_descs[384 / 4];
     uint8_t smm_enabled;
-    PIIX3IrqState *irq_state;
+    PIIX3State *piix3;
 };
 
 static void i440fx_addr_writel(void* opaque, uint32_t addr, uint32_t val)
@@ -167,7 +163,7 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
 
     if (version_id == 2)
         for (i = 0; i < 4; i++)
-            d->irq_state->piix3->pci_irq_levels[i] = qemu_get_be32(f);
+            d->piix3->pci_irq_levels[i] = qemu_get_be32(f);
 
     return 0;
 }
@@ -232,23 +228,24 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
     PCIBus *b;
     PCIDevice *d;
     I440FXState *s;
-    PIIX3IrqState *irq_state = qemu_malloc(sizeof(*irq_state));
+    PIIX3State *piix3;
 
-    irq_state->pic = pic;
     dev = qdev_create(NULL, "i440FX-pcihost");
     s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
-    b = pci_register_bus(&s->busdev.qdev, "pci.0",
-                         piix3_set_irq, pci_slot_get_pirq, irq_state, 0, 4);
+    b = pci_bus_new(&s->busdev.qdev, NULL, 0);
     s->bus = b;
     qdev_init(dev);
 
     d = pci_create_simple(b, 0, "i440FX");
     *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
-    (*pi440fx_state)->irq_state = irq_state;
 
-    irq_state->piix3 = DO_UPCAST(PIIX3State, dev,
+    piix3 = DO_UPCAST(PIIX3State, dev,
                                  pci_create_simple(b, -1, "PIIX3"));
-    *piix3_devfn = irq_state->piix3->dev.devfn;
+    piix3->pic = pic;
+    pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
+    (*pi440fx_state)->piix3 = piix3;
+
+    *piix3_devfn = piix3->dev.devfn;
 
     return b;
 }
@@ -258,22 +255,22 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
 static void piix3_set_irq(void *opaque, int irq_num, int level)
 {
     int i, pic_irq, pic_level;
-    PIIX3IrqState *irq_state = opaque;
+    PIIX3State *piix3 = opaque;
 
-    irq_state->piix3->pci_irq_levels[irq_num] = level;
+    piix3->pci_irq_levels[irq_num] = level;
 
     /* now we change the pic irq level according to the piix irq mappings */
     /* XXX: optimize */
-    pic_irq = irq_state->piix3->dev.config[0x60 + irq_num];
+    pic_irq = piix3->dev.config[0x60 + irq_num];
     if (pic_irq < 16) {
         /* The pic level is the logical OR of all the PCI irqs mapped
            to it */
         pic_level = 0;
         for (i = 0; i < 4; i++) {
-            if (pic_irq == irq_state->piix3->dev.config[0x60 + i])
-                pic_level |= irq_state->piix3->pci_irq_levels[i];
+            if (pic_irq == piix3->dev.config[0x60 + i])
+                pic_level |= piix3->pci_irq_levels[i];
         }
-        qemu_set_irq(irq_state->pic[pic_irq], pic_level);
+        qemu_set_irq(piix3->pic[pic_irq], pic_level);
     }
 }
 
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 0/8] qdev: allow embedded bus structs

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> This patch series allows to have qdev bus structs embedded into another
> struct.  Typically that would be the state struct of the parent device.
> The first patch provides the qdev infrastructure, the other ones which
> over some bus implementations.
>
> New in v2:
>   Patch #2 which adds a qbus_free() function and winds it up correctly.
>
> cheers,
>   Gerd

First five look good to me.  Gotta run, intend to check out the last
three later.