Re: [libvirt] [PATCH 4/4] qemu: don't be as insistent about adding dmi-to-pci-bridge or pci-bridge

On 04/25/2016 05:28 PM, Laine Stump wrote: > On 04/23/2016 12:46 PM, Cole Robinson wrote: >> On 04/21/2016 02:48 PM, Laine Stump wrote: >>> Previously there was no way to have a Q35 domain that didn't have >>> these two controllers. This patch skips their creation as long as >>> there are some other kinds of pci controllers at index 1 and 2 >>> (e.g. some pcie-root-port controllers). >>> >>> I'm hoping that soon we won't add them at all, plugging all devices >>> into auto-added pcie-*-port ports instead, but in the meantime this >>> makes it easier to experiment with alternative bus hierarchies. >>> --- >>> src/qemu/qemu_domain.c | 18 +++++++++++------- >>> 1 file changed, 11 insertions(+), 7 deletions(-) >>> >>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >>> index 86b7d13..0b342e2 100644 >>> --- a/src/qemu/qemu_domain.c >>> +++ b/src/qemu/qemu_domain.c >>> @@ -1581,14 +1581,18 @@ >>> qemuDomainDefAddDefaultDevices(virDomainDefPtr def, >>> VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) { >>> goto cleanup; >>> } >>> - if (virDomainDefMaybeAddController( >>> - def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1, >>> - VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 || >>> - virDomainDefMaybeAddController( >>> - def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2, >>> - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0) { >>> + /* add a dmi-to-pci-bridge and a pci-bridge if there are >>> no pci controllers >>> + * other than the pcie-root. This is so that there will be >>> hot-pluggable >>> + * PCI slots available >>> + */ >>> + if (virDomainControllerFind(def, >>> VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1) < 0 && >>> + !virDomainDefAddController(def, >>> VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1, >>> + VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE)) >>> + goto cleanup; >>> + if (virDomainControllerFind(def, >>> VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2) < 0 && >>> + !virDomainDefAddController(def, >>> VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2, >>> + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE)) >>> goto cleanup; >>> - } >>> } >>> if (addDefaultMemballoon && !def->memballoon) { >>> >> Sounds like another qemuxml2xml test case candidate > > ... and it turns out this patch doesn't go far enough to have any > useful effect. The problem is that the code that automatically > assigns PCI addresses does its best to always be sure that there is at > least one empty hot-pluggable "standard PCI" slot on the system, so > even if we don't explicitly add any pci-bridge to a Q35 domain, the > address-assignment code will automatically add one anyway; and > since the pci-bridge requires a standard PCI slot itself, we still > need a dmi-to-pci-bridge anyway (which provides 32 non-hotpluggable > standard PCI slots). > > I had been aiming to re-do how libvirt sets up the PCI controllers > for Q35, eventually eliminating the current dmi-to-pci-bridge + > pci-bridge in favor of a pure PCIe setup, using pcie-root-ports and > pcie-switch-(up|down)stream-ports, and this was going to be the first > step towards that. But after discussing that idea more with Alex > Williamson on Friday, I think we *shouldn't* do it, but just > leave the PCI controller setup essentially as it is (one possible > change - allow connecting pci-bridge directly into a pcie-root port, > thus eliminating the dmi-to-pci-bridge) > Connecting the pci-bridge directly into a pcie-root port is a little odd, but I suppose it would work. Since I am late to the party, can you please explain why do you want to eliminate the dmi-to-pci controller ? Because it doesn't support hotplug?

I have some bad news in that direction. Even the pci-bridge (devices behind it) does not support hotplug on Q35, it is on my todo list.

> What prompted the idea to change to pure PCIe controllers was that > qemu allows any PCI device to be plugged into a PCIe slot and it > apparently functions just fine, and there had been occasional > complaints that plugging everything into a pci-bridge was somehow > problematic (at one point someone suggested that virtio-net didn't > work correctly on ARM if plugged into a non-Express slot, but I > think that was later disproved). > > The problem with PCIe-only controllers, as Alex pointed out, is that > when a non-Express device is plugged into a PCIe slot, the guest OS > will see an apparently PCIe device that has no PCIe > capabilities, and while so far this hasn't caused any problem, there > is no guarantee that it won't - PCIe devices are supposed to have > PCIe capabilities. Since the only emulated qemu device that does > this is the NEC UHCI USB3 controller, it sees that we'll need to keep > setting up the pci-bridge and plugging all the rest of the devices in > there. Actually all virtio devices are now PCI Express devices if virtio-1 is enabled. The QEMU command-line is -device virtio-<dev>-pci,disable-modern=false.

The only requirement is that the virtio device would not be connected directly to pcie.0 root bus, but to a pcie root port or switch.

> > Still, it would be nice to allow *those who really want to* to have a > "pure PCIe" controller setup. We could do that if we did two things: Sure, pure PCIe setup would be nice. > > 1) modify the PCI address auto-assignment code to not insist on > always having at least one hot-pluggable standard PCI slot available. > > and one of the following: > > 2a) don't always add a dmi-to-pci-bridge, but instead only do so *if > necessary* in order to plug in a pci-bridge (which would only be > added if necessary). > > or > > 2b) permit auto-assigning a pci-bridge to plug into a port of > pcie-root, thus eliminating the need for dmi-to-pci-bridge completely. > > Does anyone have opinions about (1) or (2b)? I personally don't like 2b because would not work in 'real' world, so 1 and 2a seems fine to me. Bottom line, maybe we can tackle the dmi-to-pci bridge to cover your requirements.