On Thu, May 09, 2013 at 10:31:07AM +1000, David Gibson wrote:
> pci_find_root_bus() takes a domain parameter. Currently PCI root buses
> with domain other than 0 can't be created, so this is more or less a long
> winded way of retrieving the main PCI root bus. Numbered domains don't
> actually properly cover the (non x86) possibilities for multiple PCI root
> buses, so this patch for now enforces the domain == 0 restriction in other
> places to replace pci_find_root_bus() with an explicit
> pci_get_primary_bus().
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
Can we instead just drop the domain parameter
from pci_find_root_bus and add a comment
saying that it fails if there is more than one root?
> ---
> hw/pci/pci-hotplug-old.c | 34 +++++++++++++++++++++++++---------
> hw/pci/pci.c | 19 +++++++++++++++----
> include/hw/pci/pci.h | 2 +-
> 3 files changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> index 1aa0ab8..55441c6 100644
> --- a/hw/pci/pci-hotplug-old.c
> +++ b/hw/pci/pci-hotplug-old.c
> @@ -34,17 +34,23 @@
> #include "sysemu/blockdev.h"
> #include "qapi/error.h"
>
> -static int pci_read_devaddr(Monitor *mon, const char *addr, int *domp,
> +static int pci_read_devaddr(Monitor *mon, const char *addr,
> int *busp, unsigned *slotp)
> {
> + int dom;
> +
> /* strip legacy tag */
> if (!strncmp(addr, "pci_addr=", 9)) {
> addr += 9;
> }
> - if (pci_parse_devaddr(addr, domp, busp, slotp, NULL)) {
> + if (pci_parse_devaddr(addr, &dom, busp, slotp, NULL)) {
> monitor_printf(mon, "Invalid pci address\n");
> return -1;
> }
> + if (dom != 0) {
> + monitor_printf(mon, "Multiple PCI domains not supported, use
> device_add\n");
> + return -1;
> + }
> return 0;
> }
>
> @@ -126,18 +132,22 @@ static int scsi_hot_add(Monitor *mon, DeviceState
> *adapter,
>
> int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
> {
> - int dom, pci_bus;
> + int pci_bus;
> unsigned slot;
> + PCIBus *root = pci_get_primary_bus();
> PCIDevice *dev;
> const char *pci_addr = qdict_get_str(qdict, "pci_addr");
>
> switch (dinfo->type) {
> case IF_SCSI:
> - if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) {
> + if (!root) {
> + monitor_printf(mon, "no primary PCI bus\n");
> + goto err;
> + }
> + if (pci_read_devaddr(mon, pci_addr, &pci_bus, &slot)) {
> goto err;
> }
> - dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
> - PCI_DEVFN(slot, 0));
> + dev = pci_find_device(root, pci_bus, PCI_DEVFN(slot, 0));
> if (!dev) {
> monitor_printf(mon, "no pci device with address %s\n", pci_addr);
> goto err;
> @@ -273,16 +283,22 @@ void pci_device_hot_add(Monitor *mon, const QDict
> *qdict)
>
> static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
> {
> + PCIBus *root = pci_get_primary_bus();
> PCIDevice *d;
> - int dom, bus;
> + int bus;
> unsigned slot;
> Error *local_err = NULL;
>
> - if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
> + if (!root) {
> + monitor_printf(mon, "no primary PCI bus\n");
> + return -1;
> + }
> +
> + if (pci_read_devaddr(mon, pci_addr, &bus, &slot)) {
> return -1;
> }
>
> - d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
> + d = pci_find_device(root, bus, PCI_DEVFN(slot, 0));
> if (!d) {
> monitor_printf(mon, "slot %d empty\n", slot);
> return -1;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 9906e84..9503d56 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -246,12 +246,12 @@ static void pci_host_bus_register(int domain, PCIBus
> *bus)
> QLIST_INSERT_HEAD(&host_buses, host, next);
> }
>
> -PCIBus *pci_find_root_bus(int domain)
> +PCIBus *pci_get_primary_bus(void)
> {
> struct PCIHostBus *host;
>
> QLIST_FOREACH(host, &host_buses, next) {
> - if (host->domain == domain) {
> + if (host->domain == 0) {
> return host->bus;
> }
> }
> @@ -583,20 +583,31 @@ int pci_parse_devaddr(const char *addr, int *domp, int
> *busp,
>
> PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
> {
> + PCIBus *root = pci_get_primary_bus();
> int dom, bus;
> unsigned slot;
>
> + if (!root) {
> + fprintf(stderr, "No primary PCI bus\n");
> + return NULL;
> + }
> +
> if (!devaddr) {
> *devfnp = -1;
> - return pci_find_bus_nr(pci_find_root_bus(0), 0);
> + return pci_find_bus_nr(root, 0);
> }
>
> if (pci_parse_devaddr(devaddr, &dom, &bus, &slot, NULL) < 0) {
> return NULL;
> }
>
> + if (dom != 0) {
> + fprintf(stderr, "No support for non-zero PCI domains\n");
> + return NULL;
> + }
> +
> *devfnp = PCI_DEVFN(slot, 0);
> - return pci_find_bus_nr(pci_find_root_bus(dom), bus);
> + return pci_find_bus_nr(root, bus);
> }
>
> static void pci_init_cmask(PCIDevice *dev)
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 3ef2ee1..38682e8 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -390,7 +390,7 @@ int pci_bus_num(PCIBus *s);
> void pci_for_each_device(PCIBus *bus, int bus_num,
> void (*fn)(PCIBus *bus, PCIDevice *d, void *opaque),
> void *opaque);
> -PCIBus *pci_find_root_bus(int domain);
> +PCIBus *pci_get_primary_bus(void);
> int pci_find_domain(const PCIBus *bus);
> PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
> int pci_qdev_find_device(const char *id, PCIDevice **pdev);
> --
> 1.7.10.4
