Re: [RFC PATCH 1/1] virtio: write back features before verify

[Michael S. Tsirkin]

On Sun, Oct 03, 2021 at 07:00:30AM +0200, Halil Pasic wrote:
> On Sat, 2 Oct 2021 14:20:47 -0400
> "Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote:
> 
> > > >From my perspective the problem is that the version of the device  
> > > remains in limbo as long as the features have not yet been finalized,
> > > which means that the endianness of the config space remains in limbo as
> > > well. Both device and driver might come to different conclusions.  
> > 
> > Version === legacy versus modern?
> > It is true that feature negotiation can not be used by device to decide that
> > question simply because it happens too late.
> > So let's not use it for that then ;)
> > 
> > Yes we have VERSION_1 which looks like it should allow this, but
> > unfortunately it only helps with that for the driver, not the device.
> > 
> > In practice legacy versus modern has to be determined by
> > transport specific versioning, luckily we have that for all
> > specified transports (can't say what happens with rproc).
> 
> So if we look at ccw, you say that the revision negotiation already
> determines whether VERSION_1 is negotiated or not, and the
> feature bit VERSION_1 is superfluous?
> 
> That would also imply, that 
> 1) if revision > 0 was negotiated then the device must offer VERSION_1
> 2) if revision > 0 was negotiated and the driver cleared VERSION_1
>    the device must refuse to operate.
> 3) if revision > 0 was negotiated then the driver should reject 
>    to drive a device if it does not offer VERSION_1
> 4) if revision > 0 was negotiated the driver must accept VERSION_1
> 5) if revision > 0 was *not* negotiated then the device should not offer
>    VERSION_1 because at this point it is already certain that the device
>    can not act in accordance to the virtio 1.0 or higher interface.
> 
> Does that sound about right?

To me, it does.

> IMHO we should also change 
> https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-160003
> and the definition of VERSION_1 because both sides have to know what is
> going on before features are fully negotiated. Or?
> 
> Regards,
> Halil
> 

I guess so. And I guess we need transport-specific sections
describing this behaviour for each transport.

So something like this, for starters?

diff --git a/content.tex b/content.tex
index 1398390..c526dd3 100644
--- a/content.tex
+++ b/content.tex
@@ -140,10 +140,13 @@ \subsection{Legacy Interface: A Note on Feature
 Bits}\label{sec:Basic Facilities of a Virtio Device / Feature
 Bits / Legacy Interface: A Note on Feature Bits}
 
-Transitional Drivers MUST detect Legacy Devices by detecting that
-the feature bit VIRTIO_F_VERSION_1 is not offered.
-Transitional devices MUST detect Legacy drivers by detecting that
-VIRTIO_F_VERSION_1 has not been acknowledged by the driver.
+Transitional drivers MAY support operating legacy devices.
+Transitional devices MAY support operation by legacy drivers.
+
+Transitional drivers MUST detect legacy devices in a way that is
+transport specific.
+Transitional devices MUST detect legacy drivers in a way that
+is transport specific.
 
 In this case device is used through the legacy interface.
 
@@ -160,6 +163,25 @@ \subsection{Legacy Interface: A Note on Feature
 Specification text within these sections generally does not apply
 to non-transitional devices.
 
+\begin{note}
+The device offers different features when used through
+the legacy interface and when operated in accordance with this
+specification.
+\end{note}
+
+Transitional drivers MUST use Devices only through the legacy interface
+if the feature bit VIRTIO_F_VERSION_1 is not offered.
+Transitional devices MUST NOT offer VIRTIO_F_VERSION_1 when used through
+the legacy interface.
+
+When the driver uses a device through the legacy interface, then it
+MUST only accept the features the device offered through the
+legacy interface.
+
+When used through the legacy interface, the device SHOULD
+validate that the driver only accepted the features it
+offered through the legacy interface.
+
 \section{Notifications}\label{sec:Basic Facilities of a Virtio Device
 / Notifications}