RE: [PATCH v10 01/10] virtio: document forward compatibility guarantees

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, March 2, 2023 8:05 AM
> 
> 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>
> ---

It may be little painful, but in 10 patch series, it is worth having per patch change log.
Because I do not know my reviewed by tag in [1] was dropped because of a big change, if so what was it or it was simply missed out.

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00237.html

>  content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 0e474dd..0c2d917 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 the 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.) 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.
> +
> +In particular, this is
> +especially important for features limited to specific transports, as
> +enabling these for more transports in future versions of the
> +specification is highly likely to require changing the behaviour from
> +drivers and devices.  Drivers and devices supporting multiple
> +transports need to carefully maintain per-transport lists of allowed
> +features.
> +
>  \drivernormative{\subsection}{Feature Bits}{Basic Facilities of a Virtio Device /
> Feature Bits}  The driver MUST NOT accept a feature which the device did not
> offer,  and MUST NOT accept a feature which requires another feature which
> was  not accepted.
> 
> +The driver MUST validate the feature bits offered by the device.
> +The driver MUST ignore and MUST NOT accept any feature bit that is
> +\begin{itemize} \item not described in this specification, \item marked
> +as reserved, \item not supported for the specific transport, \item not
> +defined for the device type.
> +\end{itemize}
> +
>  The driver SHOULD go into backwards compatibility mode  if the device does
The grammar tool is suggesting s/backwards/backward.

> not offer a feature it understands, otherwise MUST  set the FAILED \field{device
> status} bit and cease initialization.
> 
> +By contrast, the driver MUST NOT fail solely because a feature it does
> +not understand has been offered by the device.
> +
Above line can be written without introducing "has been" given we have only two entities (driver, dev) to care about.

By contrast, the driver MUST NOT fail solely because it does not understand the feature offered by the device.

>  \devicenormative{\subsection}{Feature Bits}{Basic Facilities of a Virtio Device /
> 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
Extra white space between feature and which, subset and of.

> features the driver accepts, otherwise it MUST fail to set the  FEATURES_OK
> \field{device status} bit when the driver writes it.
> 
Extra white space between the FEATURE_OK

> +The device MUST NOT offer feature bits corresponding to features it
> +would not support if accepted by the driver (even if the driver is
> +prohibited from accepting the feature bits by the specification); for
> +the sake of clarity, this refers to feature bits not described in this
> +specification, reserved feature bits and feature bits reserved or not
> +supported for the specific transport or the specific device type, but
> +this does not preclude devices written to a future version of this
> +specification from offering such feature bits should such a
> +specification have a provision for devices to support the corresponding
> +features.
> +
>  If a device has successfully negotiated a set of features  at least once (by
> accepting the FEATURES_OK \field{device  status} bit during device
> initialization), then it SHOULD
> --
> MST