All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] content: clarify feature negotiation terminology and init sequence
@ 2026-04-23 22:24 Michael S. Tsirkin
  2026-04-24 12:05 ` Albert Esteve
  2026-04-27 13:38 ` Stefan Hajnoczi
  0 siblings, 2 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2026-04-23 22:24 UTC (permalink / raw)
  To: virtio-comment; +Cc: stefanha

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.

Define "negotiated" for features confirmed via FEATURES_OK.

"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>
---

changes:
address review comments by Stefan

 content.tex | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/content.tex b/content.tex
index 5de811f..0f4d662 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,16 @@ \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
+either completes the negotiation successfully (the last subset of features
+that the driver accepted is considered negotiated then) or fails,
+leaving the feature negotiation incomplete. Once the negotiation is
+complete, the only way to renegotiate is to reset the device.
 
 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 +192,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 +317,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 +538,13 @@ \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 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] content: clarify feature negotiation terminology and init sequence
  2026-04-23 22:24 [PATCH v2] content: clarify feature negotiation terminology and init sequence Michael S. Tsirkin
@ 2026-04-24 12:05 ` Albert Esteve
  2026-04-24 12:16   ` Michael S. Tsirkin
  2026-04-27 13:38 ` Stefan Hajnoczi
  1 sibling, 1 reply; 4+ messages in thread
From: Albert Esteve @ 2026-04-24 12:05 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-comment, stefanha

On Fri, Apr 24, 2026 at 12:34 AM Michael S. Tsirkin <mst@redhat.com> 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.
>
> Define "negotiated" for features confirmed via FEATURES_OK.
>
> "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>
> ---
>
> changes:
> address review comments by Stefan
>
>  content.tex | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index 5de811f..0f4d662 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,16 @@ \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
> +either completes the negotiation successfully (the last subset of features
> +that the driver accepted is considered negotiated then) or fails,
> +leaving the feature negotiation incomplete. Once the negotiation is
> +complete, the only way to renegotiate is to reset the device.
>
>  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 +192,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 +317,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 +538,13 @@ \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

As I understand it, this should be SHALL (mandatory requirement) -- or
MUST, not SHOULD (which reads as a recommendation). Otherwise it would
violate the spec, and probably cause a hard BUG() panic in the kernel
if the driver queries a feature it never declared in its
feature_table, or result in silent data corruption if it bypasses
feature checks entirely and reads conditional config fields directly.

> +   fields conditional on them.  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
>
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] content: clarify feature negotiation terminology and init sequence
  2026-04-24 12:05 ` Albert Esteve
@ 2026-04-24 12:16   ` Michael S. Tsirkin
  0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2026-04-24 12:16 UTC (permalink / raw)
  To: Albert Esteve; +Cc: virtio-comment, stefanha

On Fri, Apr 24, 2026 at 02:05:46PM +0200, Albert Esteve wrote:
> On Fri, Apr 24, 2026 at 12:34 AM Michael S. Tsirkin <mst@redhat.com> 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.
> >
> > Define "negotiated" for features confirmed via FEATURES_OK.
> >
> > "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>
> > ---
> >
> > changes:
> > address review comments by Stefan
> >
> >  content.tex | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/content.tex b/content.tex
> > index 5de811f..0f4d662 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,16 @@ \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
> > +either completes the negotiation successfully (the last subset of features
> > +that the driver accepted is considered negotiated then) or fails,
> > +leaving the feature negotiation incomplete. Once the negotiation is
> > +complete, the only way to renegotiate is to reset the device.
> >
> >  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 +192,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 +317,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 +538,13 @@ \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
> 
> As I understand it, this should be SHALL (mandatory requirement) -- or
> MUST, not SHOULD (which reads as a recommendation). Otherwise it would
> violate the spec, and probably cause a hard BUG() panic in the kernel
> if the driver queries a feature it never declared in its
> feature_table, or result in silent data corruption if it bypasses
> feature checks entirely and reads conditional config fields directly.


Unfortunately, we never required this in the past.

So we can recommend but we can not make it a MUST now. Devices have to
cope with existing drivers.



> > +   fields conditional on them.  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
> >
> >


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] content: clarify feature negotiation terminology and init sequence
  2026-04-23 22:24 [PATCH v2] content: clarify feature negotiation terminology and init sequence Michael S. Tsirkin
  2026-04-24 12:05 ` Albert Esteve
@ 2026-04-27 13:38 ` Stefan Hajnoczi
  1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2026-04-27 13:38 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-comment

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

On Thu, Apr 23, 2026 at 06:24:12PM -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.
> 
> Define "negotiated" for features confirmed via FEATURES_OK.
> 
> "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>
> ---
> 
> changes:
> address review comments by Stefan
> 
>  content.tex | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-27 13:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-23 22:24 [PATCH v2] content: clarify feature negotiation terminology and init sequence Michael S. Tsirkin
2026-04-24 12:05 ` Albert Esteve
2026-04-24 12:16   ` Michael S. Tsirkin
2026-04-27 13:38 ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.