Re: [Qemu-devel] [PATCH RFC v4 00/16] qemu: towards virtio-1 host support

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Thu, 27 Nov 2014 18:35:40 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Nov 27, 2014 at 05:28:42PM +0100, Cornelia Huck wrote:
> > On Thu, 27 Nov 2014 18:18:25 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Thu, Nov 27, 2014 at 05:06:51PM +0100, Cornelia Huck wrote:
> > 
> > > > So we should have a per-device callback into the transport layer, say
> > > > check_legacy()?
> > > 
> > > I would just have 2 masks: legacy_features and features.
> > 
> > But these belong to the device type and the transport just needs to
> > trigger usage of the right one, right?
> 
> Yes.
> 
> > > 
> > > > For ccw, this would check for the negotiated revision; for mmio, it
> > > > could check a device property configured with the device; and for pci,
> > > > whatever the mechanism is there :)
> > > > 
> > > > A transport not implementing this callback is simply considered
> > > > legacy-only.
> > > 
> > > I dislike callbacks. Let's just give all info to core,
> > > and have it DTRT.
> > > 
> > Have a is_legacy flag in the vdev that is initialized to 1, and
> > transports can unset it when the revision is negotiated or during init?
> 
> I would say have modern_features, legacy_features, and set host_features
> correctly.
> 

Started digging through the code a bit:

Currently the only time where the transport is actually mucking with
the feature bits is when the device is plugged, when it adds its
feature bits and calls virtio_bus_get_vdev_features() to get the device
specific bits. Only mmio knows at this point in time whether it has a
v1.0 or a legacy device. Other transports will need to call this
function again when the guest has actually set the revision.

My plan is the following:

- have virtio_bus_get_vdev_features_rev() that takes
  a virtio-rev parameter (0: legacy, 1: v1.0)
- unconverted transports keep calling virtio_bus_get_vdev_features()
  which calls virtio_bus_get_vdev_features_rev(..., 0)
- the rev parameter gets passed to the device, which hands over the
  right feature set (I don't think that we can use static feature
  tables, as the device may need to calulate the desired feature set
  dynamically)

Sounds reasonable?