[PATCH V3 Resend] virtio-transport: Clarify requirements

[PATCH V3 Resend] virtio-transport: Clarify requirements

[Viresh Kumar]

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.
+
+\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.
 
 \input{transport-pci.tex}
 \input{transport-mmio.tex}
-- 
2.31.1.272.g89b43f80a514

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

[Parav Pandit]

Michael,

> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Monday, May 20, 2024 3:00 PM
> To: virtio-comment@lists.linux.dev; virtio-dev@lists.linux.dev

There is still cross posting to virtio-dev.
Is the list open to community now?
If not, can you please send out the guidance and timeline to open it on virtio-comment list?

This way contributors can avoid resending it multiple times.

> Cc: Viresh Kumar <viresh.kumar@linaro.org>; Vincent Guittot
> <vincent.guittot@linaro.org>; Alex Bennée <alex.bennee@linaro.org>; stratos-
> dev@op-lists.linaro.org; Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org>; Cornelia Huck <cohuck@redhat.com>;
> Michael S. Tsirkin <mst@redhat.com>
> Subject: [PATCH V3 Resend] virtio-transport: Clarify requirements
> 
> 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.
> +
> +\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.
> 
>  \input{transport-pci.tex}
>  \input{transport-mmio.tex}
> --
> 2.31.1.272.g89b43f80a514
> 

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

[Stefano Garzarella]

Hi Viresh,
I haven't followed v1 and v2, so apologies in advance if my comments 
have already been discussed.

On Mon, May 20, 2024 at 02:59:40PM GMT, 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.
>+
>+\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.

This seems a little too stringent. For example, does MMIO offer a 
discovery mechanism? IIRC we have to pass the address via device tree or 
kernel command line.

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

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

Should we also talk about the possibility of resetting the entire 
device?

Thanks,
Stefano

>+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.
>
> \input{transport-pci.tex}
> \input{transport-mmio.tex}
>-- 
>2.31.1.272.g89b43f80a514
>
>

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

[Michael S. Tsirkin]

On Mon, May 20, 2024 at 09:42:02AM +0000, Parav Pandit wrote:
> Michael,
> 
> > From: Viresh Kumar <viresh.kumar@linaro.org>
> > Sent: Monday, May 20, 2024 3:00 PM
> > To: virtio-comment@lists.linux.dev; virtio-dev@lists.linux.dev
> 
> There is still cross posting to virtio-dev.

Well we say "do not do this" in the list subscription form.
Viresh, what gives?

> Is the list open to community now?

It's been open for a week. I think it's fair to say whoever wanted to
subscribe, has subscribed.

> If not, can you please send out the guidance and timeline to open it on virtio-comment list?
> 
> This way contributors can avoid resending it multiple times.
> 
> > Cc: Viresh Kumar <viresh.kumar@linaro.org>; Vincent Guittot
> > <vincent.guittot@linaro.org>; Alex Bennée <alex.bennee@linaro.org>; stratos-
> > dev@op-lists.linaro.org; Manos Pitsidianakis
> > <manos.pitsidianakis@linaro.org>; Cornelia Huck <cohuck@redhat.com>;
> > Michael S. Tsirkin <mst@redhat.com>
> > Subject: [PATCH V3 Resend] virtio-transport: Clarify requirements
> > 
> > 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.
> > +
> > +\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.
> > 
> >  \input{transport-pci.tex}
> >  \input{transport-mmio.tex}
> > --
> > 2.31.1.272.g89b43f80a514
> > 
> 

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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, May 20, 2024 5:55 PM
> 
> On Mon, May 20, 2024 at 09:42:02AM +0000, Parav Pandit wrote:
> > Michael,
> >
> > > From: Viresh Kumar <viresh.kumar@linaro.org>
> > > Sent: Monday, May 20, 2024 3:00 PM
> > > To: virtio-comment@lists.linux.dev; virtio-dev@lists.linux.dev
> >
> > There is still cross posting to virtio-dev.
> 
> Well we say "do not do this" in the list subscription form.
> Viresh, what gives?
> 
> > Is the list open to community now?
> 
> It's been open for a week. I think it's fair to say whoever wanted to subscribe,
> has subscribed.
> 
True. I meant to ask that do contributors need to wait for patch [1] to be merged before posting patches here?
You indicated there might be voting in [2], so trying to find what is next for contributors?

[1] https://lore.kernel.org/virtio-comment/te6tzbegqky7uz4skrag3yowet3phtq6nn7tegyrillgb4juwn@m4gi5ohfuk7m/T/#t
[2] https://lore.kernel.org/virtio-comment/te6tzbegqky7uz4skrag3yowet3phtq6nn7tegyrillgb4juwn@m4gi5ohfuk7m/T/#m7b5d6fc76d05df528c36a70cff30afe06bb44917

> > If not, can you please send out the guidance and timeline to open it on virtio-
> comment list?
> >
> > This way contributors can avoid resending it multiple times.
> >
> > > Cc: Viresh Kumar <viresh.kumar@linaro.org>; Vincent Guittot
> > > <vincent.guittot@linaro.org>; Alex Bennée <alex.bennee@linaro.org>;
> > > stratos- dev@op-lists.linaro.org; Manos Pitsidianakis
> > > <manos.pitsidianakis@linaro.org>; Cornelia Huck <cohuck@redhat.com>;
> > > Michael S. Tsirkin <mst@redhat.com>
> > > Subject: [PATCH V3 Resend] virtio-transport: Clarify requirements
> > >
> > > 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.
> > > +
> > > +\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.
> > >
> > >  \input{transport-pci.tex}
> > >  \input{transport-mmio.tex}
> > > --
> > > 2.31.1.272.g89b43f80a514
> > >
> >

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

[Michael S. Tsirkin]

On Mon, May 20, 2024 at 12:29:35PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, May 20, 2024 5:55 PM
> > 
> > On Mon, May 20, 2024 at 09:42:02AM +0000, Parav Pandit wrote:
> > > Michael,
> > >
> > > > From: Viresh Kumar <viresh.kumar@linaro.org>
> > > > Sent: Monday, May 20, 2024 3:00 PM
> > > > To: virtio-comment@lists.linux.dev; virtio-dev@lists.linux.dev
> > >
> > > There is still cross posting to virtio-dev.
> > 
> > Well we say "do not do this" in the list subscription form.
> > Viresh, what gives?
> > 
> > > Is the list open to community now?
> > 
> > It's been open for a week. I think it's fair to say whoever wanted to subscribe,
> > has subscribed.
> > 
> True. I meant to ask that do contributors need to wait for patch [1] to be merged before posting patches here?
> You indicated there might be voting in [2], so trying to find what is next for contributors?
> 
> [1] https://lore.kernel.org/virtio-comment/te6tzbegqky7uz4skrag3yowet3phtq6nn7tegyrillgb4juwn@m4gi5ohfuk7m/T/#t
> [2] https://lore.kernel.org/virtio-comment/te6tzbegqky7uz4skrag3yowet3phtq6nn7tegyrillgb4juwn@m4gi5ohfuk7m/T/#m7b5d6fc76d05df528c36a70cff30afe06bb44917

I think we can assume existing contributors are aware of the process and
we do not need to vote. I spent a day on it but could not fix the voting
scripts yet.


> > > If not, can you please send out the guidance and timeline to open it on virtio-
> > comment list?
> > >
> > > This way contributors can avoid resending it multiple times.
> > >
> > > > Cc: Viresh Kumar <viresh.kumar@linaro.org>; Vincent Guittot
> > > > <vincent.guittot@linaro.org>; Alex Bennée <alex.bennee@linaro.org>;
> > > > stratos- dev@op-lists.linaro.org; Manos Pitsidianakis
> > > > <manos.pitsidianakis@linaro.org>; Cornelia Huck <cohuck@redhat.com>;
> > > > Michael S. Tsirkin <mst@redhat.com>
> > > > Subject: [PATCH V3 Resend] virtio-transport: Clarify requirements
> > > >
> > > > 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.
> > > > +
> > > > +\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.
> > > >
> > > >  \input{transport-pci.tex}
> > > >  \input{transport-mmio.tex}
> > > > --
> > > > 2.31.1.272.g89b43f80a514
> > > >
> > >

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

[Michael S. Tsirkin]

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

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

[Viresh Kumar]

On 20-05-24, 08:25, Michael S. Tsirkin wrote:
> Well we say "do not do this" in the list subscription form.
> Viresh, what gives?

Apologies. I didn't read the page properly.

-- 
viresh

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

[Viresh Kumar]

Dropped virtio-dev list.

On 20-05-24, 12:20, Stefano Garzarella wrote:
> I haven't followed v1 and v2, so apologies in advance if my comments have
> already been discussed.

No issues.

> On Mon, May 20, 2024 at 02:59:40PM GMT, Viresh Kumar wrote:
> > +\transportnormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options / Virtio Transport Requirements}
> > +The transport MUST provide a mechanism for the driver to discover the
> > +device.
> 
> This seems a little too stringent. For example, does MMIO offer a discovery
> mechanism? IIRC we have to pass the address via device tree or kernel
> command line.

Right. Is converting this to SHOULD okay or do you suggest some other
change ?

> > +The transport MUST provide a mechanism to implement config space
> 
> "configuration space"

Sure.

> > 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
> 
> Should we also talk about the possibility of resetting the entire device?

Do we support that today ? Maybe we can if the maintainers agree.

-- 
viresh

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

[Viresh Kumar]

On 20-05-24, 09:27, Michael S. Tsirkin wrote:
> On Mon, May 20, 2024 at 02:59:40PM +0530, Viresh Kumar wrote:
> > 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.

Yeah, looks wrongly worded. I think we can just retain the earlier
statement here:

Virtio can use various different buses, thus the standard is split
into virtio general and bus-specific sections.

> > +\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.

I am okay with wherever you and Cornelia decide to put this.

> Normative statements are for implementations not spec writers.

Hmm.

> If you think of defining lots of new transports
> (questionable)

There is just one in discussion at the moment on the ARM side,
virtio-msg (for communication between the VMs), but nothing is
finalized yet.

> I think a new non-normative section
> along the lines of newdevice.tex will make sense.

-- 
viresh

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

[Michael S. Tsirkin]

On Tue, May 21, 2024 at 04:27:06PM +0530, Viresh Kumar wrote:
> On 20-05-24, 09:27, Michael S. Tsirkin wrote:
> > On Mon, May 20, 2024 at 02:59:40PM +0530, Viresh Kumar wrote:
> > > 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.
> 
> Yeah, looks wrongly worded. I think we can just retain the earlier
> statement here:
> 
> Virtio can use various different buses, thus the standard is split
> into virtio general and bus-specific sections.
> 
> > > +\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.
> 
> I am okay with wherever you and Cornelia decide to put this.
> 
> > Normative statements are for implementations not spec writers.
> 
> Hmm.
> 
> > If you think of defining lots of new transports
> > (questionable)
> 
> There is just one in discussion at the moment on the ARM side,
> virtio-msg (for communication between the VMs), but nothing is
> finalized yet.


I feel a non-normative section is enough for this.
Just convert should/must to direct speech.

> > I think a new non-normative section
> > along the lines of newdevice.tex will make sense.
> 
> -- 
> viresh

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

[Cornelia Huck]

On Tue, May 21 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, May 21, 2024 at 04:27:06PM +0530, Viresh Kumar wrote:
>> On 20-05-24, 09:27, Michael S. Tsirkin wrote:
>> > On Mon, May 20, 2024 at 02:59:40PM +0530, Viresh Kumar wrote:
>> > > 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.
>> 
>> Yeah, looks wrongly worded. I think we can just retain the earlier
>> statement here:
>> 
>> Virtio can use various different buses, thus the standard is split
>> into virtio general and bus-specific sections.

I don't really like the "bus" term here: PCI is a bus, but there is no
need for a transport to be implemented via a bus.

Maybe

"The standard contains sections describing the transport-agnostic parts
of virtio, and sections describing how individual transports implement
virtio."

>> 
>> > > +\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.
>> 
>> I am okay with wherever you and Cornelia decide to put this.
>> 
>> > Normative statements are for implementations not spec writers.
>> 
>> Hmm.

So anyone implementing a transport needs to be able to figure out
conformance just by looking at the individual transport's normative
statements? Have we cross-checked whether things are clear? Because
then...

>> 
>> > If you think of defining lots of new transports
>> > (questionable)
>> 
>> There is just one in discussion at the moment on the ARM side,
>> virtio-msg (for communication between the VMs), but nothing is
>> finalized yet.
>
>
> I feel a non-normative section is enough for this.
> Just convert should/must to direct speech.

...I think this would be fine.

>
>> > I think a new non-normative section
>> > along the lines of newdevice.tex will make sense.

[Generally, I'm not sure how consistent we are for the individual
transports. PCI is what most people are focusing on; CCW does not really
see much development anymore (at least in the projects that I'm aware
of); not sure how well-loved MMIO still is.]

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

[Viresh Kumar]

On 21-05-24, 07:40, Michael S. Tsirkin wrote:
> I feel a non-normative section is enough for this.
> Just convert should/must to direct speech.

Like this ?

diff --git a/content.tex b/content.tex
index 0a62dce5f65f..8af005398877 100644
--- a/content.tex
+++ b/content.tex
@@ -631,8 +631,84 @@ \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 contains sections describing the transport-agnostic parts
+of virtio, and sections describing how individual transports implement
+virtio.
+
+\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements}
+
+\subsection{Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / Transport Requirements}
+
+The transport provides a mechanism for the driver to discover the
+device.
+
+The transport provides a mechanism for the driver to identify the device
+type.
+
+The transport provides a mechanism for communicating virtqueue
+configurations between the device and the driver.
+
+The transport allows multiple virtqueues per device. The number of
+virtqueues for a pair of device-driver are governed by the individual
+device protocol.
+
+The transport provides a mechanism that the device and the driver use to
+access memory for implementing virtqueues.
+
+The transport provides 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 provides 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 provides a mechanism to implement configuration space
+between the device and the driver.
+
+\subsection{Device Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / Device Requirements}
+
+The device keeps any data associated with a device-initiated transaction
+accessible to the driver until the driver acknowledges the transaction
+to be complete.
+
+The device doesn't 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 doesn't access or modify buffers on a virtqueue after it has
+notified the driver about their availability.
+
+The device resets the virtqueues if requested by the driver, in a
+transport defined way, if the transport provides such a method.
+
+\subsection{Driver Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / Driver Requirements}
+
+The driver acknowledges device notifications, as mandated by the
+transport.
+
+The driver doesn't access virtqueue contents before the device notifies
+about the readiness of the same.
+
+The driver doesn't 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.
 
 \input{transport-pci.tex}
 \input{transport-mmio.tex}

-- 
viresh

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

[Stefano Garzarella]

On Tue, May 21, 2024 at 03:40:23PM GMT, Viresh Kumar wrote:
>Dropped virtio-dev list.
>
>On 20-05-24, 12:20, Stefano Garzarella wrote:
>> I haven't followed v1 and v2, so apologies in advance if my comments have
>> already been discussed.
>
>No issues.
>
>> On Mon, May 20, 2024 at 02:59:40PM GMT, Viresh Kumar wrote:
>> > +\transportnormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options / Virtio Transport Requirements}
>> > +The transport MUST provide a mechanism for the driver to discover the
>> > +device.
>>
>> This seems a little too stringent. For example, does MMIO offer a discovery
>> mechanism? IIRC we have to pass the address via device tree or kernel
>> command line.
>
>Right. Is converting this to SHOULD okay or do you suggest some other
>change ?

Or MAY ?

>
>> > +The transport MUST provide a mechanism to implement config space
>>
>> "configuration space"
>
>Sure.
>
>> > 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
>>
>> Should we also talk about the possibility of resetting the entire device?
>
>Do we support that today ? Maybe we can if the maintainers agree.

mmm, I had interpreted this statement as if we could:

   The transport MAY provide a mechanism for the driver to initiate a
   reset of the virtqueues and device.

IIUC device reset is always supported (see `\section{Device Reset}` in 
content.txt) and it's transport specific. While single virtqueue reset 
depends on VIRTIO_F_RING_RESET.

Thanks,
Stefano

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

[Matias Ezequiel Vara Larsen]

Hello, 

I added very minor comments. 

On Wed, May 29, 2024 at 03:54:24PM +0530, Viresh Kumar wrote:
> On 21-05-24, 07:40, Michael S. Tsirkin wrote:
> > I feel a non-normative section is enough for this.
> > Just convert should/must to direct speech.
> 
> Like this ?
> 
> diff --git a/content.tex b/content.tex
> index 0a62dce5f65f..8af005398877 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -631,8 +631,84 @@ \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 contains sections describing the transport-agnostic parts
> +of virtio, and sections describing how individual transports implement
> +virtio.
> +
> +\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements}
> +
> +\subsection{Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / Transport Requirements}
> +
> +The transport provides a mechanism for the driver to discover the
> +device.
> +
> +The transport provides a mechanism for the driver to identify the device
> +type.
> +
> +The transport provides a mechanism for communicating virtqueue
> +configurations between the device and the driver.
> +
> +The transport allows multiple virtqueues per device. The number of
> +virtqueues for a pair of device-driver are governed by the individual
> +device protocol.
> +
> +The transport provides a mechanism that the device and the driver use to
> +access memory for implementing virtqueues.
> +
> +The transport provides 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 provides 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 provides a mechanism to implement configuration space
> +between the device and the driver.
> +
> +\subsection{Device Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / Device Requirements}
> +
> +The device keeps any data associated with a device-initiated transaction
> +accessible to the driver until the driver acknowledges the transaction
> +to be complete.
> +
> +The device doesn't 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 doesn't access or modify buffers on a virtqueue after it has
> +notified the driver about their availability.
> +
> +The device resets the virtqueues if requested by the driver, in a
> +transport defined way, if the transport provides such a method.
> +
> +\subsection{Driver Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / Driver Requirements}
> +
> +The driver acknowledges device notifications, as mandated by the
> +transport.
> +
> +The driver doesn't access virtqueue contents before the device notifies
> +about the readiness of the same.
> +
> +The driver doesn't 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.
> +

I think last sentence could be rewritten as:
- The driver accesses queued buffers after the device has processed them
  and notified the driver of their availability. This mechanism is
  transport defined.


> +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.
>  

I would rewrite last sentence as:
- The driver asks 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.


With that, LGTM!

Matias

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

[Viresh Kumar]

On 29-05-24, 13:12, Matias Ezequiel Vara Larsen wrote:
> > +The driver doesn't 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.
> > +
> 
> I think last sentence could be rewritten as:
> - The driver accesses queued buffers after the device has processed them
>   and notified the driver of their availability. This mechanism is
>   transport defined.
> 
> 
> > +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.
> >  
> 
> I would rewrite last sentence as:
> - The driver asks 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.
> 
> 
> With that, LGTM!

Done. Thanks.

-- 
viresh

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

[Viresh Kumar]

On 29-05-24, 12:42, Stefano Garzarella wrote:
> mmm, I had interpreted this statement as if we could:
> 
>   The transport MAY provide a mechanism for the driver to initiate a
>   reset of the virtqueues and device.
> 
> IIUC device reset is always supported (see `\section{Device Reset}` in
> content.txt) and it's transport specific. While single virtqueue reset
> depends on VIRTIO_F_RING_RESET.

Done, thanks.

-- 
viresh