Skip to contentSkip to conversation

use realizefn for SysBusDevice, part 1

PATCH Superseded Hu Tao 5 participants 47 messages Jun 22, 2013 – Jul 1, 2013

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Hu Tao

author
#0 lore raw

cover use realizefn for SysBusDevice, part 1

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(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH 1/26 ohci: use realize for ohci

Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ohci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
hw/usb/hcd-ohci.c +7 -9
index 51241cd..79ef41b 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
#2 lore raw

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

Hi Hu,

Quoted 11 lines · Hu Tao
Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ohci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
hw/usb/hcd-ohci.c +0 -0
index 51241cd..79ef41b 100644

hw/usb/hcd-ohci.c typedef struct { · line 1881

Quoted 8 lines
hw/usb/hcd-ohci.c +2 -2
@@ -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?

hw/usb/hcd-ohci.c typedef struct { · line 1886

Quoted 6 lines
hw/usb/hcd-ohci.c +2 -1
+    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

Quoted 24 lines
hw/usb/hcd-ohci.c +3 -7
-    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

author
#3 lore raw

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

Quoted 17 lines · Peter Crosthwaite

Hi Hu,

Quoted 14 lines · Hu Tao
Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ohci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
hw/usb/hcd-ohci.c +0 -0
index 51241cd..79ef41b 100644
@@ -1876,17 +1876,16 @@ typedef struct {
     dma_addr_t dma_offset;
 } OHCISysBusState;
 
Quoted 8 lines

hw/usb/hcd-ohci.c typedef struct { · line 1881

Quoted 5 lines
hw/usb/hcd-ohci.c +2 -2
-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?

Quoted 10 lines

hw/usb/hcd-ohci.c typedef struct { · line 1886

Quoted 7 lines
hw/usb/hcd-ohci.c +2 -1
+    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.

#4 lore raw

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

Hi Hu,

Quoted 27 lines · Hu Tao
Quoted 17 lines · Peter Crosthwaite

Hi Hu,

Quoted 14 lines · Hu Tao
Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ohci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
hw/usb/hcd-ohci.c +0 -0
index 51241cd..79ef41b 100644
@@ -1876,17 +1876,16 @@ typedef struct {
     dma_addr_t dma_offset;
 } OHCISysBusState;
 
Quoted 8 lines

hw/usb/hcd-ohci.c typedef struct { · line 1881

Quoted 5 lines
hw/usb/hcd-ohci.c +2 -2
-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?

Quoted 3 lines

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

Quoted 16 lines
Quoted 10 lines

hw/usb/hcd-ohci.c typedef struct { · line 1886

Quoted 7 lines
hw/usb/hcd-ohci.c +2 -1
+    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.

author
#5 lore raw

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

Quoted 30 lines · Peter Crosthwaite

Hi Hu,

Quoted 27 lines · Hu Tao
Quoted 25 lines · Peter Crosthwaite

Hi Hu,

Quoted 11 lines · Hu Tao
Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ohci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
hw/usb/hcd-ohci.c +0 -0
index 51241cd..79ef41b 100644

hw/usb/hcd-ohci.c typedef struct { · line 1881

Quoted 8 lines
hw/usb/hcd-ohci.c +2 -2
@@ -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?

Quoted 6 lines
Quoted 3 lines

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.

Quoted 4 lines

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.

Quoted 20 lines

Regards,
Peter

Quoted 16 lines
Quoted 10 lines

hw/usb/hcd-ohci.c typedef struct { · line 1886

Quoted 7 lines
hw/usb/hcd-ohci.c +2 -1
+    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.

#6 lore raw

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

In reply to Hu Tao
Quoted 15 lines · Hu Tao
Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ohci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
hw/usb/hcd-ohci.c +0 -1
index 51241cd..79ef41b 100644
@@ -1876,17 +1876,16 @@ typedef struct {
     dma_addr_t dma_offset;
 } OHCISysBusState;
 
-static int ohci_init_pxa(SysBusDevice *dev)

hw/usb/hcd-ohci.c typedef struct { · line 1886

Quoted 10 lines
hw/usb/hcd-ohci.c +4 -2
+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)

Quoted 24 lines
hw/usb/hcd-ohci.c +3 -6
-    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;
 }
Signature · 1 line
1.8.3.1
Signature · 1 line
Eduardo
author
#7 lore raw

PATCH 2/26 ohci: QOM'ify some more

In reply to Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ohci.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
hw/usb/hcd-ohci.c +10 -5
index 79ef41b..2025593 100644
@@ -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,
Signature · 1 line
1.8.3.1
author
#8 lore raw

PATCH 3/26 i440fx-pcihost: use realize for i440fx-pcihost

In reply to Hu Tao
Cc Anthony Liguori <[email protected]>
Cc "Andreas Färber" <[email protected]>
Cc "Michael S. Tsirkin" <[email protected]>
Cc Laszlo Ersek <[email protected]>
Cc Paolo Bonzini <[email protected]>
---
 hw/pci-host/piix.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
hw/pci-host/piix.c +15 -10
index f9e68c3..801341a 100644
@@ -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,
 };
 
Signature · 1 line
1.8.3.1
author
#9 lore raw

PATCH 4/26 i440fx: use type-safe cast instead of directly access of parent dev

In reply to Hu Tao
Cc Anthony Liguori <[email protected]>
Cc "Andreas Färber" <[email protected]>
Cc "Michael S. Tsirkin" <[email protected]>
Cc Laszlo Ersek <[email protected]>
Cc Paolo Bonzini <[email protected]>
---
 hw/pci-host/piix.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
hw/pci-host/piix.c +7 -4
index 801341a..e2437df 100644
@@ -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);
Signature · 1 line
1.8.3.1
author
#10 lore raw

PATCH 5/26 q35: use realize for q35 host

In reply to Hu Tao
Cc "Michael S. Tsirkin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Alex Williamson <[email protected]>
Cc Isaku Yamahata <[email protected]>
Cc Jason Baron <[email protected]>
---
 hw/pci-host/q35.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)
hw/pci-host/q35.c +22 -23
index 24df6b5..bbecee6 100644
@@ -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);
Signature · 1 line
1.8.3.1
author
#11 lore raw

PATCH 6/26 q35: use type-safe cast instead of directly access of parent dev

In reply to Hu Tao

And remove variables if possible.

Cc "Michael S. Tsirkin" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Alex Williamson <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Jason Baron <[email protected]>
---
 hw/pci-host/q35.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
hw/pci-host/q35.c +10 -7
index bbecee6..bb5d506 100644
@@ -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();
 }
Signature · 1 line
1.8.3.1
author
#12 lore raw

PATCH 7/26 fdc: use realize for fdc.

In reply to Hu Tao
Cc Kevin Wolf <[email protected]>
Cc Stefan Hajnoczi <[email protected]>
---
 hw/block/fdc.c | 60 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 21 deletions(-)
hw/block/fdc.c +39 -21
index 930f191..8b8f60e 100644
@@ -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,
 };
 
Signature · 1 line
1.8.3.1
author
#13 lore raw

PATCH 8/26 fdc: QOM'ify some more

In reply to Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc Kevin Wolf <[email protected]>
Cc Stefan Hajnoczi <[email protected]>
---
 hw/block/fdc.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)
hw/block/fdc.c +15 -13
index 8b8f60e..4f5f8a8 100644
@@ -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,
Signature · 1 line
1.8.3.1
author
#14 lore raw

PATCH 9/26 pflash_cfi01: use realize for pflash_cfi01

In reply to Hu Tao
Cc Kevin Wolf <[email protected]>
Cc Stefan Hajnoczi <[email protected]>
---
 hw/block/pflash_cfi01.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
hw/block/pflash_cfi01.c +6 -8
index 63d7c99..aa04a2e 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
author
#15 lore raw

PATCH 10/26 pflash-cfi01: QOM'ify some more

In reply to Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc Kevin Wolf <[email protected]>
Cc Stefan Hajnoczi <[email protected]>
---
 hw/block/pflash_cfi01.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
hw/block/pflash_cfi01.c +8 -8
index aa04a2e..cc27f92 100644
@@ -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)
Signature · 1 line
1.8.3.1
author
#16 lore raw

PATCH 11/26 pflash_cfi02: use realize for pflash_cfi02

In reply to Hu Tao
Cc Kevin Wolf <[email protected]>
Cc Stefan Hajnoczi <[email protected]>
---
 hw/block/pflash_cfi02.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
hw/block/pflash_cfi02.c +8 -9
index 5f25246..45db415 100644
@@ -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;
 }
 
Signature · 1 line
1.8.3.1
author
#17 lore raw

PATCH 12/26 pflash-cfi02: QOM'ify some more

In reply to Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc Kevin Wolf <[email protected]>
Cc Stefan Hajnoczi <[email protected]>
---
 hw/block/pflash_cfi02.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
hw/block/pflash_cfi02.c +8 -8
index 45db415..412de7f 100644
@@ -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);
 }
Signature · 1 line
1.8.3.1
author
#18 lore raw

PATCH 13/26 ahci: use realize for ahci

In reply to Hu Tao
Cc Kevin Wolf <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Jason Baron <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc David Gibson <[email protected]>
---
 hw/ide/ahci.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
hw/ide/ahci.c +6 -8
index eab6096..c2e4b8c 100644
@@ -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;
Signature · 1 line
1.8.3.1
author
#19 lore raw

PATCH 14/26 ahci: QOM'ify some more

In reply to Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc Kevin Wolf <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Jason Baron <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc "Andreas Färber" <[email protected]>
---
 hw/ide/ahci.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
hw/ide/ahci.c +7 -4
index c2e4b8c..d37c094 100644
@@ -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,
Signature · 1 line
1.8.3.1
author
#20 lore raw

PATCH 15/26 fwcfg: use realize for fwcfg

In reply to Hu Tao
Cc Markus Armbruster <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Blue Swirl <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc "Andreas Färber" <[email protected]>
---
 hw/nvram/fw_cfg.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
hw/nvram/fw_cfg.c +16 -10
index 3c255ce..f5c85ec 100644
@@ -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,
 };
 
Signature · 1 line
1.8.3.1
author
#21 lore raw

PATCH 16/26 fwcfg: QOM'ify some more

In reply to Hu Tao

Use type constant if possible and avoid DO_UPCAST().

Cc Markus Armbruster <[email protected]>
Cc Blue Swirl <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc "Andreas Färber" <[email protected]>
---
 hw/nvram/fw_cfg.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
hw/nvram/fw_cfg.c +7 -7
index f5c85ec..e580481 100644
@@ -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)
Signature · 1 line
1.8.3.1
author
#22 lore raw

PATCH 17/26 scsi esp: use realize for scsi esp

In reply to Hu Tao
Cc Paolo Bonzini <[email protected]>
---
 hw/scsi/esp.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)
hw/scsi/esp.c +21 -11
index 0c81a50..0207dba 100644
@@ -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,
 };
 
Signature · 1 line
1.8.3.1
author
#23 lore raw

PATCH 18/26 scsi esp: QOM'ify some more

In reply to Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc Paolo Bonzini <[email protected]>
---
 hw/scsi/esp.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
hw/scsi/esp.c +10 -8
index 0207dba..aea2c94 100644
@@ -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,
Signature · 1 line
1.8.3.1
author
#24 lore raw

PATCH 19/26 hpet: use realize for hpet

In reply to Hu Tao
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc "Andreas Färber" <[email protected]>
Cc Avi Kivity <[email protected]>
---
 hw/timer/hpet.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)
hw/timer/hpet.c +17 -12
index 95dd01d..95aad3a 100644
@@ -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,
 };
 
Signature · 1 line
1.8.3.1
author
#25 lore raw

PATCH 20/26 hpet: QOM'ify some more

In reply to Hu Tao

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

Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc "Andreas Färber" <[email protected]>
Cc Avi Kivity <[email protected]>
---
 hw/timer/hpet.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
hw/timer/hpet.c +11 -7
index 95aad3a..c2c4504 100644
@@ -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,
Signature · 1 line
1.8.3.1
author
#26 lore raw

PATCH 21/26 kvmclock: use realize for kvmclock

In reply to Hu Tao
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/i386/kvm/clock.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
hw/i386/kvm/clock.c +3 -5
index 87d4d0f..74aa240 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
#27 lore raw

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

Quoted 16 lines · On Sat, 22 Jun 2013 16:50:33 +0800 Hu Tao <[email protected]> wrote:
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/i386/kvm/clock.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
hw/i386/kvm/clock.c +0 -0
index 87d4d0f..74aa240 100644

hw/i386/kvm/clock.c static void kvmclock_vm_state_change(void *opaque, int running, · line 92

Quoted 8 lines
hw/i386/kvm/clock.c +2 -2
@@ -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

Quoted 17 lines
hw/i386/kvm/clock.c +1 -3
     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;
 }
author
#28 lore raw

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

Quoted 22 lines · Igor Mammedov
Quoted 19 lines · On Sat, 22 Jun 2013 16:50:33 +0800 Hu Tao <[email protected]> wrote:
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/i386/kvm/clock.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
hw/i386/kvm/clock.c +0 -0
index 87d4d0f..74aa240 100644
@@ -87,12 +87,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
     }
 }
 
Quoted 6 lines

hw/i386/kvm/clock.c static void kvmclock_vm_state_change(void *opaque, int running, · line 92

Quoted 5 lines
hw/i386/kvm/clock.c +2 -2
-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.

#29 lore raw

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

In reply to Hu Tao
Quoted 31 lines · Hu Tao
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/i386/kvm/clock.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
hw/i386/kvm/clock.c +2 -3
index 87d4d0f..74aa240 100644
@@ -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 = {

hw/i386/kvm/clock.c static const VMStateDescription kvmclock_vmsd = { · line 114

Quoted 7 lines
hw/i386/kvm/clock.c +1 -2
@@ -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?

Quoted 1 line

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?

Quoted 7 lines
     dc->no_user = 1;
     dc->vmsd = &kvmclock_vmsd;

}

Signature · 1 line
1.8.3.1
Signature · 1 line
Eduardo
author
#30 lore raw

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

Quoted 50 lines · Eduardo Habkost
Quoted 31 lines · Hu Tao
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Eduardo Habkost <[email protected]>
---
 hw/i386/kvm/clock.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
hw/i386/kvm/clock.c +2 -3
index 87d4d0f..74aa240 100644
@@ -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 = {

hw/i386/kvm/clock.c static const VMStateDescription kvmclock_vmsd = { · line 114

Quoted 7 lines
hw/i386/kvm/clock.c +1 -2
@@ -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.

Quoted 3 lines

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.

#31 lore raw

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

On Tue, Jun 25, 2013 at 10:20:08AM +0800, Hu Tao wrote:
[...]

Quoted 14 lines
Quoted 10 lines

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.

Quoted 3 lines

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?

Quoted 7 lines

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.

Signature · 1 line
Eduardo
#32 lore raw

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

Am 25.06.2013 19:45, schrieb Eduardo Habkost:

Quoted 12 lines · Hu Tao

[...]

Quoted 10 lines
Quoted 10 lines

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.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

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. :)

Quoted 4 lines
Quoted 4 lines

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

Quoted 8 lines
Quoted 3 lines

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

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#33 lore raw

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

Quoted 33 lines · Andreas Färber

Am 25.06.2013 19:45, schrieb Eduardo Habkost:

Quoted 12 lines · Hu Tao

[...]

Quoted 10 lines
Quoted 10 lines

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.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

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. :)

Quoted 4 lines
Quoted 4 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 19 lines

* ... 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

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#34 lore raw

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

Am 01.07.2013 11:31, schrieb Hu Tao:

Quoted 41 lines · Andreas Färber
Quoted 33 lines

Am 25.06.2013 19:45, schrieb Eduardo Habkost:

Quoted 12 lines · Hu Tao

[...]

Quoted 10 lines
Quoted 10 lines

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.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

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. :)

Quoted 4 lines
Quoted 4 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

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

Quoted 28 lines
Quoted 6 lines

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.

Quoted 19 lines

* ... 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

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#35 lore raw

PATCH 22/26 kvmclock: QOM'ify some more

In reply to Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Anthony Liguori <[email protected]>
---
 hw/i386/kvm/clock.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
hw/i386/kvm/clock.c +7 -4
index 74aa240..eaad880 100644
@@ -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);
     }
 }
 
Signature · 1 line
1.8.3.1
Reviewed-by on v1 22/26
#36 lore raw

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

Quoted 9 lines · Hu Tao

Introduce type constant and avoid DO_UPCAST().

Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Igor Mammedov <[email protected]>
Cc Anthony Liguori <[email protected]>

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.

Quoted 56 lines
---
 hw/i386/kvm/clock.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
hw/i386/kvm/clock.c +7 -4
index 74aa240..eaad880 100644
@@ -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);
     }
 }
 
Signature · 1 line
1.8.3.1
Signature · 1 line
Eduardo
author
#37 lore raw

PATCH 23/26 kvmvapic realize

In reply to Hu Tao
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Blue Swirl <[email protected]>
Cc Jan Kiszka <[email protected]>
---
 hw/i386/kvmvapic.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
hw/i386/kvmvapic.c +5 -7
index 655483b..06d290c 100644
@@ -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 = {
Signature · 1 line
1.8.3.1
author
#38 lore raw

PATCH 24/26 ioapic: use realize for ioapic

In reply to Hu Tao
Cc Paolo Bonzini <[email protected]>
Cc "Andreas Färber" <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Igor Mammedov <[email protected]>
---
 hw/intc/ioapic_common.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
hw/intc/ioapic_common.c +4 -6
index 5c5bb3c..6077024 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
author
#39 lore raw

PATCH 25/26 isa bus: use realize for isa bus

In reply to Hu Tao
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Avi Kivity <[email protected]>
Cc Jim Meyering <[email protected]>
---
 hw/isa/isa-bus.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
hw/isa/isa-bus.c +3 -4
index 136d17e..287f941 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
#40 lore raw

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

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:

Quoted 5 lines
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Avi Kivity <[email protected]>

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.

Quoted 11 lines
Cc Jim Meyering <[email protected]>
---
 hw/isa/isa-bus.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
hw/isa/isa-bus.c +0 -0
index 136d17e..287f941 100644

hw/isa/isa-bus.c static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent) · line 198

Quoted 9 lines
hw/isa/isa-bus.c +2 -2
@@ -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.

hw/isa/isa-bus.c static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent) · line 208

Quoted 12 lines
hw/isa/isa-bus.c +1 -2
 }
 
 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

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#41 lore raw

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

Quoted 11 lines · 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:

Quoted 5 lines
Cc "Andreas Färber" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Anthony Liguori <[email protected]>
Cc Avi Kivity <[email protected]>

This will bounce, please drop. ;)

Noticed that. :)

Quoted 4 lines

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 <[email protected]> (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.

Quoted 31 lines
Quoted 12 lines
Cc Jim Meyering <[email protected]>
---
 hw/isa/isa-bus.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
hw/isa/isa-bus.c +0 -0
index 136d17e..287f941 100644

hw/isa/isa-bus.c static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent) · line 198

Quoted 9 lines
hw/isa/isa-bus.c +2 -2
@@ -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.

Quoted 7 lines
hw/isa/isa-bus.c +0 -1
 }
 
 static void isabus_bridge_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
Quoted 8 lines

hw/isa/isa-bus.c static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent) · line 208

Quoted 5 lines
hw/isa/isa-bus.c +1 -1
-    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.

Quoted 9 lines

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

Andreas

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#42 lore raw

PATCH 26/26 ehci: use realize for ehci

In reply to Hu Tao
Cc Gerd Hoffmann <[email protected]>
---
 hw/usb/hcd-ehci-sysbus.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
hw/usb/hcd-ehci-sysbus.c +5 -6
index b68a66a..8cb594d 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
#43 lore raw

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

Am 22.06.2013 10:50, schrieb Hu Tao:

Quoted 5 lines
Cc Gerd Hoffmann <[email protected]>
---
 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

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#44 lore raw

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

Am 22.06.2013 12:38, schrieb Andreas Färber:

Quoted 8 lines

Am 22.06.2013 10:50, schrieb Hu Tao:

Quoted 5 lines
Cc Gerd Hoffmann <[email protected]>
---
 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

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#45 lore raw
In reply to Hu Tao

Hi,

Am 22.06.2013 10:50, schrieb Hu Tao:

Quoted 7 lines

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

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
author
#46 lore raw
Quoted 11 lines · Andreas Färber

Hi,

Am 22.06.2013 10:50, schrieb Hu Tao:

Quoted 7 lines

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.

Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 7 lines

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!