* [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses
@ 2013-05-09 0:31 David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c David Gibson
` (9 more replies)
0 siblings, 10 replies; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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.
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-23 11:11 ` Michael S. Tsirkin
2013-05-23 14:54 ` Paolo Bonzini
2013-05-09 0:31 ` [Qemu-devel] [PATCH 2/8] pci: Move pci_read_devaddr to pci-hotplug-old.c David Gibson
` (8 subsequent siblings)
9 siblings, 2 replies; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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 | 290 +++++++++++++++++++++++++++++++++++
hw/pci/pci-hotplug.c | 292 ------------------------------------
7 files changed, 295 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..2ad32b6 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
+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..724a80b
--- /dev/null
+++ b/hw/pci/pci-hotplug-old.c
@@ -0,0 +1,290 @@
+/*
+ * 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"
+
+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.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 2/8] pci: Move pci_read_devaddr to pci-hotplug-old.c
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus() David Gibson
` (7 subsequent siblings)
9 siblings, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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 724a80b..1aa0ab8 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -34,6 +34,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 d5257ed..9906e84 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.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus()
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 2/8] pci: Move pci_read_devaddr to pci-hotplug-old.c David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-23 11:26 ` Michael S. Tsirkin
2013-05-09 0:31 ` [Qemu-devel] [PATCH 4/8] pci: Use helper o find device's root bus in pci_find_domain() David Gibson
` (6 subsequent siblings)
9 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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_get_primary_bus().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/pci/pci-hotplug-old.c | 34 +++++++++++++++++++++++++---------
hw/pci/pci.c | 19 +++++++++++++++----
include/hw/pci/pci.h | 2 +-
3 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index 1aa0ab8..55441c6 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -34,17 +34,23 @@
#include "sysemu/blockdev.h"
#include "qapi/error.h"
-static int pci_read_devaddr(Monitor *mon, const char *addr, int *domp,
+static int pci_read_devaddr(Monitor *mon, const char *addr,
int *busp, unsigned *slotp)
{
+ int dom;
+
/* strip legacy tag */
if (!strncmp(addr, "pci_addr=", 9)) {
addr += 9;
}
- if (pci_parse_devaddr(addr, domp, busp, slotp, NULL)) {
+ if (pci_parse_devaddr(addr, &dom, busp, slotp, NULL)) {
monitor_printf(mon, "Invalid pci address\n");
return -1;
}
+ if (dom != 0) {
+ monitor_printf(mon, "Multiple PCI domains not supported, use device_add\n");
+ return -1;
+ }
return 0;
}
@@ -126,18 +132,22 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
{
- int dom, pci_bus;
+ int pci_bus;
unsigned slot;
+ PCIBus *root = pci_get_primary_bus();
PCIDevice *dev;
const char *pci_addr = qdict_get_str(qdict, "pci_addr");
switch (dinfo->type) {
case IF_SCSI:
- if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) {
+ if (!root) {
+ monitor_printf(mon, "no primary PCI bus\n");
+ goto err;
+ }
+ if (pci_read_devaddr(mon, pci_addr, &pci_bus, &slot)) {
goto err;
}
- dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
- PCI_DEVFN(slot, 0));
+ dev = pci_find_device(root, pci_bus, PCI_DEVFN(slot, 0));
if (!dev) {
monitor_printf(mon, "no pci device with address %s\n", pci_addr);
goto err;
@@ -273,16 +283,22 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
{
+ PCIBus *root = pci_get_primary_bus();
PCIDevice *d;
- int dom, bus;
+ int bus;
unsigned slot;
Error *local_err = NULL;
- if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
+ if (!root) {
+ monitor_printf(mon, "no primary PCI bus\n");
+ return -1;
+ }
+
+ if (pci_read_devaddr(mon, pci_addr, &bus, &slot)) {
return -1;
}
- d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
+ d = pci_find_device(root, bus, PCI_DEVFN(slot, 0));
if (!d) {
monitor_printf(mon, "slot %d empty\n", slot);
return -1;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 9906e84..9503d56 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -246,12 +246,12 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
QLIST_INSERT_HEAD(&host_buses, host, next);
}
-PCIBus *pci_find_root_bus(int domain)
+PCIBus *pci_get_primary_bus(void)
{
struct PCIHostBus *host;
QLIST_FOREACH(host, &host_buses, next) {
- if (host->domain == domain) {
+ if (host->domain == 0) {
return host->bus;
}
}
@@ -583,20 +583,31 @@ int pci_parse_devaddr(const char *addr, int *domp, int *busp,
PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
{
+ PCIBus *root = pci_get_primary_bus();
int dom, bus;
unsigned slot;
+ if (!root) {
+ fprintf(stderr, "No primary PCI bus\n");
+ return NULL;
+ }
+
if (!devaddr) {
*devfnp = -1;
- return pci_find_bus_nr(pci_find_root_bus(0), 0);
+ return pci_find_bus_nr(root, 0);
}
if (pci_parse_devaddr(devaddr, &dom, &bus, &slot, NULL) < 0) {
return NULL;
}
+ if (dom != 0) {
+ fprintf(stderr, "No support for non-zero PCI domains\n");
+ return NULL;
+ }
+
*devfnp = PCI_DEVFN(slot, 0);
- return pci_find_bus_nr(pci_find_root_bus(dom), bus);
+ return pci_find_bus_nr(root, bus);
}
static void pci_init_cmask(PCIDevice *dev)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 3ef2ee1..38682e8 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -390,7 +390,7 @@ int pci_bus_num(PCIBus *s);
void pci_for_each_device(PCIBus *bus, int bus_num,
void (*fn)(PCIBus *bus, PCIDevice *d, void *opaque),
void *opaque);
-PCIBus *pci_find_root_bus(int domain);
+PCIBus *pci_get_primary_bus(void);
int pci_find_domain(const PCIBus *bus);
PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
int pci_qdev_find_device(const char *id, PCIDevice **pdev);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 4/8] pci: Use helper o find device's root bus in pci_find_domain()
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
` (2 preceding siblings ...)
2013-05-09 0:31 ` [Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus() David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
` (5 subsequent siblings)
9 siblings, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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 55441c6..98b4c18 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -274,7 +274,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 9503d56..f1cee73 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -259,18 +259,24 @@ PCIBus *pci_get_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;
}
}
@@ -2002,7 +2008,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;
@@ -2157,7 +2163,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 38682e8..1383cfe 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_get_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.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
` (3 preceding siblings ...)
2013-05-09 0:31 ` [Qemu-devel] [PATCH 4/8] pci: Use helper o find device's root bus in pci_find_domain() David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-23 11:04 ` Michael S. Tsirkin
2013-05-09 0:31 ` [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus David Gibson
` (4 subsequent siblings)
9 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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/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 ++++++++++
8 files changed, 57 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/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index 98b4c18..d26674d 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -273,8 +273,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 f1cee73..a3c192c 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,
@@ -2005,10 +2007,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;
@@ -2142,30 +2144,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 1383cfe..1b50dc0 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_get_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.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
` (4 preceding siblings ...)
2013-05-09 0:31 ` [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-23 11:01 ` Michael S. Tsirkin
2013-05-23 11:22 ` Michael S. Tsirkin
2013-05-09 0:31 ` [Qemu-devel] [PATCH 7/8] pci: Remove domain from PCIHostBus David Gibson
` (3 subsequent siblings)
9 siblings, 2 replies; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: qemu-devel, David Gibson
Currently pci_get_primary_bus() searches the list of root buses for one
with domain 0. But since host buses are always registered with domain 0,
this just amounts to finding the only PCI host bus.
This simplifies the implementation by defining the primary PCI bus to
be the first one registered, using a global variable to track it.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/pci/pci.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index a3c192c..b25a1a1 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -96,6 +96,7 @@ struct PCIHostBus {
QLIST_ENTRY(PCIHostBus) next;
};
static QLIST_HEAD(, PCIHostBus) host_buses;
+static PCIBus *pci_primary_bus;
static const VMStateDescription vmstate_pcibus = {
.name = "PCIBUS",
@@ -241,6 +242,12 @@ static int pcibus_reset(BusState *qbus)
static void pci_host_bus_register(int domain, PCIBus *bus)
{
struct PCIHostBus *host;
+
+ /* If this is the first one, assume it's the primary bus */
+ if (!pci_primary_bus) {
+ pci_primary_bus = bus;
+ }
+
host = g_malloc0(sizeof(*host));
host->domain = domain;
host->bus = bus;
@@ -249,15 +256,7 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
PCIBus *pci_get_primary_bus(void)
{
- struct PCIHostBus *host;
-
- QLIST_FOREACH(host, &host_buses, next) {
- if (host->domain == 0) {
- return host->bus;
- }
- }
-
- return NULL;
+ return pci_primary_bus;
}
PCIBus *pci_device_root_bus(const PCIDevice *d)
@@ -300,6 +299,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
/* host bridge */
QLIST_INIT(&bus->child);
+
pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
vmstate_register(NULL, -1, &vmstate_pcibus, bus);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 7/8] pci: Remove domain from PCIHostBus
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
` (5 preceding siblings ...)
2013-05-09 0:31 ` [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 8/8] pci: Fold host_buses list into PCIHostState functionality David Gibson
` (2 subsequent siblings)
9 siblings, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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 | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b25a1a1..716f856 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;
};
@@ -239,7 +238,7 @@ 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;
@@ -249,7 +248,6 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
}
host = g_malloc0(sizeof(*host));
- host->domain = domain;
host->bus = bus;
QLIST_INSERT_HEAD(&host_buses, host, next);
}
@@ -300,7 +298,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
/* host bridge */
QLIST_INIT(&bus->child);
- pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
+ pci_host_bus_register(bus);
vmstate_register(NULL, -1, &vmstate_pcibus, bus);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH 8/8] pci: Fold host_buses list into PCIHostState functionality
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
` (6 preceding siblings ...)
2013-05-09 0:31 ` [Qemu-devel] [PATCH 7/8] pci: Remove domain from PCIHostBus David Gibson
@ 2013-05-09 0:31 ` David Gibson
2013-05-14 10:53 ` [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses Michael S. Tsirkin
2013-05-23 11:12 ` Michael S. Tsirkin
9 siblings, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-09 0:31 UTC (permalink / raw)
To: aliguori, mst; +Cc: 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 | 28 +++++++++++-----------------
include/hw/pci/pci_host.h | 2 ++
2 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 716f856..cb18862 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 PCIBus *pci_primary_bus;
static const VMStateDescription vmstate_pcibus = {
@@ -238,18 +234,16 @@ 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;
+ PCIHostState *host_bridge = PCI_HOST_BRIDGE(parent);
/* If this is the first one, assume it's the primary bus */
if (!pci_primary_bus) {
pci_primary_bus = bus;
}
- host = g_malloc0(sizeof(*host));
- host->bus = bus;
- QLIST_INSERT_HEAD(&host_buses, host, next);
+ QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
}
PCIBus *pci_get_primary_bus(void)
@@ -298,7 +292,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);
}
@@ -1531,11 +1525,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) {
@@ -2199,11 +2193,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.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
` (7 preceding siblings ...)
2013-05-09 0:31 ` [Qemu-devel] [PATCH 8/8] pci: Fold host_buses list into PCIHostState functionality David Gibson
@ 2013-05-14 10:53 ` Michael S. Tsirkin
2013-05-23 11:12 ` Michael S. Tsirkin
9 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-14 10:53 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 09, 2013 at 10:31:04AM +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.
I absolutely agree this area needs to be cleaned up.
Going on vacation and haven't had a chance to look at the
patches yet. Sorry. Will respond early next week, hopefully.
Thanks for your patience,
--
MST
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-09 0:31 ` [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus David Gibson
@ 2013-05-23 11:01 ` Michael S. Tsirkin
2013-05-23 12:16 ` David Gibson
2013-05-23 11:22 ` Michael S. Tsirkin
1 sibling, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 11:01 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> Currently pci_get_primary_bus() searches the list of root buses for one
> with domain 0. But since host buses are always registered with domain 0,
> this just amounts to finding the only PCI host bus.
>
> This simplifies the implementation by defining the primary PCI bus to
> be the first one registered, using a global variable to track it.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This is the only part that I dislike.
How about an explicit API to set the primary bus?
Let machine types set it.
> ---
> hw/pci/pci.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index a3c192c..b25a1a1 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -96,6 +96,7 @@ struct PCIHostBus {
> QLIST_ENTRY(PCIHostBus) next;
> };
> static QLIST_HEAD(, PCIHostBus) host_buses;
> +static PCIBus *pci_primary_bus;
>
> static const VMStateDescription vmstate_pcibus = {
> .name = "PCIBUS",
> @@ -241,6 +242,12 @@ static int pcibus_reset(BusState *qbus)
> static void pci_host_bus_register(int domain, PCIBus *bus)
> {
> struct PCIHostBus *host;
> +
> + /* If this is the first one, assume it's the primary bus */
> + if (!pci_primary_bus) {
> + pci_primary_bus = bus;
> + }
> +
> host = g_malloc0(sizeof(*host));
> host->domain = domain;
> host->bus = bus;
> @@ -249,15 +256,7 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
>
> PCIBus *pci_get_primary_bus(void)
> {
> - struct PCIHostBus *host;
> -
> - QLIST_FOREACH(host, &host_buses, next) {
> - if (host->domain == 0) {
> - return host->bus;
> - }
> - }
> -
> - return NULL;
> + return pci_primary_bus;
> }
>
> PCIBus *pci_device_root_bus(const PCIDevice *d)
> @@ -300,6 +299,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>
> /* host bridge */
> QLIST_INIT(&bus->child);
> +
> pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
>
> vmstate_register(NULL, -1, &vmstate_pcibus, bus);
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-09 0:31 ` [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
@ 2013-05-23 11:04 ` Michael S. Tsirkin
2013-05-23 12:21 ` David Gibson
2013-05-23 14:51 ` Paolo Bonzini
0 siblings, 2 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 11:04 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 09, 2013 at 10:31:09AM +1000, David Gibson wrote:
> 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>
AFAIK PC is the only one with working migration, yes, but
we have Q35 as well which can be migrated.
> ---
> hw/pci-host/piix.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 ++++++++++
> 8 files changed, 57 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/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> index 98b4c18..d26674d 100644
> --- a/hw/pci/pci-hotplug-old.c
> +++ b/hw/pci/pci-hotplug-old.c
> @@ -273,8 +273,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 f1cee73..a3c192c 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,
> @@ -2005,10 +2007,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;
> @@ -2142,30 +2144,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 1383cfe..1b50dc0 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_get_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.7.10.4
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c
2013-05-09 0:31 ` [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c David Gibson
@ 2013-05-23 11:11 ` Michael S. Tsirkin
2013-05-23 12:23 ` David Gibson
2013-05-24 7:44 ` David Gibson
2013-05-23 14:54 ` Paolo Bonzini
1 sibling, 2 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 11:11 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 09, 2013 at 10:31:05AM +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 | 290 +++++++++++++++++++++++++++++++++++
> hw/pci/pci-hotplug.c | 292 ------------------------------------
> 7 files changed, 295 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..2ad32b6 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
> +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..724a80b
> --- /dev/null
> +++ b/hw/pci/pci-hotplug-old.c
Please use git format-patch -M so renames are
properly shown. I can't see whether anything changed here.
> @@ -0,0 +1,290 @@
> +/*
> + * QEMU PCI hotplug support
I don't particularly mind whether it's called pci-hotplug or
pci-hotplug-old, but I'm prepared to go with you on this.
However, what we really should do is fix the comment
to match reality.
I think this needs two commits:
- fix comment
- rename
to make format-patch -M detect this as a rename
> + *
> + * 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.7.10.4
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
` (8 preceding siblings ...)
2013-05-14 10:53 ` [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses Michael S. Tsirkin
@ 2013-05-23 11:12 ` Michael S. Tsirkin
9 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 11:12 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 09, 2013 at 10:31:04AM +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.
I agree with the direction this patchset takes.
I sent some comments on the individual patches separately.
Thanks!
--
MST
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-09 0:31 ` [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus David Gibson
2013-05-23 11:01 ` Michael S. Tsirkin
@ 2013-05-23 11:22 ` Michael S. Tsirkin
2013-05-23 12:16 ` David Gibson
1 sibling, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 11:22 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> Currently pci_get_primary_bus() searches the list of root buses for one
> with domain 0. But since host buses are always registered with domain 0,
> this just amounts to finding the only PCI host bus.
>
> This simplifies the implementation by defining the primary PCI bus to
> be the first one registered, using a global variable to track it.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Or better: can we just fail if there is more than
one root?
> ---
> hw/pci/pci.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index a3c192c..b25a1a1 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -96,6 +96,7 @@ struct PCIHostBus {
> QLIST_ENTRY(PCIHostBus) next;
> };
> static QLIST_HEAD(, PCIHostBus) host_buses;
> +static PCIBus *pci_primary_bus;
>
> static const VMStateDescription vmstate_pcibus = {
> .name = "PCIBUS",
> @@ -241,6 +242,12 @@ static int pcibus_reset(BusState *qbus)
> static void pci_host_bus_register(int domain, PCIBus *bus)
> {
> struct PCIHostBus *host;
> +
> + /* If this is the first one, assume it's the primary bus */
> + if (!pci_primary_bus) {
> + pci_primary_bus = bus;
> + }
> +
> host = g_malloc0(sizeof(*host));
> host->domain = domain;
> host->bus = bus;
> @@ -249,15 +256,7 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
>
> PCIBus *pci_get_primary_bus(void)
> {
> - struct PCIHostBus *host;
> -
> - QLIST_FOREACH(host, &host_buses, next) {
> - if (host->domain == 0) {
> - return host->bus;
> - }
> - }
> -
> - return NULL;
> + return pci_primary_bus;
> }
>
> PCIBus *pci_device_root_bus(const PCIDevice *d)
> @@ -300,6 +299,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>
> /* host bridge */
> QLIST_INIT(&bus->child);
> +
> pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
>
> vmstate_register(NULL, -1, &vmstate_pcibus, bus);
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus()
2013-05-09 0:31 ` [Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus() David Gibson
@ 2013-05-23 11:26 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 11:26 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 09, 2013 at 10:31:07AM +1000, David Gibson wrote:
> pci_find_root_bus() takes a domain parameter. Currently PCI root buses
> with domain other than 0 can't be created, so this is more or less a long
> winded way of retrieving the main PCI root bus. Numbered domains don't
> actually properly cover the (non x86) possibilities for multiple PCI root
> buses, so this patch for now enforces the domain == 0 restriction in other
> places to replace pci_find_root_bus() with an explicit
> pci_get_primary_bus().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Can we instead just drop the domain parameter
from pci_find_root_bus and add a comment
saying that it fails if there is more than one root?
> ---
> hw/pci/pci-hotplug-old.c | 34 +++++++++++++++++++++++++---------
> hw/pci/pci.c | 19 +++++++++++++++----
> include/hw/pci/pci.h | 2 +-
> 3 files changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> index 1aa0ab8..55441c6 100644
> --- a/hw/pci/pci-hotplug-old.c
> +++ b/hw/pci/pci-hotplug-old.c
> @@ -34,17 +34,23 @@
> #include "sysemu/blockdev.h"
> #include "qapi/error.h"
>
> -static int pci_read_devaddr(Monitor *mon, const char *addr, int *domp,
> +static int pci_read_devaddr(Monitor *mon, const char *addr,
> int *busp, unsigned *slotp)
> {
> + int dom;
> +
> /* strip legacy tag */
> if (!strncmp(addr, "pci_addr=", 9)) {
> addr += 9;
> }
> - if (pci_parse_devaddr(addr, domp, busp, slotp, NULL)) {
> + if (pci_parse_devaddr(addr, &dom, busp, slotp, NULL)) {
> monitor_printf(mon, "Invalid pci address\n");
> return -1;
> }
> + if (dom != 0) {
> + monitor_printf(mon, "Multiple PCI domains not supported, use device_add\n");
> + return -1;
> + }
> return 0;
> }
>
> @@ -126,18 +132,22 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
>
> int pci_drive_hot_add(Monitor *mon, const QDict *qdict, DriveInfo *dinfo)
> {
> - int dom, pci_bus;
> + int pci_bus;
> unsigned slot;
> + PCIBus *root = pci_get_primary_bus();
> PCIDevice *dev;
> const char *pci_addr = qdict_get_str(qdict, "pci_addr");
>
> switch (dinfo->type) {
> case IF_SCSI:
> - if (pci_read_devaddr(mon, pci_addr, &dom, &pci_bus, &slot)) {
> + if (!root) {
> + monitor_printf(mon, "no primary PCI bus\n");
> + goto err;
> + }
> + if (pci_read_devaddr(mon, pci_addr, &pci_bus, &slot)) {
> goto err;
> }
> - dev = pci_find_device(pci_find_root_bus(dom), pci_bus,
> - PCI_DEVFN(slot, 0));
> + dev = pci_find_device(root, pci_bus, PCI_DEVFN(slot, 0));
> if (!dev) {
> monitor_printf(mon, "no pci device with address %s\n", pci_addr);
> goto err;
> @@ -273,16 +283,22 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
>
> static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
> {
> + PCIBus *root = pci_get_primary_bus();
> PCIDevice *d;
> - int dom, bus;
> + int bus;
> unsigned slot;
> Error *local_err = NULL;
>
> - if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
> + if (!root) {
> + monitor_printf(mon, "no primary PCI bus\n");
> + return -1;
> + }
> +
> + if (pci_read_devaddr(mon, pci_addr, &bus, &slot)) {
> return -1;
> }
>
> - d = pci_find_device(pci_find_root_bus(dom), bus, PCI_DEVFN(slot, 0));
> + d = pci_find_device(root, bus, PCI_DEVFN(slot, 0));
> if (!d) {
> monitor_printf(mon, "slot %d empty\n", slot);
> return -1;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 9906e84..9503d56 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -246,12 +246,12 @@ static void pci_host_bus_register(int domain, PCIBus *bus)
> QLIST_INSERT_HEAD(&host_buses, host, next);
> }
>
> -PCIBus *pci_find_root_bus(int domain)
> +PCIBus *pci_get_primary_bus(void)
> {
> struct PCIHostBus *host;
>
> QLIST_FOREACH(host, &host_buses, next) {
> - if (host->domain == domain) {
> + if (host->domain == 0) {
> return host->bus;
> }
> }
> @@ -583,20 +583,31 @@ int pci_parse_devaddr(const char *addr, int *domp, int *busp,
>
> PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr)
> {
> + PCIBus *root = pci_get_primary_bus();
> int dom, bus;
> unsigned slot;
>
> + if (!root) {
> + fprintf(stderr, "No primary PCI bus\n");
> + return NULL;
> + }
> +
> if (!devaddr) {
> *devfnp = -1;
> - return pci_find_bus_nr(pci_find_root_bus(0), 0);
> + return pci_find_bus_nr(root, 0);
> }
>
> if (pci_parse_devaddr(devaddr, &dom, &bus, &slot, NULL) < 0) {
> return NULL;
> }
>
> + if (dom != 0) {
> + fprintf(stderr, "No support for non-zero PCI domains\n");
> + return NULL;
> + }
> +
> *devfnp = PCI_DEVFN(slot, 0);
> - return pci_find_bus_nr(pci_find_root_bus(dom), bus);
> + return pci_find_bus_nr(root, bus);
> }
>
> static void pci_init_cmask(PCIDevice *dev)
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 3ef2ee1..38682e8 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -390,7 +390,7 @@ int pci_bus_num(PCIBus *s);
> void pci_for_each_device(PCIBus *bus, int bus_num,
> void (*fn)(PCIBus *bus, PCIDevice *d, void *opaque),
> void *opaque);
> -PCIBus *pci_find_root_bus(int domain);
> +PCIBus *pci_get_primary_bus(void);
> int pci_find_domain(const PCIBus *bus);
> PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
> int pci_qdev_find_device(const char *id, PCIDevice **pdev);
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-23 11:01 ` Michael S. Tsirkin
@ 2013-05-23 12:16 ` David Gibson
2013-05-23 14:39 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-05-23 12:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 993 bytes --]
On Thu, May 23, 2013 at 02:01:57PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > Currently pci_get_primary_bus() searches the list of root buses for one
> > with domain 0. But since host buses are always registered with domain 0,
> > this just amounts to finding the only PCI host bus.
> >
> > This simplifies the implementation by defining the primary PCI bus to
> > be the first one registered, using a global variable to track it.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> This is the only part that I dislike.
> How about an explicit API to set the primary bus?
> Let machine types set it.
I guess, though I was hoping to avoid changing every bit of platform
code that sets up a PCI bus.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-23 11:22 ` Michael S. Tsirkin
@ 2013-05-23 12:16 ` David Gibson
2013-05-29 9:43 ` David Gibson
0 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-05-23 12:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 881 bytes --]
On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > Currently pci_get_primary_bus() searches the list of root buses for one
> > with domain 0. But since host buses are always registered with domain 0,
> > this just amounts to finding the only PCI host bus.
> >
> > This simplifies the implementation by defining the primary PCI bus to
> > be the first one registered, using a global variable to track it.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> Or better: can we just fail if there is more than
> one root?
That might work, I'll look into doing that.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-23 11:04 ` Michael S. Tsirkin
@ 2013-05-23 12:21 ` David Gibson
2013-05-23 14:51 ` Paolo Bonzini
1 sibling, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-23 12:21 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]
On Thu, May 23, 2013 at 02:04:08PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 09, 2013 at 10:31:09AM +1000, David Gibson wrote:
> > 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>
>
> AFAIK PC is the only one with working migration, yes, but
> we have Q35 as well which can be migrated.
Good point, I'll add a similar hook to q35.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c
2013-05-23 11:11 ` Michael S. Tsirkin
@ 2013-05-23 12:23 ` David Gibson
2013-05-24 7:44 ` David Gibson
1 sibling, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-23 12:23 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]
On Thu, May 23, 2013 at 02:11:35PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 09, 2013 at 10:31:05AM +1000, David Gibson wrote:
[snip]
> > diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> > new file mode 100644
> > index 0000000..724a80b
> > --- /dev/null
> > +++ b/hw/pci/pci-hotplug-old.c
>
> Please use git format-patch -M so renames are
> properly shown. I can't see whether anything changed here.
Ah, sorry. Forgot that wasn't the default.
> > @@ -0,0 +1,290 @@
> > +/*
> > + * QEMU PCI hotplug support
>
> I don't particularly mind whether it's called pci-hotplug or
> pci-hotplug-old, but I'm prepared to go with you on this.
> However, what we really should do is fix the comment
> to match reality.
Good point.
> I think this needs two commits:
> - fix comment
> - rename
>
> to make format-patch -M detect this as a rename
Hrm, I think it should detect the rename, even with a change to the
comment. But I'll check to make sure, and split if necessary.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-23 12:16 ` David Gibson
@ 2013-05-23 14:39 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 14:39 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel
On Thu, May 23, 2013 at 10:16:13PM +1000, David Gibson wrote:
> On Thu, May 23, 2013 at 02:01:57PM +0300, Michael S. Tsirkin wrote:
> > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > with domain 0. But since host buses are always registered with domain 0,
> > > this just amounts to finding the only PCI host bus.
> > >
> > > This simplifies the implementation by defining the primary PCI bus to
> > > be the first one registered, using a global variable to track it.
> > >
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > This is the only part that I dislike.
> > How about an explicit API to set the primary bus?
> > Let machine types set it.
>
> I guess, though I was hoping to avoid changing every bit of platform
> code that sets up a PCI bus.
Yes, that's a lot of churn.
Maybe we don't need a primary bus at all?
If there's one root, it's simple: check
it's the only one and return.
> --
> 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-23 11:04 ` Michael S. Tsirkin
2013-05-23 12:21 ` David Gibson
@ 2013-05-23 14:51 ` Paolo Bonzini
2013-05-23 14:57 ` Michael S. Tsirkin
1 sibling, 1 reply; 40+ messages in thread
From: Paolo Bonzini @ 2013-05-23 14:51 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, David Gibson
Il 23/05/2013 13:04, Michael S. Tsirkin ha scritto:
>> > 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>
> AFAIK PC is the only one with working migration, yes, but
> we have Q35 as well which can be migrated.
Are we already supporting backwards/forwards migration with Q35?
Paolo
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c
2013-05-09 0:31 ` [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c David Gibson
2013-05-23 11:11 ` Michael S. Tsirkin
@ 2013-05-23 14:54 ` Paolo Bonzini
2013-05-24 7:46 ` David Gibson
1 sibling, 1 reply; 40+ messages in thread
From: Paolo Bonzini @ 2013-05-23 14:54 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, mst
Il 09/05/2013 02:31, David Gibson ha scritto:
> diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> index a7fb9d0..2ad32b6 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
> +obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o
With these changes, pci-hotplug-old.o can be compiled just once. Please
s/^obj-/common-obj-/.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-23 14:51 ` Paolo Bonzini
@ 2013-05-23 14:57 ` Michael S. Tsirkin
2013-05-23 15:00 ` Paolo Bonzini
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 14:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: aliguori, qemu-devel, David Gibson
On Thu, May 23, 2013 at 04:51:17PM +0200, Paolo Bonzini wrote:
> Il 23/05/2013 13:04, Michael S. Tsirkin ha scritto:
> >> > 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>
> > AFAIK PC is the only one with working migration, yes, but
> > we have Q35 as well which can be migrated.
>
> Are we already supporting backwards/forwards migration with Q35?
>
> Paolo
We released 1.5 with Q35 so we better ...
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-23 14:57 ` Michael S. Tsirkin
@ 2013-05-23 15:00 ` Paolo Bonzini
2013-05-23 15:06 ` Michael S. Tsirkin
2013-05-24 7:40 ` David Gibson
0 siblings, 2 replies; 40+ messages in thread
From: Paolo Bonzini @ 2013-05-23 15:00 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, David Gibson
Il 23/05/2013 16:57, Michael S. Tsirkin ha scritto:
> On Thu, May 23, 2013 at 04:51:17PM +0200, Paolo Bonzini wrote:
>> Il 23/05/2013 13:04, Michael S. Tsirkin ha scritto:
>>>>> 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>
>>> AFAIK PC is the only one with working migration, yes, but
>>> we have Q35 as well which can be migrated.
>>
>> Are we already supporting backwards/forwards migration with Q35?
>
> We released 1.5 with Q35 so we better ...
We released 1.5 with Q35 migration disabled by default, IIRC to enable
it you need to remove the AHCI controller with -nodefaults. I think
bending the rules is still reasonable.
Paolo
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-23 15:00 ` Paolo Bonzini
@ 2013-05-23 15:06 ` Michael S. Tsirkin
2013-05-24 7:40 ` David Gibson
1 sibling, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-23 15:06 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, agraf, aliguori, qemu-devel, David Gibson
On Thu, May 23, 2013 at 05:00:39PM +0200, Paolo Bonzini wrote:
> Il 23/05/2013 16:57, Michael S. Tsirkin ha scritto:
> > On Thu, May 23, 2013 at 04:51:17PM +0200, Paolo Bonzini wrote:
> >> Il 23/05/2013 13:04, Michael S. Tsirkin ha scritto:
> >>>>> 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>
> >>> AFAIK PC is the only one with working migration, yes, but
> >>> we have Q35 as well which can be migrated.
> >>
> >> Are we already supporting backwards/forwards migration with Q35?
> >
> > We released 1.5 with Q35 so we better ...
>
> We released 1.5 with Q35 migration disabled by default, IIRC to enable
> it you need to remove the AHCI controller with -nodefaults. I think
> bending the rules is still reasonable.
>
> Paolo
Which reminds me, does someone plan to look into fixing
migration for AHCI?
--
MST
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path()
2013-05-23 15:00 ` Paolo Bonzini
2013-05-23 15:06 ` Michael S. Tsirkin
@ 2013-05-24 7:40 ` David Gibson
1 sibling, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-24 7:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: aliguori, qemu-devel, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 2261 bytes --]
On Thu, May 23, 2013 at 05:00:39PM +0200, Paolo Bonzini wrote:
> Il 23/05/2013 16:57, Michael S. Tsirkin ha scritto:
> > On Thu, May 23, 2013 at 04:51:17PM +0200, Paolo Bonzini wrote:
> >> Il 23/05/2013 13:04, Michael S. Tsirkin ha scritto:
> >>>>> 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>
> >>> AFAIK PC is the only one with working migration, yes, but
> >>> we have Q35 as well which can be migrated.
> >>
> >> Are we already supporting backwards/forwards migration with Q35?
> >
> > We released 1.5 with Q35 so we better ...
>
> We released 1.5 with Q35 migration disabled by default, IIRC to enable
> it you need to remove the AHCI controller with -nodefaults. I think
> bending the rules is still reasonable.
For my purposes, it doesn't much matter. Naming the PCI root buses by
domain number makes sense for Q35, as it does for PIIX. I've revised
my patch to add a suitable root_bus_path hook for q35.
Also, I've set up a github tree to publish this amongst other things:
git://github.com/dgibson/qemu.git
Branch 'pci'.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c
2013-05-23 11:11 ` Michael S. Tsirkin
2013-05-23 12:23 ` David Gibson
@ 2013-05-24 7:44 ` David Gibson
1 sibling, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-24 7:44 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]
On Thu, May 23, 2013 at 02:11:35PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 09, 2013 at 10:31:05AM +1000, David Gibson wrote:
[snip]
> > diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> > index a7fb9d0..2ad32b6 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
> > +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..724a80b
> > --- /dev/null
> > +++ b/hw/pci/pci-hotplug-old.c
>
> Please use git format-patch -M so renames are
> properly shown. I can't see whether anything changed here.
>
> > @@ -0,0 +1,290 @@
> > +/*
> > + * QEMU PCI hotplug support
>
> I don't particularly mind whether it's called pci-hotplug or
> pci-hotplug-old, but I'm prepared to go with you on this.
> However, what we really should do is fix the comment
> to match reality.
Good point. Comment updated in my tree.
> I think this needs two commits:
> - fix comment
> - rename
>
> to make format-patch -M detect this as a rename
I verified that even with the comment change and rename in the same
patch, git format-patch -M detects the rename.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c
2013-05-23 14:54 ` Paolo Bonzini
@ 2013-05-24 7:46 ` David Gibson
0 siblings, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-24 7:46 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: aliguori, qemu-devel, mst
[-- Attachment #1: Type: text/plain, Size: 877 bytes --]
On Thu, May 23, 2013 at 04:54:17PM +0200, Paolo Bonzini wrote:
> Il 09/05/2013 02:31, David Gibson ha scritto:
> > diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> > index a7fb9d0..2ad32b6 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
> > +obj-$(CONFIG_PCI_HOTPLUG_OLD) += pci-hotplug-old.o
>
> With these changes, pci-hotplug-old.o can be compiled just once. Please
> s/^obj-/common-obj-/.
Done, thanks.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-23 12:16 ` David Gibson
@ 2013-05-29 9:43 ` David Gibson
2013-05-29 9:47 ` David Gibson
2013-05-29 9:55 ` Michael S. Tsirkin
0 siblings, 2 replies; 40+ messages in thread
From: David Gibson @ 2013-05-29 9:43 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]
On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > with domain 0. But since host buses are always registered with domain 0,
> > > this just amounts to finding the only PCI host bus.
> > >
> > > This simplifies the implementation by defining the primary PCI bus to
> > > be the first one registered, using a global variable to track it.
> > >
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > Or better: can we just fail if there is more than
> > one root?
>
> That might work, I'll look into doing that.
So, the difficulty with this is that then any machine with multiple
PCI bridges could not use pci_nic_init(), since it calls
pci_get_bus_devfn() which calls pci_find_primary_bus() which would
always fail. And using pci_nic_init() is more or less mandatory in
the machine_init function to support old-style nic configuration.
Suggestions?
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-29 9:43 ` David Gibson
@ 2013-05-29 9:47 ` David Gibson
2013-05-29 9:55 ` Michael S. Tsirkin
1 sibling, 0 replies; 40+ messages in thread
From: David Gibson @ 2013-05-29 9:47 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > with domain 0. But since host buses are always registered with domain 0,
> > > > this just amounts to finding the only PCI host bus.
> > > >
> > > > This simplifies the implementation by defining the primary PCI bus to
> > > > be the first one registered, using a global variable to track it.
> > > >
> > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > >
> > > Or better: can we just fail if there is more than
> > > one root?
> >
> > That might work, I'll look into doing that.
>
> So, the difficulty with this is that then any machine with multiple
> PCI bridges could not use pci_nic_init(), since it calls
> pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> always fail. And using pci_nic_init() is more or less mandatory in
> the machine_init function to support old-style nic configuration.
>
> Suggestions?
Oh, fwiw, my latest work-in-progress can be had at
git://github.com/dgibson/qemu.git ('pci' branch)
This is the only comment remaining to be addressed from the last
round, so I'm hoping we can come to some consensus here and I'll
repost.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-29 9:43 ` David Gibson
2013-05-29 9:47 ` David Gibson
@ 2013-05-29 9:55 ` Michael S. Tsirkin
2013-05-29 10:06 ` David Gibson
1 sibling, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-29 9:55 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, David Gibson
On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > with domain 0. But since host buses are always registered with domain 0,
> > > > this just amounts to finding the only PCI host bus.
> > > >
> > > > This simplifies the implementation by defining the primary PCI bus to
> > > > be the first one registered, using a global variable to track it.
> > > >
> > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > >
> > > Or better: can we just fail if there is more than
> > > one root?
> >
> > That might work, I'll look into doing that.
>
> So, the difficulty with this is that then any machine with multiple
> PCI bridges could not use pci_nic_init(), since it calls
> pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> always fail. And using pci_nic_init() is more or less mandatory in
> the machine_init function to support old-style nic configuration.
>
> Suggestions?
You mean multiple PCI roots?
Well, there are no legacy machines with multiple roots to support, are
there? So why do we need to support legacy flags for these new
configurations?
> --
> 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-29 9:55 ` Michael S. Tsirkin
@ 2013-05-29 10:06 ` David Gibson
2013-05-29 10:17 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-05-29 10:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, David Gibson
[-- Attachment #1: Type: text/plain, Size: 1926 bytes --]
On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > > with domain 0. But since host buses are always registered with domain 0,
> > > > > this just amounts to finding the only PCI host bus.
> > > > >
> > > > > This simplifies the implementation by defining the primary PCI bus to
> > > > > be the first one registered, using a global variable to track it.
> > > > >
> > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > >
> > > > Or better: can we just fail if there is more than
> > > > one root?
> > >
> > > That might work, I'll look into doing that.
> >
> > So, the difficulty with this is that then any machine with multiple
> > PCI bridges could not use pci_nic_init(), since it calls
> > pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> > always fail. And using pci_nic_init() is more or less mandatory in
> > the machine_init function to support old-style nic configuration.
> >
> > Suggestions?
>
> You mean multiple PCI roots?
> Well, there are no legacy machines with multiple roots to support, are
> there? So why do we need to support legacy flags for these new
> configurations?
Because people expect them. Plus on spapr we already support the
legacy nic options; it would be very strange for them to suddenly
break when we add a second host bridge.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-29 10:06 ` David Gibson
@ 2013-05-29 10:17 ` Michael S. Tsirkin
2013-05-29 11:04 ` David Gibson
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-29 10:17 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, David Gibson
On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > > > with domain 0. But since host buses are always registered with domain 0,
> > > > > > this just amounts to finding the only PCI host bus.
> > > > > >
> > > > > > This simplifies the implementation by defining the primary PCI bus to
> > > > > > be the first one registered, using a global variable to track it.
> > > > > >
> > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > >
> > > > > Or better: can we just fail if there is more than
> > > > > one root?
> > > >
> > > > That might work, I'll look into doing that.
> > >
> > > So, the difficulty with this is that then any machine with multiple
> > > PCI bridges could not use pci_nic_init(), since it calls
> > > pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> > > always fail. And using pci_nic_init() is more or less mandatory in
> > > the machine_init function to support old-style nic configuration.
> > >
> > > Suggestions?
> >
> > You mean multiple PCI roots?
> > Well, there are no legacy machines with multiple roots to support, are
> > there? So why do we need to support legacy flags for these new
> > configurations?
>
> Because people expect them.
People can learn, somehow they will learn to add a new root, so they can
learn to use -device too.
So let's make it fail on multiple roots, and output a message along the
lines of "please use -device virtio-net-pci instead".
> Plus on spapr we already support the
> legacy nic options; it would be very strange for them to suddenly
> break when we add a second host bridge.
Not sure who "we" is here. IMHO user should ask for a new
machine type with two roots explicitly.
> --
> 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-29 10:17 ` Michael S. Tsirkin
@ 2013-05-29 11:04 ` David Gibson
2013-05-29 12:22 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-05-29 11:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, David Gibson
[-- Attachment #1: Type: text/plain, Size: 3530 bytes --]
On Wed, May 29, 2013 at 01:17:13PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> > On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > > > > with domain 0. But since host buses are always registered with domain 0,
> > > > > > > this just amounts to finding the only PCI host bus.
> > > > > > >
> > > > > > > This simplifies the implementation by defining the primary PCI bus to
> > > > > > > be the first one registered, using a global variable to track it.
> > > > > > >
> > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > > >
> > > > > > Or better: can we just fail if there is more than
> > > > > > one root?
> > > > >
> > > > > That might work, I'll look into doing that.
> > > >
> > > > So, the difficulty with this is that then any machine with multiple
> > > > PCI bridges could not use pci_nic_init(), since it calls
> > > > pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> > > > always fail. And using pci_nic_init() is more or less mandatory in
> > > > the machine_init function to support old-style nic configuration.
> > > >
> > > > Suggestions?
> > >
> > > You mean multiple PCI roots?
> > > Well, there are no legacy machines with multiple roots to support, are
> > > there? So why do we need to support legacy flags for these new
> > > configurations?
> >
> > Because people expect them.
>
> People can learn, somehow they will learn to add a new root, so they can
> learn to use -device too.
Hrm. I'd kind of like a second (third?) opinion on that. Anthony?
> So let's make it fail on multiple roots, and output a message along the
> lines of "please use -device virtio-net-pci instead".
How to produce a meaningful error like that isn't totally obvious,
since the test for multiple roots is down in find_primary_pci_bus()
(or whatever), and once we get back up to pci_nic_init() we just know
that pci_get_bus_devfn() failed for some reason.
> > Plus on spapr we already support the
> > legacy nic options; it would be very strange for them to suddenly
> > break when we add a second host bridge.
>
> Not sure who "we" is here. IMHO user should ask for a new
> machine type with two roots explicitly.
You seem to be thinking of the number of host bridges as a fixed
property of the platform, which it isn't on spapr. PCI host bridges
are just another device. Large scale real hardware can easily have
dozens of them. In qemu we create one always as a convenience, but
users can (and will have to, for vfio) create additional ones
trivially with -device.
[Which raises another complication as a tangent. People (and libvirt)
don't generally expect -nodefaults to remove the PCI bridge, but
arguably it should on spapr, since a PAPR guest with no PCI is
perfectly viable but there's currently no way to specify such a
thing.]
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-29 11:04 ` David Gibson
@ 2013-05-29 12:22 ` Michael S. Tsirkin
2013-05-30 3:34 ` David Gibson
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-29 12:22 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, David Gibson
On Wed, May 29, 2013 at 09:04:00PM +1000, David Gibson wrote:
> On Wed, May 29, 2013 at 01:17:13PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> > > On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > > > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > > > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > > > > > with domain 0. But since host buses are always registered with domain 0,
> > > > > > > > this just amounts to finding the only PCI host bus.
> > > > > > > >
> > > > > > > > This simplifies the implementation by defining the primary PCI bus to
> > > > > > > > be the first one registered, using a global variable to track it.
> > > > > > > >
> > > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > > > >
> > > > > > > Or better: can we just fail if there is more than
> > > > > > > one root?
> > > > > >
> > > > > > That might work, I'll look into doing that.
> > > > >
> > > > > So, the difficulty with this is that then any machine with multiple
> > > > > PCI bridges could not use pci_nic_init(), since it calls
> > > > > pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> > > > > always fail. And using pci_nic_init() is more or less mandatory in
> > > > > the machine_init function to support old-style nic configuration.
> > > > >
> > > > > Suggestions?
> > > >
> > > > You mean multiple PCI roots?
> > > > Well, there are no legacy machines with multiple roots to support, are
> > > > there? So why do we need to support legacy flags for these new
> > > > configurations?
> > >
> > > Because people expect them.
> >
> > People can learn, somehow they will learn to add a new root, so they can
> > learn to use -device too.
>
> Hrm. I'd kind of like a second (third?) opinion on that. Anthony?
>
> > So let's make it fail on multiple roots, and output a message along the
> > lines of "please use -device virtio-net-pci instead".
>
> How to produce a meaningful error like that isn't totally obvious,
> since the test for multiple roots is down in find_primary_pci_bus()
> (or whatever), and once we get back up to pci_nic_init() we just know
> that pci_get_bus_devfn() failed for some reason.
What other possible reason for it to fail?
> > > Plus on spapr we already support the
> > > legacy nic options; it would be very strange for them to suddenly
> > > break when we add a second host bridge.
> >
> > Not sure who "we" is here. IMHO user should ask for a new
> > machine type with two roots explicitly.
>
> You seem to be thinking of the number of host bridges as a fixed
> property of the platform, which it isn't on spapr. PCI host bridges
> are just another device. Large scale real hardware can easily have
> dozens of them.
Absolutely. I'm not thinking of it as fixed.
I'm thinking of the *default* number of pci host bridges
as fixed. If a user is smart enough to use -device to create
a host bridge, said user can learn about -device for creating
a nic.
> In qemu we create one always as a convenience, but
> users can (and will have to, for vfio) create additional ones
> trivially with -device.
So they know about -device then.
> [Which raises another complication as a tangent. People (and libvirt)
> don't generally expect -nodefaults to remove the PCI bridge, but
> arguably it should on spapr, since a PAPR guest with no PCI is
> perfectly viable but there's currently no way to specify such a
> thing.]
I guess the problem is not what they expect generally,
but specifically that some users might rely on spapr with
-nodefaults having PCI?
I don't have any ideas besides introducing a new machine type
that is same as spapr but without the default PCI host bridge.
> --
> 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-29 12:22 ` Michael S. Tsirkin
@ 2013-05-30 3:34 ` David Gibson
2013-05-30 5:02 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-05-30 3:34 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, David Gibson
[-- Attachment #1: Type: text/plain, Size: 4885 bytes --]
On Wed, May 29, 2013 at 03:22:29PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 29, 2013 at 09:04:00PM +1000, David Gibson wrote:
> > On Wed, May 29, 2013 at 01:17:13PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> > > > On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > > > > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > > > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > > > > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > > > > > > with domain 0. But since host buses are always registered with domain 0,
> > > > > > > > > this just amounts to finding the only PCI host bus.
> > > > > > > > >
> > > > > > > > > This simplifies the implementation by defining the primary PCI bus to
> > > > > > > > > be the first one registered, using a global variable to track it.
> > > > > > > > >
> > > > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > > > > >
> > > > > > > > Or better: can we just fail if there is more than
> > > > > > > > one root?
> > > > > > >
> > > > > > > That might work, I'll look into doing that.
> > > > > >
> > > > > > So, the difficulty with this is that then any machine with multiple
> > > > > > PCI bridges could not use pci_nic_init(), since it calls
> > > > > > pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> > > > > > always fail. And using pci_nic_init() is more or less mandatory in
> > > > > > the machine_init function to support old-style nic configuration.
> > > > > >
> > > > > > Suggestions?
> > > > >
> > > > > You mean multiple PCI roots?
> > > > > Well, there are no legacy machines with multiple roots to support, are
> > > > > there? So why do we need to support legacy flags for these new
> > > > > configurations?
> > > >
> > > > Because people expect them.
> > >
> > > People can learn, somehow they will learn to add a new root, so they can
> > > learn to use -device too.
> >
> > Hrm. I'd kind of like a second (third?) opinion on that. Anthony?
> >
> > > So let's make it fail on multiple roots, and output a message along the
> > > lines of "please use -device virtio-net-pci instead".
> >
> > How to produce a meaningful error like that isn't totally obvious,
> > since the test for multiple roots is down in find_primary_pci_bus()
> > (or whatever), and once we get back up to pci_nic_init() we just know
> > that pci_get_bus_devfn() failed for some reason.
>
> What other possible reason for it to fail?
Unparseable address (it can be user specified) or internal failure to
initialize the device are the first two that spring to mind..
> > > > Plus on spapr we already support the
> > > > legacy nic options; it would be very strange for them to suddenly
> > > > break when we add a second host bridge.
> > >
> > > Not sure who "we" is here. IMHO user should ask for a new
> > > machine type with two roots explicitly.
> >
> > You seem to be thinking of the number of host bridges as a fixed
> > property of the platform, which it isn't on spapr. PCI host bridges
> > are just another device. Large scale real hardware can easily have
> > dozens of them.
>
> Absolutely. I'm not thinking of it as fixed.
> I'm thinking of the *default* number of pci host bridges
> as fixed. If a user is smart enough to use -device to create
> a host bridge, said user can learn about -device for creating
> a nic.
Hm, I guess. I'm still uncomfortable with breaking a documented
option, even if its not the preferred method these days.
> > In qemu we create one always as a convenience, but
> > users can (and will have to, for vfio) create additional ones
> > trivially with -device.
>
> So they know about -device then.
>
> > [Which raises another complication as a tangent. People (and libvirt)
> > don't generally expect -nodefaults to remove the PCI bridge, but
> > arguably it should on spapr, since a PAPR guest with no PCI is
> > perfectly viable but there's currently no way to specify such a
> > thing.]
>
> I guess the problem is not what they expect generally,
> but specifically that some users might rely on spapr with
> -nodefaults having PCI?
I'm pretty sure libvirt will rely on that, if nothing else.
> I don't have any ideas besides introducing a new machine type
> that is same as spapr but without the default PCI host bridge.
--
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: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-30 3:34 ` David Gibson
@ 2013-05-30 5:02 ` Michael S. Tsirkin
2013-06-06 7:39 ` David Gibson
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-05-30 5:02 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, David Gibson
On Thu, May 30, 2013 at 01:34:41PM +1000, David Gibson wrote:
> On Wed, May 29, 2013 at 03:22:29PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 29, 2013 at 09:04:00PM +1000, David Gibson wrote:
> > > On Wed, May 29, 2013 at 01:17:13PM +0300, Michael S. Tsirkin wrote:
> > > > On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> > > > > On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > > > > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > > > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > > > > > > > Currently pci_get_primary_bus() searches the list of root buses for one
> > > > > > > > > > with domain 0. But since host buses are always registered with domain 0,
> > > > > > > > > > this just amounts to finding the only PCI host bus.
> > > > > > > > > >
> > > > > > > > > > This simplifies the implementation by defining the primary PCI bus to
> > > > > > > > > > be the first one registered, using a global variable to track it.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > > > > > > >
> > > > > > > > > Or better: can we just fail if there is more than
> > > > > > > > > one root?
> > > > > > > >
> > > > > > > > That might work, I'll look into doing that.
> > > > > > >
> > > > > > > So, the difficulty with this is that then any machine with multiple
> > > > > > > PCI bridges could not use pci_nic_init(), since it calls
> > > > > > > pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> > > > > > > always fail. And using pci_nic_init() is more or less mandatory in
> > > > > > > the machine_init function to support old-style nic configuration.
> > > > > > >
> > > > > > > Suggestions?
> > > > > >
> > > > > > You mean multiple PCI roots?
> > > > > > Well, there are no legacy machines with multiple roots to support, are
> > > > > > there? So why do we need to support legacy flags for these new
> > > > > > configurations?
> > > > >
> > > > > Because people expect them.
> > > >
> > > > People can learn, somehow they will learn to add a new root, so they can
> > > > learn to use -device too.
> > >
> > > Hrm. I'd kind of like a second (third?) opinion on that. Anthony?
> > >
> > > > So let's make it fail on multiple roots, and output a message along the
> > > > lines of "please use -device virtio-net-pci instead".
> > >
> > > How to produce a meaningful error like that isn't totally obvious,
> > > since the test for multiple roots is down in find_primary_pci_bus()
> > > (or whatever), and once we get back up to pci_nic_init() we just know
> > > that pci_get_bus_devfn() failed for some reason.
> >
> > What other possible reason for it to fail?
>
> Unparseable address (it can be user specified) or internal failure to
> initialize the device are the first two that spring to mind..
Well, let's change the API in some way. How about we
pass root to pci_get_bus_devfn?
> > > > > Plus on spapr we already support the
> > > > > legacy nic options; it would be very strange for them to suddenly
> > > > > break when we add a second host bridge.
> > > >
> > > > Not sure who "we" is here. IMHO user should ask for a new
> > > > machine type with two roots explicitly.
> > >
> > > You seem to be thinking of the number of host bridges as a fixed
> > > property of the platform, which it isn't on spapr. PCI host bridges
> > > are just another device. Large scale real hardware can easily have
> > > dozens of them.
> >
> > Absolutely. I'm not thinking of it as fixed.
> > I'm thinking of the *default* number of pci host bridges
> > as fixed. If a user is smart enough to use -device to create
> > a host bridge, said user can learn about -device for creating
> > a nic.
>
> Hm, I guess. I'm still uncomfortable with breaking a documented
> option, even if its not the preferred method these days.
Let's add
> > > In qemu we create one always as a convenience, but
> > > users can (and will have to, for vfio) create additional ones
> > > trivially with -device.
> >
> > So they know about -device then.
> >
> > > [Which raises another complication as a tangent. People (and libvirt)
> > > don't generally expect -nodefaults to remove the PCI bridge, but
> > > arguably it should on spapr, since a PAPR guest with no PCI is
> > > perfectly viable but there's currently no way to specify such a
> > > thing.]
> >
> > I guess the problem is not what they expect generally,
> > but specifically that some users might rely on spapr with
> > -nodefaults having PCI?
>
> I'm pretty sure libvirt will rely on that, if nothing else.
>
> > I don't have any ideas besides introducing a new machine type
> > that is same as spapr but without the default PCI host bridge.
>
> --
> 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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-05-30 5:02 ` Michael S. Tsirkin
@ 2013-06-06 7:39 ` David Gibson
2013-06-06 8:18 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: David Gibson @ 2013-06-06 7:39 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, David Gibson
[-- Attachment #1: Type: text/plain, Size: 3119 bytes --]
On Thu, May 30, 2013 at 08:02:27AM +0300, Michael S. Tsirkin wrote:
> On Thu, May 30, 2013 at 01:34:41PM +1000, David Gibson wrote:
> > On Wed, May 29, 2013 at 03:22:29PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 29, 2013 at 09:04:00PM +1000, David Gibson wrote:
> > > > On Wed, May 29, 2013 at 01:17:13PM +0300, Michael S. Tsirkin wrote:
> > > > > On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> > > > > > On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > > > > > > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > > > > > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > > > > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > > > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David
> Gibson wrote:
[snip]
> > > > > So let's make it fail on multiple roots, and output a message along the
> > > > > lines of "please use -device virtio-net-pci instead".
> > > >
> > > > How to produce a meaningful error like that isn't totally obvious,
> > > > since the test for multiple roots is down in find_primary_pci_bus()
> > > > (or whatever), and once we get back up to pci_nic_init() we just know
> > > > that pci_get_bus_devfn() failed for some reason.
> > >
> > > What other possible reason for it to fail?
> >
> > Unparseable address (it can be user specified) or internal failure to
> > initialize the device are the first two that spring to mind..
>
> Well, let's change the API in some way. How about we
> pass root to pci_get_bus_devfn?
Alrighty, that I can do. I was initially hesitant, since at least
notionally the given PCI address string can include a domain, but
we're already pretty much explicitly disabling that, and none of the
built-in examples use it, so I think it's fine.
> > > > > > Plus on spapr we already support the
> > > > > > legacy nic options; it would be very strange for them to suddenly
> > > > > > break when we add a second host bridge.
> > > > >
> > > > > Not sure who "we" is here. IMHO user should ask for a new
> > > > > machine type with two roots explicitly.
> > > >
> > > > You seem to be thinking of the number of host bridges as a fixed
> > > > property of the platform, which it isn't on spapr. PCI host bridges
> > > > are just another device. Large scale real hardware can easily have
> > > > dozens of them.
> > >
> > > Absolutely. I'm not thinking of it as fixed.
> > > I'm thinking of the *default* number of pci host bridges
> > > as fixed. If a user is smart enough to use -device to create
> > > a host bridge, said user can learn about -device for creating
> > > a nic.
> >
> > Hm, I guess. I'm still uncomfortable with breaking a documented
> > option, even if its not the preferred method these days.
>
> Let's add
Uh.. was there supposed to be the rest of a sentence there?
--
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] 40+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
2013-06-06 7:39 ` David Gibson
@ 2013-06-06 8:18 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2013-06-06 8:18 UTC (permalink / raw)
To: David Gibson; +Cc: aliguori, qemu-devel, David Gibson
On Thu, Jun 06, 2013 at 05:39:11PM +1000, David Gibson wrote:
> On Thu, May 30, 2013 at 08:02:27AM +0300, Michael S. Tsirkin wrote:
> > On Thu, May 30, 2013 at 01:34:41PM +1000, David Gibson wrote:
> > > On Wed, May 29, 2013 at 03:22:29PM +0300, Michael S. Tsirkin wrote:
> > > > On Wed, May 29, 2013 at 09:04:00PM +1000, David Gibson wrote:
> > > > > On Wed, May 29, 2013 at 01:17:13PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> > > > > > > On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > > > > > > > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > > > > > > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > > > > > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > > > > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David
> > Gibson wrote:
> [snip]
> > > > > > So let's make it fail on multiple roots, and output a message along the
> > > > > > lines of "please use -device virtio-net-pci instead".
> > > > >
> > > > > How to produce a meaningful error like that isn't totally obvious,
> > > > > since the test for multiple roots is down in find_primary_pci_bus()
> > > > > (or whatever), and once we get back up to pci_nic_init() we just know
> > > > > that pci_get_bus_devfn() failed for some reason.
> > > >
> > > > What other possible reason for it to fail?
> > >
> > > Unparseable address (it can be user specified) or internal failure to
> > > initialize the device are the first two that spring to mind..
> >
> > Well, let's change the API in some way. How about we
> > pass root to pci_get_bus_devfn?
>
> Alrighty, that I can do. I was initially hesitant, since at least
> notionally the given PCI address string can include a domain, but
> we're already pretty much explicitly disabling that, and none of the
> built-in examples use it, so I think it's fine.
>
> > > > > > > Plus on spapr we already support the
> > > > > > > legacy nic options; it would be very strange for them to suddenly
> > > > > > > break when we add a second host bridge.
> > > > > >
> > > > > > Not sure who "we" is here. IMHO user should ask for a new
> > > > > > machine type with two roots explicitly.
> > > > >
> > > > > You seem to be thinking of the number of host bridges as a fixed
> > > > > property of the platform, which it isn't on spapr. PCI host bridges
> > > > > are just another device. Large scale real hardware can easily have
> > > > > dozens of them.
> > > >
> > > > Absolutely. I'm not thinking of it as fixed.
> > > > I'm thinking of the *default* number of pci host bridges
> > > > as fixed. If a user is smart enough to use -device to create
> > > > a host bridge, said user can learn about -device for creating
> > > > a nic.
> > >
> > > Hm, I guess. I'm still uncomfortable with breaking a documented
> > > option, even if its not the preferred method these days.
> >
> > Let's add
>
> Uh.. was there supposed to be the rest of a sentence there?
I meant let's add documentation that says -net nic is deprecated,
and not supported with multiple root devices, and to use
-device instead.
> --
> 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
^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2013-06-06 8:18 UTC | newest]
Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-09 0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c David Gibson
2013-05-23 11:11 ` Michael S. Tsirkin
2013-05-23 12:23 ` David Gibson
2013-05-24 7:44 ` David Gibson
2013-05-23 14:54 ` Paolo Bonzini
2013-05-24 7:46 ` David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 2/8] pci: Move pci_read_devaddr to pci-hotplug-old.c David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus() David Gibson
2013-05-23 11:26 ` Michael S. Tsirkin
2013-05-09 0:31 ` [Qemu-devel] [PATCH 4/8] pci: Use helper o find device's root bus in pci_find_domain() David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
2013-05-23 11:04 ` Michael S. Tsirkin
2013-05-23 12:21 ` David Gibson
2013-05-23 14:51 ` Paolo Bonzini
2013-05-23 14:57 ` Michael S. Tsirkin
2013-05-23 15:00 ` Paolo Bonzini
2013-05-23 15:06 ` Michael S. Tsirkin
2013-05-24 7:40 ` David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus David Gibson
2013-05-23 11:01 ` Michael S. Tsirkin
2013-05-23 12:16 ` David Gibson
2013-05-23 14:39 ` Michael S. Tsirkin
2013-05-23 11:22 ` Michael S. Tsirkin
2013-05-23 12:16 ` David Gibson
2013-05-29 9:43 ` David Gibson
2013-05-29 9:47 ` David Gibson
2013-05-29 9:55 ` Michael S. Tsirkin
2013-05-29 10:06 ` David Gibson
2013-05-29 10:17 ` Michael S. Tsirkin
2013-05-29 11:04 ` David Gibson
2013-05-29 12:22 ` Michael S. Tsirkin
2013-05-30 3:34 ` David Gibson
2013-05-30 5:02 ` Michael S. Tsirkin
2013-06-06 7:39 ` David Gibson
2013-06-06 8:18 ` Michael S. Tsirkin
2013-05-09 0:31 ` [Qemu-devel] [PATCH 7/8] pci: Remove domain from PCIHostBus David Gibson
2013-05-09 0:31 ` [Qemu-devel] [PATCH 8/8] pci: Fold host_buses list into PCIHostState functionality David Gibson
2013-05-14 10:53 ` [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses Michael S. Tsirkin
2013-05-23 11:12 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.