On 12/08/2015 02:22 AM, Pavel Fedin wrote:
Hello!
For AARCH64, though... well, if you want to know why it's added for that machinetype, I guess you'd need to talk to the person who turned on addPCIeRoot for AARCH64 :-). That was me, and i simply reused the existing code.
Yeah, I know :-) (or rather "git-blame-mode" in emacs knows!) Just some good-natured poking :-) (actually I wouldn't be surprised if it was me who reviewed your patch, and just assumed that if you were adding the other controllers, that you must actually want them).
Actually, initially i tried to use addPCIRoot first, but it ended up in a plain PCI bus which does not support MSI-X. Or, even, it did not work at all, because libvirt refers to bus#0 as to just "pci", which does not work with virt.
qemu has hardcoded the PCI root bus with the name "pci.0" and the PCIe root complex with the name "pcie.0". libvirt sticks by those names. (but names all optional buses as "pci.n")
I don't remember the exact outcome. So, i decided to use addPCIeRoot instead, and everything just worked.
Except the extra stuff. The origin of the problem was in my overloading "addPCIeRoot" to indicate that the other two controllers should also be added. You only made the mistake of thinking that the name of the variable was actually an accurate/complete description of what it did :-)
I actually wondered about that recently when I was tinkering with auto-adding USB2 controllers when machinetype is Q35 (i.e. "why are we adding an Intel/x86-specific controller to AARCH64 machines?") I guess we could tweak it so that on non-x86 we have only pcie-root without all the downstream chain.
That sounds fine to me...
For qemu it will perfectly work, and actually in my XML i put all my devices to bus#0, so that MSI-X works. I don't use downstream buses at all. But, indeed, i never tested hotplugging.
...as long as something is done about hotplug (assuming aarch64 supports that, or plans to support it) (I wonder if maybe we could/should add an option somewhere saying "hotplugCapable=yes|no". No idea where or how that would go, or *exactly* what it would do, but I guess it would end up being a short hand for adding a controller or two, and would then be swallowed by the parser...)
I didn't even know aarch64 migration was working... It should work out of the box with GICv2, and for GICv3 i posted RFC patches. RFC because kernel API for that is not ready yet.
Is this only a problem on aarch64, or is there a migration problem with pci-bridge on x86 as well? (It's possible there is but it hasn't been noticed, because pci-bridge likely isn't used much outside of q35 machines, and q35 was prohibited from migrating until qemu 2.4 due to the embedded sata driver which didn't support migration.) I think it should be a generic problem because i don't see how PCI bridge implementation could contain any arch-specific code.
BTW, does the aarch64 virt machinetype support any controller aside from the embedded pcie-root? No. The actual controller used by "virt" machine is called "generic PCI host", and it registers itself as "pcie.0".
But apparently qemu is accepting "-device i82801b11-bridge" and -device pci-bridge, since that's what you get from libvirt. If these devices aren't supported for aarch64, they should be disabled in qemu (and not listed in the devices when libvirt asks). (An aside - it's really annoying that so many of the different qemu binaries don't accept "-device ?" to return a list of valid devices; this is very useful when exploring...)
Right now we will auto-add only a pci-bridge if no available slot is found for a pci device, but we will (should anyway) auto-assign a slot on an *existing* PCIe controller if the device has PCIE as the preferred slot type. AFAIR we already do this. The only problem is that preferred slot type for the device is currently hardcoded to be PCIe for video cards and, IIRC, SCSI; and plain PCI for everything else.
Right (at least for video, don't know about SCSI - if that's the case then it was added by someone after my initial implementation). These are hardcoded because there is currently no information available by qemu introspection that can tell us which type of bus is better for any device (there was a thread about that on the qemu mailing list a year or so ago, but I don't think anything ever came of it). Since we can't learn the proper information from qemu, the best we can do is modify that one horribly ugly function so that it is more horrible and even uglier (and bigger) to change the preferred bus type for each device depending on the arch/machinetype and the device itself.