Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> Currently pci_get_primary_bus() searches the list of root buses for one
> with domain 0.  But since host buses are always registered with domain 0,
> this just amounts to finding the only PCI host bus.
> 
> This simplifies the implementation by defining the primary PCI bus to
> be the first one registered, using a global variable to track it.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

This is the only part that I dislike.
How about an explicit API to set the primary bus?
Let machine types set it.

> ---
>  hw/pci/pci.c |   18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index a3c192c..b25a1a1 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -96,6 +96,7 @@ struct PCIHostBus {
>      QLIST_ENTRY(PCIHostBus) next;
>  };
>  static QLIST_HEAD(, PCIHostBus) host_buses;
> +static PCIBus *pci_primary_bus;
>  
>  static const VMStateDescription vmstate_pcibus = {
>      .name = "PCIBUS",
> @@ -241,6 +242,12 @@ static int pcibus_reset(BusState *qbus)
>  static void pci_host_bus_register(int domain, PCIBus *bus)
>  {
>      struct PCIHostBus *host;
> +
> +    /* If this is the first one, assume it's the primary bus */
> +    if (!pci_primary_bus) {
> +        pci_primary_bus = bus;
> +    }
> +
>      host = g_malloc0(sizeof(*host));
>      host->domain = domain;
>      host->bus = bus;
> @@ -249,15 +256,7 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
>  
>  PCIBus *pci_get_primary_bus(void)
>  {
> -    struct PCIHostBus *host;
> -
> -    QLIST_FOREACH(host, &host_buses, next) {
> -        if (host->domain == 0) {
> -            return host->bus;
> -        }
> -    }
> -
> -    return NULL;
> +    return pci_primary_bus;
>  }
>  
>  PCIBus *pci_device_root_bus(const PCIDevice *d)
> @@ -300,6 +299,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>  
>      /* host bridge */
>      QLIST_INIT(&bus->child);
> +
>      pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
>  
>      vmstate_register(NULL, -1, &vmstate_pcibus, bus);
> -- 
> 1.7.10.4