On Mon, Oct 15, 2018 at 11:45:56AM +0200, Cornelia Huck wrote:
> On Fri, 12 Oct 2018 23:54:35 -0300
> Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> > 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 is 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-0.9: legacy virtio device
> > - Supports Conventional PCI buses only, because
> > it has a PIO BAR
> > - virtio-*-pci-1.0-transitional: virtio-1.0 device supporting legacy drivers
> > - Supports Conventional PCI buses only, because
> > it has a PIO BAR
> > - virtio-*-pci-1.0: modern-only
> > - Supports both Conventional PCI and PCI Express buses
> >
> > All the types above will inherit from an abstract
> > "virtio-*-pci-base" type, so existing code that doesn't care
> > about the virtio version can use "virtio-*-pci-base" on type
> > casts.
>
> I get a crash when running 'make check'; might be missing a change to
> virtio-input-host?
>
> TEST: tests/device-introspect-test... (pid=28626)
> /s390x/device/introspect/list: OK
> /s390x/device/introspect/list-fields: OK
> /s390x/device/introspect/none: OK
> /s390x/device/introspect/abstract: OK
> /s390x/device/introspect/abstract-interfaces: OK
> /s390x/device/introspect/concrete/defaults/none:
> /home/cohuck/git/qemu/hw/virtio/virtio-pci.c:2730:virtio_host_initfn: Object
> 0x5633a43a1480 is not an instance of type virtio-input-host-device-base
> Broken pipe
> /home/cohuck/git/qemu/tests/libqtest.c:125: kill_qemu() detected QEMU death
> from signal 6 (Aborted) (core dumped)
Oops. I will fix it in v2, thanks.
>
[...]
> >
> > A simple test script (tests/acceptance/virtio_version.py) is
> > included, to check if the new device types are equivalent to
> > using the `disable-legacy` and `disable-modern` options.
> >
> > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> > ---
> > Reference to previous discussion that originated this idea:
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg558389.html
> > ---
> > hw/virtio/virtio-pci.h | 93 +++++++++---
> > hw/display/virtio-gpu-pci.c | 8 +-
> > hw/display/virtio-vga.c | 11 +-
> > hw/virtio/virtio-crypto-pci.c | 8 +-
> > hw/virtio/virtio-pci.c | 225 +++++++++++++++++++++--------
> > tests/acceptance/virtio_version.py | 138 ++++++++++++++++++
> > 6 files changed, 390 insertions(+), 93 deletions(-)
> > create mode 100644 tests/acceptance/virtio_version.py
>
> The approach makes sense to me, but I second the suggestion to use
> something like 'modern' (or 'standard'?) instead of '1.0'.
As noted on the reply I sent to Michael: the whole point of this
patch is to provide unambiguous device type names. What would be
the advantage of having another ambiguous device type named
"virtio-*-modern"?
>
> Also, before someone asks: I don't think we need something like that
> for virtio-ccw, as we (a) only support fencing newer revisions, not
> older ones, and (b) the implications of using different virtio
> revisions are localized to virtio-ccw only and do not have further
> implications as for virtio-pci.
Thanks! I was planning to ask about it later. :)
--
Eduardo
