conf: Prefer pcie-to-pci-bridge to dmi-to-pci-bridge

Both pcie-to-pci-bridge and dmi-to-pci-bridge can be used to
create a traditional PCI topology in a pure PCIe guest such as
those using the x86_64/q35 or aarch64/virt machine type;
however, the former should be preferred, as it doesn't need to
obey limitation of real hardware and is completely
architecture-agnostic.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1520821Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

docs/formatdomain.html.in

@@ -4114,10 +4114,14 @@
       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, usually at the defacto standard location of slot=0x1e. A
+      hotplugged. If traditional PCI devices are present in the guest
+      configuration, a <code>pcie-to-pci-bridge</code> controller will
+      automatically be added: this controller, which plugs into a
+      <code>pcie-root-port</code>, provides 31 usable PCI slots (1-31) with
+      hotplug support (<span class="since">since 4.3.0</span>). If the QEMU
+      binary doesn't support the corresponding device, then a
+      <code>dmi-to-pci-bridge</code> controller will be added instead,
+      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

src/conf/domain_addr.c

@@ -416,6 +416,7 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,
     size_t i;
     int model;
     bool needDMIToPCIBridge = false;
+    bool needPCIeToPCIBridge = false;
 
     add = addr->bus - addrs->nbuses + 1;
     if (add <= 0)
@@ -436,27 +437,41 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,
             model = VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE;
 
             /* if there aren't yet any buses that will accept a
-             * pci-bridge, and the caller is asking for one, we'll need to
-             * add a dmi-to-pci-bridge first.
+             * pci-bridge, but we need one for the device's PCI address
+             * to make sense, it means the guest only has a PCIe topology
+             * configured so far, and we need to create a traditional PCI
+             * topology to accomodate the new device.
              */
             needDMIToPCIBridge = true;
+            needPCIeToPCIBridge = true;
             for (i = 0; i < addrs->nbuses; i++) {
                 if (addrs->buses[i].flags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE) {
                     needDMIToPCIBridge = false;
+                    needPCIeToPCIBridge = false;
                     break;
                 }
             }
-            if (needDMIToPCIBridge && add == 1) {
+
+            /* Prefer pcie-to-pci-bridge, fall back to dmi-to-pci-bridge */
+            if (addrs->isPCIeToPCIBridgeSupported)
+                needDMIToPCIBridge = false;
+            else
+                needPCIeToPCIBridge = false;
+
+            if ((needDMIToPCIBridge || needPCIeToPCIBridge) && add == 1) {
                 /* We need to add a single pci-bridge to provide the bus
                  * our legacy PCI device will be plugged into; however, we
                  * have also determined that there isn't yet any proper
-                 * place to connect that pci-bridge we're about to add (on
-                 * a system with pcie-root, that "proper place" would be a
-                 * dmi-to-pci-bridge". So, to give the pci-bridge a place
-                 * to connect, we increase the count of buses to add,
-                 * while also incrementing the bus number in the address
-                 * for the device (since the pci-bridge will now be at an
-                 * index 1 higher than the caller had anticipated).
+                 * place to connect that pci-bridge we're about to add,
+                 * which means we're dealing with a pure PCIe guest. We
+                 * need to create a traditional PCI topology, and for that
+                 * we have two options: dmi-to-pci-bridge + pci-bridge or
+                 * pcie-root-port + pcie-to-pci-bridge (the latter of which
+                 * is pretty much a pci-bridge as far as devices attached
+                 * to it are concerned and as such makes the pci-bridge
+                 * unnecessary). Either way, there's going to be one more
+                 * controller than initially expected, and the 'bus' part
+                 * of the device's address will need to be bumped.
                  */
                 add++;
                 addr->bus++;
@@ -534,6 +549,30 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,
         }
     }
 
+    if (needPCIeToPCIBridge) {
+        /* We need a pcie-root-port to plug pcie-to-pci-bridge into; however,
+         * qemuDomainAssignPCIAddresses() will, in some cases, create a dummy
+         * PCIe device and reserve an address for it in order to leave the
+         * user with an empty pcie-root-port ready for hotplugging, and if
+         * we didn't do anything other than adding the pcie-root-port here
+         * it would be used for that, which we don't want. So we change the
+         * connect flags to make sure only the pcie-to-pci-bridge will be
+         * connected to the pcie-root-port we just added, and another one
+         * will be allocated for the dummy PCIe device later on.
+         */
+        if (virDomainPCIAddressBusSetModel(&addrs->buses[i],
+                                           VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT) < 0) {
+            return -1;
+        }
+        addrs->buses[i].flags = VIR_PCI_CONNECT_TYPE_PCIE_TO_PCI_BRIDGE;
+        i++;
+
+        if (virDomainPCIAddressBusSetModel(&addrs->buses[i++],
+                                           VIR_DOMAIN_CONTROLLER_MODEL_PCIE_TO_PCI_BRIDGE) < 0) {
+            return -1;
+        }
+    }
+
     for (; i < addrs->nbuses; i++) {
         if (virDomainPCIAddressBusSetModel(&addrs->buses[i], model) < 0)
             return -1;