Re: [PATCH] content: clarify feature negotiation terminology and init sequence

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

On Wed, Apr 22, 2026 at 02:05:27PM -0400, Stefan Hajnoczi wrote:
> On Wed, Apr 22, 2026 at 11:38:55AM -0400, Michael S. Tsirkin wrote:
> > Make several clarifications to the init sequence documentation:
> > 
> > The Linux virtio core (drivers/virtio/virtio.c) initializes devices
> > as follows:
> >   1. Intersect driver and device feature bits
> >   2. finalize_features() - write accepted features to the device
> >   3. drv->validate() - read config space, may clear feature bits
> >      (e.g. virtio-net clears VIRTIO_NET_F_MTU if mtu < MIN_MTU,
> >      balloon clears PAGE_POISON if guest does not init pages)
> >   4. If validate changed any features, finalize_features() again
> >   5. virtio_features_ok() - set FEATURES_OK, confirm with device
> > 
> > this allows the device to know which fields will be read:
> > recommend this in the spec.
> > 
> > Legacy driver detection is specified using a mechanism that
> > does not work on all transports. Make it clear that it's an
> > example: what matters is that devices do detection in some way
> > and are compatible with legacy drivers.
> > 
> > Clarify the distinction between features written to the
> > device ("accepted") and features confirmed via FEATURES_OK
> > ("negotiated").
> > 
> > "acknowledged" is used as a synonym for "accepted", but only in two
> > places. Just use "accepted" consistently.
> > 
> > Spec describes multiple moving pieces then ends with "before accepting
> > it" - vague, and is overloading "accept". Replace with a reference to
> > FEATURES_OK.
> > 
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/241
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  content.tex | 31 ++++++++++++++++++++++++-------
> >  1 file changed, 24 insertions(+), 7 deletions(-)
> > 
> > diff --git a/content.tex b/content.tex
> > index 5de811f..3dcba31 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -39,7 +39,7 @@ \section{\field{Device Status} Field}\label{sec:Basic Facilities of a Virtio Dev
> >  \item[DRIVER_OK (4)] Indicates that the driver is set up and ready to
> >    drive the device.
> >  
> > -\item[FEATURES_OK (8)] Indicates that the driver has acknowledged all the
> > +\item[FEATURES_OK (8)] Indicates that the driver has accepted all the
> >    features it understands, and feature negotiation is complete.
> >  
> >  \item[SUSPEND (16)] When VIRTIO_F_SUSPEND is negotiated, indicates that the
> > @@ -89,13 +89,21 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
> >  
> >  Each virtio device offers all the features it understands.  During
> >  device initialization, the driver reads this and tells the device the
> > -subset that it accepts.  The only way to renegotiate is to reset
> > -the device.
> > +subset that it accepts.  The device validates this subset and
> > +accepts it to complete the negotiation, or rejects it, leaving
> > +the negotiation incomplete. Once the negotiation is complete,
> > +the only way to renegotiate is to reset the device.
> > +
> > +A feature bit is \emph{accepted} once the driver has written it to the
> > +device.
> 
> The above paragraph says the device "accepts" features too. Therefore it
> is confusing to define "accepted" as only require driver action in this
> sentence. Is it implied that the driver's write only completes when the
> device has also accepted the features? I think it would be simpler to
> avoid using the word "accept" in two different steps of the feature bit
> negotiation process.

We don't really need a new verb - we have negotiated for this.


+subset that it accepts.  The device validates this subset and
+either completes the negotiation successfully (the last subset of features
that the driver accepted is consider negotiated then) or fails, leaving
+the feature negotiation incomplete. Once the negotiation is complete,
+the only way to renegotiate is to reset the device.

Hmm?


> > A feature bit is \emph{negotiated} once FEATURES_OK has been
> > +set and confirmed.  Before FEATURES_OK is confirmed, features are
> > +accepted but not yet negotiated, and the device has not confirmed that it
> > +supports the combination selected by the driver.
> >  
> >  This allows for forwards and backwards compatibility: if the device is
> >  enhanced with a new feature bit, older drivers will not write that
> >  feature bit back to the device.  Similarly, if a driver is enhanced with a feature
> > -that the device doesn't support, it see the new feature is not offered.
> > +that the device doesn't support, it will see that the new feature is not offered.
> >  
> >  Feature bits are allocated as follows:
> >  
> > @@ -189,8 +197,8 @@ \subsection{Legacy Interface: A Note on Feature
> >  
> >  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 devices MUST detect Legacy drivers, e.g. by detecting that
> > +VIRTIO_F_VERSION_1 has not been accepted by the driver.
> >  
> >  In this case device is used through the legacy interface.
> >  
> > @@ -314,6 +322,11 @@ \section{Device Configuration Space}\label{sec:Basic Facilities of a Virtio Devi
> >  greater than the specified 8-bit size.
> >  \end{note}
> >  
> > +\drivernormative{\subsection}{Device Configuration Space}{Basic Facilities of a Virtio Device / Device Configuration Space}
> > +Before reading a device-specific configuration field that is
> > +conditional on a feature bit, the driver SHOULD first accept
> > +that feature bit.
> > +
> >  \devicenormative{\subsection}{Device Configuration Space}{Basic Facilities of a Virtio Device / Device Configuration Space}
> >  The device MUST allow reading of any device-specific configuration
> >  field before FEATURES_OK is set by the driver.  This includes fields which are
> > @@ -530,7 +543,11 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
> >  \item\label{itm:General Initialization And Device Operation /
> >  Device Initialization / Read feature bits} Read device feature bits, and write the subset of feature bits
> >     understood by the OS and driver to the device.  During this step the
> > -   driver MAY read (but MUST NOT write) the device-specific configuration fields to check that it can support the device before accepting it.
> > +   driver MAY read (but MUST NOT write) the device-specific configuration
> > +   fields to check that it can support the device before setting FEATURES_OK.
> > +   The driver SHOULD accept feature bits before reading configuration
> > +   fields conditional on them.  The driver MAY then clear feature bits,
> > +   write the updated subset to the device, and repeat this process.
> 
> The last sentence is a little unclear to me. I think it's saying a
> driver can accept some feature bits, read the configuration space,
> and then change its mind and accept a subset of the previously accepted
> feature bits - all before setting FEATURES_OK.
> 
> The part that confused me was "write the updated subset to the device". Can the "accept" language be used here?
> -> "accept this new subset of feature bits"

Good point, but we need to also tell the device. Let's use just that?

  The driver MAY then accept a different subset of feature bits
  (e.g., deciding, based on the configuration fields, not to use a certain feature),
  tell the device about the updated subset, and repeat this process.


> >  
> >  \item\label{itm:General Initialization And Device Operation / Device Initialization / Set FEATURES-OK} Set the FEATURES_OK status bit.  The driver MUST NOT accept
> >     new feature bits after this step.
> > -- 
> > MST
> > 
> >