Patchwork [3/8] pci: Abolish pci_find_root_bus()

login
register
mail settings
Submitter David Gibson
Date May 9, 2013, 12:31 a.m.
Message ID <1368059472-25071-4-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/242684/
State New
Headers show

Comments

David Gibson - May 9, 2013, 12:31 a.m.
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 <david@gibson.dropbear.id.au>
---
 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(-)
Michael S. Tsirkin - May 23, 2013, 11:26 a.m.
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 <david@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

Patch

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);