On Fri, Jul 02, 2010 at 11:30:11AM +0900, Isaku Yamahata wrote:
> allocate PCIBus dynamically for PCIBridge and bug fix of
> pci_unregister_secondary_bus().
could you make the bugfix a separate patch please?
> This is a preparation for splitting out pci_bridge functions.
> Since PCIBus is private to pci.c, PCIBridge won't be able to
> contain PCIBus in its structure.
>
> Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
I think this becomes too complex: as bridge configuration affects
the bus operation, you might end up sticking a pointer to the device
in the bus. A similar arrangement is in place in with piix_pci, and I would
love to get rid of it, too.
Let's just put PCIBus in a header? It could be a new header
named pci_internals.h or something like this.
> ---
> hw/pci.c | 25 ++++++++++++++-----------
> 1 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 08652e8..fdf02d0 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -286,23 +286,27 @@ PCIBus *pci_register_bus(DeviceState *parent, const
> char *name,
> return bus;
> }
>
> -static void pci_register_secondary_bus(PCIBus *parent,
> - PCIBus *bus,
> - PCIDevice *dev,
> - pci_map_irq_fn map_irq,
> - const char *name)
> +static PCIBus *pci_register_secondary_bus(PCIBus *parent,
> + PCIDevice *dev,
> + pci_map_irq_fn map_irq,
> + const char *name)
> {
> - qbus_create_inplace(&bus->qbus, &pci_bus_info, &dev->qdev, name);
> + PCIBus *bus;
> + bus = pci_bus_new(&dev->qdev, name, 0);
> +
> bus->map_irq = map_irq;
> bus->parent_dev = dev;
>
> QLIST_INSERT_HEAD(&parent->child, bus, sibling);
> +
> + return bus;
This does more than we need: pci_bus_new
was created for host bus so it will also register in
reset and vmstate lists.
> }
>
> static void pci_unregister_secondary_bus(PCIBus *bus)
> {
> assert(QLIST_EMPTY(&bus->child));
> QLIST_REMOVE(bus, sibling);
> + qbus_free(&bus->qbus);
> }
>
> int pci_bus_num(PCIBus *s)
> @@ -1527,7 +1531,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char
> *default_model,
>
> typedef struct {
> PCIDevice dev;
> - PCIBus bus;
> + PCIBus *bus;
> uint32_t vid;
> uint32_t did;
> } PCIBridge;
> @@ -1628,8 +1632,7 @@ static int pci_bridge_initfn(PCIDevice *dev)
> static int pci_bridge_exitfn(PCIDevice *pci_dev)
> {
> PCIBridge *s = DO_UPCAST(PCIBridge, dev, pci_dev);
> - PCIBus *bus = &s->bus;
> - pci_unregister_secondary_bus(bus);
> + pci_unregister_secondary_bus(s->bus);
> return 0;
> }
>
> @@ -1646,8 +1649,8 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool
> multifunction,
> qdev_init_nofail(&dev->qdev);
>
> s = DO_UPCAST(PCIBridge, dev, dev);
> - pci_register_secondary_bus(bus, &s->bus, &s->dev, map_irq, name);
> - return &s->bus;
> + s->bus = pci_register_secondary_bus(bus, &s->dev, map_irq, name);
> + return s->bus;
> }
>
> PCIDevice *pci_bridge_get_device(PCIBus *bus)
> --
> 1.7.1.1
