Patchwork [v2,5/5] pci_bridge: introduce pci bridge library.

login
register
mail settings
Submitter Isaku Yamahata
Date July 12, 2010, 10:36 a.m.
Message ID <f8c20ad486a6d30f910674727a73880bcd6e10ae.1278930415.git.yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/58591/
State New
Headers show

Comments

Isaku Yamahata - July 12, 2010, 10:36 a.m.
introduce pci bridge library.
convert apb bridge and dec p2p bridge to use new pci bridge library.
save/restore is supported as a side effect.
This is also preparation for pci express root/upstream/downstream port.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/apb_pci.c       |   42 ++++++++++-----
 hw/dec_pci.c       |   34 +++++++++---
 hw/pci_bridge.c    |  154 +++++++++++++++++++++++++++++++++++-----------------
 hw/pci_bridge.h    |   24 +++++++--
 hw/pci_internals.h |   10 ++--
 qemu-common.h      |    1 +
 6 files changed, 185 insertions(+), 80 deletions(-)
Michael S. Tsirkin - July 12, 2010, 12:10 p.m.
On Mon, Jul 12, 2010 at 07:36:44PM +0900, Isaku Yamahata wrote:
> diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
> index ddb2c82..4697c7a 100644
> --- a/hw/pci_bridge.h
> +++ b/hw/pci_bridge.h
> @@ -29,13 +29,27 @@
>  #include "pci.h"
>  
>  PCIDevice *pci_bridge_get_device(PCIBus *bus);
> +PCIBus *pci_bridge_get_sec_bus(PCIBridge *br);
>  
> -pcibus_t pci_bridge_get_base(PCIDevice *bridge, uint8_t type);
> -pcibus_t pci_bridge_get_limit(PCIDevice *bridge, uint8_t type);
> +pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type);
> +pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type);
>  
> -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction,
> -                        uint16_t vid, uint16_t did,
> -                        pci_map_irq_fn map_irq, const char *name);
> +void pci_bridge_write_config(PCIDevice *d,
> +                             uint32_t address, uint32_t val, int len);
> +void pci_bridge_reset_reg(PCIDevice *dev);
> +void pci_bridge_reset(DeviceState *qdev);
> +
> +int pci_bridge_initfn(PCIDevice *pci_dev);
> +int pci_bridge_exitfn(PCIDevice *pci_dev);
> +
> +void pci_bridge_qdev_register(PCIDeviceInfo *info);
> +
> +PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, bool multifunction,
> +                             pci_map_irq_fn map_irq,
> +                             const char *name, const char *bus_name);
> +PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn, bool multifunction,
> +                                    pci_map_irq_fn map_irq,
> +                                    const char *name, const char *bus_name);

The APIs leave much to be desired.  _simple and regular are same?  What does _register do?

We really should just use qdev: Can't we use pci_qdev_register_many and
pci_create to create the bridge?  Long term, all pci_create variants
should go and get replaced with qdev_create.

>  
>  #endif  /* QEMU_PCI_BRIDGE_H */
>  /*
> diff --git a/hw/pci_internals.h b/hw/pci_internals.h
> index fa844ab..6502f83 100644
> --- a/hw/pci_internals.h
> +++ b/hw/pci_internals.h
> @@ -30,11 +30,13 @@ struct PCIBus {
>      int *irq_count;
>  };
>  
> -typedef struct {
> +struct PCIBridge {
>      PCIDevice dev;
> +
> +    /* private member */
>      PCIBus sec_bus;
> -    uint32_t vid;
> -    uint32_t did;
> -} PCIBridge;
> +    pci_map_irq_fn map_irq;
> +    const char *bus_name;
> +};
>  
>  #endif /* QEMU_PCI_INTERNALS_H */
> diff --git a/qemu-common.h b/qemu-common.h
> index 3fb2f0b..d735235 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -219,6 +219,7 @@ typedef struct PCIHostState PCIHostState;
>  typedef struct PCIExpressHost PCIExpressHost;
>  typedef struct PCIBus PCIBus;
>  typedef struct PCIDevice PCIDevice;
> +typedef struct PCIBridge PCIBridge;
>  typedef struct SerialState SerialState;
>  typedef struct IRQState *qemu_irq;
>  typedef struct PCMCIACardState PCMCIACardState;
> -- 
> 1.7.1.1
Isaku Yamahata - July 12, 2010, 1:28 p.m.
On Mon, Jul 12, 2010 at 03:10:00PM +0300, Michael S. Tsirkin wrote:
> On Mon, Jul 12, 2010 at 07:36:44PM +0900, Isaku Yamahata wrote:
> > diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
> > index ddb2c82..4697c7a 100644
> > --- a/hw/pci_bridge.h
> > +++ b/hw/pci_bridge.h
> > @@ -29,13 +29,27 @@
> >  #include "pci.h"
> >  
> >  PCIDevice *pci_bridge_get_device(PCIBus *bus);
> > +PCIBus *pci_bridge_get_sec_bus(PCIBridge *br);
> >  
> > -pcibus_t pci_bridge_get_base(PCIDevice *bridge, uint8_t type);
> > -pcibus_t pci_bridge_get_limit(PCIDevice *bridge, uint8_t type);
> > +pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type);
> > +pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type);
> >  
> > -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction,
> > -                        uint16_t vid, uint16_t did,
> > -                        pci_map_irq_fn map_irq, const char *name);
> > +void pci_bridge_write_config(PCIDevice *d,
> > +                             uint32_t address, uint32_t val, int len);
> > +void pci_bridge_reset_reg(PCIDevice *dev);
> > +void pci_bridge_reset(DeviceState *qdev);
> > +
> > +int pci_bridge_initfn(PCIDevice *pci_dev);
> > +int pci_bridge_exitfn(PCIDevice *pci_dev);
> > +
> > +void pci_bridge_qdev_register(PCIDeviceInfo *info);
> > +
> > +PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, bool multifunction,
> > +                             pci_map_irq_fn map_irq,
> > +                             const char *name, const char *bus_name);
> > +PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn, bool multifunction,
> > +                                    pci_map_irq_fn map_irq,
> > +                                    const char *name, const char *bus_name);
> 
> The APIs leave much to be desired.  _simple and regular are same?  What does _register do?
> 
> We really should just use qdev: Can't we use pci_qdev_register_many and
> pci_create to create the bridge?  Long term, all pci_create variants
> should go and get replaced with qdev_create.

If struct PCIBridge is exported, those three can be eliminated.
I think it is okay to export struct PCIBridge, but it would not
be a good idea to export PCIBus which is embedded in PCIBridge::sec_bus.

So how should we go?
- export both PCIBus and PCIBridge.

- make PCIBridge::sec_bus pointer, and export PCIBridge.
  And kill register, create, create_simple.
  v1 patch. Although you rejected it, I suppose you didn't see
  this issue.

- introduce wrapper functions to convert types.
  This patch.

- better alternatives?
Michael S. Tsirkin - July 12, 2010, 2:47 p.m.
On Mon, Jul 12, 2010 at 10:28:24PM +0900, Isaku Yamahata wrote:
> On Mon, Jul 12, 2010 at 03:10:00PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jul 12, 2010 at 07:36:44PM +0900, Isaku Yamahata wrote:
> > > diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
> > > index ddb2c82..4697c7a 100644
> > > --- a/hw/pci_bridge.h
> > > +++ b/hw/pci_bridge.h
> > > @@ -29,13 +29,27 @@
> > >  #include "pci.h"
> > >  
> > >  PCIDevice *pci_bridge_get_device(PCIBus *bus);
> > > +PCIBus *pci_bridge_get_sec_bus(PCIBridge *br);
> > >  
> > > -pcibus_t pci_bridge_get_base(PCIDevice *bridge, uint8_t type);
> > > -pcibus_t pci_bridge_get_limit(PCIDevice *bridge, uint8_t type);
> > > +pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type);
> > > +pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type);
> > >  
> > > -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction,
> > > -                        uint16_t vid, uint16_t did,
> > > -                        pci_map_irq_fn map_irq, const char *name);
> > > +void pci_bridge_write_config(PCIDevice *d,
> > > +                             uint32_t address, uint32_t val, int len);
> > > +void pci_bridge_reset_reg(PCIDevice *dev);
> > > +void pci_bridge_reset(DeviceState *qdev);
> > > +
> > > +int pci_bridge_initfn(PCIDevice *pci_dev);
> > > +int pci_bridge_exitfn(PCIDevice *pci_dev);
> > > +
> > > +void pci_bridge_qdev_register(PCIDeviceInfo *info);
> > > +
> > > +PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, bool multifunction,
> > > +                             pci_map_irq_fn map_irq,
> > > +                             const char *name, const char *bus_name);
> > > +PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn, bool multifunction,
> > > +                                    pci_map_irq_fn map_irq,
> > > +                                    const char *name, const char *bus_name);
> > 
> > The APIs leave much to be desired.  _simple and regular are same?  What does _register do?
> > 
> > We really should just use qdev: Can't we use pci_qdev_register_many and
> > pci_create to create the bridge?  Long term, all pci_create variants
> > should go and get replaced with qdev_create.
> 
> If struct PCIBridge is exported, those three can be eliminated.
> I think it is okay to export struct PCIBridge, but it would not
> be a good idea to export PCIBus which is embedded in PCIBridge::sec_bus.
> 
> So how should we go?
> - export both PCIBus and PCIBridge.
> 
> - make PCIBridge::sec_bus pointer, and export PCIBridge.
>   And kill register, create, create_simple.
>   v1 patch. Although you rejected it, I suppose you didn't see
>   this issue.
> 
> - introduce wrapper functions to convert types.
>   This patch.
> 
> - better alternatives?

Let's export and see where this leads us.

> -- 
> yamahata

Patch

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 88ee4a9..d446550 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -31,6 +31,7 @@ 
 #include "pci_host.h"
 #include "pci_bridge.h"
 #include "rwhandler.h"
+#include "pci_bridge.h"
 #include "apb_pci.h"
 #include "sysemu.h"
 
@@ -294,9 +295,17 @@  static void pci_apb_set_irq(void *opaque, int irq_num, int level)
     }
 }
 
-static void apb_pci_bridge_init(PCIBus *b)
+static int apb_pci_bridge_initfn(PCIDevice *dev)
 {
-    PCIDevice *dev = pci_bridge_get_device(b);
+    int rc;
+
+    rc = pci_bridge_initfn(dev);
+    if (rc < 0) {
+        return rc;
+    }
+
+    pci_config_set_vendor_id(dev->config, PCI_VENDOR_ID_SUN);
+    pci_config_set_device_id(dev->config, PCI_DEVICE_ID_SUN_SIMBA);
 
     /*
      * command register:
@@ -313,6 +322,7 @@  static void apb_pci_bridge_init(PCIBus *b)
                  PCI_STATUS_FAST_BACK | PCI_STATUS_66MHZ |
                  PCI_STATUS_DEVSEL_MEDIUM);
     pci_set_byte(dev->config + PCI_REVISION_ID, 0x11);
+    return 0;
 }
 
 PCIBus *pci_apb_init(target_phys_addr_t special_base,
@@ -323,6 +333,7 @@  PCIBus *pci_apb_init(target_phys_addr_t special_base,
     SysBusDevice *s;
     APBState *d;
     unsigned int i;
+    PCIBridge *br;
 
     /* Ultrasparc PBM main bus */
     dev = qdev_create(NULL, "pbm");
@@ -348,17 +359,15 @@  PCIBus *pci_apb_init(target_phys_addr_t special_base,
     pci_create_simple(d->bus, 0, "pbm");
 
     /* APB secondary busses */
-    *bus2 = pci_bridge_init(d->bus, PCI_DEVFN(1, 0), true,
-                            PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SIMBA,
-                            pci_apb_map_irq,
-                            "Advanced PCI Bus secondary bridge 1");
-    apb_pci_bridge_init(*bus2);
-
-    *bus3 = pci_bridge_init(d->bus, PCI_DEVFN(1, 1), true,
-                            PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_SIMBA,
-                            pci_apb_map_irq,
-                            "Advanced PCI Bus secondary bridge 2");
-    apb_pci_bridge_init(*bus3);
+    br = pci_bridge_create_simple(d->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,
+                                  pci_apb_map_irq, "pbm-bridge",
+                                  "Advanced PCI Bus secondary bridge 2");
+    *bus3 = pci_bridge_get_sec_bus(br);
 
     return d->bus;
 }
@@ -441,10 +450,17 @@  static SysBusDeviceInfo pbm_host_info = {
     .qdev.reset = pci_pbm_reset,
     .init = pci_pbm_init_device,
 };
+
+static PCIDeviceInfo pbm_pci_bridge_info = {
+    .qdev.name = "pbm-bridge",
+    .init = apb_pci_bridge_initfn,
+};
+
 static void pbm_register_devices(void)
 {
     sysbus_register_withprop(&pbm_host_info);
     pci_qdev_register(&pbm_pci_host_info);
+    pci_bridge_qdev_register(&pbm_pci_bridge_info);
 }
 
 device_init(pbm_register_devices)
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index f7a9cdc..ce84faa 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -49,18 +49,33 @@  static int dec_map_irq(PCIDevice *pci_dev, int irq_num)
     return irq_num;
 }
 
-PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
+static int dec_21154_initfn(PCIDevice *dev)
 {
-    DeviceState *dev;
-    PCIBus *ret;
+    int rc;
+
+    rc = pci_bridge_initfn(dev);
+    if (rc < 0) {
+        return rc;
+    }
+
+    pci_config_set_vendor_id(dev->config, PCI_VENDOR_ID_DEC);
+    pci_config_set_device_id(dev->config, PCI_DEVICE_ID_DEC_21154);
+    return 0;
+}
 
-    dev = qdev_create(NULL, "dec-21154");
-    qdev_init_nofail(dev);
-    ret = pci_bridge_init(parent_bus, devfn, false,
-                          PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_21154,
-                          dec_map_irq, "DEC 21154 PCI-PCI bridge");
+static PCIDeviceInfo dec_21154_pci_bridge_info = {
+    .qdev.name = "dec-21154-p2p-bridge",
+    .qdev.desc = "DEC 21154 PCI-PCI bridge",
+    .init = dec_21154_initfn,
+};
 
-    return ret;
+PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
+{
+    PCIBridge *br;
+    br = pci_bridge_create_simple(parent_bus, devfn, false, dec_map_irq,
+                                  "dec-21154-p2p-bridge",
+                                  "DEC 21154 PCI-PCI bridge");
+    return pci_bridge_get_sec_bus(br);
 }
 
 static int pci_dec_21154_init_device(SysBusDevice *dev)
@@ -99,6 +114,7 @@  static void dec_register_devices(void)
     sysbus_register_dev("dec-21154", sizeof(DECState),
                         pci_dec_21154_init_device);
     pci_qdev_register(&dec_21154_pci_host_info);
+    pci_bridge_qdev_register(&dec_21154_pci_bridge_info);
 }
 
 device_init(dec_register_devices)
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 16f930a..f369dc3 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -32,12 +32,19 @@ 
 #include "pci_bridge.h"
 #include "pci_internals.h"
 
+/* Accessor function to get parent bridge device from pci bus. */
 PCIDevice *pci_bridge_get_device(PCIBus *bus)
 {
     return bus->parent_dev;
 }
 
-static uint32_t pci_config_get_io_base(PCIDevice *d,
+/* Accessor function to get secondary bus from pci-to-pci bridge device */
+PCIBus *pci_bridge_get_sec_bus(PCIBridge *br)
+{
+    return &br->sec_bus;
+}
+
+static uint32_t pci_config_get_io_base(const PCIDevice *d,
                                        uint32_t base, uint32_t base_upper16)
 {
     uint32_t val;
@@ -49,13 +56,13 @@  static uint32_t pci_config_get_io_base(PCIDevice *d,
     return val;
 }
 
-static pcibus_t pci_config_get_memory_base(PCIDevice *d, uint32_t base)
+static pcibus_t pci_config_get_memory_base(const PCIDevice *d, uint32_t base)
 {
     return ((pcibus_t)pci_get_word(d->config + base) & PCI_MEMORY_RANGE_MASK)
         << 16;
 }
 
-static pcibus_t pci_config_get_pref_base(PCIDevice *d,
+static pcibus_t pci_config_get_pref_base(const PCIDevice *d,
                                          uint32_t base, uint32_t upper)
 {
     pcibus_t tmp;
@@ -69,7 +76,8 @@  static pcibus_t pci_config_get_pref_base(PCIDevice *d,
     return val;
 }
 
-pcibus_t pci_bridge_get_base(PCIDevice *bridge, uint8_t type)
+/* accessor function to get bridge filtering base address */
+pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type)
 {
     pcibus_t base;
     if (type & PCI_BASE_ADDRESS_SPACE_IO) {
@@ -87,7 +95,8 @@  pcibus_t pci_bridge_get_base(PCIDevice *bridge, uint8_t type)
     return base;
 }
 
-pcibus_t pci_bridge_get_limit(PCIDevice *bridge, uint8_t type)
+/* accessor funciton to get bridge filtering limit */
+pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type)
 {
     pcibus_t limit;
     if (type & PCI_BASE_ADDRESS_SPACE_IO) {
@@ -106,9 +115,11 @@  pcibus_t pci_bridge_get_limit(PCIDevice *bridge, uint8_t type)
     return limit;
 }
 
-static void pci_bridge_write_config(PCIDevice *d,
+/* default write_config function for PCI-to-PCI bridge */
+void pci_bridge_write_config(PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
+    PCIBridge *br = DO_UPCAST(PCIBridge, dev, d);
     pci_default_write_config(d, address, val, len);
 
     if (/* io base/limit */
@@ -117,16 +128,45 @@  static void pci_bridge_write_config(PCIDevice *d,
         /* memory base/limit, prefetchable base/limit and
            io base/limit upper 16 */
         ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) {
-        pci_bridge_update_mappings(d->bus);
+        pci_bridge_update_mappings(&br->sec_bus);
     }
 }
 
-static int pci_bridge_initfn(PCIDevice *dev)
+/* reset bridge specific configuration registers */
+void pci_bridge_reset_reg(PCIDevice *dev)
 {
-    PCIBridge *s = DO_UPCAST(PCIBridge, dev, dev);
+    uint8_t *conf = dev->config;
+
+    conf[PCI_PRIMARY_BUS] = 0;
+    conf[PCI_SECONDARY_BUS] = 0;
+    conf[PCI_SUBORDINATE_BUS] = 0;
+    conf[PCI_SEC_LATENCY_TIMER] = 0;
+
+    conf[PCI_IO_BASE] = 0;
+    conf[PCI_IO_LIMIT] = 0;
+    pci_set_word(conf + PCI_MEMORY_BASE, 0);
+    pci_set_word(conf + PCI_MEMORY_LIMIT, 0);
+    pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0);
+    pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0);
+    pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
+    pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
+
+    pci_set_word(conf + PCI_BRIDGE_CONTROL, 0);
+}
 
-    pci_config_set_vendor_id(s->dev.config, s->vid);
-    pci_config_set_device_id(s->dev.config, s->did);
+/* default reset function for PCI-to-PCI bridge */
+void pci_bridge_reset(DeviceState *qdev)
+{
+    PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
+    pci_bridge_reset_reg(dev);
+}
+
+/* default qdev initialization function for PCI-to-PCI bridge */
+int pci_bridge_initfn(PCIDevice *dev)
+{
+    PCIBus *parent = dev->bus;
+    PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
+    PCIBus *sec_bus = &br->sec_bus;
 
     pci_set_word(dev->config + PCI_STATUS,
                  PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
@@ -136,58 +176,74 @@  static int pci_bridge_initfn(PCIDevice *dev)
         PCI_HEADER_TYPE_BRIDGE;
     pci_set_word(dev->config + PCI_SEC_STATUS,
                  PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
+
+    qbus_create_inplace(&sec_bus->qbus, &pci_bus_info, &dev->qdev,
+                        br->bus_name);
+    sec_bus->parent_dev = dev;
+    sec_bus->map_irq = br->map_irq;
+
+    QLIST_INIT(&sec_bus->child);
+    QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
     return 0;
 }
 
-static int pci_bridge_exitfn(PCIDevice *pci_dev)
+/* default qdev clean up function for PCI-to-PCI bridge */
+int pci_bridge_exitfn(PCIDevice *pci_dev)
 {
     PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
     assert(QLIST_EMPTY(&s->sec_bus.child));
     QLIST_REMOVE(&s->sec_bus, sibling);
+    /* qbus_free() is called automatically by qdev_free() */
     return 0;
 }
 
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction,
-                        uint16_t vid, uint16_t did,
-                        pci_map_irq_fn map_irq, const char *name)
+/*
+ * convenience function of pci_qdev_register for PCI-to-PCI bridge
+ * This function fills not-initialized members with default value.
+ */
+void pci_bridge_qdev_register(PCIDeviceInfo *info)
 {
-    PCIDevice *dev;
-    PCIBridge *s;
-    PCIBus *sec_bus;
-
-    dev = pci_create_multifunction(bus, devfn, multifunction, "pci-bridge");
-    qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
-    qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
-    qdev_init_nofail(&dev->qdev);
-
-    s = DO_UPCAST(PCIBridge, dev, dev);
-    sec_bus = &s->sec_bus;
-    qbus_create_inplace(&sec_bus->qbus, &pci_bus_info, &dev->qdev, name);
-    sec_bus->parent_dev = dev;
-    sec_bus->map_irq = map_irq;
-
-    QLIST_INIT(&sec_bus->child);
-    QLIST_INSERT_HEAD(&bus->child, sec_bus, sibling);
-    return &s->sec_bus;
-}
+    if (!info->qdev.size) {
+        info->qdev.size = sizeof(PCIBridge);
+    }
+    if (!info->qdev.vmsd) {
+        info->qdev.vmsd = &vmstate_pci_device;
+    }
 
-static PCIDeviceInfo bridge_info = {
-    .qdev.name    = "pci-bridge",
-    .qdev.size    = sizeof(PCIBridge),
-    .init         = pci_bridge_initfn,
-    .exit         = pci_bridge_exitfn,
-    .config_write = pci_bridge_write_config,
-    .is_bridge    = 1,
-    .qdev.props   = (Property[]) {
-        DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
-        DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
-        DEFINE_PROP_END_OF_LIST(),
+    assert(info->init); /* vid/did must be set at least */
+    if (!info->exit) {
+        info->exit = pci_bridge_exitfn;
     }
-};
+    if (!info->config_write) {
+        info->config_write = pci_bridge_write_config;
+    }
+    if (!info->qdev.reset) {
+        info->qdev.reset = pci_bridge_reset;
+    }
+
+    info->is_bridge = 1;
+    pci_qdev_register(info);
+}
 
-static void pci_register_devices(void)
+/* transitional function to create pci-to-pci bridge */
+PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, bool multifunction,
+                             pci_map_irq_fn map_irq,
+                             const char *name, const char *bus_name)
 {
-    pci_qdev_register(&bridge_info);
+    PCIDevice *dev = pci_create_multifunction(bus, devfn, multifunction, name);
+    PCIBridge *br = DO_UPCAST(PCIBridge, dev, dev);
+    br->map_irq = map_irq;
+    br->bus_name = bus_name;
+    return br;
 }
 
-device_init(pci_register_devices)
+/* convenience function = pci_bridge_create() + qdev_init_nofail() */
+PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn, bool multifunction,
+                                    pci_map_irq_fn map_irq,
+                                    const char *name, const char *bus_name)
+{
+    PCIBridge *br = pci_bridge_create(bus, devfn, multifunction, map_irq,
+                                      name, bus_name);
+    qdev_init_nofail(&br->dev.qdev);
+    return br;
+}
diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
index ddb2c82..4697c7a 100644
--- a/hw/pci_bridge.h
+++ b/hw/pci_bridge.h
@@ -29,13 +29,27 @@ 
 #include "pci.h"
 
 PCIDevice *pci_bridge_get_device(PCIBus *bus);
+PCIBus *pci_bridge_get_sec_bus(PCIBridge *br);
 
-pcibus_t pci_bridge_get_base(PCIDevice *bridge, uint8_t type);
-pcibus_t pci_bridge_get_limit(PCIDevice *bridge, uint8_t type);
+pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type);
+pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type);
 
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction,
-                        uint16_t vid, uint16_t did,
-                        pci_map_irq_fn map_irq, const char *name);
+void pci_bridge_write_config(PCIDevice *d,
+                             uint32_t address, uint32_t val, int len);
+void pci_bridge_reset_reg(PCIDevice *dev);
+void pci_bridge_reset(DeviceState *qdev);
+
+int pci_bridge_initfn(PCIDevice *pci_dev);
+int pci_bridge_exitfn(PCIDevice *pci_dev);
+
+void pci_bridge_qdev_register(PCIDeviceInfo *info);
+
+PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, bool multifunction,
+                             pci_map_irq_fn map_irq,
+                             const char *name, const char *bus_name);
+PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn, bool multifunction,
+                                    pci_map_irq_fn map_irq,
+                                    const char *name, const char *bus_name);
 
 #endif  /* QEMU_PCI_BRIDGE_H */
 /*
diff --git a/hw/pci_internals.h b/hw/pci_internals.h
index fa844ab..6502f83 100644
--- a/hw/pci_internals.h
+++ b/hw/pci_internals.h
@@ -30,11 +30,13 @@  struct PCIBus {
     int *irq_count;
 };
 
-typedef struct {
+struct PCIBridge {
     PCIDevice dev;
+
+    /* private member */
     PCIBus sec_bus;
-    uint32_t vid;
-    uint32_t did;
-} PCIBridge;
+    pci_map_irq_fn map_irq;
+    const char *bus_name;
+};
 
 #endif /* QEMU_PCI_INTERNALS_H */
diff --git a/qemu-common.h b/qemu-common.h
index 3fb2f0b..d735235 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -219,6 +219,7 @@  typedef struct PCIHostState PCIHostState;
 typedef struct PCIExpressHost PCIExpressHost;
 typedef struct PCIBus PCIBus;
 typedef struct PCIDevice PCIDevice;
+typedef struct PCIBridge PCIBridge;
 typedef struct SerialState SerialState;
 typedef struct IRQState *qemu_irq;
 typedef struct PCMCIACardState PCMCIACardState;