[Qemu-devel] [PATCH 00/26] use realizefn for SysBusDevice, part 1

[Qemu-devel] [PATCH 00/26] use realizefn for SysBusDevice, part 1

[Hu Tao]

This series updates part of devices inheriting from SysbusDevice
to use DeviceState::realize, and QOM'ify them.

These devices are default to x86_64-softmmu. I'm planning to
make patches in the same manner, that is, each series is for
devices default to each target. After all devices are converted
to realizefn, SysBusDevice::init can be removed.


Hu Tao (26):
  ohci: use realize for ohci
  ohci: QOM'ify some more
  i440fx-pcihost: use realize for i440fx-pcihost
  i440fx: use type-safe cast instead of directly access of parent dev
  q35: use realize for q35 host
  q35: use type-safe cast instead of directly access of parent dev
  fdc: use realize for fdc.
  fdc: QOM'ify some more
  pflash_cfi01: use realize for pflash_cfi01
  pflash-cfi01: QOM'ify some more
  pflash_cfi02: use realize for pflash_cfi02
  pflash-cfi02: QOM'ify some more
  ahci: use realize for ahci
  ahci: QOM'ify some more
  fwcfg: use realize for fwcfg
  fwcfg: QOM'ify some more
  scsi esp: use realize for scsi esp
  scsi esp: QOM'ify some more
  hpet: use realize for hpet
  hpet: QOM'ify some more
  kvmclock: use realize for kvmclock
  kvmclock: QOM'ify some more
  kvmvapic realize
  ioapic: use realize for ioapic
  isa bus: use realize for isa bus
  ehci: use realize for ehci

 hw/block/fdc.c           | 84 ++++++++++++++++++++++++++++++------------------
 hw/block/pflash_cfi01.c  | 28 ++++++++--------
 hw/block/pflash_cfi02.c  | 31 +++++++++---------
 hw/i386/kvm/clock.c      | 17 +++++-----
 hw/i386/kvmvapic.c       | 12 +++----
 hw/ide/ahci.c            | 23 ++++++-------
 hw/intc/ioapic_common.c  | 10 +++---
 hw/isa/isa-bus.c         |  7 ++--
 hw/nvram/fw_cfg.c        | 38 +++++++++++++---------
 hw/pci-host/piix.c       | 36 +++++++++++++--------
 hw/pci-host/q35.c        | 60 +++++++++++++++++-----------------
 hw/scsi/esp.c            | 46 ++++++++++++++++----------
 hw/timer/hpet.c          | 43 +++++++++++++++----------
 hw/usb/hcd-ehci-sysbus.c | 11 +++----
 hw/usb/hcd-ohci.c        | 27 +++++++++-------
 15 files changed, 263 insertions(+), 210 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 01/26] ohci: use realize for ohci

[Hu Tao]

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/usb/hcd-ohci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 51241cd..79ef41b 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1876,17 +1876,16 @@ typedef struct {
     dma_addr_t dma_offset;
 } OHCISysBusState;
 
-static int ohci_init_pxa(SysBusDevice *dev)
+static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 {
-    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
+    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
 
     /* Cannot fail as we pass NULL for masterbus */
-    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
+    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
                   &dma_context_memory);
-    sysbus_init_irq(dev, &s->ohci.irq);
-    sysbus_init_mmio(dev, &s->ohci.mem);
-
-    return 0;
+    sysbus_init_irq(b, &s->ohci.irq);
+    sysbus_init_mmio(b, &s->ohci.mem);
 }
 
 static Property ohci_pci_properties[] = {
@@ -1926,9 +1925,8 @@ static Property ohci_sysbus_properties[] = {
 static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
 
-    sbc->init = ohci_init_pxa;
+    dc->realize = ohci_realize_pxa;
     dc->desc = "OHCI USB Controller";
     dc->props = ohci_sysbus_properties;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/26] ohci: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/usb/hcd-ohci.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 79ef41b..2025593 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1842,6 +1842,8 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
     return 0;
 }
 
+#define TYPE_PCI_OHCI "pci-ohci"
+#define PCI_OHCI(obj) OBJECT_CHECK(OHCIPCIState, (obj), TYPE_PCI_OHCI)
 typedef struct {
     PCIDevice pci_dev;
     OHCIState state;
@@ -1852,7 +1854,7 @@ typedef struct {
 
 static int usb_ohci_initfn_pci(struct PCIDevice *dev)
 {
-    OHCIPCIState *ohci = DO_UPCAST(OHCIPCIState, pci_dev, dev);
+    OHCIPCIState *ohci = PCI_OHCI(dev);
 
     ohci->pci_dev.config[PCI_CLASS_PROG] = 0x10; /* OHCI */
     ohci->pci_dev.config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
@@ -1869,6 +1871,9 @@ static int usb_ohci_initfn_pci(struct PCIDevice *dev)
     return 0;
 }
 
+#define TYPE_SYSBUS_OHCI "sysbus-ohci"
+#define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
+
 typedef struct {
     SysBusDevice busdev;
     OHCIState ohci;
@@ -1878,8 +1883,8 @@ typedef struct {
 
 static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 {
-    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);
-    SysBusDevice *b = SYS_BUS_DEVICE(dev);
+    OHCISysBusState *s = SYSBUS_OHCI(dev);
+    SysBusDevice *b= SYS_BUS_DEVICE(dev);
 
     /* Cannot fail as we pass NULL for masterbus */
     usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
@@ -1910,7 +1915,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ohci_pci_info = {
-    .name          = "pci-ohci",
+    .name          = TYPE_PCI_OHCI,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(OHCIPCIState),
     .class_init    = ohci_pci_class_init,
@@ -1932,7 +1937,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ohci_sysbus_info = {
-    .name          = "sysbus-ohci",
+    .name          = TYPE_SYSBUS_OHCI,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(OHCISysBusState),
     .class_init    = ohci_sysbus_class_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/26] i440fx-pcihost: use realize for i440fx-pcihost

[Hu Tao]

Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/piix.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9e68c3..801341a 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -197,21 +197,26 @@ static const VMStateDescription vmstate_i440fx = {
     }
 };
 
-static int i440fx_pcihost_initfn(SysBusDevice *dev)
+static void i440fx_pcihost_initfn(Object *obj)
 {
-    PCIHostState *s = PCI_HOST_BRIDGE(dev);
+    PCIHostState *s = PCI_HOST_BRIDGE(obj);
 
     memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s,
                           "pci-conf-idx", 4);
-    sysbus_add_io(dev, 0xcf8, &s->conf_mem);
-    sysbus_init_ioports(&s->busdev, 0xcf8, 4);
-
     memory_region_init_io(&s->data_mem, &pci_host_data_le_ops, s,
                           "pci-conf-data", 4);
-    sysbus_add_io(dev, 0xcfc, &s->data_mem);
-    sysbus_init_ioports(&s->busdev, 0xcfc, 4);
+}
 
-    return 0;
+static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
+{
+    PCIHostState *s = PCI_HOST_BRIDGE(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
+
+    sysbus_add_io(b, 0xcf8, &s->conf_mem);
+    sysbus_init_ioports(b, 0xcf8, 4);
+
+    sysbus_add_io(b, 0xcfc, &s->data_mem);
+    sysbus_init_ioports(b, 0xcfc, 4);
 }
 
 static int i440fx_initfn(PCIDevice *dev)
@@ -632,9 +637,8 @@ static const TypeInfo i440fx_info = {
 static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = i440fx_pcihost_initfn;
+    dc->realize = i440fx_pcihost_realize;
     dc->fw_name = "pci";
     dc->no_user = 1;
 }
@@ -643,6 +647,7 @@ static const TypeInfo i440fx_pcihost_info = {
     .name          = "i440FX-pcihost",
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(I440FXState),
+    .instance_init = i440fx_pcihost_initfn,
     .class_init    = i440fx_pcihost_class_init,
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/26] i440fx: use type-safe cast instead of directly access of parent dev

[Hu Tao]

Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/piix.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 801341a..e2437df 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i;
+    PCIDevice *pd = PCI_DEVICE(d);
 
     memory_region_transaction_begin();
     for (i = 0; i < 13; i++) {
         pam_update(&d->pam_regions[i], i,
-                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
+                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
     }
-    smram_update(&d->smram_region, d->dev.config[I440FX_SMRAM], d->smm_enabled);
+    smram_update(&d->smram_region, pd->config[I440FX_SMRAM], d->smm_enabled);
     memory_region_transaction_commit();
 }
 
 static void i440fx_set_smm(int val, void *arg)
 {
     PCII440FXState *d = arg;
+    PCIDevice *pd = PCI_DEVICE(d);
 
     memory_region_transaction_begin();
-    smram_set_smm(&d->smm_enabled, val, d->dev.config[I440FX_SMRAM],
+    smram_set_smm(&d->smm_enabled, val, pd->config[I440FX_SMRAM],
                   &d->smram_region);
     memory_region_transaction_commit();
 }
@@ -158,9 +160,10 @@ static void i440fx_write_config(PCIDevice *dev,
 static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
 {
     PCII440FXState *d = opaque;
+    PCIDevice *pd = PCI_DEVICE(d);
     int ret, i;
 
-    ret = pci_device_load(&d->dev, f);
+    ret = pci_device_load(pd, f);
     if (ret < 0)
         return ret;
     i440fx_update_memory_mappings(d);
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/26] q35: use realize for q35 host

[Hu Tao]

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Isaku Yamahata <yamahata@valinux.co.jp>
Cc: Jason Baron <jbaron@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/q35.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 24df6b5..bbecee6 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -34,33 +34,27 @@
  * Q35 host
  */
 
-static int q35_host_init(SysBusDevice *dev)
+static void q35_host_realize(DeviceState *dev, Error **errp)
 {
-    PCIBus *b;
-    PCIHostState *pci = FROM_SYSBUS(PCIHostState, dev);
-    Q35PCIHost *s = Q35_HOST_DEVICE(&dev->qdev);
+    PCIHostState *pci = DO_UPCAST(PCIHostState, busdev.qdev, dev);
+    Q35PCIHost *s = Q35_HOST_DEVICE(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
 
-    memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci,
-                          "pci-conf-idx", 4);
-    sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
-    sysbus_init_ioports(&pci->busdev, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
+    sysbus_add_io(b, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
+    sysbus_init_ioports(b, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
 
-    memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci,
-                          "pci-conf-data", 4);
-    sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
-    sysbus_init_ioports(&pci->busdev, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
+    sysbus_add_io(b, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
+    sysbus_init_ioports(b, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
-    if (pcie_host_init(&s->host) < 0) {
-        return -1;
+    if (pcie_host_init(PCIE_HOST_BRIDGE(pci)) < 0) {
+        error_setg(errp, "failed to initialize pcie host");
+        return;
     }
-    b = pci_bus_new(&s->host.pci.busdev.qdev, "pcie.0",
-                    s->mch.pci_address_space, s->mch.address_space_io,
-                    0, TYPE_PCIE_BUS);
-    s->host.pci.bus = b;
-    qdev_set_parent_bus(DEVICE(&s->mch), BUS(b));
+    pci->bus = pci_bus_new(dev, "pcie.0",
+                           s->mch.pci_address_space, s->mch.address_space_io,
+                           0, TYPE_PCIE_BUS);
+    qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->mch));
-
-    return 0;
 }
 
 static Property mch_props[] = {
@@ -72,9 +66,8 @@ static Property mch_props[] = {
 static void q35_host_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = q35_host_init;
+    dc->realize = q35_host_realize;
     dc->props = mch_props;
     dc->fw_name = "pci";
 }
@@ -82,6 +75,12 @@ static void q35_host_class_init(ObjectClass *klass, void *data)
 static void q35_host_initfn(Object *obj)
 {
     Q35PCIHost *s = Q35_HOST_DEVICE(obj);
+    PCIHostState *pci = PCI_HOST_BRIDGE(obj);
+
+    memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci,
+                          "pci-conf-idx", 4);
+    memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci,
+                          "pci-conf-data", 4);
 
     object_initialize(&s->mch, TYPE_MCH_PCI_DEVICE);
     object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL);
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/26] q35: use type-safe cast instead of directly access of parent dev

[Hu Tao]

And remove variables if possible.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Jason Baron <jbaron@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/q35.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index bbecee6..bb5d506 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -36,7 +36,7 @@
 
 static void q35_host_realize(DeviceState *dev, Error **errp)
 {
-    PCIHostState *pci = DO_UPCAST(PCIHostState, busdev.qdev, dev);
+    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     Q35PCIHost *s = Q35_HOST_DEVICE(dev);
     SysBusDevice *b = SYS_BUS_DEVICE(dev);
 
@@ -104,9 +104,8 @@ static const TypeInfo q35_host_info = {
 static void mch_update_pciexbar(MCHPCIState *mch)
 {
     PCIDevice *pci_dev = &mch->d;
-    BusState *bus = qdev_get_parent_bus(&pci_dev->qdev);
+    BusState *bus = qdev_get_parent_bus(DEVICE(pci_dev));
     DeviceState *qdev = bus->parent;
-    Q35PCIHost *s = Q35_HOST_DEVICE(qdev);
 
     uint64_t pciexbar;
     int enable;
@@ -138,18 +137,19 @@ static void mch_update_pciexbar(MCHPCIState *mch)
         break;
     }
     addr = pciexbar & addr_mask;
-    pcie_host_mmcfg_update(&s->host, enable, addr, length);
+    pcie_host_mmcfg_update(PCIE_HOST_BRIDGE(qdev), enable, addr, length);
 }
 
 /* PAM */
 static void mch_update_pam(MCHPCIState *mch)
 {
+    PCIDevice *pd = PCI_DEVICE(mch);
     int i;
 
     memory_region_transaction_begin();
     for (i = 0; i < 13; i++) {
         pam_update(&mch->pam_regions[i], i,
-                   mch->d.config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
+                   pd->config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
     }
     memory_region_transaction_commit();
 }
@@ -157,8 +157,10 @@ static void mch_update_pam(MCHPCIState *mch)
 /* SMRAM */
 static void mch_update_smram(MCHPCIState *mch)
 {
+    PCIDevice *pd = PCI_DEVICE(mch);
+
     memory_region_transaction_begin();
-    smram_update(&mch->smram_region, mch->d.config[MCH_HOST_BRDIGE_SMRAM],
+    smram_update(&mch->smram_region, pd->config[MCH_HOST_BRDIGE_SMRAM],
                     mch->smm_enabled);
     memory_region_transaction_commit();
 }
@@ -166,9 +168,10 @@ static void mch_update_smram(MCHPCIState *mch)
 static void mch_set_smm(int smm, void *arg)
 {
     MCHPCIState *mch = arg;
+    PCIDevice *pd = PCI_DEVICE(mch);
 
     memory_region_transaction_begin();
-    smram_set_smm(&mch->smm_enabled, smm, mch->d.config[MCH_HOST_BRDIGE_SMRAM],
+    smram_set_smm(&mch->smm_enabled, smm, pd->config[MCH_HOST_BRDIGE_SMRAM],
                     &mch->smram_region);
     memory_region_transaction_commit();
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/26] fdc: use realize for fdc.

[Hu Tao]

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/fdc.c | 60 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 930f191..8b8f60e 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2143,37 +2143,55 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
     add_boot_device_path(isa->bootindexB, dev, "/floppy@1");
 }
 
-static int sysbus_fdc_init1(SysBusDevice *dev)
+static void sysbus_fdc_initfn(Object *obj)
 {
-    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev, dev);
+    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev.parent_obj, obj);
     FDCtrl *fdctrl = &sys->state;
-    int ret;
 
     memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_ops, fdctrl, "fdc", 0x08);
-    sysbus_init_mmio(dev, &fdctrl->iomem);
-    sysbus_init_irq(dev, &fdctrl->irq);
-    qdev_init_gpio_in(&dev->qdev, fdctrl_handle_tc, 1);
-    fdctrl->dma_chann = -1;
+}
 
-    qdev_set_legacy_instance_id(&dev->qdev, 0 /* io */, 2); /* FIXME */
-    ret = fdctrl_init_common(fdctrl);
+static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
+{
+    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
+    FDCtrl *fdctrl = &sys->state;
 
-    return ret;
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &fdctrl->iomem);
+    sysbus_init_irq(SYS_BUS_DEVICE(dev), &fdctrl->irq);
+    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
+    fdctrl->dma_chann = -1;
+
+    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
+    if (fdctrl_init_common(fdctrl) < 0) {
+        error_setg(errp, "Floppy init failed.");
+        return;
+    }
 }
 
-static int sun4m_fdc_init1(SysBusDevice *dev)
+static void sun4m_fdc_initfn(Object *obj)
 {
-    FDCtrl *fdctrl = &(FROM_SYSBUS(FDCtrlSysBus, dev)->state);
+    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev.parent_obj, obj);
+    FDCtrl *fdctrl = &sys->state;
 
     memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_strict_ops, fdctrl,
                           "fdctrl", 0x08);
-    sysbus_init_mmio(dev, &fdctrl->iomem);
-    sysbus_init_irq(dev, &fdctrl->irq);
-    qdev_init_gpio_in(&dev->qdev, fdctrl_handle_tc, 1);
+}
+
+static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
+{
+    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
+    FDCtrl *fdctrl = &sys->state;
+
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &fdctrl->iomem);
+    sysbus_init_irq(SYS_BUS_DEVICE(dev), &fdctrl->irq);
+    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
 
     fdctrl->sun4m = 1;
-    qdev_set_legacy_instance_id(&dev->qdev, 0 /* io */, 2); /* FIXME */
-    return fdctrl_init_common(fdctrl);
+    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
+    if (fdctrl_init_common(fdctrl) < 0) {
+        error_setg(errp, "Floppy init failed.");
+        return;
+    }
 }
 
 FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
@@ -2244,9 +2262,8 @@ static Property sysbus_fdc_properties[] = {
 static void sysbus_fdc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = sysbus_fdc_init1;
+    dc->realize = sysbus_fdc_realize;
     dc->reset = fdctrl_external_reset_sysbus;
     dc->vmsd = &vmstate_sysbus_fdc;
     dc->props = sysbus_fdc_properties;
@@ -2256,6 +2273,7 @@ static const TypeInfo sysbus_fdc_info = {
     .name          = "sysbus-fdc",
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FDCtrlSysBus),
+    .instance_init = sysbus_fdc_initfn,
     .class_init    = sysbus_fdc_class_init,
 };
 
@@ -2267,9 +2285,8 @@ static Property sun4m_fdc_properties[] = {
 static void sun4m_fdc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = sun4m_fdc_init1;
+    dc->realize = sun4m_fdc_realize;
     dc->reset = fdctrl_external_reset_sysbus;
     dc->vmsd = &vmstate_sysbus_fdc;
     dc->props = sun4m_fdc_properties;
@@ -2279,6 +2296,7 @@ static const TypeInfo sun4m_fdc_info = {
     .name          = "SUNW,fdtwo",
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FDCtrlSysBus),
+    .instance_init = sun4m_fdc_initfn,
     .class_init    = sun4m_fdc_class_init,
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/26] fdc: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/fdc.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 8b8f60e..4f5f8a8 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -544,6 +544,9 @@ struct FDCtrl {
     uint8_t timer1;
 };
 
+#define TYPE_SYSBUS_FDC "sysbus-fdc"
+#define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
+
 typedef struct FDCtrlSysBus {
     SysBusDevice busdev;
     struct FDCtrl state;
@@ -773,10 +776,9 @@ static const VMStateDescription vmstate_fdc = {
 
 static void fdctrl_external_reset_sysbus(DeviceState *d)
 {
-    FDCtrlSysBus *sys = container_of(d, FDCtrlSysBus, busdev.qdev);
-    FDCtrl *s = &sys->state;
+    FDCtrlSysBus *sys = SYSBUS_FDC(d);
 
-    fdctrl_reset(s, 0);
+    fdctrl_reset(&sys->state, 0);
 }
 
 static void fdctrl_external_reset_isa(DeviceState *d)
@@ -2049,8 +2051,8 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
     DeviceState *dev;
     FDCtrlSysBus *sys;
 
-    dev = qdev_create(NULL, "sysbus-fdc");
-    sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
+    dev = qdev_create(NULL, TYPE_SYSBUS_FDC);
+    sys = SYSBUS_FDC(dev);
     fdctrl = &sys->state;
     fdctrl->dma_chann = dma_chann; /* FIXME */
     if (fds[0]) {
@@ -2060,8 +2062,8 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
         qdev_prop_set_drive_nofail(dev, "driveB", fds[1]->bdrv);
     }
     qdev_init_nofail(dev);
-    sysbus_connect_irq(&sys->busdev, 0, irq);
-    sysbus_mmio_map(&sys->busdev, 0, mmio_base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq);
+    sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, mmio_base);
 }
 
 void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
@@ -2075,9 +2077,9 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
         qdev_prop_set_drive_nofail(dev, "drive", fds[0]->bdrv);
     }
     qdev_init_nofail(dev);
-    sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
-    sysbus_connect_irq(&sys->busdev, 0, irq);
-    sysbus_mmio_map(&sys->busdev, 0, io_base);
+    sys = SYSBUS_FDC(dev);
+    sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq);
+    sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, io_base);
     *fdc_tc = qdev_get_gpio_in(dev, 0);
 }
 
@@ -2153,7 +2155,7 @@ static void sysbus_fdc_initfn(Object *obj)
 
 static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
 {
-    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
+    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
     FDCtrl *fdctrl = &sys->state;
 
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &fdctrl->iomem);
@@ -2170,7 +2172,7 @@ static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
 
 static void sun4m_fdc_initfn(Object *obj)
 {
-    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev.parent_obj, obj);
+    FDCtrlSysBus *sys = SYSBUS_FDC(obj);
     FDCtrl *fdctrl = &sys->state;
 
     memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_strict_ops, fdctrl,
@@ -2270,7 +2272,7 @@ static void sysbus_fdc_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo sysbus_fdc_info = {
-    .name          = "sysbus-fdc",
+    .name          = TYPE_SYSBUS_FDC,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FDCtrlSysBus),
     .instance_init = sysbus_fdc_initfn,
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/26] pflash_cfi01: use realize for pflash_cfi01

[Hu Tao]

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi01.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 63d7c99..aa04a2e 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -563,9 +563,9 @@ static const MemoryRegionOps pflash_cfi01_ops_le = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int pflash_cfi01_init(SysBusDevice *dev)
+static void pflash_cfi01_realize(DeviceState *dev, Error **err)
 {
-    pflash_t *pfl = FROM_SYSBUS(typeof(*pfl), dev);
+    pflash_t *pfl = DO_UPCAST(pflash_t, busdev.qdev, dev);
     uint64_t total_len;
     int ret;
 
@@ -583,7 +583,7 @@ static int pflash_cfi01_init(SysBusDevice *dev)
         pfl->name, total_len);
     vmstate_register_ram(&pfl->mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
-    sysbus_init_mmio(dev, &pfl->mem);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
 
     if (pfl->bs) {
         /* read the initial flash content */
@@ -592,7 +592,8 @@ static int pflash_cfi01_init(SysBusDevice *dev)
         if (ret < 0) {
             vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
             memory_region_destroy(&pfl->mem);
-            return 1;
+            error_setg (err, "failed to read the initial flash content");
+            return;
         }
     }
 
@@ -689,8 +690,6 @@ static int pflash_cfi01_init(SysBusDevice *dev)
     pfl->cfi_table[0x3c] = 0x00;
 
     pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
-
-    return 0;
 }
 
 static Property pflash_cfi01_properties[] = {
@@ -710,9 +709,8 @@ static Property pflash_cfi01_properties[] = {
 static void pflash_cfi01_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = pflash_cfi01_init;
+    dc->realize = pflash_cfi01_realize;
     dc->props = pflash_cfi01_properties;
     dc->vmsd = &vmstate_pflash;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/26] pflash-cfi01: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi01.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index aa04a2e..cc27f92 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -60,6 +60,9 @@ do {                                                        \
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
+#define TYPE_CFI_PFLASH01 "cfi.pflash01"
+#define CFI_PFLASH01(obj) OBJECT_CHECK(pflash_t, (obj), TYPE_CFI_PFLASH01)
+
 struct pflash_t {
     SysBusDevice busdev;
     BlockDriverState *bs;
@@ -565,7 +568,7 @@ static const MemoryRegionOps pflash_cfi01_ops_le = {
 
 static void pflash_cfi01_realize(DeviceState *dev, Error **err)
 {
-    pflash_t *pfl = DO_UPCAST(pflash_t, busdev.qdev, dev);
+    pflash_t *pfl = CFI_PFLASH01(dev);
     uint64_t total_len;
     int ret;
 
@@ -717,7 +720,7 @@ static void pflash_cfi01_class_init(ObjectClass *klass, void *data)
 
 
 static const TypeInfo pflash_cfi01_info = {
-    .name           = "cfi.pflash01",
+    .name           = TYPE_CFI_PFLASH01,
     .parent         = TYPE_SYS_BUS_DEVICE,
     .instance_size  = sizeof(struct pflash_t),
     .class_init     = pflash_cfi01_class_init,
@@ -738,10 +741,7 @@ pflash_t *pflash_cfi01_register(hwaddr base,
                                 uint16_t id0, uint16_t id1,
                                 uint16_t id2, uint16_t id3, int be)
 {
-    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
-    SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
-    pflash_t *pfl = (pflash_t *)object_dynamic_cast(OBJECT(dev),
-                                                    "cfi.pflash01");
+    DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
 
     if (bs && qdev_prop_set_drive(dev, "drive", bs)) {
         abort();
@@ -757,8 +757,8 @@ pflash_t *pflash_cfi01_register(hwaddr base,
     qdev_prop_set_string(dev, "name", name);
     qdev_init_nofail(dev);
 
-    sysbus_mmio_map(busdev, 0, base);
-    return pfl;
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    return CFI_PFLASH01(dev);
 }
 
 MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/26] pflash_cfi02: use realize for pflash_cfi02

[Hu Tao]

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi02.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 5f25246..45db415 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -586,9 +586,9 @@ static const MemoryRegionOps pflash_cfi02_ops_le = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int pflash_cfi02_init(SysBusDevice *dev)
+static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 {
-    pflash_t *pfl = FROM_SYSBUS(typeof(*pfl), dev);
+    pflash_t *pfl = DO_UPCAST(typeof(*pfl), busdev.qdev, dev);
     uint32_t chip_len;
     int ret;
 
@@ -610,14 +610,16 @@ static int pflash_cfi02_init(SysBusDevice *dev)
         /* read the initial flash content */
         ret = bdrv_read(pfl->bs, 0, pfl->storage, chip_len >> 9);
         if (ret < 0) {
-            g_free(pfl);
-            return 1;
+            vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl));
+            memory_region_destroy(&pfl->orig_mem);
+            error_setg (errp, "failed to read the initial flash content");
+            return;
         }
     }
 
     pflash_setup_mappings(pfl);
     pfl->rom_mode = 1;
-    sysbus_init_mmio(dev, &pfl->mem);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
 
     if (pfl->bs) {
         pfl->ro = bdrv_is_read_only(pfl->bs);
@@ -706,8 +708,6 @@ static int pflash_cfi02_init(SysBusDevice *dev)
 
     pfl->cfi_table[0x3b] = 0x00;
     pfl->cfi_table[0x3c] = 0x00;
-
-    return 0;
 }
 
 static Property pflash_cfi02_properties[] = {
@@ -730,9 +730,8 @@ static Property pflash_cfi02_properties[] = {
 static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = pflash_cfi02_init;
+    dc->realize = pflash_cfi02_realize;
     dc->props = pflash_cfi02_properties;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 12/26] pflash-cfi02: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi02.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 45db415..412de7f 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -55,6 +55,9 @@ do {                                                       \
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
+#define TYPE_CFI_PFLASH02 "cfi.pflash02"
+#define CFI_PFLASH02(obj) OBJECT_CHECK(pflash_t, (obj), TYPE_CFI_PFLASH02)
+
 struct pflash_t {
     SysBusDevice busdev;
     BlockDriverState *bs;
@@ -588,7 +591,7 @@ static const MemoryRegionOps pflash_cfi02_ops_le = {
 
 static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 {
-    pflash_t *pfl = DO_UPCAST(typeof(*pfl), busdev.qdev, dev);
+    pflash_t *pfl = CFI_PFLASH02(dev);
     uint32_t chip_len;
     int ret;
 
@@ -736,7 +739,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo pflash_cfi02_info = {
-    .name           = "cfi.pflash02",
+    .name           = TYPE_CFI_PFLASH02,
     .parent         = TYPE_SYS_BUS_DEVICE,
     .instance_size  = sizeof(struct pflash_t),
     .class_init     = pflash_cfi02_class_init,
@@ -759,10 +762,7 @@ pflash_t *pflash_cfi02_register(hwaddr base,
                                 uint16_t unlock_addr0, uint16_t unlock_addr1,
                                 int be)
 {
-    DeviceState *dev = qdev_create(NULL, "cfi.pflash02");
-    SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
-    pflash_t *pfl = (pflash_t *)object_dynamic_cast(OBJECT(dev),
-                                                    "cfi.pflash02");
+    DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH02);
 
     if (bs && qdev_prop_set_drive(dev, "drive", bs)) {
         abort();
@@ -781,6 +781,6 @@ pflash_t *pflash_cfi02_register(hwaddr base,
     qdev_prop_set_string(dev, "name", name);
     qdev_init_nofail(dev);
 
-    sysbus_mmio_map(busdev, 0, base);
-    return pfl;
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    return CFI_PFLASH02(dev);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 13/26] ahci: use realize for ahci

[Hu Tao]

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/ide/ahci.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index eab6096..c2e4b8c 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1296,14 +1296,13 @@ static void sysbus_ahci_reset(DeviceState *dev)
     ahci_reset(&s->ahci);
 }
 
-static int sysbus_ahci_init(SysBusDevice *dev)
+static void sysbus_ahci_realize(DeviceState *dev, Error **errp)
 {
-    SysbusAHCIState *s = FROM_SYSBUS(SysbusAHCIState, dev);
-    ahci_init(&s->ahci, &dev->qdev, NULL, s->num_ports);
+    SysbusAHCIState *s = DO_UPCAST(SysbusAHCIState, busdev.qdev, dev);
+    ahci_init(&s->ahci, dev, NULL, s->num_ports);
 
-    sysbus_init_mmio(dev, &s->ahci.mem);
-    sysbus_init_irq(dev, &s->ahci.irq);
-    return 0;
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->ahci.mem);
+    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->ahci.irq);
 }
 
 static Property sysbus_ahci_properties[] = {
@@ -1313,10 +1312,9 @@ static Property sysbus_ahci_properties[] = {
 
 static void sysbus_ahci_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    sbc->init = sysbus_ahci_init;
+    dc->realize = sysbus_ahci_realize;
     dc->vmsd = &vmstate_sysbus_ahci;
     dc->props = sysbus_ahci_properties;
     dc->reset = sysbus_ahci_reset;
-- 
1.8.3.1

[Qemu-devel] [PATCH 14/26] ahci: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/ide/ahci.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index c2e4b8c..d37c094 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1274,6 +1274,9 @@ const VMStateDescription vmstate_ahci = {
     },
 };
 
+#define TYPE_SYSBUS_AHCI "sysbus-ahci"
+#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
+
 typedef struct SysbusAHCIState {
     SysBusDevice busdev;
     AHCIState ahci;
@@ -1281,7 +1284,7 @@ typedef struct SysbusAHCIState {
 } SysbusAHCIState;
 
 static const VMStateDescription vmstate_sysbus_ahci = {
-    .name = "sysbus-ahci",
+    .name = TYPE_SYSBUS_AHCI,
     .unmigratable = 1, /* Still buggy under I/O load */
     .fields = (VMStateField []) {
         VMSTATE_AHCI(ahci, AHCIPCIState),
@@ -1291,14 +1294,14 @@ static const VMStateDescription vmstate_sysbus_ahci = {
 
 static void sysbus_ahci_reset(DeviceState *dev)
 {
-    SysbusAHCIState *s = DO_UPCAST(SysbusAHCIState, busdev.qdev, dev);
+    SysbusAHCIState *s = SYSBUS_AHCI(dev);
 
     ahci_reset(&s->ahci);
 }
 
 static void sysbus_ahci_realize(DeviceState *dev, Error **errp)
 {
-    SysbusAHCIState *s = DO_UPCAST(SysbusAHCIState, busdev.qdev, dev);
+    SysbusAHCIState *s = SYSBUS_AHCI(dev);
     ahci_init(&s->ahci, dev, NULL, s->num_ports);
 
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->ahci.mem);
@@ -1321,7 +1324,7 @@ static void sysbus_ahci_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo sysbus_ahci_info = {
-    .name          = "sysbus-ahci",
+    .name          = TYPE_SYSBUS_AHCI,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysbusAHCIState),
     .class_init    = sysbus_ahci_class_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH 15/26] fwcfg: use realize for fwcfg

[Hu Tao]

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/nvram/fw_cfg.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 3c255ce..f5c85ec 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -522,31 +522,37 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     return s;
 }
 
-static int fw_cfg_init1(SysBusDevice *dev)
+static void fw_cfg_initfn(Object *obj)
 {
-    FWCfgState *s = FROM_SYSBUS(FWCfgState, dev);
+    FWCfgState *s = DO_UPCAST(FWCfgState, busdev.qdev.parent_obj, obj);
 
     memory_region_init_io(&s->ctl_iomem, &fw_cfg_ctl_mem_ops, s,
                           "fwcfg.ctl", FW_CFG_SIZE);
-    sysbus_init_mmio(dev, &s->ctl_iomem);
     memory_region_init_io(&s->data_iomem, &fw_cfg_data_mem_ops, s,
                           "fwcfg.data", FW_CFG_DATA_SIZE);
-    sysbus_init_mmio(dev, &s->data_iomem);
     /* In case ctl and data overlap: */
     memory_region_init_io(&s->comb_iomem, &fw_cfg_comb_mem_ops, s,
                           "fwcfg", FW_CFG_SIZE);
+}
+
+static void fw_cfg_realize(DeviceState *dev, Error **errp)
+{
+    FWCfgState *s = DO_UPCAST(FWCfgState, busdev.qdev, dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
+
+    sysbus_init_mmio(b, &s->ctl_iomem);
+    sysbus_init_mmio(b, &s->data_iomem);
 
     if (s->ctl_iobase + 1 == s->data_iobase) {
-        sysbus_add_io(dev, s->ctl_iobase, &s->comb_iomem);
+        sysbus_add_io(b, s->ctl_iobase, &s->comb_iomem);
     } else {
         if (s->ctl_iobase) {
-            sysbus_add_io(dev, s->ctl_iobase, &s->ctl_iomem);
+            sysbus_add_io(b, s->ctl_iobase, &s->ctl_iomem);
         }
         if (s->data_iobase) {
-            sysbus_add_io(dev, s->data_iobase, &s->data_iomem);
+            sysbus_add_io(b, s->data_iobase, &s->data_iomem);
         }
     }
-    return 0;
 }
 
 static Property fw_cfg_properties[] = {
@@ -564,9 +570,8 @@ FWCfgState *fw_cfg_find(void)
 static void fw_cfg_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = fw_cfg_init1;
+    dc->realize = fw_cfg_realize;
     dc->no_user = 1;
     dc->reset = fw_cfg_reset;
     dc->vmsd = &vmstate_fw_cfg;
@@ -577,6 +582,7 @@ static const TypeInfo fw_cfg_info = {
     .name          = TYPE_FW_CFG,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FWCfgState),
+    .instance_init = fw_cfg_initfn,
     .class_init    = fw_cfg_class_init,
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 16/26] fwcfg: QOM'ify some more

[Hu Tao]

Use type constant if possible and avoid DO_UPCAST().

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/nvram/fw_cfg.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index f5c85ec..e580481 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -35,6 +35,7 @@
 #define TYPE_FW_CFG "fw_cfg"
 #define FW_CFG_NAME "fw_cfg"
 #define FW_CFG_PATH "/machine/" FW_CFG_NAME
+#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
 
 typedef struct FWCfgEntry {
     uint32_t len;
@@ -326,7 +327,7 @@ static const MemoryRegionOps fw_cfg_comb_mem_ops = {
 
 static void fw_cfg_reset(DeviceState *d)
 {
-    FWCfgState *s = DO_UPCAST(FWCfgState, busdev.qdev, d);
+    FWCfgState *s = FW_CFG(d);
 
     fw_cfg_select(s, 0);
 }
@@ -365,7 +366,7 @@ static bool is_version_1(void *opaque, int version_id)
 }
 
 static const VMStateDescription vmstate_fw_cfg = {
-    .name = "fw_cfg",
+    .name = TYPE_FW_CFG,
     .version_id = 2,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
@@ -489,12 +490,12 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     SysBusDevice *d;
     FWCfgState *s;
 
-    dev = qdev_create(NULL, "fw_cfg");
+    dev = qdev_create(NULL, TYPE_FW_CFG);
     qdev_prop_set_uint32(dev, "ctl_iobase", ctl_port);
     qdev_prop_set_uint32(dev, "data_iobase", data_port);
     d = SYS_BUS_DEVICE(dev);
 
-    s = DO_UPCAST(FWCfgState, busdev.qdev, dev);
+    s = FW_CFG(dev);
 
     assert(!object_resolve_path(FW_CFG_PATH, NULL));
 
@@ -524,7 +525,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
 
 static void fw_cfg_initfn(Object *obj)
 {
-    FWCfgState *s = DO_UPCAST(FWCfgState, busdev.qdev.parent_obj, obj);
+    FWCfgState *s = FW_CFG(obj);
 
     memory_region_init_io(&s->ctl_iomem, &fw_cfg_ctl_mem_ops, s,
                           "fwcfg.ctl", FW_CFG_SIZE);
@@ -563,8 +564,7 @@ static Property fw_cfg_properties[] = {
 
 FWCfgState *fw_cfg_find(void)
 {
-    return OBJECT_CHECK(FWCfgState, object_resolve_path(FW_CFG_PATH, NULL),
-                        TYPE_FW_CFG);
+    return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
 }
 
 static void fw_cfg_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

[Qemu-devel] [PATCH 17/26] scsi esp: use realize for scsi esp

[Hu Tao]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/scsi/esp.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 0c81a50..0207dba 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -666,23 +666,33 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
     }
 }
 
-static int sysbus_esp_init(SysBusDevice *dev)
+static void sysbus_esp_init(Object *obj)
 {
-    SysBusESPState *sysbus = FROM_SYSBUS(SysBusESPState, dev);
+    SysBusESPState *sysbus = DO_UPCAST(SysBusESPState, busdev.qdev.parent_obj, obj);
+
+    memory_region_init_io(&sysbus->iomem, &sysbus_esp_mem_ops, sysbus,
+                          "esp", ESP_REGS << sysbus->it_shift);
+}
+
+static void sysbus_esp_realize(DeviceState *dev, Error **errp)
+{
+    SysBusESPState *sysbus = DO_UPCAST(SysBusESPState, busdev.qdev, dev);
+    SysBusDevice *sb = SYS_BUS_DEVICE(dev);
     ESPState *s = &sysbus->esp;
 
-    sysbus_init_irq(dev, &s->irq);
+    sysbus_init_irq(sb, &s->irq);
     assert(sysbus->it_shift != -1);
 
     s->chip_id = TCHI_FAS100A;
-    memory_region_init_io(&sysbus->iomem, &sysbus_esp_mem_ops, sysbus,
-                          "esp", ESP_REGS << sysbus->it_shift);
-    sysbus_init_mmio(dev, &sysbus->iomem);
+    sysbus_init_mmio(sb, &sysbus->iomem);
 
-    qdev_init_gpio_in(&dev->qdev, sysbus_esp_gpio_demux, 2);
+    qdev_init_gpio_in(dev, sysbus_esp_gpio_demux, 2);
 
-    scsi_bus_new(&s->bus, &dev->qdev, &esp_scsi_info, NULL);
-    return scsi_bus_legacy_handle_cmdline(&s->bus);
+    scsi_bus_new(&s->bus, dev, &esp_scsi_info, NULL);
+    if (scsi_bus_legacy_handle_cmdline(&s->bus) < 0) {
+        error_setg(errp, "handling scsi bus failed");
+        return;
+    }
 }
 
 static void sysbus_esp_hard_reset(DeviceState *dev)
@@ -705,9 +715,8 @@ static const VMStateDescription vmstate_sysbus_esp_scsi = {
 static void sysbus_esp_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = sysbus_esp_init;
+    dc->realize = sysbus_esp_realize;
     dc->reset = sysbus_esp_hard_reset;
     dc->vmsd = &vmstate_sysbus_esp_scsi;
 }
@@ -716,6 +725,7 @@ static const TypeInfo sysbus_esp_info = {
     .name          = "esp",
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusESPState),
+    .instance_init = sysbus_esp_init,
     .class_init    = sysbus_esp_class_init,
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 18/26] scsi esp: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/scsi/esp.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 0207dba..aea2c94 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -578,6 +578,9 @@ const VMStateDescription vmstate_esp = {
     }
 };
 
+#define TYPE_ESP "esp"
+#define ESP(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
+
 typedef struct {
     SysBusDevice busdev;
     MemoryRegion iomem;
@@ -623,8 +626,8 @@ void esp_init(hwaddr espaddr, int it_shift,
     SysBusESPState *sysbus;
     ESPState *esp;
 
-    dev = qdev_create(NULL, "esp");
-    sysbus = DO_UPCAST(SysBusESPState, busdev.qdev, dev);
+    dev = qdev_create(NULL, TYPE_ESP);
+    sysbus = ESP(dev);
     esp = &sysbus->esp;
     esp->dma_memory_read = dma_memory_read;
     esp->dma_memory_write = dma_memory_write;
@@ -652,8 +655,7 @@ static const struct SCSIBusInfo esp_scsi_info = {
 
 static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
 {
-    DeviceState *d = opaque;
-    SysBusESPState *sysbus = container_of(d, SysBusESPState, busdev.qdev);
+    SysBusESPState *sysbus = opaque;
     ESPState *s = &sysbus->esp;
 
     switch (irq) {
@@ -668,7 +670,7 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
 
 static void sysbus_esp_init(Object *obj)
 {
-    SysBusESPState *sysbus = DO_UPCAST(SysBusESPState, busdev.qdev.parent_obj, obj);
+    SysBusESPState *sysbus = ESP(obj);
 
     memory_region_init_io(&sysbus->iomem, &sysbus_esp_mem_ops, sysbus,
                           "esp", ESP_REGS << sysbus->it_shift);
@@ -676,7 +678,7 @@ static void sysbus_esp_init(Object *obj)
 
 static void sysbus_esp_realize(DeviceState *dev, Error **errp)
 {
-    SysBusESPState *sysbus = DO_UPCAST(SysBusESPState, busdev.qdev, dev);
+    SysBusESPState *sysbus = ESP(dev);
     SysBusDevice *sb = SYS_BUS_DEVICE(dev);
     ESPState *s = &sysbus->esp;
 
@@ -697,7 +699,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error **errp)
 
 static void sysbus_esp_hard_reset(DeviceState *dev)
 {
-    SysBusESPState *sysbus = DO_UPCAST(SysBusESPState, busdev.qdev, dev);
+    SysBusESPState *sysbus = ESP(dev);
     esp_hard_reset(&sysbus->esp);
 }
 
@@ -722,7 +724,7 @@ static void sysbus_esp_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo sysbus_esp_info = {
-    .name          = "esp",
+    .name          = TYPE_ESP,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusESPState),
     .instance_init = sysbus_esp_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH 19/26] hpet: use realize for hpet

[Hu Tao]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Avi Kivity <avi@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/timer/hpet.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 95dd01d..95aad3a 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -679,9 +679,16 @@ static void hpet_handle_legacy_irq(void *opaque, int n, int level)
     }
 }
 
-static int hpet_init(SysBusDevice *dev)
+static void hpet_init(Object *obj)
 {
-    HPETState *s = FROM_SYSBUS(HPETState, dev);
+    HPETState *s = DO_UPCAST(HPETState, busdev.qdev.parent_obj, obj);
+
+    memory_region_init_io(&s->iomem, &hpet_ram_ops, s, "hpet", 0x400);
+}
+
+static void hpet_realize(DeviceState *dev, Error **errp)
+{
+    HPETState *s = DO_UPCAST(HPETState, busdev.qdev, dev);
     int i;
     HPETTimer *timer;
 
@@ -691,14 +698,14 @@ static int hpet_init(SysBusDevice *dev)
     }
 
     if (hpet_cfg.count == 8) {
-        fprintf(stderr, "Only 8 instances of HPET is allowed\n");
-        return -1;
+        error_setg(errp, "Only 8 instances of HPET is allowed");
+        return;
     }
 
     s->hpet_id = hpet_cfg.count++;
 
     for (i = 0; i < HPET_NUM_IRQ_ROUTES; i++) {
-        sysbus_init_irq(dev, &s->irqs[i]);
+        sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irqs[i]);
     }
 
     if (s->num_timers < HPET_MIN_TIMERS) {
@@ -718,13 +725,11 @@ static int hpet_init(SysBusDevice *dev)
     s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
     s->capability |= ((HPET_CLK_PERIOD) << 32);
 
-    qdev_init_gpio_in(&dev->qdev, hpet_handle_legacy_irq, 2);
-    qdev_init_gpio_out(&dev->qdev, &s->pit_enabled, 1);
+    qdev_init_gpio_in(dev, hpet_handle_legacy_irq, 2);
+    qdev_init_gpio_out(dev, &s->pit_enabled, 1);
 
     /* HPET Area */
-    memory_region_init_io(&s->iomem, &hpet_ram_ops, s, "hpet", 0x400);
-    sysbus_init_mmio(dev, &s->iomem);
-    return 0;
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
 }
 
 static Property hpet_device_properties[] = {
@@ -736,9 +741,8 @@ static Property hpet_device_properties[] = {
 static void hpet_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = hpet_init;
+    dc->realize = hpet_realize;
     dc->no_user = 1;
     dc->reset = hpet_reset;
     dc->vmsd = &vmstate_hpet;
@@ -749,6 +753,7 @@ static const TypeInfo hpet_device_info = {
     .name          = "hpet",
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(HPETState),
+    .instance_init = hpet_init,
     .class_init    = hpet_device_class_init,
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 20/26] hpet: QOM'ify some more

[Hu Tao]

Introduce type constant, avoid DO_UPCAST() and
FROM_SYSBUS().

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Avi Kivity <avi@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/timer/hpet.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 95aad3a..c2c4504 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -42,6 +42,9 @@
 
 #define HPET_MSI_SUPPORT        0
 
+#define TYPE_HPET "hpet"
+#define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
+
 struct HPETState;
 typedef struct HPETTimer {  /* timers */
     uint8_t tn;             /*timer number*/
@@ -278,7 +281,7 @@ static const VMStateDescription vmstate_hpet_timer = {
 };
 
 static const VMStateDescription vmstate_hpet = {
-    .name = "hpet",
+    .name = TYPE_HPET,
     .version_id = 2,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
@@ -634,7 +637,8 @@ static const MemoryRegionOps hpet_ram_ops = {
 
 static void hpet_reset(DeviceState *d)
 {
-    HPETState *s = FROM_SYSBUS(HPETState, SYS_BUS_DEVICE(d));
+    HPETState *s = HPET(d);
+    SysBusDevice *bus = SYS_BUS_DEVICE(d);
     int i;
 
     for (i = 0; i < s->num_timers; i++) {
@@ -657,7 +661,7 @@ static void hpet_reset(DeviceState *d)
     s->hpet_offset = 0ULL;
     s->config = 0ULL;
     hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
-    hpet_cfg.hpet[s->hpet_id].address = SYS_BUS_DEVICE(d)->mmio[0].addr;
+    hpet_cfg.hpet[s->hpet_id].address = bus->mmio[0].addr;
 
     /* to document that the RTC lowers its output on reset as well */
     s->rtc_irq_level = 0;
@@ -665,7 +669,7 @@ static void hpet_reset(DeviceState *d)
 
 static void hpet_handle_legacy_irq(void *opaque, int n, int level)
 {
-    HPETState *s = FROM_SYSBUS(HPETState, opaque);
+    HPETState *s = opaque;
 
     if (n == HPET_LEGACY_PIT_INT) {
         if (!hpet_in_legacy_mode(s)) {
@@ -681,14 +685,14 @@ static void hpet_handle_legacy_irq(void *opaque, int n, int level)
 
 static void hpet_init(Object *obj)
 {
-    HPETState *s = DO_UPCAST(HPETState, busdev.qdev.parent_obj, obj);
+    HPETState *s = HPET(obj);
 
     memory_region_init_io(&s->iomem, &hpet_ram_ops, s, "hpet", 0x400);
 }
 
 static void hpet_realize(DeviceState *dev, Error **errp)
 {
-    HPETState *s = DO_UPCAST(HPETState, busdev.qdev, dev);
+    HPETState *s = HPET(dev);
     int i;
     HPETTimer *timer;
 
@@ -750,7 +754,7 @@ static void hpet_device_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo hpet_device_info = {
-    .name          = "hpet",
+    .name          = TYPE_HPET,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(HPETState),
     .instance_init = hpet_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Hu Tao]

Cc: qemu-devel@nongnu.org
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/kvm/clock.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 87d4d0f..74aa240 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -87,12 +87,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
     }
 }
 
-static int kvmclock_init(SysBusDevice *dev)
+static void kvmclock_realize(DeviceState *dev, Error **errp)
 {
-    KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
+    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
 
     qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
-    return 0;
 }
 
 static const VMStateDescription kvmclock_vmsd = {
@@ -111,9 +110,8 @@ static const VMStateDescription kvmclock_vmsd = {
 static void kvmclock_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = kvmclock_init;
+    dc->realize = kvmclock_realize;
     dc->no_user = 1;
     dc->vmsd = &kvmclock_vmsd;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 22/26] kvmclock: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Cc: qemu-devel@nongnu.org
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/kvm/clock.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 74aa240..eaad880 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -22,6 +22,9 @@
 #include <linux/kvm.h>
 #include <linux/kvm_para.h>
 
+#define TYPE_KVM_CLOCK "kvmclock"
+#define KVM_CLOCK(obj) OBJECT_CHECK(KVMClockState, (obj), TYPE_KVM_CLOCK)
+
 typedef struct KVMClockState {
     SysBusDevice busdev;
     uint64_t clock;
@@ -89,13 +92,13 @@ static void kvmclock_vm_state_change(void *opaque, int running,
 
 static void kvmclock_realize(DeviceState *dev, Error **errp)
 {
-    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
+    KVMClockState *s = KVM_CLOCK(dev);
 
     qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
 }
 
 static const VMStateDescription kvmclock_vmsd = {
-    .name = "kvmclock",
+    .name = TYPE_KVM_CLOCK,
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
@@ -117,7 +120,7 @@ static void kvmclock_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo kvmclock_info = {
-    .name          = "kvmclock",
+    .name          = TYPE_KVM_CLOCK,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(KVMClockState),
     .class_init    = kvmclock_class_init,
@@ -129,7 +132,7 @@ void kvmclock_create(void)
     if (kvm_enabled() &&
         first_cpu->features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
                                          (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
-        sysbus_create_simple("kvmclock", -1, NULL);
+        sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
     }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 23/26] kvmvapic realize

[Hu Tao]

Cc: qemu-devel@nongnu.org
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/kvmvapic.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 655483b..06d290c 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -698,19 +698,18 @@ static const MemoryRegionOps vapic_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int vapic_init(SysBusDevice *dev)
+static void vapic_realize(DeviceState *dev, Error **errp)
 {
     VAPICROMState *s = VAPIC(dev);
+    SysBusDevice *bus = SYS_BUS_DEVICE(dev);
 
     memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2);
-    sysbus_add_io(dev, VAPIC_IO_PORT, &s->io);
-    sysbus_init_ioports(dev, VAPIC_IO_PORT, 2);
+    sysbus_add_io(bus, VAPIC_IO_PORT, &s->io);
+    sysbus_init_ioports(bus, VAPIC_IO_PORT, 2);
 
     option_rom[nb_option_roms].name = "kvmvapic.bin";
     option_rom[nb_option_roms].bootindex = -1;
     nb_option_roms++;
-
-    return 0;
 }
 
 static void do_vapic_enable(void *data)
@@ -806,13 +805,12 @@ static const VMStateDescription vmstate_vapic = {
 
 static void vapic_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->no_user = 1;
     dc->reset   = vapic_reset;
     dc->vmsd    = &vmstate_vapic;
-    sc->init    = vapic_init;
+    dc->realize = vapic_realize;
 }
 
 static const TypeInfo vapic_type = {
-- 
1.8.3.1

[Qemu-devel] [PATCH 24/26] ioapic: use realize for ioapic

[Hu Tao]

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/intc/ioapic_common.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 5c5bb3c..6077024 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -57,14 +57,15 @@ static int ioapic_dispatch_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static int ioapic_init_common(SysBusDevice *dev)
+static void ioapic_common_realize(DeviceState *dev, Error **errp)
 {
     IOAPICCommonState *s = IOAPIC_COMMON(dev);
     IOAPICCommonClass *info;
     static int ioapic_no;
 
     if (ioapic_no >= MAX_IOAPICS) {
-        return -1;
+        error_setg (errp, "invalid ioapic number: %d", ioapic_no);
+        return;
     }
 
     info = IOAPIC_COMMON_GET_CLASS(s);
@@ -72,8 +73,6 @@ static int ioapic_init_common(SysBusDevice *dev)
 
     sysbus_init_mmio(&s->busdev, &s->io_memory);
     ioapic_no++;
-
-    return 0;
 }
 
 static const VMStateDescription vmstate_ioapic_common = {
@@ -95,10 +94,9 @@ static const VMStateDescription vmstate_ioapic_common = {
 
 static void ioapic_common_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    sc->init = ioapic_init_common;
+    dc->realize = ioapic_common_realize;
     dc->vmsd = &vmstate_ioapic_common;
     dc->no_user = 1;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 25/26] isa bus: use realize for isa bus

[Hu Tao]

Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Avi Kivity <avi@redhat.com>
Cc: Jim Meyering <meyering@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/isa/isa-bus.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 136d17e..287f941 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -192,18 +192,17 @@ static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     }
 }
 
-static int isabus_bridge_init(SysBusDevice *dev)
+static void isabus_bridge_realize(DeviceState *dev, Error **errp)
 {
     /* nothing */
-    return 0;
+    return;
 }
 
 static void isabus_bridge_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = isabus_bridge_init;
+    dc->realize = isabus_bridge_realize;
     dc->fw_name = "isa";
     dc->no_user = 1;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 26/26] ehci: use realize for ehci

[Hu Tao]

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/usb/hcd-ehci-sysbus.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index b68a66a..8cb594d 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -32,9 +32,10 @@ static Property ehci_sysbus_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
+static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
 {
     EHCISysBusState *i = SYS_BUS_EHCI(dev);
+    SysBusDevice *bus = SYS_BUS_DEVICE(dev);
     SysBusEHCIClass *sec = SYS_BUS_EHCI_GET_CLASS(dev);
     EHCIState *s = &i->ehci;
 
@@ -43,17 +44,15 @@ static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
     s->dma = &dma_context_memory;
 
     usb_ehci_initfn(s, DEVICE(dev));
-    sysbus_init_irq(dev, &s->irq);
-    sysbus_init_mmio(dev, &s->mem);
-    return 0;
+    sysbus_init_irq(bus, &s->irq);
+    sysbus_init_mmio(bus, &s->mem);
 }
 
 static void ehci_sysbus_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = usb_ehci_sysbus_initfn;
+    dc->realize = usb_ehci_sysbus_realize;
     dc->vmsd = &vmstate_ehci_sysbus;
     dc->props = ehci_sysbus_properties;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 26/26] ehci: use realize for ehci

[Andreas Färber]

Am 22.06.2013 10:50, schrieb Hu Tao:
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/usb/hcd-ehci-sysbus.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

I had already done this iirc, it should be in Gerd's USB queue, which I
was waiting for to be merged to continue my Tegra work.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 01/26] ohci: use realize for ohci

[Peter Crosthwaite]

Hi Hu,

On Sat, Jun 22, 2013 at 6:50 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/usb/hcd-ohci.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 51241cd..79ef41b 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -1876,17 +1876,16 @@ typedef struct {
>      dma_addr_t dma_offset;
>  } OHCISysBusState;
>
> -static int ohci_init_pxa(SysBusDevice *dev)
> +static void ohci_realize_pxa(DeviceState *dev, Error **errp)
>  {
> -    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
> +    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);

I don't think this is an improvement. Until a QOM cast macro is
available, FROM_SYSBUS is preferable to a DO_UPCAST I think?

> +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
>
>      /* Cannot fail as we pass NULL for masterbus */
> -    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
> +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
>                    &dma_context_memory);

Rebase required due to Paolos IOMMU patches going in removing
dma_context_memory.

Regards,
Peter

> -    sysbus_init_irq(dev, &s->ohci.irq);
> -    sysbus_init_mmio(dev, &s->ohci.mem);
> -
> -    return 0;
> +    sysbus_init_irq(b, &s->ohci.irq);
> +    sysbus_init_mmio(b, &s->ohci.mem);
>  }
>
>  static Property ohci_pci_properties[] = {
> @@ -1926,9 +1925,8 @@ static Property ohci_sysbus_properties[] = {
>  static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
>
> -    sbc->init = ohci_init_pxa;
> +    dc->realize = ohci_realize_pxa;
>      dc->desc = "OHCI USB Controller";
>      dc->props = ohci_sysbus_properties;
>  }
> --
> 1.8.3.1
>
>

Re: [Qemu-devel] [PATCH 01/26] ohci: use realize for ohci

[Hu Tao]

On Mon, Jun 24, 2013 at 03:54:31PM +1000, Peter Crosthwaite wrote:
> Hi Hu,
> 
> On Sat, Jun 22, 2013 at 6:50 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/usb/hcd-ohci.c | 16 +++++++---------
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> > index 51241cd..79ef41b 100644
> > --- a/hw/usb/hcd-ohci.c
> > +++ b/hw/usb/hcd-ohci.c
> > @@ -1876,17 +1876,16 @@ typedef struct {
> >      dma_addr_t dma_offset;
> >  } OHCISysBusState;
> >
> > -static int ohci_init_pxa(SysBusDevice *dev)
> > +static void ohci_realize_pxa(DeviceState *dev, Error **errp)
> >  {
> > -    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
> > +    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);
> 
> I don't think this is an improvement. Until a QOM cast macro is
> available, FROM_SYSBUS is preferable to a DO_UPCAST I think?

patch 2 introduces QOM macro and replaces DO_UPCAST. Instead, we can
also first do QOM then realize. Which one do you prefer?

> 
> > +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
> >
> >      /* Cannot fail as we pass NULL for masterbus */
> > -    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
> > +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
> >                    &dma_context_memory);
> 
> Rebase required due to Paolos IOMMU patches going in removing
> dma_context_memory.

Thanks for reminding. I'll do a rebase anyway, patches involving i440fx
and q35 may conflict with your `pci cleanup' series, and ehci patches
duplicates Andreas's work.

Re: [Qemu-devel] [PATCH 01/26] ohci: use realize for ohci

[Peter Crosthwaite]

Hi Hu,

On Mon, Jun 24, 2013 at 4:11 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> On Mon, Jun 24, 2013 at 03:54:31PM +1000, Peter Crosthwaite wrote:
>> Hi Hu,
>>
>> On Sat, Jun 22, 2013 at 6:50 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
>> > Cc: Gerd Hoffmann <kraxel@redhat.com>
>> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>> > ---
>> >  hw/usb/hcd-ohci.c | 16 +++++++---------
>> >  1 file changed, 7 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
>> > index 51241cd..79ef41b 100644
>> > --- a/hw/usb/hcd-ohci.c
>> > +++ b/hw/usb/hcd-ohci.c
>> > @@ -1876,17 +1876,16 @@ typedef struct {
>> >      dma_addr_t dma_offset;
>> >  } OHCISysBusState;
>> >
>> > -static int ohci_init_pxa(SysBusDevice *dev)
>> > +static void ohci_realize_pxa(DeviceState *dev, Error **errp)
>> >  {
>> > -    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
>> > +    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);
>>
>> I don't think this is an improvement. Until a QOM cast macro is
>> available, FROM_SYSBUS is preferable to a DO_UPCAST I think?
>
> patch 2 introduces QOM macro and replaces DO_UPCAST. Instead, we can
> also first do QOM then realize. Which one do you prefer?
>

Other way round I think make more sense, as no need to have this ugly
hunk for transition sake.

Squashing is another low effort option, one patch that just does it
all makes sense to me (and ive done this a few times already with
various devices).

Regards,
Peter

>>
>> > +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
>> >
>> >      /* Cannot fail as we pass NULL for masterbus */
>> > -    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
>> > +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
>> >                    &dma_context_memory);
>>
>> Rebase required due to Paolos IOMMU patches going in removing
>> dma_context_memory.
>
> Thanks for reminding. I'll do a rebase anyway, patches involving i440fx
> and q35 may conflict with your `pci cleanup' series, and ehci patches
> duplicates Andreas's work.
>
>

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Igor Mammedov]

On Sat, 22 Jun 2013 16:50:33 +0800
Hu Tao <hutao@cn.fujitsu.com> wrote:

> Cc: qemu-devel@nongnu.org
> Cc: "Andreas Färber" <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/i386/kvm/clock.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> index 87d4d0f..74aa240 100644
> --- a/hw/i386/kvm/clock.c
> +++ b/hw/i386/kvm/clock.c
> @@ -87,12 +87,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
>      }
>  }
>  
> -static int kvmclock_init(SysBusDevice *dev)
> +static void kvmclock_realize(DeviceState *dev, Error **errp)
>  {
> -    KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
> +    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
it would be better to swap this patch with 22/26 and use KVM_CLOCK() cast here

>  
>      qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
> -    return 0;
>  }
>  
>  static const VMStateDescription kvmclock_vmsd = {
> @@ -111,9 +110,8 @@ static const VMStateDescription kvmclock_vmsd = {
>  static void kvmclock_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = kvmclock_init;
> +    dc->realize = kvmclock_realize;
>      dc->no_user = 1;
>      dc->vmsd = &kvmclock_vmsd;
>  }

Re: [Qemu-devel] [PATCH 22/26] kvmclock: QOM'ify some more

[Eduardo Habkost]

On Sat, Jun 22, 2013 at 04:50:34PM +0800, Hu Tao wrote:
> Introduce type constant and avoid DO_UPCAST().
> 
> Cc: qemu-devel@nongnu.org
> Cc: "Andreas Färber" <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

But I agree with Igor that the order of 21/26 and 22/26 could be
changed, so kvmclock_realize() could use KVM_CLOCK() since the
beginning.


> ---
>  hw/i386/kvm/clock.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> index 74aa240..eaad880 100644
> --- a/hw/i386/kvm/clock.c
> +++ b/hw/i386/kvm/clock.c
> @@ -22,6 +22,9 @@
>  #include <linux/kvm.h>
>  #include <linux/kvm_para.h>
>  
> +#define TYPE_KVM_CLOCK "kvmclock"
> +#define KVM_CLOCK(obj) OBJECT_CHECK(KVMClockState, (obj), TYPE_KVM_CLOCK)
> +
>  typedef struct KVMClockState {
>      SysBusDevice busdev;
>      uint64_t clock;
> @@ -89,13 +92,13 @@ static void kvmclock_vm_state_change(void *opaque, int running,
>  
>  static void kvmclock_realize(DeviceState *dev, Error **errp)
>  {
> -    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
> +    KVMClockState *s = KVM_CLOCK(dev);
>  
>      qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
>  }
>  
>  static const VMStateDescription kvmclock_vmsd = {
> -    .name = "kvmclock",
> +    .name = TYPE_KVM_CLOCK,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
> @@ -117,7 +120,7 @@ static void kvmclock_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo kvmclock_info = {
> -    .name          = "kvmclock",
> +    .name          = TYPE_KVM_CLOCK,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(KVMClockState),
>      .class_init    = kvmclock_class_init,
> @@ -129,7 +132,7 @@ void kvmclock_create(void)
>      if (kvm_enabled() &&
>          first_cpu->features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
>                                           (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
> -        sysbus_create_simple("kvmclock", -1, NULL);
> +        sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
>      }
>  }
>  
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 01/26] ohci: use realize for ohci

[Eduardo Habkost]

On Sat, Jun 22, 2013 at 04:50:13PM +0800, Hu Tao wrote:
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/usb/hcd-ohci.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 51241cd..79ef41b 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -1876,17 +1876,16 @@ typedef struct {
>      dma_addr_t dma_offset;
>  } OHCISysBusState;
>  
> -static int ohci_init_pxa(SysBusDevice *dev)
> +static void ohci_realize_pxa(DeviceState *dev, Error **errp)
>  {
> -    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
> +    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);
> +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
>  
>      /* Cannot fail as we pass NULL for masterbus */
> -    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
> +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
>                    &dma_context_memory);

This patch doesn't apply cleanly anymore, since "[PULL 00/25]
Memory/IOMMU patches, part 3: IOMMU implementation" from Paolo was
merged (commit 576156f)


> -    sysbus_init_irq(dev, &s->ohci.irq);
> -    sysbus_init_mmio(dev, &s->ohci.mem);
> -
> -    return 0;
> +    sysbus_init_irq(b, &s->ohci.irq);
> +    sysbus_init_mmio(b, &s->ohci.mem);
>  }
>  
>  static Property ohci_pci_properties[] = {
> @@ -1926,9 +1925,8 @@ static Property ohci_sysbus_properties[] = {
>  static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    sbc->init = ohci_init_pxa;
> +    dc->realize = ohci_realize_pxa;
>      dc->desc = "OHCI USB Controller";
>      dc->props = ohci_sysbus_properties;
>  }
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Eduardo Habkost]

On Sat, Jun 22, 2013 at 04:50:33PM +0800, Hu Tao wrote:
> Cc: qemu-devel@nongnu.org
> Cc: "Andreas Färber" <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/i386/kvm/clock.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> index 87d4d0f..74aa240 100644
> --- a/hw/i386/kvm/clock.c
> +++ b/hw/i386/kvm/clock.c
> @@ -87,12 +87,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
>      }
>  }
>  
> -static int kvmclock_init(SysBusDevice *dev)
> +static void kvmclock_realize(DeviceState *dev, Error **errp)
>  {
> -    KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
> +    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
>  
>      qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
> -    return 0;
>  }
>  
>  static const VMStateDescription kvmclock_vmsd = {
> @@ -111,9 +110,8 @@ static const VMStateDescription kvmclock_vmsd = {
>  static void kvmclock_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = kvmclock_init;
> +    dc->realize = kvmclock_realize;

Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itself?

>From DeviceClass documentation:

 * If a type derived directly from TYPE_DEVICE implements @realize, it does
 * not need to implement @init and therefore does not need to store and call
 * #DeviceClass' default @realize callback.
 * For other types consult the documentation and implementation of the
 * respective parent types.

The problem is that there's no documentation about ->realize() on
SysBusDeviceClass. Can we please explicitly document SysBusDeviceClass
expectations about ->realize() first, before making those changes?


>      dc->no_user = 1;
>      dc->vmsd = &kvmclock_vmsd;
>  }
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 01/26] ohci: use realize for ohci

[Hu Tao]

On Mon, Jun 24, 2013 at 04:17:28PM +1000, Peter Crosthwaite wrote:
> Hi Hu,
> 
> On Mon, Jun 24, 2013 at 4:11 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> > On Mon, Jun 24, 2013 at 03:54:31PM +1000, Peter Crosthwaite wrote:
> >> Hi Hu,
> >>
> >> On Sat, Jun 22, 2013 at 6:50 PM, Hu Tao <hutao@cn.fujitsu.com> wrote:
> >> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >> > ---
> >> >  hw/usb/hcd-ohci.c | 16 +++++++---------
> >> >  1 file changed, 7 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> >> > index 51241cd..79ef41b 100644
> >> > --- a/hw/usb/hcd-ohci.c
> >> > +++ b/hw/usb/hcd-ohci.c
> >> > @@ -1876,17 +1876,16 @@ typedef struct {
> >> >      dma_addr_t dma_offset;
> >> >  } OHCISysBusState;
> >> >
> >> > -static int ohci_init_pxa(SysBusDevice *dev)
> >> > +static void ohci_realize_pxa(DeviceState *dev, Error **errp)
> >> >  {
> >> > -    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
> >> > +    OHCISysBusState *s = DO_UPCAST(OHCISysBusState, busdev.qdev, dev);
> >>
> >> I don't think this is an improvement. Until a QOM cast macro is
> >> available, FROM_SYSBUS is preferable to a DO_UPCAST I think?
> >
> > patch 2 introduces QOM macro and replaces DO_UPCAST. Instead, we can
> > also first do QOM then realize. Which one do you prefer?
> >
> 
> Other way round I think make more sense, as no need to have this ugly
> hunk for transition sake.

Agreed.

> 
> Squashing is another low effort option, one patch that just does it
> all makes sense to me (and ive done this a few times already with
> various devices).

But I think it's easier to review to keep it in two steps.

> 
> Regards,
> Peter
> 
> >>
> >> > +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
> >> >
> >> >      /* Cannot fail as we pass NULL for masterbus */
> >> > -    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
> >> > +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
> >> >                    &dma_context_memory);
> >>
> >> Rebase required due to Paolos IOMMU patches going in removing
> >> dma_context_memory.
> >
> > Thanks for reminding. I'll do a rebase anyway, patches involving i440fx
> > and q35 may conflict with your `pci cleanup' series, and ehci patches
> > duplicates Andreas's work.
> >
> >

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Hu Tao]

On Mon, Jun 24, 2013 at 12:14:07PM +0200, Igor Mammedov wrote:
> On Sat, 22 Jun 2013 16:50:33 +0800
> Hu Tao <hutao@cn.fujitsu.com> wrote:
> 
> > Cc: qemu-devel@nongnu.org
> > Cc: "Andreas Färber" <afaerber@suse.de>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Anthony Liguori <aliguori@us.ibm.com>
> > Cc: Igor Mammedov <imammedo@redhat.com>
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/i386/kvm/clock.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> > index 87d4d0f..74aa240 100644
> > --- a/hw/i386/kvm/clock.c
> > +++ b/hw/i386/kvm/clock.c
> > @@ -87,12 +87,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
> >      }
> >  }
> >  
> > -static int kvmclock_init(SysBusDevice *dev)
> > +static void kvmclock_realize(DeviceState *dev, Error **errp)
> >  {
> > -    KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
> > +    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
> it would be better to swap this patch with 22/26 and use KVM_CLOCK() cast here

OK. I'll swap them in next version.

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Hu Tao]

On Mon, Jun 24, 2013 at 11:01:54AM -0300, Eduardo Habkost wrote:
> On Sat, Jun 22, 2013 at 04:50:33PM +0800, Hu Tao wrote:
> > Cc: qemu-devel@nongnu.org
> > Cc: "Andreas Färber" <afaerber@suse.de>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Anthony Liguori <aliguori@us.ibm.com>
> > Cc: Igor Mammedov <imammedo@redhat.com>
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/i386/kvm/clock.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> > index 87d4d0f..74aa240 100644
> > --- a/hw/i386/kvm/clock.c
> > +++ b/hw/i386/kvm/clock.c
> > @@ -87,12 +87,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
> >      }
> >  }
> >  
> > -static int kvmclock_init(SysBusDevice *dev)
> > +static void kvmclock_realize(DeviceState *dev, Error **errp)
> >  {
> > -    KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
> > +    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
> >  
> >      qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
> > -    return 0;
> >  }
> >  
> >  static const VMStateDescription kvmclock_vmsd = {
> > @@ -111,9 +110,8 @@ static const VMStateDescription kvmclock_vmsd = {
> >  static void kvmclock_class_init(ObjectClass *klass, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> > -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> >  
> > -    k->init = kvmclock_init;
> > +    dc->realize = kvmclock_realize;
> 
> Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itself?
> 
> From DeviceClass documentation:
> 
>  * If a type derived directly from TYPE_DEVICE implements @realize, it does
>  * not need to implement @init and therefore does not need to store and call
>  * #DeviceClass' default @realize callback.
>  * For other types consult the documentation and implementation of the
>  * respective parent types.
> 
> The problem is that there's no documentation about ->realize() on
> SysBusDeviceClass. Can we please explicitly document SysBusDeviceClass
> expectations about ->realize() first, before making those changes?

IIUC, subclass's overriding ->realize should call parent's ->realize at
some point. Peter Crosthwaite has a patchset to access a object's parent
class at http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02982.html

Regarding SysBusDevice::init and SysBusDevice::realize, I think it's the
same as in the case of DeviceClass. If you agree I'll document it as in
DeviceClass.

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Eduardo Habkost]

On Tue, Jun 25, 2013 at 10:20:08AM +0800, Hu Tao wrote:
[...]
> > Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itself?
> > 
> > From DeviceClass documentation:
> > 
> >  * If a type derived directly from TYPE_DEVICE implements @realize, it does
> >  * not need to implement @init and therefore does not need to store and call
> >  * #DeviceClass' default @realize callback.
> >  * For other types consult the documentation and implementation of the
> >  * respective parent types.
> > 
> > The problem is that there's no documentation about ->realize() on
> > SysBusDeviceClass. Can we please explicitly document SysBusDeviceClass
> > expectations about ->realize() first, before making those changes?
> 
> IIUC, subclass's overriding ->realize should call parent's ->realize at
> some point. Peter Crosthwaite has a patchset to access a object's parent
> class at http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02982.html
> 
> Regarding SysBusDevice::init and SysBusDevice::realize, I think it's the
> same as in the case of DeviceClass. If you agree I'll document it as in
> DeviceClass.

I believe it is reasonable to document that SysBusDevice subclasses
don't need to call the parent ->realize() method, like on DeviceClass.
This is exactly what this patch set does, after all, and I haven't seen
anybody complaining about it yet.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Andreas Färber]

Am 25.06.2013 19:45, schrieb Eduardo Habkost:
> On Tue, Jun 25, 2013 at 10:20:08AM +0800, Hu Tao wrote:
> [...]
>>> Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itself?
>>>
>>> From DeviceClass documentation:
>>>
>>>  * If a type derived directly from TYPE_DEVICE implements @realize, it does
>>>  * not need to implement @init and therefore does not need to store and call
>>>  * #DeviceClass' default @realize callback.
>>>  * For other types consult the documentation and implementation of the
>>>  * respective parent types.
>>>
>>> The problem is that there's no documentation about ->realize() on
>>> SysBusDeviceClass. Can we please explicitly document SysBusDeviceClass
>>> expectations about ->realize() first, before making those changes?

If someone wants to add a paragraph to sysbus.h:SysBusDeviceClass
documentation I would happily ack or pick that up. :)

>> IIUC, subclass's overriding ->realize should call parent's ->realize at
>> some point. Peter Crosthwaite has a patchset to access a object's parent
>> class at http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02982.html
>>
>> Regarding SysBusDevice::init and SysBusDevice::realize, I think it's the
>> same as in the case of DeviceClass. If you agree I'll document it as in
>> DeviceClass.
> 
> I believe it is reasonable to document that SysBusDevice subclasses
> don't need to call the parent ->realize() method, like on DeviceClass.
> This is exactly what this patch set does, after all, and I haven't seen
> anybody complaining about it yet.

So the thing is that SysBusDevice's DeviceClass::init implementation,
called by DeviceState's DeviceClass::realize implementation, just calls
SysBusDeviceClass::init if non-NULL. When we introduce our own device's
realizefn to replace our SysBusDeviceClass::init implementation, there
is no point calling that effectively no-op DeviceClass::realize
implementation. And if we tried to, ...
* ... how would we decide whether to call the parent's implementation
first or last? It's not yes or no, it's no or when. Changing between
either is more than just moving one line, it affects error handling and
with knowledge about parent implementation never failing we could so far
save us some work.
* ... with the current QOM method scheme we'd go insane introducing a
FooClass per device to save SysBusDevice's DeviceClass::realize in
FooClass::parent_realize. Still waiting for Anthony on whether and how
we want to change our scheme.

Long story short: If someone wants to mess with base classes they get to
check derived classes for compatibility with their change.

Ideally qtest would help automate that to some degree.
I would be all in favor if someone wanted to add a dummy test case per
non-default, non-KVM device converted so that we can see that we didn't
screw up -device instantiation too badly.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 26/26] ehci: use realize for ehci

[Andreas Färber]

Am 22.06.2013 12:38, schrieb Andreas Färber:
> Am 22.06.2013 10:50, schrieb Hu Tao:
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>> ---
>>  hw/usb/hcd-ehci-sysbus.c | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> I had already done this iirc, it should be in Gerd's USB queue, [...]

FTR it was merged earlier this week:
http://git.qemu.org/?p=qemu.git;a=commit;h=89f204d2c60fbf3e0c5af1ff1681e57c9f057178

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 25/26] isa bus: use realize for isa bus

[Andreas Färber]

The subject is a bit misleading, suggest:
"isa-bus: Use QOM realize for ISA SysBus bridge"

Am 22.06.2013 10:50, schrieb Hu Tao:
> Cc: "Andreas Färber" <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>

> Cc: Avi Kivity <avi@redhat.com>

This will bounce, please drop. ;)

Also, if you use git-send-email together with
"scripts/get_maintainer.pl --nogit-fallback" you only need to add any
CCs missing in MAINTAINERS file per commit.

> Cc: Jim Meyering <meyering@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/isa/isa-bus.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index 136d17e..287f941 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -192,18 +192,17 @@ static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>      }
>  }
>  
> -static int isabus_bridge_init(SysBusDevice *dev)
> +static void isabus_bridge_realize(DeviceState *dev, Error **errp)
>  {
>      /* nothing */
> -    return 0;
> +    return;

Not needed.

>  }
>  
>  static void isabus_bridge_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = isabus_bridge_init;
> +    dc->realize = isabus_bridge_realize;
>      dc->fw_name = "isa";
>      dc->no_user = 1;
>  }

I would propose to simply drop the init function instead of turning it
into a realize function unless you see a future use case for it.

Historically it was not possible to omit a SysBusDevice init function:
http://git.qemu.org/?p=qemu.git;a=commit;h=4ce5dae88ecf2bafa0cd663de7e923728b1b3672

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 00/26] use realizefn for SysBusDevice, part 1

[Andreas Färber]

Hi,

Am 22.06.2013 10:50, schrieb Hu Tao:
> This series updates part of devices inheriting from SysbusDevice
> to use DeviceState::realize, and QOM'ify them.
> 
> These devices are default to x86_64-softmmu. I'm planning to
> make patches in the same manner, that is, each series is for
> devices default to each target. After all devices are converted
> to realizefn, SysBusDevice::init can be removed.

This series looks promising. As was requested by others, please rebase
(possibly on qom-next if there's conflicts with Peter M.'s patch?) and
reorder the patches so that QOM cast macros come before more functional
changes (the former can be picked up more quickly).

For ARM I am looking into a few devices surrounding cpu/a9mpcore.c.
Paolo has announced a PULL for his MemoryRegion refactorings for
beginning of the week, which is likely to conflict with realizefn work.

As a reminder for realize series (we should definitely document this!):
* QOM realize functions should not create new devices as that creates
problems with recursive realized = true and will prohibit changing
static device properties through QMP. (Therefore more review work.)
* Care must be taken with bus creation since qbus_create[_inplace]() /
qbus_realize() check for DeviceState::id, which is still NULL at
instance_init time. (OHCI looked good for instance.)

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 25/26] isa bus: use realize for isa bus

[Hu Tao]

On Sun, Jun 30, 2013 at 04:57:03PM +0200, Andreas Färber wrote:
> The subject is a bit misleading, suggest:
> "isa-bus: Use QOM realize for ISA SysBus bridge"
> 
> Am 22.06.2013 10:50, schrieb Hu Tao:
> > Cc: "Andreas Färber" <afaerber@suse.de>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Anthony Liguori <aliguori@us.ibm.com>
> 
> > Cc: Avi Kivity <avi@redhat.com>
> 
> This will bounce, please drop. ;)

Noticed that. :)

> 
> Also, if you use git-send-email together with
> "scripts/get_maintainer.pl --nogit-fallback" you only need to add any
> CCs missing in MAINTAINERS file per commit.

So don't have to CC people in MAINTAINERS? For example:

  ./scripts/get_maintainer.pl --nogit-fallback patch-01

get:

  Gerd Hoffmann <kraxel@redhat.com> (maintainer:USB)

and Gerd Hoffmann is in MAINTAINERS, there is no need to CC him in
patch-01? If so I'll update my script.

> 
> > Cc: Jim Meyering <meyering@redhat.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/isa/isa-bus.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> > index 136d17e..287f941 100644
> > --- a/hw/isa/isa-bus.c
> > +++ b/hw/isa/isa-bus.c
> > @@ -192,18 +192,17 @@ static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
> >      }
> >  }
> >  
> > -static int isabus_bridge_init(SysBusDevice *dev)
> > +static void isabus_bridge_realize(DeviceState *dev, Error **errp)
> >  {
> >      /* nothing */
> > -    return 0;
> > +    return;
> 
> Not needed.
> 
> >  }
> >  
> >  static void isabus_bridge_class_init(ObjectClass *klass, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> > -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> >  
> > -    k->init = isabus_bridge_init;
> > +    dc->realize = isabus_bridge_realize;
> >      dc->fw_name = "isa";
> >      dc->no_user = 1;
> >  }
> 
> I would propose to simply drop the init function instead of turning it
> into a realize function unless you see a future use case for it.

There are many other devices using init, so simply drop it now won't
compile. One of the goals is to remove the init after all devices are
converted from init to realize.

> 
> Historically it was not possible to omit a SysBusDevice init function:
> http://git.qemu.org/?p=qemu.git;a=commit;h=4ce5dae88ecf2bafa0cd663de7e923728b1b3672
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 00/26] use realizefn for SysBusDevice, part 1

[Hu Tao]

On Sun, Jun 30, 2013 at 06:45:37PM +0200, Andreas Färber wrote:
> Hi,
> 
> Am 22.06.2013 10:50, schrieb Hu Tao:
> > This series updates part of devices inheriting from SysbusDevice
> > to use DeviceState::realize, and QOM'ify them.
> > 
> > These devices are default to x86_64-softmmu. I'm planning to
> > make patches in the same manner, that is, each series is for
> > devices default to each target. After all devices are converted
> > to realizefn, SysBusDevice::init can be removed.
> 
> This series looks promising. As was requested by others, please rebase
> (possibly on qom-next if there's conflicts with Peter M.'s patch?) and
> reorder the patches so that QOM cast macros come before more functional
> changes (the former can be picked up more quickly).

Sure.

> 
> For ARM I am looking into a few devices surrounding cpu/a9mpcore.c.
> Paolo has announced a PULL for his MemoryRegion refactorings for
> beginning of the week, which is likely to conflict with realizefn work.
> 
> As a reminder for realize series (we should definitely document this!):
> * QOM realize functions should not create new devices as that creates
> problems with recursive realized = true and will prohibit changing
> static device properties through QMP. (Therefore more review work.)
> * Care must be taken with bus creation since qbus_create[_inplace]() /
> qbus_realize() check for DeviceState::id, which is still NULL at
> instance_init time. (OHCI looked good for instance.)

Thanks!

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Hu Tao]

On Sun, Jun 30, 2013 at 04:36:13PM +0200, Andreas Färber wrote:
> Am 25.06.2013 19:45, schrieb Eduardo Habkost:
> > On Tue, Jun 25, 2013 at 10:20:08AM +0800, Hu Tao wrote:
> > [...]
> >>> Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itself?
> >>>
> >>> From DeviceClass documentation:
> >>>
> >>>  * If a type derived directly from TYPE_DEVICE implements @realize, it does
> >>>  * not need to implement @init and therefore does not need to store and call
> >>>  * #DeviceClass' default @realize callback.
> >>>  * For other types consult the documentation and implementation of the
> >>>  * respective parent types.
> >>>
> >>> The problem is that there's no documentation about ->realize() on
> >>> SysBusDeviceClass. Can we please explicitly document SysBusDeviceClass
> >>> expectations about ->realize() first, before making those changes?
> 
> If someone wants to add a paragraph to sysbus.h:SysBusDeviceClass
> documentation I would happily ack or pick that up. :)
> 
> >> IIUC, subclass's overriding ->realize should call parent's ->realize at
> >> some point. Peter Crosthwaite has a patchset to access a object's parent
> >> class at http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02982.html
> >>
> >> Regarding SysBusDevice::init and SysBusDevice::realize, I think it's the
> >> same as in the case of DeviceClass. If you agree I'll document it as in
> >> DeviceClass.
> > 
> > I believe it is reasonable to document that SysBusDevice subclasses
> > don't need to call the parent ->realize() method, like on DeviceClass.
> > This is exactly what this patch set does, after all, and I haven't seen
> > anybody complaining about it yet.
> 
> So the thing is that SysBusDevice's DeviceClass::init implementation,
> called by DeviceState's DeviceClass::realize implementation, just calls
> SysBusDeviceClass::init if non-NULL. When we introduce our own device's
> realizefn to replace our SysBusDeviceClass::init implementation, there
> is no point calling that effectively no-op DeviceClass::realize
> implementation.

This is true because we are in transition from DeviceClass:init to
DeviceClass:realize, by calling sub-class's DeviceClass:init in
DeviceClass's realize. But once the transition is done, and
DeviceClass's (and any intermediate devired classes') realize does
do something, we can't just ignore it in overriding realize.

Fix me if i'm wrong.

>                 And if we tried to, ...
> * ... how would we decide whether to call the parent's implementation
> first or last? It's not yes or no, it's no or when. Changing between
> either is more than just moving one line, it affects error handling and
> with knowledge about parent implementation never failing we could so far
> save us some work.

Agreed.

> * ... with the current QOM method scheme we'd go insane introducing a
> FooClass per device to save SysBusDevice's DeviceClass::realize in
> FooClass::parent_realize. Still waiting for Anthony on whether and how
> we want to change our scheme.
> 
> Long story short: If someone wants to mess with base classes they get to
> check derived classes for compatibility with their change.
> 
> Ideally qtest would help automate that to some degree.
> I would be all in favor if someone wanted to add a dummy test case per
> non-default, non-KVM device converted so that we can see that we didn't
> screw up -device instantiation too badly.
> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Andreas Färber]

Am 01.07.2013 11:31, schrieb Hu Tao:
> On Sun, Jun 30, 2013 at 04:36:13PM +0200, Andreas Färber wrote:
>> Am 25.06.2013 19:45, schrieb Eduardo Habkost:
>>> On Tue, Jun 25, 2013 at 10:20:08AM +0800, Hu Tao wrote:
>>> [...]
>>>>> Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itself?
>>>>>
>>>>> From DeviceClass documentation:
>>>>>
>>>>>  * If a type derived directly from TYPE_DEVICE implements @realize, it does
>>>>>  * not need to implement @init and therefore does not need to store and call
>>>>>  * #DeviceClass' default @realize callback.
>>>>>  * For other types consult the documentation and implementation of the
>>>>>  * respective parent types.
>>>>>
>>>>> The problem is that there's no documentation about ->realize() on
>>>>> SysBusDeviceClass. Can we please explicitly document SysBusDeviceClass
>>>>> expectations about ->realize() first, before making those changes?
>>
>> If someone wants to add a paragraph to sysbus.h:SysBusDeviceClass
>> documentation I would happily ack or pick that up. :)
>>
>>>> IIUC, subclass's overriding ->realize should call parent's ->realize at
>>>> some point. Peter Crosthwaite has a patchset to access a object's parent
>>>> class at http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02982.html
>>>>
>>>> Regarding SysBusDevice::init and SysBusDevice::realize, I think it's the
>>>> same as in the case of DeviceClass. If you agree I'll document it as in
>>>> DeviceClass.
>>>
>>> I believe it is reasonable to document that SysBusDevice subclasses
>>> don't need to call the parent ->realize() method, like on DeviceClass.
>>> This is exactly what this patch set does, after all, and I haven't seen
>>> anybody complaining about it yet.
>>
>> So the thing is that SysBusDevice's DeviceClass::init implementation,
>> called by DeviceState's DeviceClass::realize implementation, just calls
>> SysBusDeviceClass::init if non-NULL. When we introduce our own device's
>> realizefn to replace our SysBusDeviceClass::init implementation, there
>> is no point calling that effectively no-op DeviceClass::realize
>> implementation.
> 
> This is true because we are in transition from DeviceClass:init to
> DeviceClass:realize, by calling sub-class's DeviceClass:init in
> DeviceClass's realize.

Correct.

> But once the transition is done, and
> DeviceClass's (and any intermediate devired classes') realize does
> do something, we can't just ignore it in overriding realize.

We have the following hierarchy:

Object
+Device
  + SysBusDevice
    + EHCI
      + FaradayEHCI

Object does not know about realize.

Device has a realizefn that calls DeviceClass::init today, nothing more.
Therefore SysBusDevice doesn't need to additionally call that today.

Since, e.g., EHCI implements a realizefn, derived types need to call
their parent's realizefn, i.e. FaradayEHCI EHCI's or if there were
another model F derived from Faraday, then F FaradayEHCI's. Correct.

Once the transition is done, I expect those four lines to go away, with
Device's realizefn seriously doing nothing, as a fallback to avoid if
(dc->realized) {...} type code.
The sysbus_get_default() assignment could easily be moved to
SysBusDevice's instance_init, so I don't see anything from qdev_create()
/ qdev_init() that would need to be moved there. Do you?

The way Paolo proposed it, realize_children would be separate from
realize and called directly from DeviceState's property setter, so it
could be overridden independently.

Andreas

>>                 And if we tried to, ...
>> * ... how would we decide whether to call the parent's implementation
>> first or last? It's not yes or no, it's no or when. Changing between
>> either is more than just moving one line, it affects error handling and
>> with knowledge about parent implementation never failing we could so far
>> save us some work.
> 
> Agreed.
> 
>> * ... with the current QOM method scheme we'd go insane introducing a
>> FooClass per device to save SysBusDevice's DeviceClass::realize in
>> FooClass::parent_realize. Still waiting for Anthony on whether and how
>> we want to change our scheme.
>>
>> Long story short: If someone wants to mess with base classes they get to
>> check derived classes for compatibility with their change.
>>
>> Ideally qtest would help automate that to some degree.
>> I would be all in favor if someone wanted to add a dummy test case per
>> non-default, non-KVM device converted so that we can see that we didn't
>> screw up -device instantiation too badly.
>>
>> Regards,
>> Andreas
>>
>> -- 
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg