Patchwork Re: [PATCH 1/2] pci/bridge: allocate PCIBus dynamically for PCIBridge.

login
register
mail settings
Submitter Isaku Yamahata
Date July 16, 2010, 1:46 a.m.
Message ID <20100716014651.GA32307@valinux.co.jp>
Download mbox | patch
Permalink /patch/59060/
State New
Headers show

Comments

Isaku Yamahata - July 16, 2010, 1:46 a.m.
On Thu, Jul 08, 2010 at 07:49:35PM +0300, Michael S. Tsirkin wrote:

> > For root bus:
> > - pci_host_bus_new()/pci_host_bus_new_inplace()
> >   qbus style api. pci segment must be specified.
> >   New code should use this.
> 
> I'd prefer a simple _init which works like _inplace.
> Allocating memory is simple enough for users.

How about the following patch?
  - replaced pci_bus_new(), pci_bus_new_inplace() with
    pci_host_bus_init()/pci_host_bus_cleanup()
  - eliminated memory allocation by embedding PCIBus into PCIHostState.

I've tested it with only piix4 yet.
If this direction is okay, I'll give it more tests and post it.


From d54dffc884c93bc4f71c506a933891d404e5bf81 Mon Sep 17 00:00:00 2001
Message-Id: <d54dffc884c93bc4f71c506a933891d404e5bf81.1279244480.git.yamahata@valinux.co.jp>
In-Reply-To: <cover.1279244480.git.yamahata@valinux.co.jp>
References: <cover.1279244480.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Tue, 13 Jul 2010 17:35:00 +0900
Subject: [PATCH] pci/pci_host: pci host bus initialization clean up.

Embed PCIBus into PCIHostState and clean up of pci host bus initialization.
And Embed PCIHostState into each devices.
Especially pci host bus creation must be aware of pci segment, usually 0.
Although some boards doesn't use PCIHostState at the moment,
in long term enhance PCIHostState and convert them.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/apb_pci.c       |   21 ++++++++---------
 hw/bonito.c        |   25 +++++++++++----------
 hw/grackle_pci.c   |   11 ++++-----
 hw/gt64xxx.c       |   14 ++++++------
 hw/pci.c           |   61 ++++++++++++++++++++++++++++++++++------------------
 hw/pci.h           |   18 ++++++++++-----
 hw/pci_host.c      |   14 ++++-------
 hw/pci_host.h      |   13 ++++++++++-
 hw/pcie_host.c     |   12 +++++-----
 hw/piix_pci.c      |    5 ++-
 hw/ppc4xx_pci.c    |   11 ++++-----
 hw/ppce500_pci.c   |   12 ++++------
 hw/prep_pci.c      |   20 ++++++++--------
 hw/sh_pci.c        |   14 ++++++------
 hw/unin_pci.c      |   30 ++++++++++++------------
 hw/versatile_pci.c |    8 ++++--
 16 files changed, 160 insertions(+), 129 deletions(-)
Michael S. Tsirkin - July 16, 2010, 7:35 a.m.
On Fri, Jul 16, 2010 at 10:46:52AM +0900, Isaku Yamahata wrote:
> On Thu, Jul 08, 2010 at 07:49:35PM +0300, Michael S. Tsirkin wrote:
> 
> > > For root bus:
> > > - pci_host_bus_new()/pci_host_bus_new_inplace()
> > >   qbus style api. pci segment must be specified.
> > >   New code should use this.
> > 
> > I'd prefer a simple _init which works like _inplace.
> > Allocating memory is simple enough for users.
> 
> How about the following patch?
>   - replaced pci_bus_new(), pci_bus_new_inplace() with
>     pci_host_bus_init()/pci_host_bus_cleanup()
>   - eliminated memory allocation by embedding PCIBus into PCIHostState.
> I've tested it with only piix4 yet.
> If this direction is okay, I'll give it more tests and post it.
> 

Looks good to me. What do others think?  Let's also make
pci_host_bus_init_simple inline.

Patch

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index d446550..027160a 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -68,7 +68,7 @@  do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
 
 typedef struct APBState {
     SysBusDevice busdev;
-    PCIBus      *bus;
+    PCIHostState pci;
     ReadWriteHandler pci_config_handler;
     uint32_t iommu[4];
     uint32_t pci_control[16];
@@ -194,7 +194,7 @@  static void apb_pci_config_write(ReadWriteHandler *h, pcibus_t addr,
 
     val = qemu_bswap_len(val, size);
     APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %x\n", __func__, addr, val);
-    pci_data_write(s->bus, addr, val, size);
+    pci_data_write(&s->pci.bus, addr, val, size);
 }
 
 static uint32_t apb_pci_config_read(ReadWriteHandler *h, pcibus_t addr,
@@ -203,7 +203,7 @@  static uint32_t apb_pci_config_read(ReadWriteHandler *h, pcibus_t addr,
     uint32_t ret;
     APBState *s = container_of(h, APBState, pci_config_handler);
 
-    ret = pci_data_read(s->bus, addr, size);
+    ret = pci_data_read(&s->pci.bus, addr, size);
     ret = qemu_bswap_len(ret, size);
     APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, ret);
     return ret;
@@ -347,29 +347,28 @@  PCIBus *pci_apb_init(target_phys_addr_t special_base,
     sysbus_mmio_map(s, 2, special_base + 0x2000000ULL);
     d = FROM_SYSBUS(APBState, s);
 
-    d->bus = pci_register_bus(&d->busdev.qdev, "pci",
-                                         pci_apb_set_irq, pci_pbm_map_irq, d,
-                                         0, 32);
-    pci_bus_set_mem_base(d->bus, mem_base);
+    pci_host_bus_init_simple(&d->pci, &d->busdev.qdev, 0, "pci",
+                             pci_apb_set_irq, pci_pbm_map_irq, d, 0, 32);
+    pci_bus_set_mem_base(&d->pci.bus, mem_base);
 
     for (i = 0; i < 32; i++) {
         sysbus_connect_irq(s, i, pic[i]);
     }
 
-    pci_create_simple(d->bus, 0, "pbm");
+    pci_create_simple(&d->pci.bus, 0, "pbm");
 
     /* APB secondary busses */
-    br = pci_bridge_create_simple(d->bus, PCI_DEVFN(1, 0), true,
+    br = pci_bridge_create_simple(&d->pci.bus, PCI_DEVFN(1, 0), true,
                                   pci_apb_map_irq, "pbm-bridge",
                                   "Advanced PCI Bus secondary bridge 1");
     *bus2 = pci_bridge_get_sec_bus(br);
 
-    br = pci_bridge_create_simple(d->bus, PCI_DEVFN(1, 1), true,
+    br = pci_bridge_create_simple(&d->pci.bus, PCI_DEVFN(1, 1), true,
                                   pci_apb_map_irq, "pbm-bridge",
                                   "Advanced PCI Bus secondary bridge 2");
     *bus3 = pci_bridge_get_sec_bus(br);
 
-    return d->bus;
+    return &d->pci.bus;
 }
 
 static void pci_pbm_reset(DeviceState *d)
diff --git a/hw/bonito.c b/hw/bonito.c
index 414e0aa..a328f3a 100644
--- a/hw/bonito.c
+++ b/hw/bonito.c
@@ -444,9 +444,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(&s->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(&s->pcihost->bus), devno, funno, regno);
 
     return pciaddr;
 }
@@ -467,7 +467,7 @@  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);
+    pci_data_write(&s->pcihost->bus, s->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);
@@ -493,7 +493,7 @@  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);
+    pci_data_write(&s->pcihost->bus, s->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);
@@ -519,7 +519,7 @@  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);
+    pci_data_write(&s->pcihost->bus, s->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);
@@ -548,7 +548,7 @@  static uint32_t bonito_spciconf_readb(void *opaque, target_phys_addr_t addr)
     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(&s->pcihost->bus, s->pcihost->config_reg, 1);
 }
 
 static uint32_t bonito_spciconf_readw(void *opaque, target_phys_addr_t addr)
@@ -574,7 +574,7 @@  static uint32_t bonito_spciconf_readw(void *opaque, target_phys_addr_t addr)
     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(&s->pcihost->bus, s->pcihost->config_reg, 2);
 }
 
 static uint32_t bonito_spciconf_readl(void *opaque, target_phys_addr_t addr)
@@ -600,7 +600,7 @@  static uint32_t bonito_spciconf_readl(void *opaque, target_phys_addr_t addr)
     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(&s->pcihost->bus, s->pcihost->config_reg, 4);
 }
 
 /* south bridge PCI configure space. 0x1fe8 0000 - 0x1fef ffff */
@@ -774,11 +774,12 @@  PCIBus *bonito_init(qemu_irq *pic)
 
     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,
-                         pci_bonito_map_irq, pic, 0x28, 32);
-    pcihost->pci.bus = b;
+    pci_host_bus_init_simple(&pcihost->pci, &pcihost->busdev.qdev, 0, "pci",
+                             pci_bonito_set_irq, pci_bonito_map_irq,
+                             pic, 0x28, 32);
     qdev_init_nofail(dev);
-    pci_bus_set_mem_base(pcihost->pci.bus, 0x10000000);
+    b = &pcihost->pci.bus;
+    pci_bus_set_mem_base(b, 0x10000000);
 
     d = pci_create_simple(b, PCI_DEVFN(0, 0), "Bonito");
     s = DO_UPCAST(PCIBonitoState, dev, d);
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index 91c755f..1056ed3 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -88,17 +88,16 @@  PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(GrackleState, s);
-    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
-                                         pci_grackle_set_irq,
-                                         pci_grackle_map_irq,
-                                         pic, 0, 4);
+    pci_host_bus_init_simple(&d->host_state, &d->busdev.qdev, 0, "pci",
+                             pci_grackle_set_irq, pci_grackle_map_irq,
+                             pic, 0, 4);
 
-    pci_create_simple(d->host_state.bus, 0, "grackle");
+    pci_create_simple(&d->host_state.bus, 0, "grackle");
 
     sysbus_mmio_map(s, 0, base);
     sysbus_mmio_map(s, 1, base + 0x00200000);
 
-    return d->host_state.bus;
+    return &d->host_state.bus;
 }
 
 static int pci_grackle_init_device(SysBusDevice *dev)
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index cabf7ea..4e227ba 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -535,7 +535,7 @@  static void gt64120_writel (void *opaque, target_phys_addr_t addr,
         if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800))
             val = bswap32(val);
         if (s->pci->config_reg & (1u << 31))
-            pci_data_write(s->pci->bus, s->pci->config_reg, val, 4);
+            pci_data_write(&s->pci->bus, s->pci->config_reg, val, 4);
         break;
 
     /* Interrupts */
@@ -775,7 +775,7 @@  static uint32_t gt64120_readl (void *opaque,
         if (!(s->pci->config_reg & (1 << 31)))
             val = 0xffffffff;
         else
-            val = pci_data_read(s->pci->bus, s->pci->config_reg, 4);
+            val = pci_data_read(&s->pci->bus, s->pci->config_reg, 4);
         if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800))
             val = bswap32(val);
         break;
@@ -1113,11 +1113,11 @@  PCIBus *pci_gt64120_init(qemu_irq *pic)
     s = qemu_mallocz(sizeof(GT64120State));
     s->pci = qemu_mallocz(sizeof(GT64120PCIState));
 
-    s->pci->bus = pci_register_bus(NULL, "pci",
-                                   pci_gt64120_set_irq, pci_gt64120_map_irq,
-                                   pic, PCI_DEVFN(18, 0), 4);
+    pci_host_bus_init_simple(s->pci, NULL, 0, "pci",
+                             pci_gt64120_set_irq, pci_gt64120_map_irq,
+                             pic, PCI_DEVFN(18, 0), 4);
     s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
-    d = pci_register_device(s->pci->bus, "GT64120 PCI Bus", sizeof(PCIDevice),
+    d = pci_register_device(&s->pci->bus, "GT64120 PCI Bus", sizeof(PCIDevice),
                             0, NULL, NULL);
 
     /* FIXME: Malta specific hw assumptions ahead */
@@ -1149,5 +1149,5 @@  PCIBus *pci_gt64120_init(qemu_irq *pic)
     register_savevm(&d->qdev, "GT64120 PCI Bus", 0, 1,
                     gt64120_save, gt64120_load, d);
 
-    return s->pci->bus;
+    return &s->pci->bus;
 }
diff --git a/hw/pci.c b/hw/pci.c
index 737fbd2..71430ad 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -25,6 +25,7 @@ 
 #include "pci.h"
 #include "pci_bridge.h"
 #include "pci_internals.h"
+#include "pci_host.h"
 #include "monitor.h"
 #include "net.h"
 #include "sysemu.h"
@@ -173,6 +174,19 @@  static void pci_host_bus_register(int domain, PCIBus *bus)
     QLIST_INSERT_HEAD(&host_buses, host, next);
 }
 
+static void pci_host_bus_unregister(PCIBus *bus)
+{
+    struct PCIHostBus *host;
+    QLIST_FOREACH(host, &host_buses, next) {
+        if (host->bus == bus) {
+            QLIST_REMOVE(host, next);
+            qemu_free(host);
+            return;
+        }
+    }
+    abort();/* should not be reached */
+}
+
 PCIBus *pci_find_root_bus(int domain)
 {
     struct PCIHostBus *host;
@@ -206,29 +220,45 @@  int pci_find_domain(const PCIBus *bus)
     return -1;
 }
 
-void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
-                         const char *name, int devfn_min)
+/* initialize pci host bus. To create secondary bus use pci_bridge.c */
+void pci_host_bus_init(PCIHostState *pci_host, DeviceState *parent,
+                       uint16_t segment, const char *name, int devfn_min)
 {
-    qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
+    PCIBus *bus = &pci_host->bus;
     assert(PCI_FUNC(devfn_min) == 0);
+
+    pci_host_init(pci_host);
+    qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
     bus->devfn_min = devfn_min;
 
     /* host bridge */
     QLIST_INIT(&bus->child);
-    pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
+    pci_host_bus_register(segment, bus);
 
     vmstate_register(NULL, -1, &vmstate_pcibus, bus);
     qemu_register_reset(pci_bus_reset, bus);
 }
 
-PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min)
+void pci_host_bus_cleanup(PCIHostState *pci_host)
 {
-    PCIBus *bus;
+    PCIBus *bus = &pci_host->bus;
+    assert(bus->parent_dev == NULL);    /* host bus */
+    assert(QLIST_EMPTY(&bus->child));   /* no child bus */
 
-    bus = qemu_mallocz(sizeof(*bus));
-    bus->qbus.qdev_allocated = 1;
-    pci_bus_new_inplace(bus, parent, name, devfn_min);
-    return bus;
+    qemu_unregister_reset(pci_bus_reset, bus);
+    vmstate_unregister(NULL, &vmstate_pcibus, bus);
+    pci_host_bus_unregister(bus);
+    /* pci_host_cleanup(pci_host) */
+}
+
+/* convenience function to ease conversion from pci_register_bus(). */
+void pci_host_bus_init_simple(PCIHostState *pci_host, DeviceState *parent,
+                              uint16_t segment, const char *name,
+                              pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+                              void *irq_opaque, int devfn_min, int nirq)
+{
+    pci_host_bus_init(pci_host, parent, segment, name, devfn_min);
+    pci_bus_irqs(&pci_host->bus, set_irq, map_irq, irq_opaque, nirq);
 }
 
 void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
@@ -253,17 +283,6 @@  void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base)
     bus->mem_base = base;
 }
 
-PCIBus *pci_register_bus(DeviceState *parent, const char *name,
-                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-                         void *irq_opaque, int devfn_min, int nirq)
-{
-    PCIBus *bus;
-
-    bus = pci_bus_new(parent, name, devfn_min);
-    pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
-    return bus;
-}
-
 int pci_bus_num(PCIBus *s)
 {
     if (!s->parent_dev)
diff --git a/hw/pci.h b/hw/pci.h
index c551f96..a268f3c 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -204,15 +204,21 @@  int pci_device_load(PCIDevice *s, QEMUFile *f);
 typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
 typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev, int state);
-void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
-                         const char *name, int devfn_min);
-PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min);
+
+void pci_host_bus_init(PCIHostState *pci_host, DeviceState *parent,
+                       uint16_t segment, const char *name, int devfn_min);
+void pci_host_bus_cleanup(PCIHostState *pci_host);
+
+/* convenience function to ease conversion from pci_register_bus().
+   = pci_host_bus_init() + pci_bus_irqs() */
+void pci_host_bus_init_simple(PCIHostState *pci_host, DeviceState *parent,
+                              uint16_t segment, const char *name,
+                              pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+                              void *irq_opaque, int devfn_min, int nirq);
+
 void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                   void *irq_opaque, int nirq);
 void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
-PCIBus *pci_register_bus(DeviceState *parent, const char *name,
-                         pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-                         void *irq_opaque, int devfn_min, int nirq);
 
 void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base);
 
diff --git a/hw/pci_host.c b/hw/pci_host.c
index bc5b771..811454f 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -131,7 +131,7 @@  static void pci_host_data_write_swap(ReadWriteHandler *handler,
     PCI_DPRINTF("write addr %" FMT_PCIBUS " len %d val %x\n",
                 addr, len, val);
     if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg | (addr & 3), val, len);
+        pci_data_write(&s->bus, s->config_reg | (addr & 3), val, len);
 }
 
 static uint32_t pci_host_data_read_swap(ReadWriteHandler *handler,
@@ -141,7 +141,7 @@  static uint32_t pci_host_data_read_swap(ReadWriteHandler *handler,
     uint32_t val;
     if (!(s->config_reg & (1 << 31)))
         return 0xffffffff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
+    val = pci_data_read(&s->bus, s->config_reg | (addr & 3), len);
     PCI_DPRINTF("read addr %" FMT_PCIBUS " len %d val %x\n",
                 addr, len, val);
     val = qemu_bswap_len(val, len);
@@ -155,7 +155,7 @@  static void pci_host_data_write_noswap(ReadWriteHandler *handler,
     PCI_DPRINTF("write addr %" FMT_PCIBUS " len %d val %x\n",
                 addr, len, val);
     if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg | (addr & 3), val, len);
+        pci_data_write(&s->bus, s->config_reg | (addr & 3), val, len);
 }
 
 static uint32_t pci_host_data_read_noswap(ReadWriteHandler *handler,
@@ -165,13 +165,13 @@  static uint32_t pci_host_data_read_noswap(ReadWriteHandler *handler,
     uint32_t val;
     if (!(s->config_reg & (1 << 31)))
         return 0xffffffff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
+    val = pci_data_read(&s->bus, s->config_reg | (addr & 3), len);
     PCI_DPRINTF("read addr %" FMT_PCIBUS " len %d val %x\n",
                 addr, len, val);
     return val;
 }
 
-static void pci_host_init(PCIHostState *s)
+void pci_host_init(PCIHostState *s)
 {
     s->conf_handler.write = pci_host_config_write_swap;
     s->conf_handler.read = pci_host_config_read_swap;
@@ -185,7 +185,6 @@  static void pci_host_init(PCIHostState *s)
 
 int pci_host_conf_register_mmio(PCIHostState *s, int swap)
 {
-    pci_host_init(s);
     if (swap) {
         return cpu_register_io_memory_simple(&s->conf_handler);
     } else {
@@ -195,13 +194,11 @@  int pci_host_conf_register_mmio(PCIHostState *s, int swap)
 
 void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
 {
-    pci_host_init(s);
     register_ioport_simple(&s->conf_noswap_handler, ioport, 4, 4);
 }
 
 int pci_host_data_register_mmio(PCIHostState *s, int swap)
 {
-    pci_host_init(s);
     if (swap) {
         return cpu_register_io_memory_simple(&s->data_handler);
     } else {
@@ -211,7 +208,6 @@  int pci_host_data_register_mmio(PCIHostState *s, int swap)
 
 void pci_host_data_register_ioport(pio_addr_t ioport, PCIHostState *s)
 {
-    pci_host_init(s);
     register_ioport_simple(&s->data_noswap_handler, ioport, 4, 1);
     register_ioport_simple(&s->data_noswap_handler, ioport, 4, 2);
     register_ioport_simple(&s->data_noswap_handler, ioport, 4, 4);
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 46d1379..4d9c10e 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -25,11 +25,20 @@ 
 /* Worker routines for a PCI host controller that uses an {address,data}
    register pair to access PCI configuration space.  */
 
+/*
+ * TODO: there remains some boards which doesn't use PCIHostState.
+ *       Enhance PCIHostState API and convert remaining boards.
+ *       - allow custom address decoder which doesn't match with
+ *         24:16 bus, 15:8 defn, 7:0 offset.
+ *       - allow MMIO mapped direct access to configuration space
+ */
+
 #ifndef PCI_HOST_H
 #define PCI_HOST_H
 
 #include "sysbus.h"
 #include "rwhandler.h"
+#include "pci_internals.h"
 
 struct PCIHostState {
     ReadWriteHandler conf_noswap_handler;
@@ -37,9 +46,11 @@  struct PCIHostState {
     ReadWriteHandler data_noswap_handler;
     ReadWriteHandler data_handler;
     uint32_t config_reg;
-    PCIBus *bus;
+    PCIBus bus;
 };
 
+void pci_host_init(PCIHostState *s);
+
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
 
diff --git a/hw/pcie_host.c b/hw/pcie_host.c
index c4feeca..0de90cd 100644
--- a/hw/pcie_host.c
+++ b/hw/pcie_host.c
@@ -80,39 +80,39 @@  static void pcie_mmcfg_data_writeb(void *opaque,
                                    target_phys_addr_t addr, uint32_t value)
 {
     PCIExpressHost *e = opaque;
-    pcie_mmcfg_data_write(e->pci.bus, addr - e->base_addr, value, 1);
+    pcie_mmcfg_data_write(&e->pci.bus, addr - e->base_addr, value, 1);
 }
 
 static void pcie_mmcfg_data_writew(void *opaque,
                                    target_phys_addr_t addr, uint32_t value)
 {
     PCIExpressHost *e = opaque;
-    pcie_mmcfg_data_write(e->pci.bus, addr - e->base_addr, value, 2);
+    pcie_mmcfg_data_write(&e->pci.bus, addr - e->base_addr, value, 2);
 }
 
 static void pcie_mmcfg_data_writel(void *opaque,
                                    target_phys_addr_t addr, uint32_t value)
 {
     PCIExpressHost *e = opaque;
-    pcie_mmcfg_data_write(e->pci.bus, addr - e->base_addr, value, 4);
+    pcie_mmcfg_data_write(&e->pci.bus, addr - e->base_addr, value, 4);
 }
 
 static uint32_t pcie_mmcfg_data_readb(void *opaque, target_phys_addr_t addr)
 {
     PCIExpressHost *e = opaque;
-    return pcie_mmcfg_data_read(e->pci.bus, addr - e->base_addr, 1);
+    return pcie_mmcfg_data_read(&e->pci.bus, addr - e->base_addr, 1);
 }
 
 static uint32_t pcie_mmcfg_data_readw(void *opaque, target_phys_addr_t addr)
 {
     PCIExpressHost *e = opaque;
-    return pcie_mmcfg_data_read(e->pci.bus, addr - e->base_addr, 2);
+    return pcie_mmcfg_data_read(&e->pci.bus, addr - e->base_addr, 2);
 }
 
 static uint32_t pcie_mmcfg_data_readl(void *opaque, target_phys_addr_t addr)
 {
     PCIExpressHost *e = opaque;
-    return pcie_mmcfg_data_read(e->pci.bus, addr - e->base_addr, 4);
+    return pcie_mmcfg_data_read(&e->pci.bus, addr - e->base_addr, 4);
 }
 
 
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 663afe2..3d03165 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -228,9 +228,10 @@  PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
 
     dev = qdev_create(NULL, "i440FX-pcihost");
     s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
-    b = pci_bus_new(&s->busdev.qdev, NULL, 0);
-    s->pci_host.bus = b;
+
+    pci_host_bus_init(&s->pci_host, &s->busdev.qdev, 0, NULL, 0);
     qdev_init_nofail(dev);
+    b = &s->pci_host.bus;
 
     d = pci_create_simple(b, 0, "i440FX");
     *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
index 6e437e7..357eb88 100644
--- a/hw/ppc4xx_pci.c
+++ b/hw/ppc4xx_pci.c
@@ -357,12 +357,11 @@  PCIBus *ppc4xx_pci_init(CPUState *env, qemu_irq pci_irqs[4],
 
     controller = qemu_mallocz(sizeof(PPC4xxPCIState));
 
-    controller->pci_state.bus = pci_register_bus(NULL, "pci",
-                                                 ppc4xx_pci_set_irq,
-                                                 ppc4xx_pci_map_irq,
-                                                 pci_irqs, 0, 4);
+    pci_host_bus_init_simple(&controller->pci_state, NULL, 0, "pci",
+                             ppc4xx_pci_set_irq, ppc4xx_pci_map_irq,
+                             pci_irqs, 0, 4);
 
-    controller->pci_dev = pci_register_device(controller->pci_state.bus,
+    controller->pci_dev = pci_register_device(&controller->pci_state.bus,
                                               "host bridge", sizeof(PCIDevice),
                                               0, NULL, NULL);
     pci_conf = controller->pci_dev->config;
@@ -395,7 +394,7 @@  PCIBus *ppc4xx_pci_init(CPUState *env, qemu_irq pci_irqs[4],
     register_savevm(&controller->pci_dev->qdev, "ppc4xx_pci", ppc4xx_pci_id++,
                     1, ppc4xx_pci_save, ppc4xx_pci_load, controller);
 
-    return controller->pci_state.bus;
+    return &controller->pci_state.bus;
 
 free:
     printf("%s error\n", __func__);
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 8ac99f2..8db470d 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -276,12 +276,10 @@  PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
 
     controller = qemu_mallocz(sizeof(PPCE500PCIState));
 
-    controller->pci_state.bus = pci_register_bus(NULL, "pci",
-                                                 mpc85xx_pci_set_irq,
-                                                 mpc85xx_pci_map_irq,
-                                                 pci_irqs, PCI_DEVFN(0x11, 0),
-                                                 4);
-    d = pci_register_device(controller->pci_state.bus,
+    pci_host_bus_init_simple(&controller->pci_state, NULL, 0, "pci",
+                             mpc85xx_pci_set_irq, mpc85xx_pci_map_irq,
+                             pci_irqs, PCI_DEVFN(0x11, 0), 4);
+    d = pci_register_device(&controller->pci_state.bus,
                             "host bridge", sizeof(PCIDevice),
                             0, NULL, NULL);
 
@@ -314,7 +312,7 @@  PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
     register_savevm(&d->qdev, "ppce500_pci", ppce500_pci_id++,
                     1, ppce500_pci_save, ppce500_pci_load, controller);
 
-    return controller->pci_state.bus;
+    return &controller->pci_state.bus;
 
 free:
     printf("%s error\n", __func__);
diff --git a/hw/prep_pci.c b/hw/prep_pci.c
index 0c2afe9..f3d0655 100644
--- a/hw/prep_pci.c
+++ b/hw/prep_pci.c
@@ -43,28 +43,28 @@  static inline uint32_t PPC_PCIIO_config(target_phys_addr_t addr)
 static void PPC_PCIIO_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)
 {
     PREPPCIState *s = opaque;
-    pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 1);
+    pci_data_write(&s->bus, PPC_PCIIO_config(addr), val, 1);
 }
 
 static void PPC_PCIIO_writew (void *opaque, target_phys_addr_t addr, uint32_t val)
 {
     PREPPCIState *s = opaque;
     val = bswap16(val);
-    pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 2);
+    pci_data_write(&s->bus, PPC_PCIIO_config(addr), val, 2);
 }
 
 static void PPC_PCIIO_writel (void *opaque, target_phys_addr_t addr, uint32_t val)
 {
     PREPPCIState *s = opaque;
     val = bswap32(val);
-    pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 4);
+    pci_data_write(&s->bus, PPC_PCIIO_config(addr), val, 4);
 }
 
 static uint32_t PPC_PCIIO_readb (void *opaque, target_phys_addr_t addr)
 {
     PREPPCIState *s = opaque;
     uint32_t val;
-    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 1);
+    val = pci_data_read(&s->bus, PPC_PCIIO_config(addr), 1);
     return val;
 }
 
@@ -72,7 +72,7 @@  static uint32_t PPC_PCIIO_readw (void *opaque, target_phys_addr_t addr)
 {
     PREPPCIState *s = opaque;
     uint32_t val;
-    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 2);
+    val = pci_data_read(&s->bus, PPC_PCIIO_config(addr), 2);
     val = bswap16(val);
     return val;
 }
@@ -81,7 +81,7 @@  static uint32_t PPC_PCIIO_readl (void *opaque, target_phys_addr_t addr)
 {
     PREPPCIState *s = opaque;
     uint32_t val;
-    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 4);
+    val = pci_data_read(&s->bus, PPC_PCIIO_config(addr), 4);
     val = bswap32(val);
     return val;
 }
@@ -117,8 +117,8 @@  PCIBus *pci_prep_init(qemu_irq *pic)
     int PPC_io_memory;
 
     s = qemu_mallocz(sizeof(PREPPCIState));
-    s->bus = pci_register_bus(NULL, "pci",
-                              prep_set_irq, prep_map_irq, pic, 0, 4);
+    pci_host_bus_init_simple(s, NULL, 0, "pci",
+                             prep_set_irq, prep_map_irq, pic, 0, 4);
 
     pci_host_conf_register_ioport(0xcf8, s);
 
@@ -129,7 +129,7 @@  PCIBus *pci_prep_init(qemu_irq *pic)
     cpu_register_physical_memory(0x80800000, 0x00400000, PPC_io_memory);
 
     /* PCI host bridge */
-    d = pci_register_device(s->bus, "PREP Host Bridge - Motorola Raven",
+    d = pci_register_device(&s->bus, "PREP Host Bridge - Motorola Raven",
                             sizeof(PCIDevice), 0, NULL, NULL);
     pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_MOTOROLA);
     pci_config_set_device_id(d->config, PCI_DEVICE_ID_MOTOROLA_RAVEN);
@@ -139,5 +139,5 @@  PCIBus *pci_prep_init(qemu_irq *pic)
     d->config[0x0D] = 0x10; // latency_timer
     d->config[0x34] = 0x00; // capabilities_pointer
 
-    return s->bus;
+    return &s->bus;
 }
diff --git a/hw/sh_pci.c b/hw/sh_pci.c
index cc2f190..798501d 100644
--- a/hw/sh_pci.c
+++ b/hw/sh_pci.c
@@ -29,7 +29,7 @@ 
 #include "bswap.h"
 
 typedef struct {
-    PCIBus *bus;
+    PCIHostState pci;
     PCIDevice *dev;
     uint32_t par;
     uint32_t mbr;
@@ -58,7 +58,7 @@  static void sh_pci_reg_write (void *p, target_phys_addr_t addr, uint32_t val)
         }
         break;
     case 0x220:
-        pci_data_write(pcic->bus, pcic->par, val, 4);
+        pci_data_write(&pcic->pci.bus, pcic->par, val, 4);
         break;
     }
 }
@@ -76,7 +76,7 @@  static uint32_t sh_pci_reg_read (void *p, target_phys_addr_t addr)
     case 0x1c8:
         return pcic->iobr;
     case 0x220:
-        return pci_data_read(pcic->bus, pcic->par, 4);
+        return pci_data_read(&pcic->pci.bus, pcic->par, 4);
     }
     return 0;
 }
@@ -98,10 +98,10 @@  PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
     int reg;
 
     p = qemu_mallocz(sizeof(SHPCIC));
-    p->bus = pci_register_bus(NULL, "pci",
-                              set_irq, map_irq, opaque, devfn_min, nirq);
+    pci_host_bus_init_simple(&p->pci, NULL, 0, "pci",
+                             set_irq, map_irq, opaque, devfn_min, nirq);
 
-    p->dev = pci_register_device(p->bus, "SH PCIC", sizeof(PCIDevice),
+    p->dev = pci_register_device(&p->pci.bus, "SH PCIC", sizeof(PCIDevice),
                                  -1, NULL, NULL);
     reg = cpu_register_io_memory(sh_pci_reg.r, sh_pci_reg.w, p);
     cpu_register_physical_memory(0x1e200000, 0x224, reg);
@@ -117,5 +117,5 @@  PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
     p->dev->config[0x06] = 0x90;
     p->dev->config[0x07] = 0x02;
 
-    return p->bus;
+    return &p->pci.bus;
 }
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index 1310211..9439cf2 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -123,7 +123,7 @@  static void unin_data_write(ReadWriteHandler *handler,
     UNINState *s = container_of(handler, UNINState, data_handler);
     val = qemu_bswap_len(val, len);
     UNIN_DPRINTF("write addr %" FMT_PCIBUS " len %d val %x\n", addr, len, val);
-    pci_data_write(s->host_state.bus,
+    pci_data_write(&s->host_state.bus,
                    unin_get_config_reg(s->host_state.config_reg, addr),
                    val, len);
 }
@@ -134,7 +134,7 @@  static uint32_t unin_data_read(ReadWriteHandler *handler,
     UNINState *s = container_of(handler, UNINState, data_handler);
     uint32_t val;
 
-    val = pci_data_read(s->host_state.bus,
+    val = pci_data_read(&s->host_state.bus,
                         unin_get_config_reg(s->host_state.config_reg, addr),
                         len);
     UNIN_DPRINTF("read addr %" FMT_PCIBUS " len %d val %x\n", addr, len, val);
@@ -228,12 +228,12 @@  PCIBus *pci_pmac_init(qemu_irq *pic)
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(UNINState, s);
-    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
-                                         pci_unin_set_irq, pci_unin_map_irq,
-                                         pic, PCI_DEVFN(11, 0), 4);
+    pci_host_bus_init_simple(&d->host_state, &d->busdev.qdev, 0, "pci",
+                             pci_unin_set_irq, pci_unin_map_irq,
+                             pic, PCI_DEVFN(11, 0), 4);
 
 #if 0
-    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north");
+    pci_create_simple(&d->host_state.bus, PCI_DEVFN(11, 0), "uni-north");
 #endif
 
     sysbus_mmio_map(s, 0, 0xf2800000);
@@ -242,11 +242,11 @@  PCIBus *pci_pmac_init(qemu_irq *pic)
     /* DEC 21154 bridge */
 #if 0
     /* XXX: not activated as PPC BIOS doesn't handle multiple buses properly */
-    pci_create_simple(d->host_state.bus, PCI_DEVFN(12, 0), "dec-21154");
+    pci_create_simple(&d->host_state.bus, PCI_DEVFN(12, 0), "dec-21154");
 #endif
 
     /* Uninorth AGP bus */
-    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north-agp");
+    pci_create_simple(&d->host_state.bus, PCI_DEVFN(11, 0), "uni-north-agp");
     dev = qdev_create(NULL, "uni-north-agp");
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
@@ -256,7 +256,7 @@  PCIBus *pci_pmac_init(qemu_irq *pic)
     /* Uninorth internal bus */
 #if 0
     /* XXX: not needed for now */
-    pci_create_simple(d->host_state.bus, PCI_DEVFN(14, 0), "uni-north-pci");
+    pci_create_simple(&d->host_state.bus, PCI_DEVFN(14, 0), "uni-north-pci");
     dev = qdev_create(NULL, "uni-north-pci");
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
@@ -264,7 +264,7 @@  PCIBus *pci_pmac_init(qemu_irq *pic)
     sysbus_mmio_map(s, 1, 0xf4c00000);
 #endif
 
-    return d->host_state.bus;
+    return &d->host_state.bus;
 }
 
 PCIBus *pci_pmac_u3_init(qemu_irq *pic)
@@ -280,16 +280,16 @@  PCIBus *pci_pmac_u3_init(qemu_irq *pic)
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(UNINState, s);
 
-    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
-                                         pci_unin_set_irq, pci_unin_map_irq,
-                                         pic, PCI_DEVFN(11, 0), 4);
+    pci_host_bus_init_simple(&d->host_state, &d->busdev.qdev, 0, "pci",
+                             pci_unin_set_irq, pci_unin_map_irq,
+                             pic, PCI_DEVFN(11, 0), 4);
 
     sysbus_mmio_map(s, 0, 0xf0800000);
     sysbus_mmio_map(s, 1, 0xf0c00000);
 
-    pci_create_simple(d->host_state.bus, 11 << 3, "u3-agp");
+    pci_create_simple(&d->host_state.bus, 11 << 3, "u3-agp");
 
-    return d->host_state.bus;
+    return &d->host_state.bus;
 }
 
 static int unin_main_pci_host_init(PCIDevice *d)
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index a76bdfa..4cc588b 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -16,6 +16,7 @@  typedef struct {
     qemu_irq irq[4];
     int realview;
     int mem_config;
+    PCIHostState pci;
 } PCIVPBState;
 
 static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr)
@@ -125,9 +126,10 @@  static int pci_vpb_init(SysBusDevice *dev)
     for (i = 0; i < 4; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
-    bus = pci_register_bus(&dev->qdev, "pci",
-                           pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
-                           PCI_DEVFN(11, 0), 4);
+    pci_host_bus_init_simple(&s->pci, &dev->qdev, 0, "pci",
+                             pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
+                             PCI_DEVFN(11, 0), 4);
+    bus = &s->pci.bus;
 
     /* ??? Register memory space.  */