Virtualization - Nov 2014 - [PATCH RFC v4 00/16] qemu: towards virtio-1 host support

On Thu, 27 Nov 2014 18:18:25 +0200
"Michael S. Tsirkin" <mst at 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?
> 
> > 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?

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 at 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.

-- 
MST

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

Also, let's focus on one device, e.g. -net for now.
Then probably virtio scsi.
That's because blk needs to be reworked to support ANY_LAYOUT.

-- 
MST

On Thu, 27 Nov 2014 18:35:40 +0200
"Michael S. Tsirkin" <mst at 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 at 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?

On Thu, 27 Nov 2014 18:38:59 +0200
"Michael S. Tsirkin" <mst at redhat.com> wrote:
> Also, let's focus on one device, e.g. -net for now.
Yes, I'm currently looking at net.
> Then probably virtio scsi.
> That's because blk needs to be reworked to support ANY_LAYOUT.
I had focused on blk because that's the other device used on every s390
image :) But the ANY_LAYOUT stuff looks like a bit of work :/