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

[Eduardo Habkost]

On Thu, Nov 15, 2018 at 05:29:24PM +0100, Andrea Bolognani wrote:
> 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 :)

That's possibly my fault.  I described it completely wrong in one
message in the v1 thread.


> 
> 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?

Exactly.

> 
> 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 :)

I like that suggestion.  Makes the device names more explicit
_and_ shorter.  I'll do that in v3.

-- 
Eduardo