[Qemu-devel] Re: [PATCH] pci: make pci_bus_new() aware of pci domain

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

On Thu, Jan 20, 2011 at 03:57:56PM +0900, Isaku Yamahata wrote:
> This patch makes pci bus creation aware of pci domain.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

IMO domain support needs some thought,
before we jump into it. Specifically:

I am not sure a simple number is enough to address a domain. In linux,
what seems to happen is that the pci segment reported by pci is used.
What happens without acpi? I'll have to look - do you know? What will we
do for acpi? We can load firmware or add a hardware interface to get the domains.
How will migration work?

Thanks,

> ---
>  hw/pci.c      |   19 ++++++++++++++-----
>  hw/pci.h      |    7 ++++---
>  hw/piix_pci.c |    2 +-
>  3 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 86af0ee..e1e7b25 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -246,8 +246,11 @@ int pci_find_domain(const PCIBus *bus)
>      return -1;
>  }
>  
> +/* create root pci bus.
> + * If secondary pci bus is wanted, use pci_bridge_initfn()
> + */
>  void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
> -                         const char *name, int devfn_min)
> +                         const char *name, int domain, int devfn_min)
>  {
>      qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
>      assert(PCI_FUNC(devfn_min) == 0);
> @@ -255,18 +258,19 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
>  
>      /* host bridge */
>      QLIST_INIT(&bus->child);
> -    pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
> +    pci_host_bus_register(domain, bus);
>  
>      vmstate_register(NULL, -1, &vmstate_pcibus, bus);
>  }
>  
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min)
> +PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +                    int domain, int devfn_min)
>  {
>      PCIBus *bus;
>  
>      bus = qemu_mallocz(sizeof(*bus));
>      bus->qbus.qdev_allocated = 1;
> -    pci_bus_new_inplace(bus, parent, name, devfn_min);
> +    pci_bus_new_inplace(bus, parent, name, domain, devfn_min);
>      return bus;
>  }
>  
> @@ -292,13 +296,18 @@ void pci_bus_set_mem_base(PCIBus *bus, target_phys_addr_t base)
>      bus->mem_base = base;
>  }
>  
> +/* deprecated: kept for compatility of existing codes.
> + * pci_bus_new() and pci_bus_irqs() should be used for root pci bus
> + * like i440fx_init().
> + * pci_bridge_initfn() should be used for secondary pci bus
> + */
>  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);
> +    bus = pci_bus_new(parent, name, 0 /* domain = 0 for compat */, devfn_min);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
>      return bus;
>  }
> diff --git a/hw/pci.h b/hw/pci.h
> index bc8d5bb..a0fd953 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -229,14 +229,15 @@ typedef enum {
>  typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev,
>                                PCIHotplugState 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);
> +                         const char *name, int domain, int devfn_min);
> +PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +                    int domain, int devfn_min);
>  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 *irq_opaque, int devfn_min, int nirq); /* deprecated */
>  void pci_device_reset(PCIDevice *dev);
>  void pci_bus_reset(PCIBus *bus);
>  
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 358da58..718983d 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -226,7 +226,7 @@ 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);
> +    b = pci_bus_new(&s->busdev.qdev, NULL, 0, 0);
>      s->bus = b;
>      qdev_init_nofail(dev);
>  
> -- 
> 1.7.1.1