qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2)
@ 2013-06-06  8:48 David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 01/10] pci: Cleanup configuration for pci-hotplug.c David Gibson
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel

The current PCI subsystem has kind of half-hearted support for
multiple independent root buses - aka PCI domains - in the form of the
PCIHostBus structure and its domain field.  However, it doesn't quite
work because pci_host_bus_register() is always called with a domain of
0.

Worse, though, the whole concept of numbered domains isn't general
enough.  Many platforms can have independent root buses (usually on
wholly independent host bridges), but only x86 gives them a
hardware-significant domain number, essentially as a hack to allow all
the separate config spaces to be accessed via the same IO ports.
Linux guests on other platforms will show domain numbers in lspci, but
these are purely guest assigned, so qemu won't know about them.

This patch series, therefore, removes the broken-as-is domain concept
from qemu, and replaces it with a different way of handling multiple
root buses, based on a host bridge class method to provide a
identifier for the root bus.  This hook is designed in such a way as
to allow a single bridge object to support mutiple root buses with
future changes, which will allow future implementations of x86 north
bridges with multiple domains to be supported correctly, and in way
that matches the existing practice for all external interfaces.

v2:
  * Rework concept of "primary" bus in response to Michael Tsirkin's
    comments.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 01/10] pci: Cleanup configuration for pci-hotplug.c
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  9:57   ` Michael S. Tsirkin
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 02/10] pci: Move pci_read_devaddr to pci-hotplug-old.c David Gibson
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

pci-hotplug.c and the CONFIG_PCI_HOTPLUG variable which controls its
compilation are misnamed.  They're not about PCI hotplug in general, but
rather about the pci_add/pci_del interface which are now deprecated in
favour of the more general device_add/device_del interface.  This patch
therefore renames them to pci-hotplug-old.c and CONFIG_PCI_HOTPLUG_OLD.

CONFIG_PCI_HOTPLUG=y was listed twice in {i386,x86_64}-softmmu.make for no
particular reason, so we clean that up too.  In addition it was included in
ppc64-softmmu.mak for which the old hotplug interface was never used and is
unsuitable, so we remove that too.

Most of pci-hotplug.c was additionaly protected by #ifdef TARGET_I386.  The
small piece which wasn't is only called from the pci_add and pci_del hooks
in hmp-commands.hx, which themselves were protected by #ifdef TARGET_I386.
This patch therefore also removes the #ifdef from pci-hotplug-old.c,
and changes the ifdefs in hmp-commands.hx to use CONFIG_PCI_HOTPLUG_OLD.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 default-configs/i386-softmmu.mak   |   3 +-
 default-configs/ppc64-softmmu.mak  |   2 -
 default-configs/x86_64-softmmu.mak |   3 +-
 hmp-commands.hx                    |   4 +-
 hw/pci/Makefile.objs               |   2 +-
 hw/pci/pci-hotplug-old.c           | 292 +++++++++++++++++++++++++++++++++++++
 hw/pci/pci-hotplug.c               | 292 -------------------------------------
 7 files changed, 297 insertions(+), 301 deletions(-)
 create mode 100644 hw/pci/pci-hotplug-old.c
 delete mode 100644 hw/pci/pci-hotplug.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 03deca2..4a0fc9c 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -28,11 +28,10 @@ CONFIG_APPLESMC=y
 CONFIG_I8259=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_TPM_TIS=$(CONFIG_TPM)
-CONFIG_PCI_HOTPLUG=y
+CONFIG_PCI_HOTPLUG_OLD=y
 CONFIG_MC146818RTC=y
 CONFIG_PAM=y
 CONFIG_PCI_PIIX=y
-CONFIG_PCI_HOTPLUG=y
 CONFIG_WDT_IB700=y
 CONFIG_PC_SYSFW=y
 CONFIG_XEN_I386=$(CONFIG_XEN)
diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index 884ea8a..d7140c4 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -44,7 +44,5 @@ CONFIG_XILINX_ETHLITE=y
 CONFIG_OPENPIC=y
 CONFIG_PSERIES=$(CONFIG_FDT)
 CONFIG_E500=$(CONFIG_FDT)
-# For pSeries
-CONFIG_PCI_HOTPLUG=y
 # For PReP
 CONFIG_MC146818RTC=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 599b630..10bb0c6 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -28,11 +28,10 @@ CONFIG_APPLESMC=y
 CONFIG_I8259=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_TPM_TIS=$(CONFIG_TPM)
-CONFIG_PCI_HOTPLUG=y
+CONFIG_PCI_HOTPLUG_OLD=y
 CONFIG_MC146818RTC=y
 CONFIG_PAM=y
 CONFIG_PCI_PIIX=y
-CONFIG_PCI_HOTPLUG=y
 CONFIG_WDT_IB700=y
 CONFIG_PC_SYSFW=y
 CONFIG_XEN_I386=$(CONFIG_XEN)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9cea415..1d88320 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1077,7 +1077,7 @@ STEXI
 Add drive to PCI storage controller.
 ETEXI
 
-#if defined(TARGET_I386)
+#if defined(CONFIG_PCI_HOTPLUG_OLD)
     {
         .name       = "pci_add",
         .args_type  = "pci_addr:s,type:s,opts:s?",
@@ -1093,7 +1093,7 @@ STEXI
 Hot-add PCI device.
 ETEXI
 
-#if defined(TARGET_I386)
+#if defined(CONFIG_PCI_HOTPLUG_OLD)
     {
         .name       = "pci_del",
         .args_type  = "pci_addr:s",
diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
index a7fb9d0..720f438 100644
--- a/hw/pci/Makefile.objs
+++ b/hw/pci/Makefile.objs
@@ -8,4 +8,4 @@ common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
 common-obj-$(CONFIG_NO_PCI) += pci-stub.o
 common-obj-$(CONFIG_ALL) += pci-stub.o
 
-obj-$(CONFIG_PCI_HOTPLUG) += pci-hotplug.o
+common-obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
new file mode 100644
index 0000000..b3c233c
--- /dev/null
+++ b/hw/pci/pci-hotplug-old.c
@@ -0,0 +1,292 @@
+/*
+ * Deprecated PCI hotplug interface support
+ * This covers the old pci_add / pci_del command, whereas the more general
+ * device_add / device_del commands are now preferred.
+ *
+ * Copyright (c) 2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/hw.h"
+#include "hw/boards.h"
+#include "hw/pci/pci.h"
+#include "net/net.h"
+#include "hw/i386/pc.h"
+#include "monitor/monitor.h"
+#include "hw/scsi/scsi.h"
+#include "hw/virtio/virtio-blk.h"
+#include "qemu/config-file.h"
+#include "sysemu/blockdev.h"
+#include "qapi/error.h"
+
+static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
+                                       const char *devaddr,
+                                       const char *opts_str)
+{
+    Error *local_err = NULL;
+    QemuOpts *opts;
+    PCIBus *bus;
+    int ret, devfn;
+
+    bus = pci_get_bus_devfn(&devfn, devaddr);
+    if (!bus) {
+        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
+        return NULL;
+    }
+    if (!((BusState*)bus)->allow_hotplug) {
+        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
+        return NULL;
+    }
+
+    opts = qemu_opts_parse(qemu_find_opts("net"), opts_str ? opts_str : "", 0);
+    if (!opts) {
+        return NULL;
+    }
+
+    qemu_opt_set(opts, "type", "nic");
+
+    ret = net_client_init(opts, 0, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return NULL;
+    }
+    if (nd_table[ret].devaddr) {
+        monitor_printf(mon, "Parameter addr not supported\n");
+        return NULL;
+    }
+    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
+}
+
+static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
+                        DriveInfo *dinfo, int printinfo)
+{
+    SCSIBus *scsibus;
+    SCSIDevice *scsidev;
+
+    scsibus = (SCSIBus *)
+        object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
+                            TYPE_SCSI_BUS);
+    if (!scsibus) {
+	error_report("Device is not a SCSI adapter");
+	return -1;
+    }
+
+    /*
+     * drive_init() tries to find a default for dinfo->unit.  Doesn't
+     * work at all for hotplug though as we assign the device to a
+     * specific bus instead of the first bus with spare scsi ids.
+     *
+     * Ditch the calculated value and reload from option string (if
+     * specified).
+     */
+    dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
+    dinfo->bus = scsibus->busnr;
+    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
+                                        false, -1, NULL);
+    if (!scsidev) {
+        return -1;
+    }
+    dinfo->unit = scsidev->id;
+
+    if (printinfo)
+        monitor_printf(mon, "OK bus %d, unit %d\n",
+                       scsibus->busnr, scsidev->id);
+    return 0;
+}
+
+int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
+{
+    int dom, pci_bus;
+    unsigned slot;
+    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)) {
+            goto err;
+        }
+        dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
+                              PCI_DEVFN(slot, 0));
+        if (!dev) {
+            monitor_printf(mon, "no pci device with address %s\n", pci_addr);
+            goto err;
+        }
+        if (scsi_hot_add(mon, &dev->qdev, dinfo, 1) != 0) {
+            goto err;
+        }
+        break;
+    default:
+        monitor_printf(mon, "Can't hot-add drive to type %d\n", dinfo->type);
+        goto err;
+    }
+
+    return 0;
+err:
+    return -1;
+}
+
+static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
+                                           const char *devaddr,
+                                           const char *opts)
+{
+    PCIDevice *dev;
+    DriveInfo *dinfo = NULL;
+    int type = -1;
+    char buf[128];
+    PCIBus *bus;
+    int devfn;
+
+    if (get_param_value(buf, sizeof(buf), "if", opts)) {
+        if (!strcmp(buf, "scsi"))
+            type = IF_SCSI;
+        else if (!strcmp(buf, "virtio")) {
+            type = IF_VIRTIO;
+        } else {
+            monitor_printf(mon, "type %s not a hotpluggable PCI device.\n", buf);
+            return NULL;
+        }
+    } else {
+        monitor_printf(mon, "no if= specified\n");
+        return NULL;
+    }
+
+    if (get_param_value(buf, sizeof(buf), "file", opts)) {
+        dinfo = add_init_drive(opts);
+        if (!dinfo)
+            return NULL;
+        if (dinfo->devaddr) {
+            monitor_printf(mon, "Parameter addr not supported\n");
+            return NULL;
+        }
+    } else {
+        dinfo = NULL;
+    }
+
+    bus = pci_get_bus_devfn(&devfn, devaddr);
+    if (!bus) {
+        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
+        return NULL;
+    }
+    if (!((BusState*)bus)->allow_hotplug) {
+        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
+        return NULL;
+    }
+
+    switch (type) {
+    case IF_SCSI:
+        dev = pci_create(bus, devfn, "lsi53c895a");
+        if (qdev_init(&dev->qdev) < 0)
+            dev = NULL;
+        if (dev && dinfo) {
+            if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
+                qdev_unplug(&dev->qdev, NULL);
+                dev = NULL;
+            }
+        }
+        break;
+    case IF_VIRTIO:
+        if (!dinfo) {
+            monitor_printf(mon, "virtio requires a backing file/device.\n");
+            return NULL;
+        }
+        dev = pci_create(bus, devfn, "virtio-blk-pci");
+        if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
+            qdev_free(&dev->qdev);
+            dev = NULL;
+            break;
+        }
+        if (qdev_init(&dev->qdev) < 0)
+            dev = NULL;
+        break;
+    default:
+        dev = NULL;
+    }
+    return dev;
+}
+
+void pci_device_hot_add(Monitor *mon, const QDict *qdict)
+{
+    PCIDevice *dev = NULL;
+    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
+    const char *type = qdict_get_str(qdict, "type");
+    const char *opts = qdict_get_try_str(qdict, "opts");
+
+    /* strip legacy tag */
+    if (!strncmp(pci_addr, "pci_addr=", 9)) {
+        pci_addr += 9;
+    }
+
+    if (!opts) {
+        opts = "";
+    }
+
+    if (!strcmp(pci_addr, "auto"))
+        pci_addr = NULL;
+
+    if (strcmp(type, "nic") == 0) {
+        dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
+    } else if (strcmp(type, "storage") == 0) {
+        dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
+    } else {
+        monitor_printf(mon, "invalid type: %s\n", type);
+    }
+
+    if (dev) {
+        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
+                       pci_find_domain(dev->bus),
+                       pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
+                       PCI_FUNC(dev->devfn));
+    } else
+        monitor_printf(mon, "failed to add %s\n", opts);
+}
+
+static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
+{
+    PCIDevice *d;
+    int dom, bus;
+    unsigned slot;
+    Error *local_err = NULL;
+
+    if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
+        return -1;
+    }
+
+    d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
+    if (!d) {
+        monitor_printf(mon, "slot %d empty\n", slot);
+        return -1;
+    }
+
+    qdev_unplug(&d->qdev, &local_err);
+    if (error_is_set(&local_err)) {
+        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
+        error_free(local_err);
+        return -1;
+    }
+
+    return 0;
+}
+
+void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
+{
+    pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
+}
diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c
deleted file mode 100644
index 12287d1..0000000
--- a/hw/pci/pci-hotplug.c
+++ /dev/null
@@ -1,292 +0,0 @@
-/*
- * QEMU PCI hotplug support
- *
- * Copyright (c) 2004 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include "hw/hw.h"
-#include "hw/boards.h"
-#include "hw/pci/pci.h"
-#include "net/net.h"
-#include "hw/i386/pc.h"
-#include "monitor/monitor.h"
-#include "hw/scsi/scsi.h"
-#include "hw/virtio/virtio-blk.h"
-#include "qemu/config-file.h"
-#include "sysemu/blockdev.h"
-#include "qapi/error.h"
-
-#if defined(TARGET_I386)
-static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
-                                       const char *devaddr,
-                                       const char *opts_str)
-{
-    Error *local_err = NULL;
-    QemuOpts *opts;
-    PCIBus *bus;
-    int ret, devfn;
-
-    bus = pci_get_bus_devfn(&devfn, devaddr);
-    if (!bus) {
-        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
-        return NULL;
-    }
-    if (!((BusState*)bus)->allow_hotplug) {
-        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
-        return NULL;
-    }
-
-    opts = qemu_opts_parse(qemu_find_opts("net"), opts_str ? opts_str : "", 0);
-    if (!opts) {
-        return NULL;
-    }
-
-    qemu_opt_set(opts, "type", "nic");
-
-    ret = net_client_init(opts, 0, &local_err);
-    if (error_is_set(&local_err)) {
-        qerror_report_err(local_err);
-        error_free(local_err);
-        return NULL;
-    }
-    if (nd_table[ret].devaddr) {
-        monitor_printf(mon, "Parameter addr not supported\n");
-        return NULL;
-    }
-    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
-}
-
-static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
-                        DriveInfo *dinfo, int printinfo)
-{
-    SCSIBus *scsibus;
-    SCSIDevice *scsidev;
-
-    scsibus = (SCSIBus *)
-        object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
-                            TYPE_SCSI_BUS);
-    if (!scsibus) {
-	error_report("Device is not a SCSI adapter");
-	return -1;
-    }
-
-    /*
-     * drive_init() tries to find a default for dinfo->unit.  Doesn't
-     * work at all for hotplug though as we assign the device to a
-     * specific bus instead of the first bus with spare scsi ids.
-     *
-     * Ditch the calculated value and reload from option string (if
-     * specified).
-     */
-    dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
-    dinfo->bus = scsibus->busnr;
-    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
-                                        false, -1, NULL);
-    if (!scsidev) {
-        return -1;
-    }
-    dinfo->unit = scsidev->id;
-
-    if (printinfo)
-        monitor_printf(mon, "OK bus %d, unit %d\n",
-                       scsibus->busnr, scsidev->id);
-    return 0;
-}
-
-int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
-{
-    int dom, pci_bus;
-    unsigned slot;
-    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)) {
-            goto err;
-        }
-        dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
-                              PCI_DEVFN(slot, 0));
-        if (!dev) {
-            monitor_printf(mon, "no pci device with address %s\n", pci_addr);
-            goto err;
-        }
-        if (scsi_hot_add(mon, &dev->qdev, dinfo, 1) != 0) {
-            goto err;
-        }
-        break;
-    default:
-        monitor_printf(mon, "Can't hot-add drive to type %d\n", dinfo->type);
-        goto err;
-    }
-
-    return 0;
-err:
-    return -1;
-}
-
-static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
-                                           const char *devaddr,
-                                           const char *opts)
-{
-    PCIDevice *dev;
-    DriveInfo *dinfo = NULL;
-    int type = -1;
-    char buf[128];
-    PCIBus *bus;
-    int devfn;
-
-    if (get_param_value(buf, sizeof(buf), "if", opts)) {
-        if (!strcmp(buf, "scsi"))
-            type = IF_SCSI;
-        else if (!strcmp(buf, "virtio")) {
-            type = IF_VIRTIO;
-        } else {
-            monitor_printf(mon, "type %s not a hotpluggable PCI device.\n", buf);
-            return NULL;
-        }
-    } else {
-        monitor_printf(mon, "no if= specified\n");
-        return NULL;
-    }
-
-    if (get_param_value(buf, sizeof(buf), "file", opts)) {
-        dinfo = add_init_drive(opts);
-        if (!dinfo)
-            return NULL;
-        if (dinfo->devaddr) {
-            monitor_printf(mon, "Parameter addr not supported\n");
-            return NULL;
-        }
-    } else {
-        dinfo = NULL;
-    }
-
-    bus = pci_get_bus_devfn(&devfn, devaddr);
-    if (!bus) {
-        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
-        return NULL;
-    }
-    if (!((BusState*)bus)->allow_hotplug) {
-        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
-        return NULL;
-    }
-
-    switch (type) {
-    case IF_SCSI:
-        dev = pci_create(bus, devfn, "lsi53c895a");
-        if (qdev_init(&dev->qdev) < 0)
-            dev = NULL;
-        if (dev && dinfo) {
-            if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
-                qdev_unplug(&dev->qdev, NULL);
-                dev = NULL;
-            }
-        }
-        break;
-    case IF_VIRTIO:
-        if (!dinfo) {
-            monitor_printf(mon, "virtio requires a backing file/device.\n");
-            return NULL;
-        }
-        dev = pci_create(bus, devfn, "virtio-blk-pci");
-        if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
-            qdev_free(&dev->qdev);
-            dev = NULL;
-            break;
-        }
-        if (qdev_init(&dev->qdev) < 0)
-            dev = NULL;
-        break;
-    default:
-        dev = NULL;
-    }
-    return dev;
-}
-
-void pci_device_hot_add(Monitor *mon, const QDict *qdict)
-{
-    PCIDevice *dev = NULL;
-    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
-    const char *type = qdict_get_str(qdict, "type");
-    const char *opts = qdict_get_try_str(qdict, "opts");
-
-    /* strip legacy tag */
-    if (!strncmp(pci_addr, "pci_addr=", 9)) {
-        pci_addr += 9;
-    }
-
-    if (!opts) {
-        opts = "";
-    }
-
-    if (!strcmp(pci_addr, "auto"))
-        pci_addr = NULL;
-
-    if (strcmp(type, "nic") == 0) {
-        dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
-    } else if (strcmp(type, "storage") == 0) {
-        dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
-    } else {
-        monitor_printf(mon, "invalid type: %s\n", type);
-    }
-
-    if (dev) {
-        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
-                       pci_find_domain(dev->bus),
-                       pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
-                       PCI_FUNC(dev->devfn));
-    } else
-        monitor_printf(mon, "failed to add %s\n", opts);
-}
-#endif
-
-static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
-{
-    PCIDevice *d;
-    int dom, bus;
-    unsigned slot;
-    Error *local_err = NULL;
-
-    if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
-        return -1;
-    }
-
-    d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
-    if (!d) {
-        monitor_printf(mon, "slot %d empty\n", slot);
-        return -1;
-    }
-
-    qdev_unplug(&d->qdev, &local_err);
-    if (error_is_set(&local_err)) {
-        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
-        error_free(local_err);
-        return -1;
-    }
-
-    return 0;
-}
-
-void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
-{
-    pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
-}
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 02/10] pci: Move pci_read_devaddr to pci-hotplug-old.c
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 01/10] pci: Cleanup configuration for pci-hotplug.c David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 03/10] pci: Abolish pci_find_root_bus() David Gibson
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

pci_read_devaddr() is only used by the legacy functions for the old PCI
hotplug interface in pci-hotplug-old.c.  So we move the function there,
and make it static.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci/pci-hotplug-old.c | 14 ++++++++++++++
 hw/pci/pci.c             | 16 +---------------
 include/hw/pci/pci.h     |  4 ++--
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index b3c233c..a0b5558 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -36,6 +36,20 @@
 #include "sysemu/blockdev.h"
 #include "qapi/error.h"
 
+static int pci_read_devaddr(Monitor *mon, const char *addr, int *domp,
+                            int *busp, unsigned *slotp)
+{
+    /* strip legacy tag */
+    if (!strncmp(addr, "pci_addr=", 9)) {
+        addr += 9;
+    }
+    if (pci_parse_devaddr(addr, domp, busp, slotp, NULL)) {
+        monitor_printf(mon, "Invalid pci address\n");
+        return -1;
+    }
+    return 0;
+}
+
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
                                        const char *devaddr,
                                        const char *opts_str)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index bb3879b..f0005c6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -522,7 +522,7 @@ static void pci_set_default_subsystem_id(PCIDevice *pci_dev)
  * Parse [[<domain>:]<bus>:]<slot>, return -1 on error if funcp == NULL
  *       [[<domain>:]<bus>:]<slot>.<func>, return -1 on error
  */
-static int pci_parse_devaddr(const char *addr, int *domp, int *busp,
+int pci_parse_devaddr(const char *addr, int *domp, int *busp,
                       unsigned int *slotp, unsigned int *funcp)
 {
     const char *p;
@@ -581,20 +581,6 @@ static int pci_parse_devaddr(const char *addr, int *domp, int *busp,
     return 0;
 }
 
-int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,
-                     unsigned *slotp)
-{
-    /* strip legacy tag */
-    if (!strncmp(addr, "pci_addr=", 9)) {
-        addr += 9;
-    }
-    if (pci_parse_devaddr(addr, domp, busp, slotp, NULL)) {
-        monitor_printf(mon, "Invalid pci address\n");
-        return -1;
-    }
-    return 0;
-}
-
 PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
 {
     int dom, bus;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 8d075ab..3ef2ee1 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -396,8 +396,8 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
 int pci_qdev_find_device(const char *id, PCIDevice **pdev);
 PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr);
 
-int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,
-                     unsigned *slotp);
+int pci_parse_devaddr(const char *addr, int *domp, int *busp,
+                      unsigned int *slotp, unsigned int *funcp);
 
 void pci_device_deassert_intx(PCIDevice *dev);
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 03/10] pci: Abolish pci_find_root_bus()
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 01/10] pci: Cleanup configuration for pci-hotplug.c David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 02/10] pci: Move pci_read_devaddr to pci-hotplug-old.c David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 04/10] pci: Use helper to find device's root bus in pci_find_domain() David Gibson
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

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_find_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(-)

diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index a0b5558..7a47d6b 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -36,17 +36,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;
 }
 
@@ -128,18 +134,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_find_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;
@@ -275,16 +285,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_find_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 f0005c6..80b0d7e 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_find_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_find_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..c26d704 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_find_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.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 04/10] pci: Use helper to find device's root bus in pci_find_domain()
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (2 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 03/10] pci: Abolish pci_find_root_bus() David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 05/10] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

Currently pci_find_domain() performs two functions - it locates the PCI
root bus above the given bus, then looks up that root bus's domain number.
This patch adds a helper function to perform the first task, finding the
root bus for a given PCI device.  This is then used in pci_find_domain().
This changes pci_find_domain()'s signature slightly, taking a PCIDevice
instead of a PCIBus - since all callers passed something of the form
dev->bus, this simplifies things slightly.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci/pci-hotplug-old.c |  2 +-
 hw/pci/pci.c             | 20 +++++++++++++-------
 hw/pci/pcie_aer.c        |  3 +--
 include/hw/pci/pci.h     |  3 ++-
 4 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index 7a47d6b..37e0720 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -276,7 +276,7 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
 
     if (dev) {
         monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
-                       pci_find_domain(dev->bus),
+                       pci_find_domain(dev),
                        pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
                        PCI_FUNC(dev->devfn));
     } else
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 80b0d7e..8cb7e5d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -259,18 +259,24 @@ PCIBus *pci_find_primary_bus(void)
     return NULL;
 }
 
-int pci_find_domain(const PCIBus *bus)
+PCIBus *pci_device_root_bus(const PCIDevice *d)
 {
-    PCIDevice *d;
-    struct PCIHostBus *host;
+    PCIBus *bus = d->bus;
 
-    /* obtain root bus */
     while ((d = bus->parent_dev) != NULL) {
         bus = d->bus;
     }
 
+    return bus;
+}
+
+int pci_find_domain(const PCIDevice *dev)
+{
+    const PCIBus *rootbus = pci_device_root_bus(dev);
+    struct PCIHostBus *host;
+
     QLIST_FOREACH(host, &host_buses, next) {
-        if (host->bus == bus) {
+        if (host->bus == rootbus) {
             return host->domain;
         }
     }
@@ -2000,7 +2006,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
                 fprintf(stderr, "ERROR: %04x:%02x:%02x.%x "
                         "Attempt to add PCI capability %x at offset "
                         "%x overlaps existing capability %x at offset %x\n",
-                        pci_find_domain(pdev->bus), pci_bus_num(pdev->bus),
+                        pci_find_domain(pdev), pci_bus_num(pdev->bus),
                         PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
                         cap_id, offset, overlapping_cap, i);
                 return -EINVAL;
@@ -2155,7 +2161,7 @@ static char *pcibus_get_dev_path(DeviceState *dev)
     path[path_len] = '\0';
 
     /* First field is the domain. */
-    s = snprintf(domain, sizeof domain, "%04x:00", pci_find_domain(d->bus));
+    s = snprintf(domain, sizeof domain, "%04x:00", pci_find_domain(d));
     assert(s == domain_len);
     memcpy(path, domain, domain_len);
 
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 1ce72ce..06f77ac 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -1022,8 +1022,7 @@ int do_pcie_aer_inject_error(Monitor *mon,
     *ret_data = qobject_from_jsonf("{'id': %s, "
                                    "'domain': %d, 'bus': %d, 'devfn': %d, "
                                    "'ret': %d}",
-                                   id,
-                                   pci_find_domain(dev->bus),
+                                   id, pci_find_domain(dev),
                                    pci_bus_num(dev->bus), dev->devfn,
                                    ret);
     assert(*ret_data);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index c26d704..7376b9e 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -391,7 +391,8 @@ void pci_for_each_device(PCIBus *bus, int bus_num,
                          void (*fn)(PCIBus *bus, PCIDevice *d, void *opaque),
                          void *opaque);
 PCIBus *pci_find_primary_bus(void);
-int pci_find_domain(const PCIBus *bus);
+PCIBus *pci_device_root_bus(const PCIDevice *d);
+int pci_find_domain(const PCIDevice *dev);
 PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
 int pci_qdev_find_device(const char *id, PCIDevice **pdev);
 PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 05/10] pci: Replace pci_find_domain() with more general pci_root_bus_path()
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (3 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 04/10] pci: Use helper to find device's root bus in pci_find_domain() David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 06/10] pci: Add root bus argument to pci_get_bus_devfn() David Gibson
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

pci_find_domain() is used in a number of places where we want an id for a
whole PCI domain (i.e. the subtree under a PCI root bus).  The trouble is
that many platforms may support multiple independent host bridges with no
hardware supplied notion of domain number.

This patch, therefore, replaces calls to pci_find_domain() with calls to
a new pci_root_bus_path() returning a string.  The new call is implemented
in terms of a new callback in the host bridge class, so it can be defined
in some way that's well defined for the platform.  When no callback is
available we fall back on the qbus name.

Most current uses of pci_find_domain() are for error or informational
messages, so the change in identifiers should be harmless.  The exception
is pci_get_dev_path(), whose results form part of migration streams.  To
maintain compatibility with old migration streams, the PIIX PCI host is
altered to always supply "0000" for this path, which matches the old domain
number (since the code didn't actually support domains other than 0).

For the pseries (spapr) PCI bridge we use a different platform-unique
identifier (pseries machines can routinely have dozens of PCI host
bridges).  Theoretically that breaks migration streams, but given that we
don't yet have migration support for pseries, it doesn't matter.

Any other machines that have working migration support including PCI
devices will need to be updated to maintain migration stream compatibility.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci-host/piix.c        |  9 +++++++++
 hw/pci-host/q35.c         |  9 +++++++++
 hw/pci/pci-hotplug-old.c  |  4 ++--
 hw/pci/pci.c              | 38 ++++++++++++++++++++------------------
 hw/pci/pci_host.c         |  1 +
 hw/pci/pcie_aer.c         |  8 ++++----
 hw/ppc/spapr_pci.c        | 10 ++++++++++
 include/hw/pci/pci.h      |  2 +-
 include/hw/pci/pci_host.h | 10 ++++++++++
 9 files changed, 66 insertions(+), 25 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9e68c3..c36e725 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -629,11 +629,20 @@ static const TypeInfo i440fx_info = {
     .class_init    = i440fx_class_init,
 };
 
+static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
+                                                PCIBus *rootbus)
+{
+    /* For backwards compat with old device paths */
+    return "0000";
+}
+
 static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
 
+    hc->root_bus_path = i440fx_pcihost_root_bus_path;
     k->init = i440fx_pcihost_initfn;
     dc->fw_name = "pci";
     dc->no_user = 1;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 24df6b5..d515e27 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -63,6 +63,13 @@ static int q35_host_init(SysBusDevice *dev)
     return 0;
 }
 
+static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
+                                          PCIBus *rootbus)
+{
+    /* For backwards compat with old device paths */
+    return "0000";
+}
+
 static Property mch_props[] = {
     DEFINE_PROP_UINT64("MCFG", Q35PCIHost, host.base_addr,
                         MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT),
@@ -73,7 +80,9 @@ static void q35_host_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
 
+    hc->root_bus_path = q35_host_root_bus_path;
     k->init = q35_host_init;
     dc->props = mch_props;
     dc->fw_name = "pci";
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index 37e0720..e251810 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -275,8 +275,8 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
     }
 
     if (dev) {
-        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
-                       pci_find_domain(dev),
+        monitor_printf(mon, "OK root bus %s, bus %d, slot %d, function %d\n",
+                       pci_root_bus_path(dev),
                        pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
                        PCI_FUNC(dev->devfn));
     } else
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8cb7e5d..354cf08 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -25,6 +25,7 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_bus.h"
+#include "hw/pci/pci_host.h"
 #include "monitor/monitor.h"
 #include "net/net.h"
 #include "sysemu/sysemu.h"
@@ -270,19 +271,20 @@ PCIBus *pci_device_root_bus(const PCIDevice *d)
     return bus;
 }
 
-int pci_find_domain(const PCIDevice *dev)
+const char *pci_root_bus_path(PCIDevice *dev)
 {
-    const PCIBus *rootbus = pci_device_root_bus(dev);
-    struct PCIHostBus *host;
+    PCIBus *rootbus = pci_device_root_bus(dev);
+    PCIHostState *host_bridge = PCI_HOST_BRIDGE(rootbus->qbus.parent);
+    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_GET_CLASS(host_bridge);
 
-    QLIST_FOREACH(host, &host_buses, next) {
-        if (host->bus == rootbus) {
-            return host->domain;
-        }
+    assert(!rootbus->parent_dev);
+    assert(host_bridge->bus == rootbus);
+
+    if (hc->root_bus_path) {
+        return (*hc->root_bus_path)(host_bridge, rootbus);
     }
 
-    abort();    /* should not be reached */
-    return -1;
+    return rootbus->qbus.name;
 }
 
 static void pci_bus_init(PCIBus *bus, DeviceState *parent,
@@ -2003,10 +2005,10 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
         for (i = offset; i < offset + size; i++) {
             overlapping_cap = pci_find_capability_at_offset(pdev, i);
             if (overlapping_cap) {
-                fprintf(stderr, "ERROR: %04x:%02x:%02x.%x "
+                fprintf(stderr, "ERROR: %s:%02x:%02x.%x "
                         "Attempt to add PCI capability %x at offset "
                         "%x overlaps existing capability %x at offset %x\n",
-                        pci_find_domain(pdev), pci_bus_num(pdev->bus),
+                        pci_root_bus_path(pdev), pci_bus_num(pdev->bus),
                         PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
                         cap_id, offset, overlapping_cap, i);
                 return -EINVAL;
@@ -2140,30 +2142,30 @@ static char *pcibus_get_dev_path(DeviceState *dev)
      * domain:Bus:Slot.Func for systems without nested PCI bridges.
      * Slot.Function list specifies the slot and function numbers for all
      * devices on the path from root to the specific device. */
-    char domain[] = "DDDD:00";
+    const char *root_bus_path;
+    int root_bus_len;
     char slot[] = ":SS.F";
-    int domain_len = sizeof domain - 1 /* For '\0' */;
     int slot_len = sizeof slot - 1 /* For '\0' */;
     int path_len;
     char *path, *p;
     int s;
 
+    root_bus_path = pci_root_bus_path(d);
+    root_bus_len = strlen(root_bus_path);
+
     /* Calculate # of slots on path between device and root. */;
     slot_depth = 0;
     for (t = d; t; t = t->bus->parent_dev) {
         ++slot_depth;
     }
 
-    path_len = domain_len + slot_len * slot_depth;
+    path_len = root_bus_len + slot_len * slot_depth;
 
     /* Allocate memory, fill in the terminating null byte. */
     path = g_malloc(path_len + 1 /* For '\0' */);
     path[path_len] = '\0';
 
-    /* First field is the domain. */
-    s = snprintf(domain, sizeof domain, "%04x:00", pci_find_domain(d));
-    assert(s == domain_len);
-    memcpy(path, domain, domain_len);
+    memcpy(path, root_bus_path, root_bus_len);
 
     /* Fill in slot numbers. We walk up from device to root, so need to print
      * them in the reverse order, last to first. */
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 12254b1..7dd9b25 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -169,6 +169,7 @@ static const TypeInfo pci_host_type_info = {
     .name = TYPE_PCI_HOST_BRIDGE,
     .parent = TYPE_SYS_BUS_DEVICE,
     .abstract = true,
+    .class_size = sizeof(PCIHostBridgeClass),
     .instance_size = sizeof(PCIHostState),
 };
 
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 06f77ac..ca762ab 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -817,9 +817,9 @@ void pcie_aer_inject_error_print(Monitor *mon, const QObject *data)
     qdict = qobject_to_qdict(data);
 
     devfn = (int)qdict_get_int(qdict, "devfn");
-    monitor_printf(mon, "OK id: %s domain: %x, bus: %x devfn: %x.%x\n",
+    monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
                    qdict_get_str(qdict, "id"),
-                   (int) qdict_get_int(qdict, "domain"),
+                   qdict_get_str(qdict, "root_bus"),
                    (int) qdict_get_int(qdict, "bus"),
                    PCI_SLOT(devfn), PCI_FUNC(devfn));
 }
@@ -1020,9 +1020,9 @@ int do_pcie_aer_inject_error(Monitor *mon,
 
     ret = pcie_aer_inject_error(dev, &err);
     *ret_data = qobject_from_jsonf("{'id': %s, "
-                                   "'domain': %d, 'bus': %d, 'devfn': %d, "
+                                   "'root_bus': %s, 'bus': %d, 'devfn': %d, "
                                    "'ret': %d}",
-                                   id, pci_find_domain(dev),
+                                   id, pci_root_bus_path(dev),
                                    pci_bus_num(dev->bus), dev->devfn,
                                    ret);
     assert(*ret_data);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 62ff323..2d102f5 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -693,11 +693,21 @@ static Property spapr_phb_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static const char *spapr_phb_root_bus_path(PCIHostState *host_bridge,
+                                           PCIBus *rootbus)
+{
+    sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(host_bridge);
+
+    return sphb->dtbusname;
+}
+
 static void spapr_phb_class_init(ObjectClass *klass, void *data)
 {
+    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
     SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    hc->root_bus_path = spapr_phb_root_bus_path;
     sdc->init = spapr_phb_init;
     dc->props = spapr_phb_properties;
     dc->reset = spapr_phb_reset;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 7376b9e..962b7a3 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -392,7 +392,7 @@ void pci_for_each_device(PCIBus *bus, int bus_num,
                          void *opaque);
 PCIBus *pci_find_primary_bus(void);
 PCIBus *pci_device_root_bus(const PCIDevice *d);
-int pci_find_domain(const PCIDevice *dev);
+const char *pci_root_bus_path(PCIDevice *dev);
 PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
 int pci_qdev_find_device(const char *id, PCIDevice **pdev);
 PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr);
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index 236cd0f..44052f2 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -33,6 +33,10 @@
 #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge"
 #define PCI_HOST_BRIDGE(obj) \
     OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST_BRIDGE)
+#define PCI_HOST_BRIDGE_CLASS(klass) \
+     OBJECT_CLASS_CHECK(PCIHostBridgeClass, (klass), TYPE_PCI_HOST_BRIDGE)
+#define PCI_HOST_BRIDGE_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(PCIHostBridgeClass, (obj), TYPE_PCI_HOST_BRIDGE)
 
 struct PCIHostState {
     SysBusDevice busdev;
@@ -44,6 +48,12 @@ struct PCIHostState {
     PCIBus *bus;
 };
 
+typedef struct PCIHostBridgeClass {
+    SysBusDeviceClass parent_class;
+
+    const char *(*root_bus_path)(PCIHostState *, PCIBus *);
+} PCIHostBridgeClass;
+
 /* common internal helpers for PCI/PCIe hosts, cut off overflows */
 void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
                                   uint32_t limit, uint32_t val, uint32_t len);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 06/10] pci: Add root bus argument to pci_get_bus_devfn()
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (4 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 05/10] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 07/10] pci: Add root bus parameter to pci_nic_init() David Gibson
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

pci_get_bus_devfn() interprets a full PCI address string to give a PCIBus *
and device/function number within that bus.  Currently it assumes it is
working on an address under the primary PCI root bus.  This patch extends
it to allow the caller to specify a root bus.  This might seem a little odd
since the supplied address can (theoretically) include a PCI domain number.
However, attempting to use a non-zero domain number there is currently an
error, so that shouldn't really cause problems.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci/pci-hotplug-old.c | 4 ++--
 hw/pci/pci.c             | 7 ++++---
 include/hw/pci/pci.h     | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index e251810..e92d646 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -65,7 +65,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
     PCIBus *bus;
     int ret, devfn;
 
-    bus = pci_get_bus_devfn(&devfn, devaddr);
+    bus = pci_get_bus_devfn(&devfn, pci_find_primary_bus(), devaddr);
     if (!bus) {
         monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
         return NULL;
@@ -205,7 +205,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         dinfo = NULL;
     }
 
-    bus = pci_get_bus_devfn(&devfn, devaddr);
+    bus = pci_get_bus_devfn(&devfn, pci_find_primary_bus(), devaddr);
     if (!bus) {
         monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
         return NULL;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 354cf08..fb92061 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -589,12 +589,13 @@ int pci_parse_devaddr(const char *addr, int *domp, int *busp,
     return 0;
 }
 
-PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
+PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr)
 {
-    PCIBus *root = pci_find_primary_bus();
     int dom, bus;
     unsigned slot;
 
+    assert(!root->parent_dev);
+
     if (!root) {
         fprintf(stderr, "No primary PCI bus\n");
         return NULL;
@@ -1591,7 +1592,7 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
     if (i < 0)
         return NULL;
 
-    bus = pci_get_bus_devfn(&devfn, devaddr);
+    bus = pci_get_bus_devfn(&devfn, pci_find_primary_bus(), devaddr);
     if (!bus) {
         error_report("Invalid PCI device address %s for device %s",
                      devaddr, pci_nic_names[i]);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 962b7a3..3b22898 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -395,7 +395,7 @@ PCIBus *pci_device_root_bus(const PCIDevice *d);
 const char *pci_root_bus_path(PCIDevice *dev);
 PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
 int pci_qdev_find_device(const char *id, PCIDevice **pdev);
-PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr);
+PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr);
 
 int pci_parse_devaddr(const char *addr, int *domp, int *busp,
                       unsigned int *slotp, unsigned int *funcp);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 07/10] pci: Add root bus parameter to pci_nic_init()
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (5 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 06/10] pci: Add root bus argument to pci_get_bus_devfn() David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 08/10] pci: Simpler implementation of primary PCI bus David Gibson
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

At present, pci_nic_init() and pci_nic_init_nofail() assume that they will
only create a NIC under the primary PCI root.  As we add support for
multiple PCI roots, that may no longer be the case.  This patch adds a root
bus parameter to pci_nic_init() (and updates callers accordingly) to allow
the machine init code using it to specify the right PCI root for NICs
created by old-style -net nic parameters.  NICs created new-style, with
-device can of course be put anywhere.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/alpha/dp264.c         |  2 +-
 hw/arm/realview.c        |  6 ++++--
 hw/arm/versatilepb.c     |  2 +-
 hw/i386/pc.c             |  2 +-
 hw/mips/mips_fulong2e.c  |  6 +++---
 hw/mips/mips_malta.c     |  6 +++---
 hw/pci/pci-hotplug-old.c |  3 ++-
 hw/pci/pci.c             | 10 ++++++----
 hw/ppc/e500.c            |  2 +-
 hw/ppc/mac_newworld.c    |  2 +-
 hw/ppc/mac_oldworld.c    |  2 +-
 hw/ppc/ppc440_bamboo.c   |  2 +-
 hw/ppc/prep.c            |  2 +-
 hw/ppc/spapr.c           |  2 +-
 hw/sh4/r2d.c             |  5 ++++-
 hw/sparc64/sun4u.c       |  2 +-
 include/hw/pci/pci.h     |  6 ++++--
 17 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 8695efb..8dad08f 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -89,7 +89,7 @@ static void clipper_init(QEMUMachineInitArgs *args)
 
     /* Network setup.  e1000 is good enough, failing Tulip support.  */
     for (i = 0; i < nb_nics; i++) {
-        pci_nic_init_nofail(&nd_table[i], "e1000", NULL);
+        pci_nic_init_nofail(&nd_table[i], pci_bus, "e1000", NULL);
     }
 
     /* IDE disk setup.  */
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index d6f47bf..036a188 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -59,7 +59,7 @@ static void realview_init(QEMUMachineInitArgs *args,
     qemu_irq *irqp;
     qemu_irq pic[64];
     qemu_irq mmc_irq[2];
-    PCIBus *pci_bus;
+    PCIBus *pci_bus = NULL;
     NICInfo *nd;
     i2c_bus *i2c;
     int n;
@@ -250,7 +250,9 @@ static void realview_init(QEMUMachineInitArgs *args,
             }
             done_nic = 1;
         } else {
-            pci_nic_init_nofail(nd, "rtl8139", NULL);
+            if (pci_bus) {
+                pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL);
+            }
         }
     }
 
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 753757e..15eb086 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -244,7 +244,7 @@ static void versatile_init(QEMUMachineInitArgs *args, int board_id)
             smc91c111_init(nd, 0x10010000, sic[25]);
             done_smc = 1;
         } else {
-            pci_nic_init_nofail(nd, "rtl8139", NULL);
+            pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL);
         }
     }
     if (usb_enabled(false)) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4844a6b..61f5ac2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1228,7 +1228,7 @@ void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus)
         if (!pci_bus || (nd->model && strcmp(nd->model, "ne2k_isa") == 0)) {
             pc_init_ne2k_isa(isa_bus, nd);
         } else {
-            pci_nic_init_nofail(nd, "e1000", NULL);
+            pci_nic_init_nofail(nd, pci_bus, "e1000", NULL);
         }
     }
 }
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 1aac93a..cf2bd62 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -231,7 +231,7 @@ static void audio_init (PCIBus *pci_bus)
 }
 
 /* Network support */
-static void network_init (void)
+static void network_init (PCIBus *pci_bus)
 {
     int i;
 
@@ -244,7 +244,7 @@ static void network_init (void)
             default_devaddr = "07";
         }
 
-        pci_nic_init_nofail(nd, "rtl8139", default_devaddr);
+        pci_nic_init_nofail(nd, pci_bus, "rtl8139", default_devaddr);
     }
 }
 
@@ -393,7 +393,7 @@ static void mips_fulong2e_init(QEMUMachineInitArgs *args)
     /* Sound card */
     audio_init(pci_bus);
     /* Network card */
-    network_init();
+    network_init(pci_bus);
 }
 
 static QEMUMachine mips_fulong2e_machine = {
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 5033d51..05e59b4 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -468,7 +468,7 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion *address_space,
 }
 
 /* Network support */
-static void network_init(void)
+static void network_init(PCIBus *pci_bus)
 {
     int i;
 
@@ -480,7 +480,7 @@ static void network_init(void)
             /* The malta board has a PCNet card using PCI SLOT 11 */
             default_devaddr = "0b";
 
-        pci_nic_init_nofail(nd, "pcnet", default_devaddr);
+        pci_nic_init_nofail(nd, pci_bus, "pcnet", default_devaddr);
     }
 }
 
@@ -985,7 +985,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
     fdctrl_init_isa(isa_bus, fd);
 
     /* Network card */
-    network_init();
+    network_init(pci_bus);
 
     /* Optional PCI video card */
     pci_vga_init(pci_bus);
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index e92d646..807260c 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -92,7 +92,8 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
         monitor_printf(mon, "Parameter addr not supported\n");
         return NULL;
     }
-    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
+    return pci_nic_init(&nd_table[ret], pci_find_primary_bus(),
+                        "rtl8139", devaddr);
 }
 
 static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index fb92061..abdd45b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1578,7 +1578,8 @@ static const char * const pci_nic_names[] = {
 
 /* Initialize a PCI NIC.  */
 /* FIXME callers should check for failure, but don't */
-PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
+PCIDevice *pci_nic_init(NICInfo *nd, PCIBus *rootbus,
+                        const char *default_model,
                         const char *default_devaddr)
 {
     const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
@@ -1592,7 +1593,7 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
     if (i < 0)
         return NULL;
 
-    bus = pci_get_bus_devfn(&devfn, pci_find_primary_bus(), devaddr);
+    bus = pci_get_bus_devfn(&devfn, rootbus, devaddr);
     if (!bus) {
         error_report("Invalid PCI device address %s for device %s",
                      devaddr, pci_nic_names[i]);
@@ -1607,7 +1608,8 @@ PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
     return pci_dev;
 }
 
-PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
+PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
+                               const char *default_model,
                                const char *default_devaddr)
 {
     PCIDevice *res;
@@ -1615,7 +1617,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
     if (qemu_show_nic_models(nd->model, pci_nic_models))
         exit(0);
 
-    res = pci_nic_init(nd, default_model, default_devaddr);
+    res = pci_nic_init(nd, rootbus, default_model, default_devaddr);
     if (!res)
         exit(1);
     return res;
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index c9ae512..302f608 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -626,7 +626,7 @@ void ppce500_init(PPCE500Params *params)
     if (pci_bus) {
         /* Register network interfaces. */
         for (i = 0; i < nb_nics; i++) {
-            pci_nic_init_nofail(&nd_table[i], "virtio", NULL);
+            pci_nic_init_nofail(&nd_table[i], pci_bus, "virtio", NULL);
         }
     }
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index ce44e95..af5b077 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -362,7 +362,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
                              escc_mem, 0, memory_region_size(escc_mem));
 
     for(i = 0; i < nb_nics; i++)
-        pci_nic_init_nofail(&nd_table[i], "ne2k_pci", NULL);
+        pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
 
     ide_drive_get(hd, MAX_IDE_BUS);
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 3acca94..2a43027 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -259,7 +259,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
                              escc_mem, 0, memory_region_size(escc_mem));
 
     for(i = 0; i < nb_nics; i++)
-        pci_nic_init_nofail(&nd_table[i], "ne2k_pci", NULL);
+        pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
 
 
     ide_drive_get(hd, MAX_IDE_BUS);
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index a55e717..8bb9343 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -245,7 +245,7 @@ static void bamboo_init(QEMUMachineInitArgs *args)
         for (i = 0; i < nb_nics; i++) {
             /* There are no PCI NICs on the Bamboo board, but there are
              * PCI slots, so we can pick whatever default model we want. */
-            pci_nic_init_nofail(&nd_table[i], "e1000", NULL);
+            pci_nic_init_nofail(&nd_table[i], pcibus, "e1000", NULL);
         }
     }
 
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index be8a50e..4691fc1 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -627,7 +627,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
             isa_ne2000_init(isa_bus, ne2000_io[i], ne2000_irq[i],
                             &nd_table[i]);
         } else {
-            pci_nic_init_nofail(&nd_table[i], "ne2k_pci", NULL);
+            pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
         }
     }
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 218ea23..41a2e49 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -887,7 +887,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
         if (strcmp(nd->model, "ibmveth") == 0) {
             spapr_vlan_create(spapr->vio_bus, nd);
         } else {
-            pci_nic_init_nofail(&nd_table[i], nd->model, NULL);
+            pci_nic_init_nofail(&nd_table[i], phb->bus, nd->model, NULL);
         }
     }
 
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 3e4818e..dd81d75 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -236,6 +236,7 @@ static void r2d_init(QEMUMachineInitArgs *args)
     DeviceState *dev;
     SysBusDevice *busdev;
     MemoryRegion *address_space_mem = get_system_memory();
+    PCIBus *pci_bus;
 
     if (cpu_model == NULL) {
         cpu_model = "SH7751R";
@@ -264,6 +265,7 @@ static void r2d_init(QEMUMachineInitArgs *args)
     dev = qdev_create(NULL, "sh_pci");
     busdev = SYS_BUS_DEVICE(dev);
     qdev_init_nofail(dev);
+    pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci"));
     sysbus_mmio_map(busdev, 0, P4ADDR(0x1e200000));
     sysbus_mmio_map(busdev, 1, A7ADDR(0x1e200000));
     sysbus_connect_irq(busdev, 0, irq[PCI_INTA]);
@@ -295,7 +297,8 @@ static void r2d_init(QEMUMachineInitArgs *args)
 
     /* NIC: rtl8139 on-board, and 2 slots. */
     for (i = 0; i < nb_nics; i++)
-        pci_nic_init_nofail(&nd_table[i], "rtl8139", i==0 ? "2" : NULL);
+        pci_nic_init_nofail(&nd_table[i], pci_bus,
+                            "rtl8139", i==0 ? "2" : NULL);
 
     /* USB keyboard */
     usbdevice_create("keyboard");
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 2c2a111..df975f9 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -855,7 +855,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     }
 
     for(i = 0; i < nb_nics; i++)
-        pci_nic_init_nofail(&nd_table[i], "ne2k_pci", NULL);
+        pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
 
     ide_drive_get(hd, MAX_IDE_BUS);
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 3b22898..ecceeba 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -379,9 +379,11 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
 void pci_device_reset(PCIDevice *dev);
 void pci_bus_reset(PCIBus *bus);
 
-PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
+PCIDevice *pci_nic_init(NICInfo *nd, PCIBus *rootbus,
+                        const char *default_model,
                         const char *default_devaddr);
-PCIDevice *pci_nic_init_nofail(NICInfo *nd, const char *default_model,
+PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
+                               const char *default_model,
                                const char *default_devaddr);
 
 PCIDevice *pci_vga_init(PCIBus *bus);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 08/10] pci: Simpler implementation of primary PCI bus
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (6 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 07/10] pci: Add root bus parameter to pci_nic_init() David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 09/10] pci: Remove domain from PCIHostBus David Gibson
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

Currently pci_find_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.  The only remaining
users of pci_find_primary_bus() are in pci-hotplug-old.c, which implements
the old style pci_add/pci_del commands.

Therefore, this patch redefines pci_find_primary_bus() to find the only
PCI root bus, returning an error if there are multiple roots.  The callers
in pci-hotplug-old.c are updated correspondingly, to produce sensible
error messages.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci/pci-hotplug-old.c | 26 ++++++++++++++++++++------
 hw/pci/pci.c             |  9 ++++++---
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index 807260c..8077289 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -62,10 +62,17 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
 {
     Error *local_err = NULL;
     QemuOpts *opts;
+    PCIBus *root = pci_find_primary_bus();
     PCIBus *bus;
     int ret, devfn;
 
-    bus = pci_get_bus_devfn(&devfn, pci_find_primary_bus(), devaddr);
+    if (!root) {
+        monitor_printf(mon, "no primary PCI bus (if there are multiple"
+                       " PCI roots, you must use device_add instead)");
+        return NULL;
+    }
+
+    bus = pci_get_bus_devfn(&devfn, root, devaddr);
     if (!bus) {
         monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
         return NULL;
@@ -92,8 +99,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
         monitor_printf(mon, "Parameter addr not supported\n");
         return NULL;
     }
-    return pci_nic_init(&nd_table[ret], pci_find_primary_bus(),
-                        "rtl8139", devaddr);
+    return pci_nic_init(&nd_table[ret], root, "rtl8139", devaddr);
 }
 
 static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
@@ -144,7 +150,8 @@ int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
     switch (dinfo->type) {
     case IF_SCSI:
         if (!root) {
-            monitor_printf(mon, "no primary PCI bus\n");
+            monitor_printf(mon, "no primary PCI bus (if there are multiple"
+                           " PCI roots, you must use device_add instead)");
             goto err;
         }
         if (pci_read_devaddr(mon, pci_addr, &pci_bus, &slot)) {
@@ -177,6 +184,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
     DriveInfo *dinfo = NULL;
     int type = -1;
     char buf[128];
+    PCIBus *root = pci_find_primary_bus();
     PCIBus *bus;
     int devfn;
 
@@ -206,7 +214,12 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         dinfo = NULL;
     }
 
-    bus = pci_get_bus_devfn(&devfn, pci_find_primary_bus(), devaddr);
+    if (!root) {
+        monitor_printf(mon, "no primary PCI bus (if there are multiple"
+                       " PCI roots, you must use device_add instead)");
+        return NULL;
+    }
+    bus = pci_get_bus_devfn(&devfn, root, devaddr);
     if (!bus) {
         monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
         return NULL;
@@ -293,7 +306,8 @@ static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
     Error *local_err = NULL;
 
     if (!root) {
-        monitor_printf(mon, "no primary PCI bus\n");
+        monitor_printf(mon, "no primary PCI bus (if there are multiple"
+                       " PCI roots, you must use device_del instead)");
         return -1;
     }
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index abdd45b..48cc086 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -249,15 +249,18 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
 
 PCIBus *pci_find_primary_bus(void)
 {
+    PCIBus *primary_bus = NULL;
     struct PCIHostBus *host;
 
     QLIST_FOREACH(host, &host_buses, next) {
-        if (host->domain == 0) {
-            return host->bus;
+        if (primary_bus) {
+            /* We have multiple root buses, refuse to select a primary */
+            return NULL;
         }
+        primary_bus = host->bus;
     }
 
-    return NULL;
+    return primary_bus;
 }
 
 PCIBus *pci_device_root_bus(const PCIDevice *d)
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 09/10] pci: Remove domain from PCIHostBus
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (7 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 08/10] pci: Simpler implementation of primary PCI bus David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 10/10] pci: Fold host_buses list into PCIHostState functionality David Gibson
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

There are now no users of the domain field of PCIHostBus, so remove it
from the structure, and as a parameter from the pci_host_bus_register()
function which sets it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci/pci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 48cc086..b5e3eb8 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -91,7 +91,6 @@ static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
 static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
 
 struct PCIHostBus {
-    int domain;
     struct PCIBus *bus;
     QLIST_ENTRY(PCIHostBus) next;
 };
@@ -238,11 +237,10 @@ static int pcibus_reset(BusState *qbus)
     return 1;
 }
 
-static void pci_host_bus_register(int domain, PCIBus *bus)
+static void pci_host_bus_register(PCIBus *bus)
 {
     struct PCIHostBus *host;
     host = g_malloc0(sizeof(*host));
-    host->domain = domain;
     host->bus = bus;
     QLIST_INSERT_HEAD(&host_buses, host, next);
 }
@@ -303,7 +301,8 @@ 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 */
+
+    pci_host_bus_register(bus);
 
     vmstate_register(NULL, -1, &vmstate_pcibus, bus);
 }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 10/10] pci: Fold host_buses list into PCIHostState functionality
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (8 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 09/10] pci: Remove domain from PCIHostBus David Gibson
@ 2013-06-06  8:48 ` David Gibson
  2013-06-06 10:01 ` [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) Michael S. Tsirkin
  2013-06-17 21:37 ` Michael S. Tsirkin
  11 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-06  8:48 UTC (permalink / raw)
  To: aliguori, mst; +Cc: bonzini, qemu-devel, David Gibson

The host_buses list is an odd structure - a list of pointers to PCI root
buses existing in parallel to the normal qdev tree structure.  This patch
removes it, instead putting the link pointers into the PCIHostState
structure, which have a 1:1 relationship to PCIHostBus structures anyway.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci/pci.c              | 33 ++++++++++++++-------------------
 include/hw/pci/pci_host.h |  2 ++
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b5e3eb8..8d4e983 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -90,11 +90,7 @@ static void pci_del_option_rom(PCIDevice *pdev);
 static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
 static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
 
-struct PCIHostBus {
-    struct PCIBus *bus;
-    QLIST_ENTRY(PCIHostBus) next;
-};
-static QLIST_HEAD(, PCIHostBus) host_buses;
+static QLIST_HEAD(, PCIHostState) pci_host_bridges;
 
 static const VMStateDescription vmstate_pcibus = {
     .name = "PCIBUS",
@@ -237,20 +233,19 @@ static int pcibus_reset(BusState *qbus)
     return 1;
 }
 
-static void pci_host_bus_register(PCIBus *bus)
+static void pci_host_bus_register(PCIBus *bus, DeviceState *parent)
 {
-    struct PCIHostBus *host;
-    host = g_malloc0(sizeof(*host));
-    host->bus = bus;
-    QLIST_INSERT_HEAD(&host_buses, host, next);
+    PCIHostState *host_bridge = PCI_HOST_BRIDGE(parent);
+
+    QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
 }
 
 PCIBus *pci_find_primary_bus(void)
 {
     PCIBus *primary_bus = NULL;
-    struct PCIHostBus *host;
+    PCIHostState *host;
 
-    QLIST_FOREACH(host, &host_buses, next) {
+    QLIST_FOREACH(host, &pci_host_bridges, next) {
         if (primary_bus) {
             /* We have multiple root buses, refuse to select a primary */
             return NULL;
@@ -302,7 +297,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
     /* host bridge */
     QLIST_INIT(&bus->child);
 
-    pci_host_bus_register(bus);
+    pci_host_bus_register(bus, parent);
 
     vmstate_register(NULL, -1, &vmstate_pcibus, bus);
 }
@@ -1536,11 +1531,11 @@ static PciInfo *qmp_query_pci_bus(PCIBus *bus, int bus_num)
 PciInfoList *qmp_query_pci(Error **errp)
 {
     PciInfoList *info, *head = NULL, *cur_item = NULL;
-    struct PCIHostBus *host;
+    PCIHostState *host_bridge;
 
-    QLIST_FOREACH(host, &host_buses, next) {
+    QLIST_FOREACH(host_bridge, &pci_host_bridges, next) {
         info = g_malloc0(sizeof(*info));
-        info->value = qmp_query_pci_bus(host->bus, 0);
+        info->value = qmp_query_pci_bus(host_bridge->bus, 0);
 
         /* XXX: waiting for the qapi to support GSList */
         if (!cur_item) {
@@ -2204,11 +2199,11 @@ static int pci_qdev_find_recursive(PCIBus *bus,
 
 int pci_qdev_find_device(const char *id, PCIDevice **pdev)
 {
-    struct PCIHostBus *host;
+    PCIHostState *host_bridge;
     int rc = -ENODEV;
 
-    QLIST_FOREACH(host, &host_buses, next) {
-        int tmp = pci_qdev_find_recursive(host->bus, id, pdev);
+    QLIST_FOREACH(host_bridge, &pci_host_bridges, next) {
+        int tmp = pci_qdev_find_recursive(host_bridge->bus, id, pdev);
         if (!tmp) {
             rc = 0;
             break;
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index 44052f2..ba31595 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -46,6 +46,8 @@ struct PCIHostState {
     MemoryRegion mmcfg;
     uint32_t config_reg;
     PCIBus *bus;
+
+    QLIST_ENTRY(PCIHostState) next;
 };
 
 typedef struct PCIHostBridgeClass {
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 01/10] pci: Cleanup configuration for pci-hotplug.c
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 01/10] pci: Cleanup configuration for pci-hotplug.c David Gibson
@ 2013-06-06  9:57   ` Michael S. Tsirkin
  0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2013-06-06  9:57 UTC (permalink / raw)
  To: David Gibson; +Cc: pbonzini, aliguori, qemu-devel

On Thu, Jun 06, 2013 at 06:48:45PM +1000, David Gibson wrote:
> pci-hotplug.c and the CONFIG_PCI_HOTPLUG variable which controls its
> compilation are misnamed.  They're not about PCI hotplug in general, but
> rather about the pci_add/pci_del interface which are now deprecated in
> favour of the more general device_add/device_del interface.  This patch
> therefore renames them to pci-hotplug-old.c and CONFIG_PCI_HOTPLUG_OLD.
> 
> CONFIG_PCI_HOTPLUG=y was listed twice in {i386,x86_64}-softmmu.make for no
> particular reason, so we clean that up too.  In addition it was included in
> ppc64-softmmu.mak for which the old hotplug interface was never used and is
> unsuitable, so we remove that too.
> 
> Most of pci-hotplug.c was additionaly protected by #ifdef TARGET_I386.  The
> small piece which wasn't is only called from the pci_add and pci_del hooks
> in hmp-commands.hx, which themselves were protected by #ifdef TARGET_I386.
> This patch therefore also removes the #ifdef from pci-hotplug-old.c,
> and changes the ifdefs in hmp-commands.hx to use CONFIG_PCI_HOTPLUG_OLD.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  default-configs/i386-softmmu.mak   |   3 +-
>  default-configs/ppc64-softmmu.mak  |   2 -
>  default-configs/x86_64-softmmu.mak |   3 +-
>  hmp-commands.hx                    |   4 +-
>  hw/pci/Makefile.objs               |   2 +-
>  hw/pci/pci-hotplug-old.c           | 292 +++++++++++++++++++++++++++++++++++++
>  hw/pci/pci-hotplug.c               | 292 -------------------------------------
>  7 files changed, 297 insertions(+), 301 deletions(-)
>  create mode 100644 hw/pci/pci-hotplug-old.c
>  delete mode 100644 hw/pci/pci-hotplug.c
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 03deca2..4a0fc9c 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -28,11 +28,10 @@ CONFIG_APPLESMC=y
>  CONFIG_I8259=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_TPM_TIS=$(CONFIG_TPM)
> -CONFIG_PCI_HOTPLUG=y
> +CONFIG_PCI_HOTPLUG_OLD=y
>  CONFIG_MC146818RTC=y
>  CONFIG_PAM=y
>  CONFIG_PCI_PIIX=y
> -CONFIG_PCI_HOTPLUG=y
>  CONFIG_WDT_IB700=y
>  CONFIG_PC_SYSFW=y
>  CONFIG_XEN_I386=$(CONFIG_XEN)
> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> index 884ea8a..d7140c4 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -44,7 +44,5 @@ CONFIG_XILINX_ETHLITE=y
>  CONFIG_OPENPIC=y
>  CONFIG_PSERIES=$(CONFIG_FDT)
>  CONFIG_E500=$(CONFIG_FDT)
> -# For pSeries
> -CONFIG_PCI_HOTPLUG=y
>  # For PReP
>  CONFIG_MC146818RTC=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 599b630..10bb0c6 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -28,11 +28,10 @@ CONFIG_APPLESMC=y
>  CONFIG_I8259=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_TPM_TIS=$(CONFIG_TPM)
> -CONFIG_PCI_HOTPLUG=y
> +CONFIG_PCI_HOTPLUG_OLD=y
>  CONFIG_MC146818RTC=y
>  CONFIG_PAM=y
>  CONFIG_PCI_PIIX=y
> -CONFIG_PCI_HOTPLUG=y
>  CONFIG_WDT_IB700=y
>  CONFIG_PC_SYSFW=y
>  CONFIG_XEN_I386=$(CONFIG_XEN)
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 9cea415..1d88320 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1077,7 +1077,7 @@ STEXI
>  Add drive to PCI storage controller.
>  ETEXI
>  
> -#if defined(TARGET_I386)
> +#if defined(CONFIG_PCI_HOTPLUG_OLD)
>      {
>          .name       = "pci_add",
>          .args_type  = "pci_addr:s,type:s,opts:s?",
> @@ -1093,7 +1093,7 @@ STEXI
>  Hot-add PCI device.
>  ETEXI
>  
> -#if defined(TARGET_I386)
> +#if defined(CONFIG_PCI_HOTPLUG_OLD)
>      {
>          .name       = "pci_del",
>          .args_type  = "pci_addr:s",
> diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> index a7fb9d0..720f438 100644
> --- a/hw/pci/Makefile.objs
> +++ b/hw/pci/Makefile.objs
> @@ -8,4 +8,4 @@ common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
>  common-obj-$(CONFIG_NO_PCI) += pci-stub.o
>  common-obj-$(CONFIG_ALL) += pci-stub.o
>  
> -obj-$(CONFIG_PCI_HOTPLUG) += pci-hotplug.o
> +common-obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o
> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> new file mode 100644
> index 0000000..b3c233c
> --- /dev/null
> +++ b/hw/pci/pci-hotplug-old.c
> @@ -0,0 +1,292 @@

Forgot --find-renames ?


> +/*
> + * Deprecated PCI hotplug interface support
> + * This covers the old pci_add / pci_del command, whereas the more general
> + * device_add / device_del commands are now preferred.
> + *
> + * Copyright (c) 2004 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw/hw.h"
> +#include "hw/boards.h"
> +#include "hw/pci/pci.h"
> +#include "net/net.h"
> +#include "hw/i386/pc.h"
> +#include "monitor/monitor.h"
> +#include "hw/scsi/scsi.h"
> +#include "hw/virtio/virtio-blk.h"
> +#include "qemu/config-file.h"
> +#include "sysemu/blockdev.h"
> +#include "qapi/error.h"
> +
> +static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
> +                                       const char *devaddr,
> +                                       const char *opts_str)
> +{
> +    Error *local_err = NULL;
> +    QemuOpts *opts;
> +    PCIBus *bus;
> +    int ret, devfn;
> +
> +    bus = pci_get_bus_devfn(&devfn, devaddr);
> +    if (!bus) {
> +        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> +        return NULL;
> +    }
> +    if (!((BusState*)bus)->allow_hotplug) {
> +        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> +        return NULL;
> +    }
> +
> +    opts = qemu_opts_parse(qemu_find_opts("net"), opts_str ? opts_str : "", 0);
> +    if (!opts) {
> +        return NULL;
> +    }
> +
> +    qemu_opt_set(opts, "type", "nic");
> +
> +    ret = net_client_init(opts, 0, &local_err);
> +    if (error_is_set(&local_err)) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +        return NULL;
> +    }
> +    if (nd_table[ret].devaddr) {
> +        monitor_printf(mon, "Parameter addr not supported\n");
> +        return NULL;
> +    }
> +    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
> +}
> +
> +static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
> +                        DriveInfo *dinfo, int printinfo)
> +{
> +    SCSIBus *scsibus;
> +    SCSIDevice *scsidev;
> +
> +    scsibus = (SCSIBus *)
> +        object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
> +                            TYPE_SCSI_BUS);
> +    if (!scsibus) {
> +	error_report("Device is not a SCSI adapter");
> +	return -1;
> +    }
> +
> +    /*
> +     * drive_init() tries to find a default for dinfo->unit.  Doesn't
> +     * work at all for hotplug though as we assign the device to a
> +     * specific bus instead of the first bus with spare scsi ids.
> +     *
> +     * Ditch the calculated value and reload from option string (if
> +     * specified).
> +     */
> +    dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
> +    dinfo->bus = scsibus->busnr;
> +    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
> +                                        false, -1, NULL);
> +    if (!scsidev) {
> +        return -1;
> +    }
> +    dinfo->unit = scsidev->id;
> +
> +    if (printinfo)
> +        monitor_printf(mon, "OK bus %d, unit %d\n",
> +                       scsibus->busnr, scsidev->id);
> +    return 0;
> +}
> +
> +int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
> +{
> +    int dom, pci_bus;
> +    unsigned slot;
> +    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)) {
> +            goto err;
> +        }
> +        dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
> +                              PCI_DEVFN(slot, 0));
> +        if (!dev) {
> +            monitor_printf(mon, "no pci device with address %s\n", pci_addr);
> +            goto err;
> +        }
> +        if (scsi_hot_add(mon, &dev->qdev, dinfo, 1) != 0) {
> +            goto err;
> +        }
> +        break;
> +    default:
> +        monitor_printf(mon, "Can't hot-add drive to type %d\n", dinfo->type);
> +        goto err;
> +    }
> +
> +    return 0;
> +err:
> +    return -1;
> +}
> +
> +static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
> +                                           const char *devaddr,
> +                                           const char *opts)
> +{
> +    PCIDevice *dev;
> +    DriveInfo *dinfo = NULL;
> +    int type = -1;
> +    char buf[128];
> +    PCIBus *bus;
> +    int devfn;
> +
> +    if (get_param_value(buf, sizeof(buf), "if", opts)) {
> +        if (!strcmp(buf, "scsi"))
> +            type = IF_SCSI;
> +        else if (!strcmp(buf, "virtio")) {
> +            type = IF_VIRTIO;
> +        } else {
> +            monitor_printf(mon, "type %s not a hotpluggable PCI device.\n", buf);
> +            return NULL;
> +        }
> +    } else {
> +        monitor_printf(mon, "no if= specified\n");
> +        return NULL;
> +    }
> +
> +    if (get_param_value(buf, sizeof(buf), "file", opts)) {
> +        dinfo = add_init_drive(opts);
> +        if (!dinfo)
> +            return NULL;
> +        if (dinfo->devaddr) {
> +            monitor_printf(mon, "Parameter addr not supported\n");
> +            return NULL;
> +        }
> +    } else {
> +        dinfo = NULL;
> +    }
> +
> +    bus = pci_get_bus_devfn(&devfn, devaddr);
> +    if (!bus) {
> +        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> +        return NULL;
> +    }
> +    if (!((BusState*)bus)->allow_hotplug) {
> +        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> +        return NULL;
> +    }
> +
> +    switch (type) {
> +    case IF_SCSI:
> +        dev = pci_create(bus, devfn, "lsi53c895a");
> +        if (qdev_init(&dev->qdev) < 0)
> +            dev = NULL;
> +        if (dev && dinfo) {
> +            if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
> +                qdev_unplug(&dev->qdev, NULL);
> +                dev = NULL;
> +            }
> +        }
> +        break;
> +    case IF_VIRTIO:
> +        if (!dinfo) {
> +            monitor_printf(mon, "virtio requires a backing file/device.\n");
> +            return NULL;
> +        }
> +        dev = pci_create(bus, devfn, "virtio-blk-pci");
> +        if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
> +            qdev_free(&dev->qdev);
> +            dev = NULL;
> +            break;
> +        }
> +        if (qdev_init(&dev->qdev) < 0)
> +            dev = NULL;
> +        break;
> +    default:
> +        dev = NULL;
> +    }
> +    return dev;
> +}
> +
> +void pci_device_hot_add(Monitor *mon, const QDict *qdict)
> +{
> +    PCIDevice *dev = NULL;
> +    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> +    const char *type = qdict_get_str(qdict, "type");
> +    const char *opts = qdict_get_try_str(qdict, "opts");
> +
> +    /* strip legacy tag */
> +    if (!strncmp(pci_addr, "pci_addr=", 9)) {
> +        pci_addr += 9;
> +    }
> +
> +    if (!opts) {
> +        opts = "";
> +    }
> +
> +    if (!strcmp(pci_addr, "auto"))
> +        pci_addr = NULL;
> +
> +    if (strcmp(type, "nic") == 0) {
> +        dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
> +    } else if (strcmp(type, "storage") == 0) {
> +        dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
> +    } else {
> +        monitor_printf(mon, "invalid type: %s\n", type);
> +    }
> +
> +    if (dev) {
> +        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
> +                       pci_find_domain(dev->bus),
> +                       pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
> +                       PCI_FUNC(dev->devfn));
> +    } else
> +        monitor_printf(mon, "failed to add %s\n", opts);
> +}
> +
> +static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
> +{
> +    PCIDevice *d;
> +    int dom, bus;
> +    unsigned slot;
> +    Error *local_err = NULL;
> +
> +    if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
> +        return -1;
> +    }
> +
> +    d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
> +    if (!d) {
> +        monitor_printf(mon, "slot %d empty\n", slot);
> +        return -1;
> +    }
> +
> +    qdev_unplug(&d->qdev, &local_err);
> +    if (error_is_set(&local_err)) {
> +        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
> +        error_free(local_err);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
> +{
> +    pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
> +}
> diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c
> deleted file mode 100644
> index 12287d1..0000000
> --- a/hw/pci/pci-hotplug.c
> +++ /dev/null
> @@ -1,292 +0,0 @@
> -/*
> - * QEMU PCI hotplug support
> - *
> - * Copyright (c) 2004 Fabrice Bellard
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -#include "hw/hw.h"
> -#include "hw/boards.h"
> -#include "hw/pci/pci.h"
> -#include "net/net.h"
> -#include "hw/i386/pc.h"
> -#include "monitor/monitor.h"
> -#include "hw/scsi/scsi.h"
> -#include "hw/virtio/virtio-blk.h"
> -#include "qemu/config-file.h"
> -#include "sysemu/blockdev.h"
> -#include "qapi/error.h"
> -
> -#if defined(TARGET_I386)
> -static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
> -                                       const char *devaddr,
> -                                       const char *opts_str)
> -{
> -    Error *local_err = NULL;
> -    QemuOpts *opts;
> -    PCIBus *bus;
> -    int ret, devfn;
> -
> -    bus = pci_get_bus_devfn(&devfn, devaddr);
> -    if (!bus) {
> -        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> -        return NULL;
> -    }
> -    if (!((BusState*)bus)->allow_hotplug) {
> -        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> -        return NULL;
> -    }
> -
> -    opts = qemu_opts_parse(qemu_find_opts("net"), opts_str ? opts_str : "", 0);
> -    if (!opts) {
> -        return NULL;
> -    }
> -
> -    qemu_opt_set(opts, "type", "nic");
> -
> -    ret = net_client_init(opts, 0, &local_err);
> -    if (error_is_set(&local_err)) {
> -        qerror_report_err(local_err);
> -        error_free(local_err);
> -        return NULL;
> -    }
> -    if (nd_table[ret].devaddr) {
> -        monitor_printf(mon, "Parameter addr not supported\n");
> -        return NULL;
> -    }
> -    return pci_nic_init(&nd_table[ret], "rtl8139", devaddr);
> -}
> -
> -static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
> -                        DriveInfo *dinfo, int printinfo)
> -{
> -    SCSIBus *scsibus;
> -    SCSIDevice *scsidev;
> -
> -    scsibus = (SCSIBus *)
> -        object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
> -                            TYPE_SCSI_BUS);
> -    if (!scsibus) {
> -	error_report("Device is not a SCSI adapter");
> -	return -1;
> -    }
> -
> -    /*
> -     * drive_init() tries to find a default for dinfo->unit.  Doesn't
> -     * work at all for hotplug though as we assign the device to a
> -     * specific bus instead of the first bus with spare scsi ids.
> -     *
> -     * Ditch the calculated value and reload from option string (if
> -     * specified).
> -     */
> -    dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
> -    dinfo->bus = scsibus->busnr;
> -    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
> -                                        false, -1, NULL);
> -    if (!scsidev) {
> -        return -1;
> -    }
> -    dinfo->unit = scsidev->id;
> -
> -    if (printinfo)
> -        monitor_printf(mon, "OK bus %d, unit %d\n",
> -                       scsibus->busnr, scsidev->id);
> -    return 0;
> -}
> -
> -int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
> -{
> -    int dom, pci_bus;
> -    unsigned slot;
> -    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)) {
> -            goto err;
> -        }
> -        dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
> -                              PCI_DEVFN(slot, 0));
> -        if (!dev) {
> -            monitor_printf(mon, "no pci device with address %s\n", pci_addr);
> -            goto err;
> -        }
> -        if (scsi_hot_add(mon, &dev->qdev, dinfo, 1) != 0) {
> -            goto err;
> -        }
> -        break;
> -    default:
> -        monitor_printf(mon, "Can't hot-add drive to type %d\n", dinfo->type);
> -        goto err;
> -    }
> -
> -    return 0;
> -err:
> -    return -1;
> -}
> -
> -static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
> -                                           const char *devaddr,
> -                                           const char *opts)
> -{
> -    PCIDevice *dev;
> -    DriveInfo *dinfo = NULL;
> -    int type = -1;
> -    char buf[128];
> -    PCIBus *bus;
> -    int devfn;
> -
> -    if (get_param_value(buf, sizeof(buf), "if", opts)) {
> -        if (!strcmp(buf, "scsi"))
> -            type = IF_SCSI;
> -        else if (!strcmp(buf, "virtio")) {
> -            type = IF_VIRTIO;
> -        } else {
> -            monitor_printf(mon, "type %s not a hotpluggable PCI device.\n", buf);
> -            return NULL;
> -        }
> -    } else {
> -        monitor_printf(mon, "no if= specified\n");
> -        return NULL;
> -    }
> -
> -    if (get_param_value(buf, sizeof(buf), "file", opts)) {
> -        dinfo = add_init_drive(opts);
> -        if (!dinfo)
> -            return NULL;
> -        if (dinfo->devaddr) {
> -            monitor_printf(mon, "Parameter addr not supported\n");
> -            return NULL;
> -        }
> -    } else {
> -        dinfo = NULL;
> -    }
> -
> -    bus = pci_get_bus_devfn(&devfn, devaddr);
> -    if (!bus) {
> -        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
> -        return NULL;
> -    }
> -    if (!((BusState*)bus)->allow_hotplug) {
> -        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
> -        return NULL;
> -    }
> -
> -    switch (type) {
> -    case IF_SCSI:
> -        dev = pci_create(bus, devfn, "lsi53c895a");
> -        if (qdev_init(&dev->qdev) < 0)
> -            dev = NULL;
> -        if (dev && dinfo) {
> -            if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
> -                qdev_unplug(&dev->qdev, NULL);
> -                dev = NULL;
> -            }
> -        }
> -        break;
> -    case IF_VIRTIO:
> -        if (!dinfo) {
> -            monitor_printf(mon, "virtio requires a backing file/device.\n");
> -            return NULL;
> -        }
> -        dev = pci_create(bus, devfn, "virtio-blk-pci");
> -        if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
> -            qdev_free(&dev->qdev);
> -            dev = NULL;
> -            break;
> -        }
> -        if (qdev_init(&dev->qdev) < 0)
> -            dev = NULL;
> -        break;
> -    default:
> -        dev = NULL;
> -    }
> -    return dev;
> -}
> -
> -void pci_device_hot_add(Monitor *mon, const QDict *qdict)
> -{
> -    PCIDevice *dev = NULL;
> -    const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> -    const char *type = qdict_get_str(qdict, "type");
> -    const char *opts = qdict_get_try_str(qdict, "opts");
> -
> -    /* strip legacy tag */
> -    if (!strncmp(pci_addr, "pci_addr=", 9)) {
> -        pci_addr += 9;
> -    }
> -
> -    if (!opts) {
> -        opts = "";
> -    }
> -
> -    if (!strcmp(pci_addr, "auto"))
> -        pci_addr = NULL;
> -
> -    if (strcmp(type, "nic") == 0) {
> -        dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
> -    } else if (strcmp(type, "storage") == 0) {
> -        dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
> -    } else {
> -        monitor_printf(mon, "invalid type: %s\n", type);
> -    }
> -
> -    if (dev) {
> -        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
> -                       pci_find_domain(dev->bus),
> -                       pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
> -                       PCI_FUNC(dev->devfn));
> -    } else
> -        monitor_printf(mon, "failed to add %s\n", opts);
> -}
> -#endif
> -
> -static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
> -{
> -    PCIDevice *d;
> -    int dom, bus;
> -    unsigned slot;
> -    Error *local_err = NULL;
> -
> -    if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
> -        return -1;
> -    }
> -
> -    d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
> -    if (!d) {
> -        monitor_printf(mon, "slot %d empty\n", slot);
> -        return -1;
> -    }
> -
> -    qdev_unplug(&d->qdev, &local_err);
> -    if (error_is_set(&local_err)) {
> -        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
> -        error_free(local_err);
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
> -void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
> -{
> -    pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"));
> -}
> -- 
> 1.8.1.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2)
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (9 preceding siblings ...)
  2013-06-06  8:48 ` [Qemu-devel] [PATCH 10/10] pci: Fold host_buses list into PCIHostState functionality David Gibson
@ 2013-06-06 10:01 ` Michael S. Tsirkin
  2013-06-06 15:04   ` Anthony Liguori
  2013-06-12  7:18   ` Alexey Kardashevskiy
  2013-06-17 21:37 ` Michael S. Tsirkin
  11 siblings, 2 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2013-06-06 10:01 UTC (permalink / raw)
  To: David Gibson; +Cc: pbonzini, aliguori, qemu-devel

On Thu, Jun 06, 2013 at 06:48:44PM +1000, David Gibson wrote:
> The current PCI subsystem has kind of half-hearted support for
> multiple independent root buses - aka PCI domains - in the form of the
> PCIHostBus structure and its domain field.  However, it doesn't quite
> work because pci_host_bus_register() is always called with a domain of
> 0.
> 
> Worse, though, the whole concept of numbered domains isn't general
> enough.  Many platforms can have independent root buses (usually on
> wholly independent host bridges), but only x86 gives them a
> hardware-significant domain number, essentially as a hack to allow all
> the separate config spaces to be accessed via the same IO ports.
> Linux guests on other platforms will show domain numbers in lspci, but
> these are purely guest assigned, so qemu won't know about them.
> 
> This patch series, therefore, removes the broken-as-is domain concept
> from qemu, and replaces it with a different way of handling multiple
> root buses, based on a host bridge class method to provide a
> identifier for the root bus.  This hook is designed in such a way as
> to allow a single bridge object to support mutiple root buses with
> future changes, which will allow future implementations of x86 north
> bridges with multiple domains to be supported correctly, and in way
> that matches the existing practice for all external interfaces.
> 
> v2:
>   * Rework concept of "primary" bus in response to Michael Tsirkin's
>     comments.


Looks good to me.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

I'll wait a bit so others have a chance to comment, then apply
if everyone is happy.

No need to repost for the lack of -M flag - I wish there was a way
to specify that in git config.

-- 
MST

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2)
  2013-06-06 10:01 ` [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) Michael S. Tsirkin
@ 2013-06-06 15:04   ` Anthony Liguori
  2013-06-07  0:45     ` David Gibson
  2013-06-12  7:18   ` Alexey Kardashevskiy
  1 sibling, 1 reply; 18+ messages in thread
From: Anthony Liguori @ 2013-06-06 15:04 UTC (permalink / raw)
  To: Michael S. Tsirkin, David Gibson; +Cc: pbonzini, qemu-devel

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Jun 06, 2013 at 06:48:44PM +1000, David Gibson wrote:
>> The current PCI subsystem has kind of half-hearted support for
>> multiple independent root buses - aka PCI domains - in the form of the
>> PCIHostBus structure and its domain field.  However, it doesn't quite
>> work because pci_host_bus_register() is always called with a domain of
>> 0.
>> 
>> Worse, though, the whole concept of numbered domains isn't general
>> enough.  Many platforms can have independent root buses (usually on
>> wholly independent host bridges), but only x86 gives them a
>> hardware-significant domain number, essentially as a hack to allow all
>> the separate config spaces to be accessed via the same IO ports.
>> Linux guests on other platforms will show domain numbers in lspci, but
>> these are purely guest assigned, so qemu won't know about them.
>> 
>> This patch series, therefore, removes the broken-as-is domain concept
>> from qemu, and replaces it with a different way of handling multiple
>> root buses, based on a host bridge class method to provide a
>> identifier for the root bus.  This hook is designed in such a way as
>> to allow a single bridge object to support mutiple root buses with
>> future changes, which will allow future implementations of x86 north
>> bridges with multiple domains to be supported correctly, and in way
>> that matches the existing practice for all external interfaces.
>> 
>> v2:
>>   * Rework concept of "primary" bus in response to Michael Tsirkin's
>>     comments.
>
>
> Looks good to me.
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> I'll wait a bit so others have a chance to comment, then apply
> if everyone is happy.
>
> No need to repost for the lack of -M flag - I wish there was a way
> to specify that in git config.

[diff]
        renames = true

Regards,

Anthony Liguori

> -- 
> MST

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2)
  2013-06-06 15:04   ` Anthony Liguori
@ 2013-06-07  0:45     ` David Gibson
  0 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2013-06-07  0:45 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: pbonzini, qemu-devel, Michael S. Tsirkin

[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]

On Thu, Jun 06, 2013 at 10:04:58AM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Thu, Jun 06, 2013 at 06:48:44PM +1000, David Gibson wrote:
> >> The current PCI subsystem has kind of half-hearted support for
> >> multiple independent root buses - aka PCI domains - in the form of the
> >> PCIHostBus structure and its domain field.  However, it doesn't quite
> >> work because pci_host_bus_register() is always called with a domain of
> >> 0.
> >> 
> >> Worse, though, the whole concept of numbered domains isn't general
> >> enough.  Many platforms can have independent root buses (usually on
> >> wholly independent host bridges), but only x86 gives them a
> >> hardware-significant domain number, essentially as a hack to allow all
> >> the separate config spaces to be accessed via the same IO ports.
> >> Linux guests on other platforms will show domain numbers in lspci, but
> >> these are purely guest assigned, so qemu won't know about them.
> >> 
> >> This patch series, therefore, removes the broken-as-is domain concept
> >> from qemu, and replaces it with a different way of handling multiple
> >> root buses, based on a host bridge class method to provide a
> >> identifier for the root bus.  This hook is designed in such a way as
> >> to allow a single bridge object to support mutiple root buses with
> >> future changes, which will allow future implementations of x86 north
> >> bridges with multiple domains to be supported correctly, and in way
> >> that matches the existing practice for all external interfaces.
> >> 
> >> v2:
> >>   * Rework concept of "primary" bus in response to Michael Tsirkin's
> >>     comments.
> >
> >
> > Looks good to me.
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > I'll wait a bit so others have a chance to comment, then apply
> > if everyone is happy.
> >
> > No need to repost for the lack of -M flag - I wish there was a way
> > to specify that in git config.
> 
> [diff]
>         renames = true

Oh, thanks.  I was looking for a way to configure that, but hadn't
found it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2)
  2013-06-06 10:01 ` [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) Michael S. Tsirkin
  2013-06-06 15:04   ` Anthony Liguori
@ 2013-06-12  7:18   ` Alexey Kardashevskiy
  2013-06-12 19:14     ` Michael S. Tsirkin
  1 sibling, 1 reply; 18+ messages in thread
From: Alexey Kardashevskiy @ 2013-06-12  7:18 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: pbonzini, aliguori, qemu-devel, David Gibson

On 06/06/2013 08:01 PM, Michael S. Tsirkin wrote:
> On Thu, Jun 06, 2013 at 06:48:44PM +1000, David Gibson wrote:
>> The current PCI subsystem has kind of half-hearted support for
>> multiple independent root buses - aka PCI domains - in the form of the
>> PCIHostBus structure and its domain field.  However, it doesn't quite
>> work because pci_host_bus_register() is always called with a domain of
>> 0.
>>
>> Worse, though, the whole concept of numbered domains isn't general
>> enough.  Many platforms can have independent root buses (usually on
>> wholly independent host bridges), but only x86 gives them a
>> hardware-significant domain number, essentially as a hack to allow all
>> the separate config spaces to be accessed via the same IO ports.
>> Linux guests on other platforms will show domain numbers in lspci, but
>> these are purely guest assigned, so qemu won't know about them.
>>
>> This patch series, therefore, removes the broken-as-is domain concept
>> from qemu, and replaces it with a different way of handling multiple
>> root buses, based on a host bridge class method to provide a
>> identifier for the root bus.  This hook is designed in such a way as
>> to allow a single bridge object to support mutiple root buses with
>> future changes, which will allow future implementations of x86 north
>> bridges with multiple domains to be supported correctly, and in way
>> that matches the existing practice for all external interfaces.
>>
>> v2:
>>   * Rework concept of "primary" bus in response to Michael Tsirkin's
>>     comments.
> 
> 
> Looks good to me.
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> I'll wait a bit so others have a chance to comment, then apply
> if everyone is happy.


So, did it happen? I would be happy :) When is it expected to reach
upstream? Thanks!


> No need to repost for the lack of -M flag - I wish there was a way
> to specify that in git config.
> 


-- 
Alexey

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2)
  2013-06-12  7:18   ` Alexey Kardashevskiy
@ 2013-06-12 19:14     ` Michael S. Tsirkin
  0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2013-06-12 19:14 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: pbonzini, aliguori, qemu-devel, David Gibson

On Wed, Jun 12, 2013 at 05:18:46PM +1000, Alexey Kardashevskiy wrote:
> On 06/06/2013 08:01 PM, Michael S. Tsirkin wrote:
> > On Thu, Jun 06, 2013 at 06:48:44PM +1000, David Gibson wrote:
> >> The current PCI subsystem has kind of half-hearted support for
> >> multiple independent root buses - aka PCI domains - in the form of the
> >> PCIHostBus structure and its domain field.  However, it doesn't quite
> >> work because pci_host_bus_register() is always called with a domain of
> >> 0.
> >>
> >> Worse, though, the whole concept of numbered domains isn't general
> >> enough.  Many platforms can have independent root buses (usually on
> >> wholly independent host bridges), but only x86 gives them a
> >> hardware-significant domain number, essentially as a hack to allow all
> >> the separate config spaces to be accessed via the same IO ports.
> >> Linux guests on other platforms will show domain numbers in lspci, but
> >> these are purely guest assigned, so qemu won't know about them.
> >>
> >> This patch series, therefore, removes the broken-as-is domain concept
> >> from qemu, and replaces it with a different way of handling multiple
> >> root buses, based on a host bridge class method to provide a
> >> identifier for the root bus.  This hook is designed in such a way as
> >> to allow a single bridge object to support mutiple root buses with
> >> future changes, which will allow future implementations of x86 north
> >> bridges with multiple domains to be supported correctly, and in way
> >> that matches the existing practice for all external interfaces.
> >>
> >> v2:
> >>   * Rework concept of "primary" bus in response to Michael Tsirkin's
> >>     comments.
> > 
> > 
> > Looks good to me.
> > 
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > I'll wait a bit so others have a chance to comment, then apply
> > if everyone is happy.
> 
> 
> So, did it happen? I would be happy :) When is it expected to reach
> upstream? Thanks!
> 

It will be in the next pull I send.

I'm waiting for Anthony to merge the previous pull from June 6,
once it's merged I'll send next pull within a couple of days
(I try not to flood him too much).

So ... RSN.

> > No need to repost for the lack of -M flag - I wish there was a way
> > to specify that in git config.
> > 
> 
> 
> -- 
> Alexey

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2)
  2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
                   ` (10 preceding siblings ...)
  2013-06-06 10:01 ` [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) Michael S. Tsirkin
@ 2013-06-17 21:37 ` Michael S. Tsirkin
  11 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2013-06-17 21:37 UTC (permalink / raw)
  To: David Gibson; +Cc: aliguori, qemu-devel

On Thu, Jun 06, 2013 at 06:48:44PM +1000, David Gibson wrote:
> The current PCI subsystem has kind of half-hearted support for
> multiple independent root buses - aka PCI domains - in the form of the
> PCIHostBus structure and its domain field.  However, it doesn't quite
> work because pci_host_bus_register() is always called with a domain of
> 0.
> 
> Worse, though, the whole concept of numbered domains isn't general
> enough.  Many platforms can have independent root buses (usually on
> wholly independent host bridges), but only x86 gives them a
> hardware-significant domain number, essentially as a hack to allow all
> the separate config spaces to be accessed via the same IO ports.
> Linux guests on other platforms will show domain numbers in lspci, but
> these are purely guest assigned, so qemu won't know about them.
> 
> This patch series, therefore, removes the broken-as-is domain concept
> from qemu, and replaces it with a different way of handling multiple
> root buses, based on a host bridge class method to provide a
> identifier for the root bus.  This hook is designed in such a way as
> to allow a single bridge object to support mutiple root buses with
> future changes, which will allow future implementations of x86 north
> bridges with multiple domains to be supported correctly, and in way
> that matches the existing practice for all external interfaces.

Applied, thanks.
If all's well, this shall go to Anthony in a couple of days.

> v2:
>   * Rework concept of "primary" bus in response to Michael Tsirkin's
>     comments.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2013-06-17 21:37 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-06  8:48 [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 01/10] pci: Cleanup configuration for pci-hotplug.c David Gibson
2013-06-06  9:57   ` Michael S. Tsirkin
2013-06-06  8:48 ` [Qemu-devel] [PATCH 02/10] pci: Move pci_read_devaddr to pci-hotplug-old.c David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 03/10] pci: Abolish pci_find_root_bus() David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 04/10] pci: Use helper to find device's root bus in pci_find_domain() David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 05/10] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 06/10] pci: Add root bus argument to pci_get_bus_devfn() David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 07/10] pci: Add root bus parameter to pci_nic_init() David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 08/10] pci: Simpler implementation of primary PCI bus David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 09/10] pci: Remove domain from PCIHostBus David Gibson
2013-06-06  8:48 ` [Qemu-devel] [PATCH 10/10] pci: Fold host_buses list into PCIHostState functionality David Gibson
2013-06-06 10:01 ` [Qemu-devel] [0/10] Clean up PCI code to allow for multiple root buses (v2) Michael S. Tsirkin
2013-06-06 15:04   ` Anthony Liguori
2013-06-07  0:45     ` David Gibson
2013-06-12  7:18   ` Alexey Kardashevskiy
2013-06-12 19:14     ` Michael S. Tsirkin
2013-06-17 21:37 ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).