Re: [PATCH V3 Resend] virtio-transport: Clarify requirements

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

Dropped virtio-dev this is clearly spec discussion.

On Mon, May 20, 2024 at 02:59:40PM +0530, Viresh Kumar wrote:
> The virtio documentation currently doesn't define any generic
> requirements that are applicable to all transports. They can be useful
> while adding support for a new transport.
> 
> This commit tries to define the same.
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V2->V3:
> - Minor fixes.
> - Added Reviewed by from Alex.
> 
> V1->V2:
> - Lot of changes after discussions with Alex and Cornelia.
> - Almost a rewrite of the first commit.
> - Add Transport normative sections.
> 
>  commands.tex    |  3 +-
>  conformance.tex | 14 +++++++++
>  content.tex     | 78 +++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 92 insertions(+), 3 deletions(-)
> 
> diff --git a/commands.tex b/commands.tex
> index 25ea8ee3bc78..b64b14424bd2 100644
> --- a/commands.tex
> +++ b/commands.tex
> @@ -7,7 +7,8 @@
>  % How we format a field name
>  \newcommand{\field}[1]{\emph{#1}}
>  
> -% Mark a normative section (driver or device)
> +% Mark a normative section (driver or device, or transport)
> +\newcommand{\transportnormative}[3]{#1{Transport Requirements: #2}\label{transportnormative:#3}}
>  \newcommand{\drivernormative}[3]{#1{Driver Requirements: #2}\label{drivernormative:#3}}
>  \newcommand{\devicenormative}[3]{#1{Device Requirements: #2}\label{devicenormative:#3}}
>  \newcounter{clausecounter}

> diff --git a/conformance.tex b/conformance.tex
> index dc00e84e75ae..4a873169ce63 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -11,6 +11,10 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  
>  Conformance targets:
>  \begin{description}
> +\item[Transport] A transport MUST conform to one conformance clauses:
> +  \begin{itemize}
> +    \item Clause \ref{sec:Conformance / Transport Conformance}.
> +  \end{itemize}
>  \item[Driver] A driver MUST conform to four conformance clauses:
>    \begin{itemize}
>      \item Clause \ref{sec:Conformance / Driver Conformance}.
> @@ -66,6 +70,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>    \end{itemize}
>  \end{description}
>  
> +\conformance{\section}{Transport Conformance}\label{sec:Conformance / Transport Conformance}
> +
> +A transport MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{transportnormative:Virtio Transport Options / Virtio Transport Requirements}
> +\end{itemize}
> +
>  \conformance{\section}{Driver Conformance}\label{sec:Conformance / Driver Conformance}
>  
>  A driver MUST conform to the following normative statements:
> @@ -93,6 +105,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Packed Virtqueues / Supplying Buffers to The Device / Sending Available Buffer Notifications}
>  \item \ref{drivernormative:General Initialization And Device Operation / Device Initialization}
>  \item \ref{drivernormative:General Initialization And Device Operation / Device Cleanup}
> +\item \ref{drivernormative:Virtio Transport Options / Virtio Transport Requirements}
>  \item \ref{drivernormative:Reserved Feature Bits}
>  \end{itemize}
>  
> @@ -172,6 +185,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Packed Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Packed Virtqueues / Scatter-Gather Support}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Shared Memory Regions}
> +\item \ref{devicenormative:Virtio Transport Options / Virtio Transport Requirements}
>  \item \ref{devicenormative:Reserved Feature Bits}
>  \end{itemize}
>  
> diff --git a/content.tex b/content.tex
> index 0a62dce5f65f..a79993b5ed69 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -631,8 +631,82 @@ \section{Device Cleanup}\label{sec:General Initialization And Device Operation /
>  
>  \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options}
>  
> -Virtio can use various different buses, thus the standard is split
> -into virtio general and bus-specific sections.
> +Devices and drivers can use different transport methods to enable
> +interaction, for example PCI, MMIO, or Channel I/O. The transport
> +methods define various aspects of the communication between the device
> +and the driver, like device discovery, exchanging capabilities,
> +interrupt handling, data transfer, etc. For example, in a host/guest
> +architecture, the host might expose a device to the guest on a PCI bus,
> +and the guest will use a PCI-specific driver to interact with it.
> +
> +The standard is split into sections describing general virtio
> +implementation and transport-specific sections.
> +

This makes it seem like trasport is distinct from both
device and driver. Makes no sense to me.

> +\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements}
> +
> +\transportnormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options / Virtio Transport Requirements}
> +The transport MUST provide a mechanism for the driver to discover the
> +device.
> +
> +The transport MUST provide a mechanism for the driver to identify the
> +device type.
> +
> +The transport MUST provide a mechanism for communicating virtqueue
> +configurations between the device and the driver.
> +
> +The transport MUST allow multiple virtqueues per device. The number of
> +virtqueues for a pair of device-driver are governed by the individual
> +device protocol.
> +
> +The transport MUST provide a mechanism that the device and the driver
> +use to access memory for implementing virtqueues.
> +
> +The transport MUST provide a mechanism for the device to notify the
> +driver and a mechanism for the driver to notify the device, for example
> +regarding availability of a buffer on the virtqueue.
> +
> +The transport MAY provide a mechanism for the driver to initiate a
> +reset of the virtqueues and device.
> +
> +The transport SHOULD provide a mechanism for the driver to read the
> +device status. The transport MUST provide a mechanism for the driver to
> +change the device status.
> +
> +The transport MUST provide a mechanism to implement config space between
> +the device and the driver.
> +
> +\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options / Virtio Transport Requirements}
> +
> +The device MUST keep any data associated with a device-initiated
> +transaction accessible to the driver until the driver acknowledges the
> +transaction to be complete.
> +
> +The device MUST NOT access the contents of a virtqueue before the
> +driver notifies, in a transport defined way, the device that the
> +virtqueue is ready to be accessed.
> +
> +The device MUST NOT access or modify buffers on a virtqueue after it has
> +notified the driver about their availability.
> +
> +The device MUST reset the virtqueues if requested by the driver, in a
> +transport defined way, if the transport provides such a method.
> +
> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options / Virtio Transport Requirements}
> +
> +The driver MUST acknowledge device notifications, as mandated by the
> +transport.
> +
> +The driver MUST NOT access virtqueue contents before the device notifies
> +about the readiness of the same.
> +
> +The driver MUST NOT access buffers, after it has added them to the
> +virtqueue and notified the device about their availability. The driver
> +MAY access them after the device has processed them and notified the
> +driver of their availability, in a transport defined way.
> +
> +The driver MAY ask the device to reset the virtqueues if, for example,
> +the driver times out waiting for a notification from the device for a
> +previously queued request.
>  

This makes no sense in a normative section.
Normative statements are for implementations not spec writers.
If you think of defining lots of new transports
(questionable) I think a new non-normative section
along the lines of newdevice.tex will make sense.


>  \input{transport-pci.tex}
>  \input{transport-mmio.tex}
> -- 
> 2.31.1.272.g89b43f80a514