qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] virtio: Provide version-specific variants of vi


From: Eduardo Habkost
Subject: Re: [Qemu-devel] [PATCH] virtio: Provide version-specific variants of virtio PCI devices
Date: Mon, 15 Oct 2018 20:32:42 -0300
User-agent: Mutt/1.9.2 (2017-12-15)

On Mon, Oct 15, 2018 at 11:45:56AM +0200, Cornelia Huck wrote:
> On Fri, 12 Oct 2018 23:54:35 -0300
> Eduardo Habkost <address@hidden> 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 <address@hidden>
> > ---
> > Reference to previous discussion that originated this idea:
> >   https://www.mail-archive.com/address@hidden/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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]