Re: [PATCH v9 01/10] virtio: document forward compatibility guarantees

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Nov 24, 2022 at 03:34:19PM +0800, Jason Wang wrote:
> On Thu, Nov 24, 2022 at 2:59 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Nov 24, 2022 at 12:33:52PM +0800, Jason Wang wrote:
> > > On Thu, Nov 24, 2022 at 5:08 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > Feature negotiation forms the basis of forward compatibility
> > > > guarantees of virtio but has never been properly documented.
> > > > Do it now.
> > > >
> > > > Suggested-by: Halil Pasic <pasic@linux.ibm.com>
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 42 insertions(+)
> > > >
> > > > diff --git a/content.tex b/content.tex
> > > > index 3051399..e3203be 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -114,21 +114,63 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
> > > >  In particular, new fields in the device configuration space are
> > > >  indicated by offering a new feature bit.
> > > >
> > > > +To keep te feature negotiation mechanism extensible, it is important
> > > > +that devices \em{do not} offer any feature bits that they would not be
> > > > +able to handle if the driver accepted them (even though drivers are not
> > > > +supposed to accept them in the first place even if offered, according to
> > > > +this version of the specification.)
> > >
> > > It looks to me if we want to clarify like this, feature negotiation is
> > > not sufficient. Do we need to do something similar in other basic
> > > facilities? Generally, we probably need to do this for facilities that
> > > are similar to features (status, virtqueue size and others).
> >
> > I'm not sure about "not sufficient". It's sufficient as long
> > as you just want to extend features. What triggered this
> > work is adding a transport specific feature.
> 
> E.g:
> 
> For status: Devices do not offer any status bit it would not be able to handle.
> For virtqueue size:  Devices do not offer virtqueue size it would not
> be able to handle.
> 
> ?

Jason I think what you miss here is this part:

"even though drivers are not
supposed to accept them in the first place even if offered, according to
this version of the specification"

does not apply to status and virtqueue size.


Let me clarify what all this means.
It seems safe for a device to offer a reserved feature bit
since drivers are not supposed to accept it.
This text says device must not rely on this.

How would this apply to status or vq size? I don't see.










> >
> >
> > > > Likewise, it is important that
> > > > +drivers \em{do not} accept feature bits they do not know how to handle
> > > > +(even though devices are not supposed to offer them in the first place,
> > > > +according to this version of the specification.) The preferred way for
> > > > +handling reserved and unexpected features is that the driver ignores
> > > > +them.
> > >
> > > I'm not sure this is the best way or whether we've defined the
> > > "unexpected features" before. It might be better to say that to accept
> > > the features that the driver understands.
> >
> > So specifically, if feature is documented as pci only and then
> > down the road we enable it for ccw. Does driver "understand" it? Or not?
> > It is clearly unexpected.
> > I am not too worried about using terms we didn't define this is not
> > part of a conformance statement.
> > And people seem to get the meaning ;)
> 
> Ok.
> 
> Thanks