On 10/06/2016 10:13 AM, Andrea Bolognani wrote:
On Tue, 2016-09-20 at 15:14 -0400, Laine Stump wrote:
libvirt previously assigned nearly all devices to a "hotpluggable" legacy PCI slot even on machines with a PCIe root bus (and even though most such machines don't even support hotplug on legacy PCI slots!) Forcing all devices onto legacy PCI slots means that the domain will need a dmi-to-pci-bridge (to convert from PCIe to legacy PCI) and a pci-bridge (to provide hotpluggable legacy PCI slots which, again, usually aren't hotpluggable anyway).
To help reduce the need for these legacy controllers, this patch tries to assign virtio-1.0-capable devices to PCIe slots whenever possible, by setting appropriate connectFlags in virDomainDeviceConnectFlagsInternal(). Happily, when that function was written (just a few commits ago) it was created with a "virtioFlags" argument, set by both of its callers, which is the proper connectFlags to set for any virtio-*-pci device - depending on the arch/machinetype of the domain, and whether or not the qemu binary supports virtio-1.0, that flag will have either been set to PCI or PCIE. This patch merely enables the functionality by setting the flags for the device to whatever is in virtioFlags if the device is a virtio-*-pci device.
NB: since the slot must be hotpluggable, and pcie-root (the PCIe root complex) does *not* support hotplug, this means that suitable controllers must also be in the config (i.e. either pcie-root-port, or pcie-downstream-port). For now, libvirt doesn't add those automatically, so if you put virtio devices in a config for a qemu that has PCIe-capable virtio devices, you'll need to add extra pcie-root-ports yourself. That requirement will be eliminated in a future patch, but for now, it's simple to do this:
<controller type='pci' model='pcie-root-port'/> <controller type='pci' model='pcie-root-port'/> <controller type='pci' model='pcie-root-port'/> ...
Partially Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1330024 [...]
+ <video> + <model type='virtio' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> I was initially baffled by this, because I expected it to be assigned to one of the available pcie-root-ports just like all the other virtio devices.
However, according to qemuDomainValidateDevicePCISlotsQ35() this is intentional, so I guess we're good :)
Actually, you bring up an interesting point in light of the "Should PCIe devices ever be placed directly on pcie-root?" debate on qemu-devel (I think it was in the thread about the PCI topology document that Marcel is writing). We currently always put the primary video device at 00:1 just because "we always have", and it has the nice side effect of eliminating the need for legacy-PCI controllers. But in this one case the device is PCIe - to follow Marcel's recommendation of putting only legacy devices on pcie-root, we should be putting the virtio video device on a root-port. As I recall, Marcel and Alex were the most vocal on this subject, so I'm Cc'ing them, with this bit of context - this patch auto-assigns the addresses for virtio devices to be on Express ports rather than legacy slots when appropriate, but there is a bit of Q35-specific code that overrides any of that and always places the primary video device at 00:01.0 - should we still do that for the primary video if it's virtio? Or should we put it behind a root-port?
One improvement I can recommend to this test case is to add one more pcie-root-port, so that it becomes quite clear that virtio-vga has been assigned to pcie-root on purpose and *not* due to a lack of pcie-root-ports.
Or possibly change the behavior for virtio video even if it's the primary (note to self - check if someone has done the patch to allow multiple virtio video devices yet)