[Qemu-devel] [PATCH v2 0/2] pci_host: Convert to QOM

[Qemu-devel] [PATCH v2 0/2] pci_host: Convert to QOM

[Andreas Färber]

Hello Michael,

This series, based on a recent PReP RFC series, in turn based on Anthony's and
Wanpeng's i440fx refactoring, makes pci_host a proper QOM type and in this v2
also updates all remaining host controllers beyond PReP.

Only compile-tested were typhoon, bonito, ppc440 and ppce500.
I've checked it also applies cleanly to the current pci branch.

Please review and apply.

Intended merge plan is:
Merge this preparatory series through pci branch.
Follow up with move of struct PCIBus from pci_internals.h to pci.h (currently
typedef in qemu-common.h) once qom-next QBus refactoring is merged (qom-next-2),
merge through pci branch.
Rebase Raven PCI in-place initialization on that, merge through prep branch.

Regards,
Andreas

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Anthony Liguori <anthony@codemonkey.ws>

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> (still no MAINTAINERS entry!)

Cc: Jan Kiszka <jan.kiszka@siemens.com> (FYI)

Andreas Färber (2):
  pci_host: Turn into SysBus-derived QOM type
  pci_host: Derive remaining PCI host controllers from TYPE_PCI_HOST

 hw/alpha_typhoon.c |    4 +-
 hw/bonito.c        |   73 ++++++++++++++++++++++++++++++++--------------------
 hw/dec_pci.c       |   11 ++++---
 hw/dec_pci.h       |    2 +
 hw/grackle_pci.c   |   22 ++++++++-------
 hw/gt64xxx.c       |   23 +++++++++-------
 hw/pci_host.c      |   11 ++++++++
 hw/pci_host.h      |    3 ++
 hw/piix_pci.c      |   14 +++++----
 hw/ppc4xx_pci.c    |    4 +-
 hw/ppc_mac.h       |    1 +
 hw/ppc_prep.c      |    4 +--
 hw/ppce500_pci.c   |    7 +++--
 hw/prep_pci.c      |    7 +++--
 hw/spapr_pci.c     |   17 ++++++-----
 hw/spapr_pci.h     |    6 +++-
 hw/unin_pci.c      |   13 +++++----
 17 files changed, 135 insertions(+), 87 deletions(-)

-- 
1.7.7

[Qemu-devel] [PATCH v2 1/2] pci_host: Turn into SysBus-derived QOM type

[Andreas Färber]

From: Andreas Färber <andreas.faerber@web.de>

Allows us to access PCIHostState QOM-style with PCI_HOST() macro.

Update PReP Raven PCI to derive from this type.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/pci_host.c |   11 +++++++++++
 hw/pci_host.h |    3 +++
 hw/prep_pci.c |    4 ++--
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 8041778..347bfa6 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
+static const TypeInfo pci_host_type_info = {
+    .name = TYPE_PCI_HOST,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(PCIHostState),
+};
+
+static void pci_host_register_types(void)
+{
+    type_register_static(&pci_host_type_info);
+}
 
+type_init(pci_host_register_types)
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 359e38f..fba9fde 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -30,6 +30,9 @@
 
 #include "sysbus.h"
 
+#define TYPE_PCI_HOST "pci-host"
+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
+
 struct PCIHostState {
     SysBusDevice busdev;
     MemoryRegion conf_mem;
diff --git a/hw/prep_pci.c b/hw/prep_pci.c
index 38dbff4..3e9f6b8 100644
--- a/hw/prep_pci.c
+++ b/hw/prep_pci.c
@@ -183,9 +183,9 @@ static void raven_pcihost_class_init(ObjectClass *klass, void *data)
     dc->no_user = 1;
 }
 
-static TypeInfo raven_pcihost_info = {
+static const TypeInfo raven_pcihost_info = {
     .name = "raven-pcihost",
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_PCI_HOST,
     .instance_size = sizeof(PREPPCIState),
     .class_init = raven_pcihost_class_init,
 };
-- 
1.7.7

[Qemu-devel] [PATCH v2 2/2] pci_host: Derive remaining PCI host controllers from TYPE_PCI_HOST

[Andreas Färber]

Some used a dummy "busdev" field to trick qdev upcast macros.
QOM requires the parent type's struct to be placed as first field.
Drop such bogus fields and use QOM-style casts instead.

Some typedef'ed their state to PCIHostState. Use a proper struct,
and use PCIHostState and PCI_HOST() where appropriate.

Also make TypeInfo const where touched.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/alpha_typhoon.c |    4 +-
 hw/bonito.c        |   73 ++++++++++++++++++++++++++++++++--------------------
 hw/dec_pci.c       |   11 ++++---
 hw/dec_pci.h       |    2 +
 hw/grackle_pci.c   |   22 ++++++++-------
 hw/gt64xxx.c       |   23 +++++++++-------
 hw/piix_pci.c      |   14 +++++----
 hw/ppc4xx_pci.c    |    4 +-
 hw/ppc_mac.h       |    1 +
 hw/ppc_prep.c      |    4 +--
 hw/ppce500_pci.c   |    7 +++--
 hw/prep_pci.c      |    3 +-
 hw/spapr_pci.c     |   17 ++++++-----
 hw/spapr_pci.h     |    6 +++-
 hw/unin_pci.c      |   13 +++++----
 15 files changed, 119 insertions(+), 85 deletions(-)

diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
index 872e112..3ec49b8 100644
--- a/hw/alpha_typhoon.c
+++ b/hw/alpha_typhoon.c
@@ -817,9 +817,9 @@ static void typhoon_pcihost_class_init(ObjectClass *klass, void *data)
     dc->no_user = 1;
 }
 
-static TypeInfo typhoon_pcihost_info = {
+static const TypeInfo typhoon_pcihost_info = {
     .name          = "typhoon-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(TyphoonState),
     .class_init    = typhoon_pcihost_class_init,
 };
diff --git a/hw/bonito.c b/hw/bonito.c
index 77786f8..3ef1ac0 100644
--- a/hw/bonito.c
+++ b/hw/bonito.c
@@ -180,7 +180,7 @@
 #define PCI_ADDR(busno,devno,funno,regno)  \
     ((((busno)<<16)&0xff0000) + (((devno)<<11)&0xf800) + (((funno)<<8)&0x700) + (regno))
 
-typedef PCIHostState BonitoState;
+typedef struct BonitoState BonitoState;
 
 typedef struct PCIBonitoState
 {
@@ -218,6 +218,15 @@ typedef struct PCIBonitoState
 
 } PCIBonitoState;
 
+#define TYPE_BONITO_PCI_HOST "Bonito-pcihost"
+
+#define BONITO_PCI_HOST(obj) \
+    OBJECT_CHECK(BonitoState, (obj), TYPE_BONITO_PCI_HOST)
+
+struct BonitoState {
+    PCIHostState parent_obj;
+};
+
 PCIBonitoState * bonito_state;
 
 static void bonito_writel(void *opaque, target_phys_addr_t addr,
@@ -402,6 +411,7 @@ static const MemoryRegionOps bonito_cop_ops = {
 static uint32_t bonito_sbridge_pciaddr(void *opaque, target_phys_addr_t addr)
 {
     PCIBonitoState *s = opaque;
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
     uint32_t cfgaddr;
     uint32_t idsel;
     uint32_t devno;
@@ -427,9 +437,9 @@ static uint32_t bonito_sbridge_pciaddr(void *opaque, target_phys_addr_t addr)
             ",pcimap_cfg=%x\n", addr, s->regs[BONITO_PCIMAP_CFG]);
         exit(1);
     }
-    pciaddr = PCI_ADDR(pci_bus_num(s->pcihost->bus), devno, funno, regno);
+    pciaddr = PCI_ADDR(pci_bus_num(pcihost->bus), devno, funno, regno);
     DPRINTF("cfgaddr %x pciaddr %x busno %x devno %d funno %d regno %d\n",
-        cfgaddr, pciaddr, pci_bus_num(s->pcihost->bus), devno, funno, regno);
+        cfgaddr, pciaddr, pci_bus_num(pcihost->bus), devno, funno, regno);
 
     return pciaddr;
 }
@@ -438,6 +448,7 @@ static void bonito_spciconf_writeb(void *opaque, target_phys_addr_t addr,
                                    uint32_t val)
 {
     PCIBonitoState *s = opaque;
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
     uint32_t pciaddr;
     uint16_t status;
 
@@ -449,8 +460,8 @@ static void bonito_spciconf_writeb(void *opaque, target_phys_addr_t addr,
     }
 
     /* set the pci address in s->config_reg */
-    s->pcihost->config_reg = (pciaddr) | (1u << 31);
-    pci_data_write(s->pcihost->bus, s->pcihost->config_reg, val & 0xff, 1);
+    pcihost->config_reg = (pciaddr) | (1u << 31);
+    pci_data_write(pcihost->bus, pcihost->config_reg, val & 0xff, 1);
 
     /* clear PCI_STATUS_REC_MASTER_ABORT and PCI_STATUS_REC_TARGET_ABORT */
     status = pci_get_word(s->dev.config + PCI_STATUS);
@@ -462,6 +473,7 @@ static void bonito_spciconf_writew(void *opaque, target_phys_addr_t addr,
                                    uint32_t val)
 {
     PCIBonitoState *s = opaque;
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
     uint32_t pciaddr;
     uint16_t status;
 
@@ -475,8 +487,8 @@ static void bonito_spciconf_writew(void *opaque, target_phys_addr_t addr,
     }
 
     /* set the pci address in s->config_reg */
-    s->pcihost->config_reg = (pciaddr) | (1u << 31);
-    pci_data_write(s->pcihost->bus, s->pcihost->config_reg, val, 2);
+    pcihost->config_reg = (pciaddr) | (1u << 31);
+    pci_data_write(pcihost->bus, pcihost->config_reg, val, 2);
 
     /* clear PCI_STATUS_REC_MASTER_ABORT and PCI_STATUS_REC_TARGET_ABORT */
     status = pci_get_word(s->dev.config + PCI_STATUS);
@@ -488,6 +500,7 @@ static void bonito_spciconf_writel(void *opaque, target_phys_addr_t addr,
                                    uint32_t val)
 {
     PCIBonitoState *s = opaque;
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
     uint32_t pciaddr;
     uint16_t status;
 
@@ -501,8 +514,8 @@ static void bonito_spciconf_writel(void *opaque, target_phys_addr_t addr,
     }
 
     /* set the pci address in s->config_reg */
-    s->pcihost->config_reg = (pciaddr) | (1u << 31);
-    pci_data_write(s->pcihost->bus, s->pcihost->config_reg, val, 4);
+    pcihost->config_reg = (pciaddr) | (1u << 31);
+    pci_data_write(pcihost->bus, pcihost->config_reg, val, 4);
 
     /* clear PCI_STATUS_REC_MASTER_ABORT and PCI_STATUS_REC_TARGET_ABORT */
     status = pci_get_word(s->dev.config + PCI_STATUS);
@@ -513,6 +526,7 @@ static void bonito_spciconf_writel(void *opaque, target_phys_addr_t addr,
 static uint32_t bonito_spciconf_readb(void *opaque, target_phys_addr_t addr)
 {
     PCIBonitoState *s = opaque;
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
     uint32_t pciaddr;
     uint16_t status;
 
@@ -524,19 +538,20 @@ static uint32_t bonito_spciconf_readb(void *opaque, target_phys_addr_t addr)
     }
 
     /* set the pci address in s->config_reg */
-    s->pcihost->config_reg = (pciaddr) | (1u << 31);
+    pcihost->config_reg = (pciaddr) | (1u << 31);
 
     /* clear PCI_STATUS_REC_MASTER_ABORT and PCI_STATUS_REC_TARGET_ABORT */
     status = pci_get_word(s->dev.config + PCI_STATUS);
     status &= ~(PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT);
     pci_set_word(s->dev.config + PCI_STATUS, status);
 
-    return pci_data_read(s->pcihost->bus, s->pcihost->config_reg, 1);
+    return pci_data_read(pcihost->bus, pcihost->config_reg, 1);
 }
 
 static uint32_t bonito_spciconf_readw(void *opaque, target_phys_addr_t addr)
 {
     PCIBonitoState *s = opaque;
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
     uint32_t pciaddr;
     uint16_t status;
 
@@ -550,19 +565,20 @@ static uint32_t bonito_spciconf_readw(void *opaque, target_phys_addr_t addr)
     }
 
     /* set the pci address in s->config_reg */
-    s->pcihost->config_reg = (pciaddr) | (1u << 31);
+    pcihost->config_reg = (pciaddr) | (1u << 31);
 
     /* clear PCI_STATUS_REC_MASTER_ABORT and PCI_STATUS_REC_TARGET_ABORT */
     status = pci_get_word(s->dev.config + PCI_STATUS);
     status &= ~(PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT);
     pci_set_word(s->dev.config + PCI_STATUS, status);
 
-    return pci_data_read(s->pcihost->bus, s->pcihost->config_reg, 2);
+    return pci_data_read(pcihost->bus, pcihost->config_reg, 2);
 }
 
 static uint32_t bonito_spciconf_readl(void *opaque, target_phys_addr_t addr)
 {
     PCIBonitoState *s = opaque;
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
     uint32_t pciaddr;
     uint16_t status;
 
@@ -576,14 +592,14 @@ static uint32_t bonito_spciconf_readl(void *opaque, target_phys_addr_t addr)
     }
 
     /* set the pci address in s->config_reg */
-    s->pcihost->config_reg = (pciaddr) | (1u << 31);
+    pcihost->config_reg = (pciaddr) | (1u << 31);
 
     /* clear PCI_STATUS_REC_MASTER_ABORT and PCI_STATUS_REC_TARGET_ABORT */
     status = pci_get_word(s->dev.config + PCI_STATUS);
     status &= ~(PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT);
     pci_set_word(s->dev.config + PCI_STATUS, status);
 
-    return pci_data_read(s->pcihost->bus, s->pcihost->config_reg, 4);
+    return pci_data_read(pcihost->bus, pcihost->config_reg, 4);
 }
 
 /* south bridge PCI configure space. 0x1fe8 0000 - 0x1fef ffff */
@@ -679,7 +695,8 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
 static int bonito_initfn(PCIDevice *dev)
 {
     PCIBonitoState *s = DO_UPCAST(PCIBonitoState, dev, dev);
-    SysBusDevice *sysbus = &s->pcihost->busdev;
+    SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
+    PCIHostState *pcihost = PCI_HOST(s->pcihost);
 
     /* Bonito North Bridge, built on FPGA, VENDOR_ID/DEVICE_ID are "undefined" */
     pci_config_set_prog_interface(dev->config, 0x00);
@@ -691,15 +708,15 @@ static int bonito_initfn(PCIDevice *dev)
     sysbus_mmio_map(sysbus, 0, BONITO_INTERNAL_REG_BASE);
 
     /* set the north bridge pci configure  mapping */
-    memory_region_init_io(&s->pcihost->conf_mem, &bonito_pciconf_ops, s,
+    memory_region_init_io(&pcihost->conf_mem, &bonito_pciconf_ops, s,
                           "north-bridge-pci-config", BONITO_PCICONFIG_SIZE);
-    sysbus_init_mmio(sysbus, &s->pcihost->conf_mem);
+    sysbus_init_mmio(sysbus, &pcihost->conf_mem);
     sysbus_mmio_map(sysbus, 1, BONITO_PCICONFIG_BASE);
 
     /* set the south bridge pci configure  mapping */
-    memory_region_init_io(&s->pcihost->data_mem, &bonito_spciconf_ops, s,
+    memory_region_init_io(&pcihost->data_mem, &bonito_spciconf_ops, s,
                           "south-bridge-pci-config", BONITO_SPCICONFIG_SIZE);
-    sysbus_init_mmio(sysbus, &s->pcihost->data_mem);
+    sysbus_init_mmio(sysbus, &pcihost->data_mem);
     sysbus_mmio_map(sysbus, 2, BONITO_SPCICONFIG_BASE);
 
     memory_region_init_io(&s->iomem_ldma, &bonito_ldma_ops, s,
@@ -743,13 +760,13 @@ PCIBus *bonito_init(qemu_irq *pic)
 {
     DeviceState *dev;
     PCIBus *b;
-    BonitoState *pcihost;
+    PCIHostState *pcihost;
     PCIBonitoState *s;
     PCIDevice *d;
 
-    dev = qdev_create(NULL, "Bonito-pcihost");
-    pcihost = FROM_SYSBUS(BonitoState, sysbus_from_qdev(dev));
-    b = pci_register_bus(&pcihost->busdev.qdev, "pci", pci_bonito_set_irq,
+    dev = qdev_create(NULL, TYPE_BONITO_PCI_HOST);
+    pcihost = PCI_HOST(dev);
+    b = pci_register_bus(dev, "pci", pci_bonito_set_irq,
                          pci_bonito_map_irq, pic, get_system_memory(),
                          get_system_io(),
                          0x28, 32);
@@ -759,7 +776,7 @@ PCIBus *bonito_init(qemu_irq *pic)
     /* set the pcihost pointer before bonito_initfn is called */
     d = pci_create(b, PCI_DEVFN(0, 0), "Bonito");
     s = DO_UPCAST(PCIBonitoState, dev, d);
-    s->pcihost = pcihost;
+    s->pcihost = BONITO_PCI_HOST(pcihost);
     bonito_state = s;
     qdev_init_nofail(&d->qdev);
 
@@ -797,9 +814,9 @@ static void bonito_pcihost_class_init(ObjectClass *klass, void *data)
     dc->no_user = 1;
 }
 
-static TypeInfo bonito_pcihost_info = {
-    .name          = "Bonito-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+static const TypeInfo bonito_pcihost_info = {
+    .name          = TYPE_BONITO_PCI_HOST,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(BonitoState),
     .class_init    = bonito_pcihost_class_init,
 };
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index 37337bf..074c35a 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -40,8 +40,9 @@
 #define DEC_DPRINTF(fmt, ...)
 #endif
 
+#define DEC_21154(obj) OBJECT_CHECK(DECState, (obj), TYPE_DEC_21154)
+
 typedef struct DECState {
-    SysBusDevice busdev;
     PCIHostState host_state;
 } DECState;
 
@@ -90,7 +91,7 @@ static int pci_dec_21154_device_init(SysBusDevice *dev)
 {
     DECState *s;
 
-    s = FROM_SYSBUS(DECState, dev);
+    s = DEC_21154(dev);
 
     memory_region_init_io(&s->host_state.conf_mem, &pci_host_conf_le_ops,
                           &s->host_state, "pci-conf-idx", 0x1000);
@@ -133,9 +134,9 @@ static void pci_dec_21154_device_class_init(ObjectClass *klass, void *data)
     sdc->init = pci_dec_21154_device_init;
 }
 
-static TypeInfo pci_dec_21154_device_info = {
-    .name          = "dec-21154-sysbus",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+static const TypeInfo pci_dec_21154_device_info = {
+    .name          = TYPE_DEC_21154,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(DECState),
     .class_init    = pci_dec_21154_device_class_init,
 };
diff --git a/hw/dec_pci.h b/hw/dec_pci.h
index 79264ba..17dc0c2 100644
--- a/hw/dec_pci.h
+++ b/hw/dec_pci.h
@@ -3,6 +3,8 @@
 
 #include "qemu-common.h"
 
+#define TYPE_DEC_21154 "dec-21154-sysbus"
+
 PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn);
 
 #endif
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index 81ff3a3..89cbddf 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -23,10 +23,9 @@
  * THE SOFTWARE.
  */
 
-#include "sysbus.h"
+#include "pci_host.h"
 #include "ppc_mac.h"
 #include "pci.h"
-#include "pci_host.h"
 
 /* debug Grackle */
 //#define DEBUG_GRACKLE
@@ -38,9 +37,12 @@
 #define GRACKLE_DPRINTF(fmt, ...)
 #endif
 
+#define GRACKLE_PCI_HOST(obj) \
+    OBJECT_CHECK(GrackleState, (obj), TYPE_GRACKLE_PCI_HOST)
+
 typedef struct GrackleState {
-    SysBusDevice busdev;
     PCIHostState host_state;
+
     MemoryRegion pci_mmio;
     MemoryRegion pci_hole;
 } GrackleState;
@@ -71,10 +73,10 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
     SysBusDevice *s;
     GrackleState *d;
 
-    dev = qdev_create(NULL, "grackle-pcihost");
+    dev = qdev_create(NULL, TYPE_GRACKLE_PCI_HOST);
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
-    d = FROM_SYSBUS(GrackleState, s);
+    d = GRACKLE_PCI_HOST(dev);
 
     memory_region_init(&d->pci_mmio, "pci-mmio", 0x100000000ULL);
     memory_region_init_alias(&d->pci_hole, "pci-hole", &d->pci_mmio,
@@ -82,7 +84,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
     memory_region_add_subregion(address_space_mem, 0x80000000ULL,
                                 &d->pci_hole);
 
-    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
+    d->host_state.bus = pci_register_bus(dev, "pci",
                                          pci_grackle_set_irq,
                                          pci_grackle_map_irq,
                                          pic,
@@ -102,7 +104,7 @@ static int pci_grackle_init_device(SysBusDevice *dev)
 {
     GrackleState *s;
 
-    s = FROM_SYSBUS(GrackleState, dev);
+    s = GRACKLE_PCI_HOST(dev);
 
     memory_region_init_io(&s->host_state.conf_mem, &pci_host_conf_le_ops,
                           &s->host_state, "pci-conf-idx", 0x1000);
@@ -150,9 +152,9 @@ static void pci_grackle_class_init(ObjectClass *klass, void *data)
     dc->no_user = 1;
 }
 
-static TypeInfo grackle_pci_host_info = {
-    .name          = "grackle-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+static const TypeInfo grackle_pci_host_info = {
+    .name          = TYPE_GRACKLE_PCI_HOST,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(GrackleState),
     .class_init    = pci_grackle_class_init,
 };
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index a2d0e5a..9ad69b8 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -229,9 +229,14 @@
     target_phys_addr_t regname ##_length;     \
     MemoryRegion regname ##_mem
 
+#define TYPE_GT64120_PCI_HOST "gt64120"
+
+#define GT64120_PCI_HOST(obj) \
+    OBJECT_CHECK(GT64120State, (obj), TYPE_GT64120_PCI_HOST)
+
 typedef struct GT64120State {
-    SysBusDevice busdev;
     PCIHostState pci;
+
     uint32_t regs[GT_REGS];
     PCI_MAPPING_ENTRY(PCI0IO);
     PCI_MAPPING_ENTRY(ISD);
@@ -1083,15 +1088,13 @@ static void gt64120_reset(void *opaque)
 
 PCIBus *gt64120_register(qemu_irq *pic)
 {
-    SysBusDevice *s;
     GT64120State *d;
     DeviceState *dev;
 
-    dev = qdev_create(NULL, "gt64120");
+    dev = qdev_create(NULL, TYPE_GT64120_PCI_HOST);
     qdev_init_nofail(dev);
-    s = sysbus_from_qdev(dev);
-    d = FROM_SYSBUS(GT64120State, s);
-    d->pci.bus = pci_register_bus(&d->busdev.qdev, "pci",
+    d = GT64120_PCI_HOST(dev);
+    d->pci.bus = pci_register_bus(dev, "pci",
                                   gt64120_pci_set_irq, gt64120_pci_map_irq,
                                   pic,
                                   get_system_memory(),
@@ -1107,7 +1110,7 @@ static int gt64120_init(SysBusDevice *dev)
 {
     GT64120State *s;
 
-    s = FROM_SYSBUS(GT64120State, dev);
+    s = GT64120_PCI_HOST(dev);
 
     /* FIXME: This value is computed from registers during reset, but some
        devices (e.g. VGA card) need to know it when they are registered.
@@ -1161,9 +1164,9 @@ static void gt64120_class_init(ObjectClass *klass, void *data)
     sdc->init = gt64120_init;
 }
 
-static TypeInfo gt64120_info = {
-    .name          = "gt64120",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+static const TypeInfo gt64120_info = {
+    .name          = TYPE_GT64120_PCI_HOST,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(GT64120State),
     .class_init    = gt64120_class_init,
 };
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 09e84f5..ee53d78 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -36,7 +36,9 @@
  * http://download.intel.com/design/chipsets/datashts/29054901.pdf
  */
 
-typedef PCIHostState I440FXState;
+typedef struct I440FXState {
+    PCIHostState parent_obj;
+} I440FXState;
 
 #define PIIX_NUM_PIC_IRQS       16      /* i8259 * 2 */
 #define PIIX_NUM_PIRQS          4ULL    /* PIRQ[A-D] */
@@ -224,7 +226,7 @@ static const VMStateDescription vmstate_i440fx = {
 
 static int i440fx_pcihost_initfn(SysBusDevice *dev)
 {
-    I440FXState *s = FROM_SYSBUS(I440FXState, dev);
+    PCIHostState *s = PCI_HOST(dev);
 
     memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s,
                           "pci-conf-idx", 4);
@@ -266,12 +268,12 @@ static PCIBus *i440fx_common_init(const char *device_name,
     DeviceState *dev;
     PCIBus *b;
     PCIDevice *d;
-    I440FXState *s;
+    PCIHostState *s;
     PIIX3State *piix3;
     PCII440FXState *f;
 
     dev = qdev_create(NULL, "i440FX-pcihost");
-    s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
+    s = PCI_HOST(dev);
     s->address_space = address_space_mem;
     b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
                     address_space_io, 0);
@@ -581,9 +583,9 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
     dc->no_user = 1;
 }
 
-static TypeInfo i440fx_pcihost_info = {
+static const TypeInfo i440fx_pcihost_info = {
     .name          = "i440FX-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(I440FXState),
     .class_init    = i440fx_pcihost_class_init,
 };
diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
index 203c3cd..334b9a6 100644
--- a/hw/ppc4xx_pci.c
+++ b/hw/ppc4xx_pci.c
@@ -393,9 +393,9 @@ static void ppc4xx_pcihost_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_ppc4xx_pci;
 }
 
-static TypeInfo ppc4xx_pcihost_info = {
+static const TypeInfo ppc4xx_pcihost_info = {
     .name          = "ppc4xx-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(PPC4xxPCIState),
     .class_init    = ppc4xx_pcihost_class_init,
 };
diff --git a/hw/ppc_mac.h b/hw/ppc_mac.h
index af75e45..277ffb6 100644
--- a/hw/ppc_mac.h
+++ b/hw/ppc_mac.h
@@ -55,6 +55,7 @@ qemu_irq *heathrow_pic_init(MemoryRegion **pmem,
                             int nb_cpus, qemu_irq **irqs);
 
 /* Grackle PCI */
+#define TYPE_GRACKLE_PCI_HOST "grackle-pcihost"
 PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
                          MemoryRegion *address_space_mem,
                          MemoryRegion *address_space_io);
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index be2b268..8bbce0e 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -470,7 +470,6 @@ static void ppc_prep_init (ram_addr_t ram_size,
     uint32_t kernel_base, initrd_base;
     long kernel_size, initrd_size;
     DeviceState *dev;
-    SysBusDevice *sys;
     PCIHostState *pcihost;
     PCIBus *pci_bus;
     PCIDevice *pci;
@@ -583,8 +582,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
     }
 
     dev = qdev_create(NULL, "raven-pcihost");
-    sys = sysbus_from_qdev(dev);
-    pcihost = DO_UPCAST(PCIHostState, busdev, sys);
+    pcihost = PCI_HOST(dev);
     pcihost->address_space = get_system_memory();
     object_property_add_child(qdev_get_machine(), "raven", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 0f60b24..e1acf50 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -74,6 +74,7 @@ struct pci_inbound {
 
 struct PPCE500PCIState {
     PCIHostState pci_state;
+
     struct pci_outbound pob[PPCE500_PCI_NR_POBS];
     struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
     uint32_t gasket_time;
@@ -310,7 +311,7 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *address_space_io = get_system_io();
 
-    h = FROM_SYSBUS(PCIHostState, sysbus_from_qdev(dev));
+    h = PCI_HOST(dev);
     s = DO_UPCAST(PPCE500PCIState, pci_state, h);
 
     for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
@@ -366,9 +367,9 @@ static void e500_pcihost_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_ppce500_pci;
 }
 
-static TypeInfo e500_pcihost_info = {
+static const TypeInfo e500_pcihost_info = {
     .name          = "e500-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(PPCE500PCIState),
     .class_init    = e500_pcihost_class_init,
 };
diff --git a/hw/prep_pci.c b/hw/prep_pci.c
index 3e9f6b8..73c9d04 100644
--- a/hw/prep_pci.c
+++ b/hw/prep_pci.c
@@ -30,6 +30,7 @@
 
 typedef struct PRePPCIState {
     PCIHostState host_state;
+
     MemoryRegion intack;
     qemu_irq irq[4];
 } PREPPCIState;
@@ -96,7 +97,7 @@ static void prep_set_irq(void *opaque, int irq_num, int level)
 
 static int raven_pcihost_init(SysBusDevice *dev)
 {
-    PCIHostState *h = FROM_SYSBUS(PCIHostState, dev);
+    PCIHostState *h = PCI_HOST(dev);
     PREPPCIState *s = DO_UPCAST(PREPPCIState, host_state, h);
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *address_space_io = get_system_io();
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index 25b400a..bb49a4b 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -267,7 +267,7 @@ static const MemoryRegionOps spapr_io_ops = {
  */
 static int spapr_phb_init(SysBusDevice *s)
 {
-    sPAPRPHBState *phb = FROM_SYSBUS(sPAPRPHBState, s);
+    sPAPRPHBState *phb = SPAPR_PCI_HOST_BRIDGE(s);
     char *namebuf;
     int i;
     PCIBus *bus;
@@ -304,7 +304,7 @@ static int spapr_phb_init(SysBusDevice *s)
     memory_region_add_subregion(get_system_memory(), phb->io_win_addr,
                                 &phb->iowindow);
 
-    bus = pci_register_bus(&phb->busdev.qdev,
+    bus = pci_register_bus(DEVICE(s),
                            phb->busname ? phb->busname : phb->dtbusname,
                            pci_spapr_set_irq, pci_spapr_map_irq, phb,
                            &phb->memspace, &phb->iospace,
@@ -354,9 +354,9 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
     spapr_rtas_register("ibm,write-pci-config", rtas_ibm_write_pci_config);
 }
 
-static TypeInfo spapr_phb_info = {
-    .name          = "spapr-pci-host-bridge",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+static const TypeInfo spapr_phb_info = {
+    .name          = TYPE_SPAPR_PCI_HOST_BRIDGE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(sPAPRPHBState),
     .class_init    = spapr_phb_class_init,
 };
@@ -368,7 +368,7 @@ void spapr_create_phb(sPAPREnvironment *spapr,
 {
     DeviceState *dev;
 
-    dev = qdev_create(NULL, spapr_phb_info.name);
+    dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
 
     if (busname) {
         qdev_prop_set_string(dev, "busname", g_strdup(busname));
@@ -474,8 +474,9 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb,
     return 0;
 }
 
-static void register_types(void)
+static void spapr_pci_register_types(void)
 {
     type_register_static(&spapr_phb_info);
 }
-type_init(register_types)
+
+type_init(spapr_pci_register_types)
diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
index f54c2e8..0244664 100644
--- a/hw/spapr_pci.h
+++ b/hw/spapr_pci.h
@@ -27,8 +27,12 @@
 #include "hw/pci_host.h"
 #include "hw/xics.h"
 
+#define TYPE_SPAPR_PCI_HOST_BRIDGE "spapr-pci-host-bridge"
+
+#define SPAPR_PCI_HOST_BRIDGE(obj) \
+    OBJECT_CHECK(sPAPRPHBState, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
+
 typedef struct sPAPRPHBState {
-    SysBusDevice busdev;
     PCIHostState host_state;
 
     uint64_t buid;
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index 409bcd4..351cdef 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -40,6 +40,7 @@ static const int unin_irq_line[] = { 0x1b, 0x1c, 0x1d, 0x1e };
 
 typedef struct UNINState {
     PCIHostState host_state;
+
     MemoryRegion pci_mmio;
     MemoryRegion pci_hole;
 } UNINState;
@@ -432,9 +433,9 @@ static void pci_u3_agp_class_init(ObjectClass *klass, void *data)
     sbc->init = pci_u3_agp_init_device;
 }
 
-static TypeInfo pci_u3_agp_info = {
+static const TypeInfo pci_u3_agp_info = {
     .name          = "u3-agp-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(UNINState),
     .class_init    = pci_u3_agp_class_init,
 };
@@ -446,9 +447,9 @@ static void pci_unin_agp_class_init(ObjectClass *klass, void *data)
     sbc->init = pci_unin_agp_init_device;
 }
 
-static TypeInfo pci_unin_agp_info = {
+static const TypeInfo pci_unin_agp_info = {
     .name          = "uni-north-agp-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(UNINState),
     .class_init    = pci_unin_agp_class_init,
 };
@@ -460,9 +461,9 @@ static void pci_unin_internal_class_init(ObjectClass *klass, void *data)
     sbc->init = pci_unin_internal_init_device;
 }
 
-static TypeInfo pci_unin_internal_info = {
+static const TypeInfo pci_unin_internal_info = {
     .name          = "uni-north-internal-pci-pcihost",
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_PCI_HOST,
     .instance_size = sizeof(UNINState),
     .class_init    = pci_unin_internal_class_init,
 };
-- 
1.7.7

Re: [Qemu-devel] [PATCH v2 1/2] pci_host: Turn into SysBus-derived QOM type

[Anthony Liguori]

On 06/10/2012 10:57 AM, Andreas Färber wrote:
> From: Andreas Färber<andreas.faerber@web.de>
>
> Allows us to access PCIHostState QOM-style with PCI_HOST() macro.
>
> Update PReP Raven PCI to derive from this type.
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li<liwp@linux.vnet.ibm.com>
> Signed-off-by: Andreas Färber<andreas.faerber@web.de>
> Reviewed-by: Anthony Liguori<aliguori@us.ibm.com>
> ---
>   hw/pci_host.c |   11 +++++++++++
>   hw/pci_host.h |    3 +++
>   hw/prep_pci.c |    4 ++--
>   3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 8041778..347bfa6 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = {
>       .endianness = DEVICE_BIG_ENDIAN,
>   };
>
> +static const TypeInfo pci_host_type_info = {
> +    .name = TYPE_PCI_HOST,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(PCIHostState),
> +};

Any reason why PCIHost can't have TYPE_DEVICE as the the parent?

Regards,

Anthony Liguori

> +
> +static void pci_host_register_types(void)
> +{
> +    type_register_static(&pci_host_type_info);
> +}
>
> +type_init(pci_host_register_types)
> diff --git a/hw/pci_host.h b/hw/pci_host.h
> index 359e38f..fba9fde 100644
> --- a/hw/pci_host.h
> +++ b/hw/pci_host.h
> @@ -30,6 +30,9 @@
>
>   #include "sysbus.h"
>
> +#define TYPE_PCI_HOST "pci-host"
> +#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
> +
>   struct PCIHostState {
>       SysBusDevice busdev;
>       MemoryRegion conf_mem;
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index 38dbff4..3e9f6b8 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -183,9 +183,9 @@ static void raven_pcihost_class_init(ObjectClass *klass, void *data)
>       dc->no_user = 1;
>   }
>
> -static TypeInfo raven_pcihost_info = {
> +static const TypeInfo raven_pcihost_info = {
>       .name = "raven-pcihost",
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +    .parent = TYPE_PCI_HOST,
>       .instance_size = sizeof(PREPPCIState),
>       .class_init = raven_pcihost_class_init,
>   };

Re: [Qemu-devel] [PATCH v2 1/2] pci_host: Turn into SysBus-derived QOM type

[Andreas Färber]

Am 10.06.2012 19:33, schrieb Anthony Liguori:
> On 06/10/2012 10:57 AM, Andreas Färber wrote:
>> From: Andreas Färber<andreas.faerber@web.de>
>>
>> Allows us to access PCIHostState QOM-style with PCI_HOST() macro.
>>
>> Update PReP Raven PCI to derive from this type.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> Signed-off-by: Wanpeng Li<liwp@linux.vnet.ibm.com>
>> Signed-off-by: Andreas Färber<andreas.faerber@web.de>
>> Reviewed-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>>   hw/pci_host.c |   11 +++++++++++
>>   hw/pci_host.h |    3 +++
>>   hw/prep_pci.c |    4 ++--
>>   3 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci_host.c b/hw/pci_host.c
>> index 8041778..347bfa6 100644
>> --- a/hw/pci_host.c
>> +++ b/hw/pci_host.c
>> @@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = {
>>       .endianness = DEVICE_BIG_ENDIAN,
>>   };
>>
>> +static const TypeInfo pci_host_type_info = {
>> +    .name = TYPE_PCI_HOST,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(PCIHostState),
>> +};
> 
> Any reason why PCIHost can't have TYPE_DEVICE as the the parent?

Many current users rely on SysBus functionality like MMIO and IRQs.

A conversion to TYPE_DEVICE can be done as part of your SysBus removal
series.

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH v2 0/2] pci_host: Convert to QOM

[Anthony Liguori]

On 06/10/2012 10:57 AM, Andreas Färber wrote:
> Hello Michael,
>
> This series, based on a recent PReP RFC series, in turn based on Anthony's and
> Wanpeng's i440fx refactoring, makes pci_host a proper QOM type and in this v2
> also updates all remaining host controllers beyond PReP.
>
> Only compile-tested were typhoon, bonito, ppc440 and ppce500.
> I've checked it also applies cleanly to the current pci branch.
>
> Please review and apply.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

>
> Intended merge plan is:
> Merge this preparatory series through pci branch.
> Follow up with move of struct PCIBus from pci_internals.h to pci.h (currently
> typedef in qemu-common.h) once qom-next QBus refactoring is merged (qom-next-2),
> merge through pci branch.
> Rebase Raven PCI in-place initialization on that, merge through prep branch.
>
> Regards,
> Andreas
>
> Cc: Michael S. Tsirkin<mst@redhat.com>
> Cc: Anthony Liguori<anthony@codemonkey.ws>
>
> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>  (still no MAINTAINERS entry!)
>
> Cc: Jan Kiszka<jan.kiszka@siemens.com>  (FYI)
>
> Andreas Färber (2):
>    pci_host: Turn into SysBus-derived QOM type
>    pci_host: Derive remaining PCI host controllers from TYPE_PCI_HOST
>
>   hw/alpha_typhoon.c |    4 +-
>   hw/bonito.c        |   73 ++++++++++++++++++++++++++++++++--------------------
>   hw/dec_pci.c       |   11 ++++---
>   hw/dec_pci.h       |    2 +
>   hw/grackle_pci.c   |   22 ++++++++-------
>   hw/gt64xxx.c       |   23 +++++++++-------
>   hw/pci_host.c      |   11 ++++++++
>   hw/pci_host.h      |    3 ++
>   hw/piix_pci.c      |   14 +++++----
>   hw/ppc4xx_pci.c    |    4 +-
>   hw/ppc_mac.h       |    1 +
>   hw/ppc_prep.c      |    4 +--
>   hw/ppce500_pci.c   |    7 +++--
>   hw/prep_pci.c      |    7 +++--
>   hw/spapr_pci.c     |   17 ++++++-----
>   hw/spapr_pci.h     |    6 +++-
>   hw/unin_pci.c      |   13 +++++----
>   17 files changed, 135 insertions(+), 87 deletions(-)
>

Re: [Qemu-devel] [PATCH v2 0/2] pci_host: Convert to QOM

[Michael S. Tsirkin]

On Sun, Jun 10, 2012 at 05:57:53PM +0200, Andreas Färber wrote:
> Hello Michael,
> 
> This series, based on a recent PReP RFC series, in turn based on Anthony's and
> Wanpeng's i440fx refactoring, makes pci_host a proper QOM type and in this v2
> also updates all remaining host controllers beyond PReP.
> 
> Only compile-tested were typhoon, bonito, ppc440 and ppce500.
> I've checked it also applies cleanly to the current pci branch.
> 
> Please review and apply.
> 
> Intended merge plan is:
> Merge this preparatory series through pci branch.
> Follow up with move of struct PCIBus from pci_internals.h to pci.h (currently
> typedef in qemu-common.h)

I admit pci_internals.h was a failure. We ended up using it
all externally as well. So let's merge it all back into pci.h
and be done with it.


> once qom-next QBus refactoring is merged (qom-next-2),
> merge through pci branch.
> Rebase Raven PCI in-place initialization on that, merge through prep branch.
> 
> Regards,
> Andreas
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Anthony Liguori <anthony@codemonkey.ws>
> 
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> (still no MAINTAINERS entry!)
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com> (FYI)
> 
> Andreas Färber (2):
>   pci_host: Turn into SysBus-derived QOM type
>   pci_host: Derive remaining PCI host controllers from TYPE_PCI_HOST
> 
>  hw/alpha_typhoon.c |    4 +-
>  hw/bonito.c        |   73 ++++++++++++++++++++++++++++++++--------------------
>  hw/dec_pci.c       |   11 ++++---
>  hw/dec_pci.h       |    2 +
>  hw/grackle_pci.c   |   22 ++++++++-------
>  hw/gt64xxx.c       |   23 +++++++++-------
>  hw/pci_host.c      |   11 ++++++++
>  hw/pci_host.h      |    3 ++
>  hw/piix_pci.c      |   14 +++++----
>  hw/ppc4xx_pci.c    |    4 +-
>  hw/ppc_mac.h       |    1 +
>  hw/ppc_prep.c      |    4 +--
>  hw/ppce500_pci.c   |    7 +++--
>  hw/prep_pci.c      |    7 +++--
>  hw/spapr_pci.c     |   17 ++++++-----
>  hw/spapr_pci.h     |    6 +++-
>  hw/unin_pci.c      |   13 +++++----
>  17 files changed, 135 insertions(+), 87 deletions(-)
> 
> -- 
> 1.7.7

Re: [Qemu-devel] [PATCH v2 1/2] pci_host: Turn into SysBus-derived QOM type

[Michael S. Tsirkin]

On Sun, Jun 10, 2012 at 05:57:54PM +0200, Andreas Färber wrote:
> From: Andreas Färber <andreas.faerber@web.de>
> 
> Allows us to access PCIHostState QOM-style with PCI_HOST() macro.
> 
> Update PReP Raven PCI to derive from this type.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Question: this is really a pci host *bridge*.
We are calling this PCIHost internally for brevity
but QOM hierarchy is user-visible, right?

> ---
>  hw/pci_host.c |   11 +++++++++++
>  hw/pci_host.h |    3 +++
>  hw/prep_pci.c |    4 ++--
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 8041778..347bfa6 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +static const TypeInfo pci_host_type_info = {
> +    .name = TYPE_PCI_HOST,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(PCIHostState),
> +};
> +
> +static void pci_host_register_types(void)
> +{
> +    type_register_static(&pci_host_type_info);
> +}
>  
> +type_init(pci_host_register_types)
> diff --git a/hw/pci_host.h b/hw/pci_host.h
> index 359e38f..fba9fde 100644
> --- a/hw/pci_host.h
> +++ b/hw/pci_host.h
> @@ -30,6 +30,9 @@
>  
>  #include "sysbus.h"
>  
> +#define TYPE_PCI_HOST "pci-host"
> +#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
> +
>  struct PCIHostState {
>      SysBusDevice busdev;
>      MemoryRegion conf_mem;
> diff --git a/hw/prep_pci.c b/hw/prep_pci.c
> index 38dbff4..3e9f6b8 100644
> --- a/hw/prep_pci.c
> +++ b/hw/prep_pci.c
> @@ -183,9 +183,9 @@ static void raven_pcihost_class_init(ObjectClass *klass, void *data)
>      dc->no_user = 1;
>  }
>  
> -static TypeInfo raven_pcihost_info = {
> +static const TypeInfo raven_pcihost_info = {
>      .name = "raven-pcihost",
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +    .parent = TYPE_PCI_HOST,
>      .instance_size = sizeof(PREPPCIState),
>      .class_init = raven_pcihost_class_init,
>  };
> -- 
> 1.7.7

Re: [Qemu-devel] [PATCH v2 1/2] pci_host: Turn into SysBus-derived QOM type

[Andreas Färber]

Am 18.06.2012 20:28, schrieb Michael S. Tsirkin:
> On Sun, Jun 10, 2012 at 05:57:54PM +0200, Andreas Färber wrote:
>> From: Andreas Färber <andreas.faerber@web.de>
>>
>> Allows us to access PCIHostState QOM-style with PCI_HOST() macro.
>>
>> Update PReP Raven PCI to derive from this type.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> Question: this is really a pci host *bridge*.
> We are calling this PCIHost internally for brevity
> but QOM hierarchy is user-visible, right?

That's a good question... I would say it's not user-visible today unless
we instantiate TYPE_PCI_HOST directly, in which case its value
"pci-host" would be visible through the "type" property that got
introduced on qom-next. My CPU modeling for instance is based on the
assumption that we can introduce intermediate types later as a
user-invisible implementation detail.

>> ---
>>  hw/pci_host.c |   11 +++++++++++
>>  hw/pci_host.h |    3 +++
>>  hw/prep_pci.c |    4 ++--
>>  3 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci_host.c b/hw/pci_host.c
>> index 8041778..347bfa6 100644
>> --- a/hw/pci_host.c
>> +++ b/hw/pci_host.c
>> @@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = {
>>      .endianness = DEVICE_BIG_ENDIAN,
>>  };
>>  
>> +static const TypeInfo pci_host_type_info = {
>> +    .name = TYPE_PCI_HOST,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(PCIHostState),
>> +};

It would in fact be better to set .abstract = true, I guess.

Anyway, I think for sPAPR I translated PHB to PCI_HOST_BRIDGE already,
so TYPE_PCI_HOST_BRIDGE "pci-host-bridge" and PCI_HOST_BRIDGE() would
fit in nicely with the otherwise clear and verbose QOM naming. But I'd
rather not rename PCIHostState everywhere... do you agree? Or would you
want to have it as PCIHostBridge or PCIHostBridgeState for consistency?

If we use TYPE_PCI_HOST_BRIDGE I should also add _BRIDGE for some of the
derived types, but we can't change the user-visible "-pcihost" type name
there for backwards compatibility.

Any further wishes? Should the second patch be split up in some way?

Andreas

>> +
>> +static void pci_host_register_types(void)
>> +{
>> +    type_register_static(&pci_host_type_info);
>> +}
>>  
>> +type_init(pci_host_register_types)
[snip]

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

Re: [Qemu-devel] [PATCH v2 1/2] pci_host: Turn into SysBus-derived QOM type

[Anthony Liguori]

On 06/18/2012 04:44 PM, Andreas Färber wrote:
> Am 18.06.2012 20:28, schrieb Michael S. Tsirkin:
>> On Sun, Jun 10, 2012 at 05:57:54PM +0200, Andreas Färber wrote:
>>> From: Andreas Färber<andreas.faerber@web.de>
>>>
>>> Allows us to access PCIHostState QOM-style with PCI_HOST() macro.
>>>
>>> Update PReP Raven PCI to derive from this type.
>>>
>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>> Signed-off-by: Wanpeng Li<liwp@linux.vnet.ibm.com>
>>> Signed-off-by: Andreas Färber<andreas.faerber@web.de>
>>> Reviewed-by: Anthony Liguori<aliguori@us.ibm.com>
>>
>> Question: this is really a pci host *bridge*.
>> We are calling this PCIHost internally for brevity
>> but QOM hierarchy is user-visible, right?
>
> That's a good question... I would say it's not user-visible today unless
> we instantiate TYPE_PCI_HOST directly, in which case its value
> "pci-host" would be visible through the "type" property that got
> introduced on qom-next. My CPU modeling for instance is based on the
> assumption that we can introduce intermediate types later as a
> user-invisible implementation detail.

Yes, we need to formulate a support statement for the 1.2 release.

My general thinking is:

1) Properties will remain ABI compatible.  A property will not change it's type 
or semantics over time.  An example is link/child properties.  A link will 
always remain a link but the link subtype made be made more specific over time. 
  Likewise with child properties.

2) A property may be removed and new properties may be added.  Applications 
should always gracefully handle the non-existence of a property.

3) Since paths are composed of properties, they are subject to the same rules. 
That is, an absolute path will always have the same semantics as long as that 
path is still valid.

4) No guarantee is made about the stability of relative paths.

Types are really just another form of properties here so changing the type of an 
object provided that we don't violate ABI rules is okay.

I actually think it's okay for a typename to change even if it's exposed by 
-device.  If something is using -device, it ought to probe for the existence of 
a type before using -device.

Regards,

Anthony Liguori

>
>>> ---
>>>   hw/pci_host.c |   11 +++++++++++
>>>   hw/pci_host.h |    3 +++
>>>   hw/prep_pci.c |    4 ++--
>>>   3 files changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/pci_host.c b/hw/pci_host.c
>>> index 8041778..347bfa6 100644
>>> --- a/hw/pci_host.c
>>> +++ b/hw/pci_host.c
>>> @@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = {
>>>       .endianness = DEVICE_BIG_ENDIAN,
>>>   };
>>>
>>> +static const TypeInfo pci_host_type_info = {
>>> +    .name = TYPE_PCI_HOST,
>>> +    .parent = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(PCIHostState),
>>> +};
>
> It would in fact be better to set .abstract = true, I guess.
>
> Anyway, I think for sPAPR I translated PHB to PCI_HOST_BRIDGE already,
> so TYPE_PCI_HOST_BRIDGE "pci-host-bridge" and PCI_HOST_BRIDGE() would
> fit in nicely with the otherwise clear and verbose QOM naming. But I'd
> rather not rename PCIHostState everywhere... do you agree? Or would you
> want to have it as PCIHostBridge or PCIHostBridgeState for consistency?
>
> If we use TYPE_PCI_HOST_BRIDGE I should also add _BRIDGE for some of the
> derived types, but we can't change the user-visible "-pcihost" type name
> there for backwards compatibility.
>
> Any further wishes? Should the second patch be split up in some way?
>
> Andreas
>
>>> +
>>> +static void pci_host_register_types(void)
>>> +{
>>> +    type_register_static(&pci_host_type_info);
>>> +}
>>>
>>> +type_init(pci_host_register_types)
> [snip]
>

Re: [Qemu-devel] [PATCH v2 1/2] pci_host: Turn into SysBus-derived QOM type

[Michael S. Tsirkin]

On Mon, Jun 18, 2012 at 05:23:32PM -0500, Anthony Liguori wrote:
> On 06/18/2012 04:44 PM, Andreas Färber wrote:
> >Am 18.06.2012 20:28, schrieb Michael S. Tsirkin:
> >>On Sun, Jun 10, 2012 at 05:57:54PM +0200, Andreas Färber wrote:
> >>>From: Andreas Färber<andreas.faerber@web.de>
> >>>
> >>>Allows us to access PCIHostState QOM-style with PCI_HOST() macro.
> >>>
> >>>Update PReP Raven PCI to derive from this type.
> >>>
> >>>Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> >>>Signed-off-by: Wanpeng Li<liwp@linux.vnet.ibm.com>
> >>>Signed-off-by: Andreas Färber<andreas.faerber@web.de>
> >>>Reviewed-by: Anthony Liguori<aliguori@us.ibm.com>
> >>
> >>Question: this is really a pci host *bridge*.
> >>We are calling this PCIHost internally for brevity
> >>but QOM hierarchy is user-visible, right?
> >
> >That's a good question... I would say it's not user-visible today unless
> >we instantiate TYPE_PCI_HOST directly, in which case its value
> >"pci-host" would be visible through the "type" property that got
> >introduced on qom-next. My CPU modeling for instance is based on the
> >assumption that we can introduce intermediate types later as a
> >user-invisible implementation detail.
> 
> Yes, we need to formulate a support statement for the 1.2 release.
> 
> My general thinking is:
> 
> 1) Properties will remain ABI compatible.  A property will not
> change it's type or semantics over time.  An example is link/child
> properties.  A link will always remain a link but the link subtype
> made be made more specific over time.  Likewise with child
> properties.
> 
> 2) A property may be removed and new properties may be added.

At will? That's a very weak statement.

> Applications should always gracefully handle the non-existence of a
> property.

They can fail gracefully but what else can they do?

> 3) Since paths are composed of properties, they are subject to the
> same rules. That is, an absolute path will always have the same
> semantics as long as that path is still valid.
> 
> 4) No guarantee is made about the stability of relative paths.
> 
> Types are really just another form of properties here so changing
> the type of an object provided that we don't violate ABI rules is
> okay.

Let's invent internal types that we don't expect user to
know about. We have x- for properties, let's do the same here.

> I actually think it's okay for a typename to change even if it's
> exposed by -device.  If something is using -device, it ought to
> probe for the existence of a type before using -device.
> 
> Regards,
> 
> Anthony Liguori

Then it'll fail gracefully but it can't work.

> >
> >>>---
> >>>  hw/pci_host.c |   11 +++++++++++
> >>>  hw/pci_host.h |    3 +++
> >>>  hw/prep_pci.c |    4 ++--
> >>>  3 files changed, 16 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/hw/pci_host.c b/hw/pci_host.c
> >>>index 8041778..347bfa6 100644
> >>>--- a/hw/pci_host.c
> >>>+++ b/hw/pci_host.c
> >>>@@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = {
> >>>      .endianness = DEVICE_BIG_ENDIAN,
> >>>  };
> >>>
> >>>+static const TypeInfo pci_host_type_info = {
> >>>+    .name = TYPE_PCI_HOST,
> >>>+    .parent = TYPE_SYS_BUS_DEVICE,
> >>>+    .instance_size = sizeof(PCIHostState),
> >>>+};
> >
> >It would in fact be better to set .abstract = true, I guess.
> >
> >Anyway, I think for sPAPR I translated PHB to PCI_HOST_BRIDGE already,
> >so TYPE_PCI_HOST_BRIDGE "pci-host-bridge" and PCI_HOST_BRIDGE() would
> >fit in nicely with the otherwise clear and verbose QOM naming. But I'd
> >rather not rename PCIHostState everywhere... do you agree? Or would you
> >want to have it as PCIHostBridge or PCIHostBridgeState for consistency?
> >
> >If we use TYPE_PCI_HOST_BRIDGE I should also add _BRIDGE for some of the
> >derived types, but we can't change the user-visible "-pcihost" type name
> >there for backwards compatibility.
> >
> >Any further wishes? Should the second patch be split up in some way?
> >
> >Andreas
> >
> >>>+
> >>>+static void pci_host_register_types(void)
> >>>+{
> >>>+    type_register_static(&pci_host_type_info);
> >>>+}
> >>>
> >>>+type_init(pci_host_register_types)
> >[snip]
> >