When support for the pcie-root and dmi-to-pci-bridge buses on a Q35
machinetype was added, I was concerned that even though qemu at the
time allowed plugging a PCI device into a PCIe port, that it might not
be supported in the future. To prevent painful backtracking in the
possible future where this happened, I disallowed such connections
except in a few specific cases requested by qemu developers (indicated
in the code with the flag VIR_PCI_CONNEcT_TYPE_EITHER_IF_CONFIG).
Now that a couple years have passed, there is a clear message from
qemu that there is no danger in allowing PCI devices to be plugged
into PCIe ports. This patch eliminates
VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG and changes the code to always
allow PCI->PCIe or PCIe->PCI connection *when the PCI address is
specified in the config. (For newly added devices that haven't yet
been given a PCI address, the auto-placement still prefers using the
correct type of bus).
---
docs/formatdomain.html.in | 18 +++++++++++-------
src/conf/domain_addr.c | 18 +++++++++++-------
src/conf/domain_addr.h | 16 ++++++++++------
src/qemu/qemu_command.c | 6 ++----
4 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4e85b51..1dca2ac 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3034,17 +3034,21 @@
bus (for example, the machine types based on the Q35 chipset),
the pcie-root controller with index=0 is auto-added to the
domain's configuration. pcie-root has also no address, provides
- 31 slots (numbered 1-31) and can only be used to attach PCIe
- devices. In order to connect standard PCI devices on a system
- which has a pcie-root controller, a pci controller
+ 31 slots (numbered 1-31) that can be used to attach PCIe or PCI
+ devices (although libvirt will never auto-assign a PCI device to
+ a PCIe slot, it will allow manual specification of such an
+ assignment). Devices connected to pcie-root cannot be
+ hotplugged. In order to make standard PCI slots available on a
+ system which has a pcie-root controller, a pci controller
with <code>model='dmi-to-pci-bridge'</code> is automatically
- added. A dmi-to-pci-bridge controller plugs into a PCIe slot (as
- provided by pcie-root), and itself provides 31 standard PCI
- slots (which are not hot-pluggable). In order to have
+ added, usually at the defacto standard location of slot=0x1e. A
+ dmi-to-pci-bridge controller plugs into a PCIe slot (as provided
+ by pcie-root), and itself provides 31 standard PCI slots (which
+ also do not support device hotplug). In order to have
hot-pluggable PCI slots in the guest system, a pci-bridge
controller will also be automatically created and connected to
one of the slots of the auto-created dmi-to-pci-bridge
- controller; all guest devices with PCI addresses that are
+ controller; all guest PCI devices with addresses that are
auto-determined by libvirt will be placed on this pci-bridge
device. (<span class="since">since 1.1.2</span>).
</p>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index d657c7f..93c6043 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1,7 +1,7 @@
/*
* domain_addr.c: helper APIs for managing domain device addresses
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -42,16 +42,21 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
{
virErrorNumber errType = (fromConfig
? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
- virDomainPCIConnectFlags flagsMatchMask = VIR_PCI_CONNECT_TYPES_MASK;
- if (fromConfig)
- flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG;
+ if (fromConfig) {
+ /* If the requested connection was manually specified in
+ * config, allow a PCI device to connect to a PCIe slot, or
+ * vice versa.
+ */
+ if (busFlags & VIR_PCI_CONNECT_TYPES_ENDPOINT)
+ busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT;
+ }
/* If this bus doesn't allow the type of connection (PCI
* vs. PCIe) required by the device, or if the device requires
* hot-plug and this bus doesn't have it, return false.
*/
- if (!(devFlags & busFlags & flagsMatchMask)) {
+ if (!(devFlags & busFlags & VIR_PCI_CONNECT_TYPES_MASK)) {
if (reportError) {
if (devFlags & VIR_PCI_CONNECT_TYPE_PCI) {
virReportError(errType,
@@ -174,8 +179,7 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
* specified in user config *and* the particular device being
* attached also allows it
*/
- bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE |
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ bus->flags = VIR_PCI_CONNECT_TYPE_PCIE;
bus->minSlot = 1;
bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
break;
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index c18e130..dcfd2e5 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -1,7 +1,7 @@
/*
* domain_addr.h: helper APIs for managing domain device addresses
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -39,10 +39,6 @@ typedef enum {
/* PCI devices can connect to this bus */
VIR_PCI_CONNECT_TYPE_PCIE = 1 << 3,
/* PCI Express devices can connect to this bus */
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG = 1 << 4,
- /* PCI *and* PCIe devices allowed, if the address
- * was specified in the config by the user
- */
} virDomainPCIConnectFlags;
typedef struct {
@@ -70,12 +66,20 @@ struct _virDomainPCIAddressSet {
typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
-/* a combination of all bit that describe the type of connections
+/* a combination of all bits that describe the type of connections
* allowed, e.g. PCI, PCIe, switch
*/
# define VIR_PCI_CONNECT_TYPES_MASK \
(VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE)
+/* combination of all bits that could be used to connect a normal
+ * endpoint device (i.e. excluding the connection possible between an
+ * upstream and downstream switch port, or a PCIe root port and a PCIe
+ * port)
+ */
+# define VIR_PCI_CONNECT_TYPES_ENDPOINT \
+ (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE)
+
char *virDomainPCIAddressAsString(virDevicePCIAddressPtr addr)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5e5cfe6..4952797 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1621,8 +1621,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI:
- flags = (VIR_PCI_CONNECT_TYPE_PCI |
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ flags = VIR_PCI_CONNECT_TYPE_PCI;
break;
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
/* should this be PCIE-only? Or do we need to allow PCI
@@ -1643,8 +1642,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
switch (device->data.sound->model) {
case VIR_DOMAIN_SOUND_MODEL_ICH6:
case VIR_DOMAIN_SOUND_MODEL_ICH9:
- flags = (VIR_PCI_CONNECT_TYPE_PCI |
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ flags = VIR_PCI_CONNECT_TYPE_PCI;
break;
}
break;
--
2.1.0