qemu: always permit PCI devices to be manually assigned to a PCIe bus

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

@@ -3060,17 +3060,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
+      31 slots (numbered 1-31) that can be used to attach PCIe or PCI
-      devices. In order to connect standard PCI devices on a system
+      devices (although libvirt will never auto-assign a PCI device to
-      which has a pcie-root controller, a pci controller
+      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
+      added, usually at the defacto standard location of slot=0x1e. A
-      provided by pcie-root), and itself provides 31 standard PCI
+      dmi-to-pci-bridge controller plugs into a PCIe slot (as provided
-      slots (which are not hot-pluggable). In order to have
+      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>

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)
+    if (fromConfig) {
-       flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG;
+        /* 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 |
+        bus->flags = VIR_PCI_CONNECT_TYPE_PCIE;
-                      VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
         bus->minSlot = 1;
         bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
         break;

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

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 |
+              flags = VIR_PCI_CONNECT_TYPE_PCI;
-                       VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
               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 |
+            flags = VIR_PCI_CONNECT_TYPE_PCI;
-                     VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
             break;
         }
         break;