Re: [virtio] Re: [virtio-comment] [PATCH RFC] virtio-pci: new configuration layout

[Rusty Russell <rusty@au1.ibm.com>]

Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> Some remarks from my side...
>
> On Tue, 27 Aug 2013 10:38:59 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> On Tue, Aug 27, 2013 at 04:24:47PM +0930, Rusty Russell wrote:
>> > "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > > This is the new configuration layout.
>> > >
>> > > Notes:
>> > > - Everything is LE
>> > > - There's a feature bit that means spec 1.0 compliant.
>> > > - Both devices and drivers can either require the 1.0 interface
>> > >   or try to include compatibility support. The spec isn't forcing
>> > >   this decision.
>> > 
>> > Hmm, this kind includes other changes already proposed, like the LE
>> > change and the framing change.  I think this conceptually splits nicely:
>> > 
>> > 1) Feature bit 32 proposal.
>> > 2) Endian change.
>> > 3) Framing change.
>> > 4) PCI layout change.
>> 
>> Right - they are mostly in different parts of the document.
>> I put it all together so it's easy to see how we intend to
>> handle the transition.
>> So is everyone OK with keeping this in a single patch?
>
> The new feature bit is supposed to cover all of this, right? Then this
> should be one patch.

Well, the new feature would be "1.0 compliant" which will cover much
more than this.  We'll have a whole document on the differences between
legacy and 1.0, in fact.

>> > > - I kept documentation of the legacy interface around, and added notes
>> > >   on transition inline. They are in separate sections each clearly marked
>> > >   "Legacy Interface" so we'll be able to separate them out
>> > >   from the final document as necessary - for now I think it's easier
>> > >   to keep it all together.
>> > 
>> > Good thinking: most of us know the current spec so it's definitely
>> > clearer.  And makes sure we're thinking about the transition.
>> > 
>> > > Only virtio PCI has been converted.
>> > > Let's discuss this on the meeting tonight, once we figure out PCI
>> > > we can do something similar for MMIO and CCW.
>> > 
>> > > @@ -137,6 +139,11 @@ Feature bits are allocated as follows:
>> > >    24 to 31: Feature bits reserved for extensions to the queue and 
>> > >    feature negotiation mechanisms
>> > >  
>> > > +  32: Feature bit must be set for any device compliant with this
>> > > +  revision of the specification, and acknowledged by all device drivers.
>
> Would it make sense to have a bit 33 "rings big endian" whose validity
> depends on bit 32 set? This would make it possible for ccw to keep its
> current endianness.

Very awkward.  Our experience with bi-endian devices on PowerPC suggests
it's far better to not negotiate endian.  It also avoids a branch
everywhere in the driver and host.

>> > > +Legacy Interface
>> > > +	is an interface specified by an earlier draft of this specification
>> > > +        (up to 0.9.X)
>> > > +Legacy Device
>> > > +	is a device implemented before this specification was released,
>> > > +        and implementing a legacy interface on the host side
>> > > +Legacy Driver
>> > > +	is a driver implemented before this specification was released,
>> > > +        and implementing a legacy interface on the guest side
>> > > +
>> > > +to simplify transition from these earlier draft interfaces,
>> > > +it is possible to implement
>> > > +
>> > > +Transitional Device
>> > > +	a device supporting both drivers conforming to this
>> > > +        specification, and legacy drivers
>> > > +
>> > > +Transitional Driver
>> > > +	a driver supporting both devices conforming to this
>> > > +	specification, and legacy devices
>
> What happens to legacy devices in the future? Current implementers
> will obviously expose legacy devices, which means future drivers need
> to be transitional or they won't work with what is currently out there.
> Will legacy stay around (for the forseeable furture)? Will legacy
> devices still be considered standard compliant (as in "compliant to the
> legacy standard")?

Legacy devices will not be spec compliant, though we will map out the
differences for implementors and to make sure that transitional devices
& drivers are still possible.  This legacy part will eventually be a
separate document.

I expect them legacy to be firmly deprecated within a few years: new
features will probably only be offered for standard devices, and for PCI
there are some nice proposals in the standard which make life easier,
eg. for bootloaders.

>> > > +
>> > > +Device and driver that require support for revision 1.0 or newer of
>> > > +the specification to function, are called non-transitional device and driver,
>> > > +respectively.
>> > > +
>> > > +Transitional Drivers can detect Legacy Devices by detecting that
>> > > +Feature bit 32 is not offered.
>> > > +Transitional devices can detect Legacy drivers by detecting that
>> > > +Feature bit 32 has not been acknowledged by driver.
>
> Will we use new feature bits for new, incompatible revisions? Or will
> we try to stay backwards compatible?

My intent is to stay backwards compatible, using feature bits.  At some
future point we may deprecate older features I suppose.

>> > > +2.1.4.2.1 Legacy Interfaces: A Note on Message Framing
>> > > +-----------------------
>> > > +Regrettably, initial driver implementations used simple layouts, and
>> > > +devices came to rely on it, despite this specification wording[10]. It
>> > > +is thus recommended that when using legacy interfaces,
>> > > +drivers should be conservative in their assumptions,
>> > > +unless the VIRTIO_F_ANY_LAYOUT feature is accepted.
>
> So ANY_LAYOUT and feature bit 32 are mutually exclusive?

I was thinking feature 32 would imply ANY_LAYOUT; there's a feature
request for that:
        https://tools.oasis-open.org/issues/browse/VIRTIO-10

Since there seems to be consensus that we remove the SCSI commands from
virtio-blk, it seems sensible to do that (which is a blocker for
ANY_LAYOUT) and then remove ANY_LAYOUT from the non-legacy wording and
make it clear that we're always independent of framing.

>> Also Revision ID is only 8 bit - it's designed for hardware where
>> making a new revision is expensive. In software we'll run out of that
>> eventually.
>
> So Revision ID is a PCI-specific thing, right? Not all transports will
> necessarily have something equivalent, so they would need to depend on
> the feature bit.

Yes.

Cheers,
Rusty.