I'm undecided if it is worthwhile to add this...
Up until now it has been legal to have something like this in the xml:
<controller type='pci' model='pcie-root' index='0'/>
<controller type='pci' model='pci-bridge' index='2'/>
(for example, see the existing test case
"usb-controller-default-q35"). This is handled in
qemuDomainPCIAddressSetCreate() when it's adding in controllers to
fill holes in the indexes.)
Since we have always added the following to every config:
<controller type='pci' model='dmi-to-pci-bridge' index='1'/>
there has always been a proper place for the unaddressed pci-bridge to
plug in.
A upcoming commit will eliminate the forced adding of a
dmi-to-pci-bridge to *every* Q35 domain config, though. This would
mean the above-mentioned test case (and one other) would begin to
fail. There are two possible solutions to this problem:
1) change the test cases, and made the above construct an error (note
that in the future it will work just fine to not list *any* pci
controller, and they will all be auto-added as needed).
or
2) This patch, which specifically looks for the case where we have a
pci-bridge (but no dmi-to-pci-bridge) in a PCIe domain config and
auto-adds the necessary dmi-to-pci-bridge. This can only work in the
case that the pci-bridge has been given an index such that there is an
unused index with a lower value for the dmi-to-pci-bridge to occupy.
This seems like a kind of odd corner case that nobody should need to
do in the future anyway. On the other hand, I already wrote the code
and it works, so I might as well send it and see what opinions (if
any) it generates.
---
src/qemu/qemu_domain_address.c | 42 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 37 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index fee93fd..ea09556 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -911,6 +911,9 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
virDomainPCIAddressSetPtr addrs;
size_t i;
bool hasPCIeRoot = false;
+ unsigned int lowestDMIToPCIBridge = nbuses;
+ unsigned int lowestUnaddressedPCIBridge = nbuses;
+ unsigned int lowestAddressedPCIBridge = nbuses;
virDomainControllerModelPCI defaultModel;
if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL)
@@ -935,8 +938,24 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], cont->model) < 0)
goto error;
- if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+ /* we'll use all this info later to determine if we need
+ * to add a dmi-to-pci-bridge due to unaddressed pci-bridge controllers
+ */
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
hasPCIeRoot = true;
+ } else if (cont->model ==
+ VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) {
+ if (lowestDMIToPCIBridge > idx)
+ lowestDMIToPCIBridge = idx;
+ } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) {
+ if (virDeviceInfoPCIAddressWanted(&cont->info)) {
+ if (lowestUnaddressedPCIBridge > idx)
+ lowestUnaddressedPCIBridge = idx;
+ } else {
+ if (lowestAddressedPCIBridge > idx)
+ lowestAddressedPCIBridge = idx;
+ }
+ }
}
if (nbuses > 0 && !addrs->buses[0].model) {
@@ -952,13 +971,21 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
/* Now fill in a reasonable model for all the buses in the set
* that don't yet have a corresponding controller in the domain
- * config.
+ * config. In the rare (and ... strange, but still allowed) case
+ * that a domain has 1) a pcie-root at index 0, 2) *no*
+ * dmi-to-pci-bridge (or pci-bridge that was manually addressed to
+ * sit directly on pcie-root), and 3) does have an unaddressed
+ * pci-bridge at an index > 1, then we need to add a
+ * dmi-to-pci-bridge.
*/
- if (hasPCIeRoot)
- defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT;
- else
+ if (!hasPCIeRoot)
defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE;
+ else if (lowestUnaddressedPCIBridge < MIN(lowestAddressedPCIBridge,
+ lowestDMIToPCIBridge))
+ defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE;
+ else
+ defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT;
for (i = 1; i < addrs->nbuses; i++) {
@@ -970,6 +997,11 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
VIR_DEBUG("Auto-adding <controller type='pci' model='%s' index='%zu'/>",
virDomainControllerModelPCITypeToString(defaultModel), i);
+ /* only add a single dmi-to-pci-bridge, then add pcie-root-port
+ * for any other unspecified controller indexes.
+ */
+ if (hasPCIeRoot)
+ defaultModel = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT;
}
if (virDomainDeviceInfoIterate(def, qemuDomainCollectPCIAddress, addrs) < 0)
--
2.7.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list