Re: [virtio] [PATCH 3/4] ccw: virtio revision vs. virtio feature bits

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

On Mon, Feb 09, 2015 at 07:09:38PM +0100, Cornelia Huck wrote:
> On Mon, 9 Feb 2015 15:49:36 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Mon, Feb 09, 2015 at 09:38:12AM +0100, Cornelia Huck wrote:
> > > On Sun, 8 Feb 2015 11:21:18 +0100
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Fri, Oct 31, 2014 at 03:26:18PM +0100, Cornelia Huck wrote:
> > > > > We don't want the VERSION_1 feature bit unless at least revision 1
> > > > > has been negotiated. Make this a requirement.
> > > > > 
> > > > > VIRTIO-119
> > > > > 
> > > > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > > > 
> > > > Sorry about the delay in response.
> > > > I think I disagree with this one.
> > > > 
> > > > I think this discusses how transitional devices work when
> > > > using the legacy interface?
> > > > In that case, this needs to go into a separate section,
> > > > explicitly labelled "legacy".
> > > 
> > > Hmm... but a legacy device does not actually know about VERSION_1, does
> > > it? Or am I misunderstanding what you mean?
> > 
> > you are describing here behaviour where negotiated
> > revision is 0, is that right?
> > If so this is transitional device/driver behaviour, isn't it?
> 
> But a legacy device/driver can't negotiate version 0, as it doesn't
> know the respective command.

We have a table in the spec that says version 0 is legacy,
do we not? I assumed some legacy drivers do negotiate revisions?

> > > > Looking into this deeper, how do devices "not accept"
> > > > bits when using a legacy interface?
> > > 
> > > A legacy device can't accept the VERSION_1 bit, as it only has 32
> > > feature bits. It will fail the ccw for the second half of the feature
> > > bits.
> > 
> > So are we talking about drivers which set guest feature bits
> > which are not set in the host feature bit mask?
> > 
> > We have this text:
> > 	2.2.2 Device Requirements: Feature Bits
> > 	The device MUST NOT offer a feature which requires another feature which
> > 	was not offered. The device
> > 	SHOULD accept any valid subset of features the driver accepts, otherwise
> > 	it MUST fail to set the FEA-
> > 	TURES_OK device status bit when the driver writes it.
> 
> But here you were asking about legacy devices, weren't you? And these
> simply don't have the second half of feature bits, and of course don't
> know of the FEATURE_OK bit.
> 
> And this section is talking about feature bit dependencies, isn't it?

I don't rightly know - I asked above whether you are
talking about drivers which set guest feature bits
which are not set in the host feature bit mask.

> > > > 
> > > > Also, the wording here seems wrong to me.
> > > 
> > > Could you elaborate?
> > 
> > "if not at least" seems confusing. do you mean
> > "negotiated revision is 0"?
> 
> "if not at least revision 1 has been negotiated" == "no revision or
> revision 0 has been negotiated" (so revision 0, as this is not about
> legacy devices/drivers)
> 
> I don't know what's confusing about "at least"?

E.g. it's unclear whether no negotiation is included.
I prefer it that we stick to positives.


> > negociated isn't in my dictionary.
> 
> Sigh, that's an obvious typo.
> 
> > > I think this all boils down to whether revision handling is enough to
> > > designate a device as non-legacy, no?
> > 
> > It's not clear to me from this text.
> > 4.3.2.1 Setting the Virtio Revision
> > says rev 0 is legacy interface, but there
> > is not conformance statement.
> 
> Hm, I meant "standard compliant device/driver using the legacy
> interface". In practice, revision 0 is unlikely to be ever negotiated.
> 
> This all boils down to what a legacy device/driver is supposed to be.
> For me, it is a device/driver not knowing about the standard.

Yes, it's an old one written before the standard, doing whatever
it always did.

> Negotiating any revision (including 0) indicates it cares about the
> standard.

So really no legacy driver/device had revision 0?
Why don't we forbid revision 0 outright?

> Talking about revisions in the legacy sections therefore
> sounds wrong to me.

Let's just say "no revision has been negotiated"?



> > Would you like to add text that says negotiating rev 0
> > MUST force legacy interface?
> > Such text should be in a separate section marked "Legacy Interface".
> > 
> > We can add text that says drivers must not accept VIRTIO_F_VERSION_1
> > in this mode, and devices must not offer VIRTIO_F_VERSION_1.
> > Seems generic, non ccw specific, does it not?
> 
> It is specific to ccw revisions, unless we describe a general revision
> mechanism with specific implementations.

The concept of legacy interface is generic.
The way to detect it is transport specific.

> > Such text should also be in a separate section marked "Legacy Interface".
> 
> Can we somehow withdraw the ballot? It seems this whole topic needs
> more discussion, and I'm currently in the middle of a move and don't
> have a head for in-depth discussions.

Sure, I can do it.
For the record, can you please add a comment in the relevant issue tracker
to explicitly confirm that you propose cancelling
this ballot:

https://www.oasis-open.org/apps/org/workgroup/virtio/ballot.php?id=2764


-- 
MST