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.