Re: [libvirt] [PATCH for-4.0 v2] virtio: Provide version-specific variants of virtio PCI devices

On Wed, 2018-11-14 at 21:38 -0200, Eduardo Habkost wrote: > Many of the current virtio-*-pci device types actually represent > 3 different types of devices: > * virtio 1.0 non-transitional devices > * virtio 1.0 transitional devices > * virtio 0.9 ("legacy device" in virtio 1.0 terminology) > > That would be just an annoyance if it didn't break our device/bus > compatibility QMP interfaces. With this multi-purpose device > type, there's no way to tell management software that > transitional devices and legacy devices require a Conventional > PCI bus. > > The multi-purpose device types would also prevent us from telling > management software what's the PCI vendor/device ID for them, > because their PCI IDs change at runtime depending on the bus > where they were plugged. > > This patch adds separate device types for each of those virtio > device flavors: > > - virtio-*-pci: the existing multi-purpose device types > - Configurable using `disable-legacy` and `disable-modern` > properties > - Legacy driver support is automatically enabled/disabled > depending on the bus where it is plugged > - Supports Conventional PCI and PCI Express buses > (but Conventional PCI is incompatible with > disable-legacy=off) > - Changes PCI vendor/device IDs at runtime > - virtio-*-pci-transitional: virtio-1.0 device supporting legacy drivers > - Supports Conventional PCI buses only, because > it has a PIO BAR > - virtio-*-pci-non-transitional: modern-only > - Supports both Conventional PCI and PCI Express buses So, my understanding was that transitional devices would be suitable for both PCI and PCIe slots and non-transitional devices could only work in PCIe slots, but based on the above it looks like I got it pretty much completely wrong? I'm not too surprised that would be the case, to be honest: keeping this stuff straight in my head has always been a bit of a challenge, so I can't possibly not welcome a proposal like this, which will spell it out a bit more :)

Let me try to map the interactions out: * virtio-*-pci-transitional + plugged into PCI slot - shows up as vendor1/device1 + plugged into PCIe slot - doesn't work * virtio-*-pci-non-transitional + plugged into PCI slot - shows up as vendor2/device2 + plugged into PCIe slot - shows up as vendor2/device2 * virtio-*-pci + plugged into PCI slot - shows up as vendor1/device1 (same as virtio-*-pci-transitional) + plugged into PCIe slot - shows up as vendor2/device2 (same as virtio-*-pci-non-transitional) Does that look about right?

Once all the various pieces have fallen into place, when adding a device to a guest running a modern OS we would find out through libosinfo that it supports vendor2/device2 (and vendor1/device1 too, I guess?) so we would choose the non-transitional variant and plug it into PCIe when possible (q35) or PCI otherwise (pc); on the other hand, an older guest OS like CentOS 6 will only advertise support for vendor1/device1, so we'd have to use the transitional variant instead and plug it into a PCI slot regardless of the machine type, which more specifically means building a pcie.0 <- pcie-root-port <- pcie-pci-bridge topology for q35 guests. If all of the above is correct, then it sounds like a feasible enough plan to me, though of course it be a long time before users and management applications can rely on these new device types being available in downstream distributions... One thing that I'm very much not convinced about is the naming, specifically leaving the virtio revision out: I get it that we Should Never Need™ another major version of the spec, but I'm afraid discounting the possibility outright might prove to be shortsighted and come back to bite us later, so I'd much rather keep it. And once that's done, "non-transitional" (while matching the language of the spec) starts to look a bit unnecessary when you could simply have virtio-*-pci virtio-*-pci-1 virtio-*-pci-1-transitional instead. But I don't feel as strongly about this as I do about keeping the virtio revision in the device name :)