Re: [PATCH V7] virtio-transport: Add a new section to clarify transport requirements

[Cornelia Huck <cohuck@redhat.com>]

On Thu, Jul 11 2024, Viresh Kumar <viresh.kumar@linaro.org> 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 under a new Appendix section.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V6->V7:
> - Remove parts that talk about accessing content of the virtqueue.
>
> V5->V6:
> - Move the changes to a new appendix section.
> - Clarify the requirements a bit more based on review comments.
>
> V4->V5:
> - s/The transport/A transport/
> - s/MUST provide/provides/
> - Added some text for transport requirements.
>
> V3->V4:
> - Remove the normative sections and use direct speech.
> - Change wording at few places.
>
> 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.
>
>  main.tex         |  2 ++
>  newtransport.tex | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
>  create mode 100644 newtransport.tex
>
> diff --git a/main.tex b/main.tex
> index b1913d65e964..6d337217a3d1 100644
> --- a/main.tex
> +++ b/main.tex
> @@ -42,6 +42,8 @@
>  
>  \input{newdevice.tex}
>  
> +\input{newtransport.tex}
> +
>  % acknowledgements
>  \input{acknowledgements.tex} 
>  
> diff --git a/newtransport.tex b/newtransport.tex
> new file mode 100644
> index 000000000000..2abd76e5b037
> --- /dev/null
> +++ b/newtransport.tex
> @@ -0,0 +1,72 @@
> +\chapter{Creating New Transports}\label{sec:Creating New Transports}
> +
> +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.
> +
> +There are some mechanisms that any transport is required to implement,
> +and some requirements that devices and drivers are required to follow.
> +
> +\section{Transport Requirements}\label{sec:Creating New Transports / Transport Requirements}
> +
> +A transport provides a mechanism for the driver to discover the device.
> +
> +A transport provides a mechanism for the driver to identify the device
> +type.
> +
> +A transport provides a mechanism for communicating virtqueue
> +configurations between the device and the driver.
> +
> +A transport allows multiple virtqueues per device. The number of
> +virtqueues for a pair of device-driver are governed by the individual
> +device protocol.

Maybe "for a given device/driver pair"?

> +
> +A transport provides a mechanism that the device and the driver use to
> +access memory for implementing virtqueues.
> +
> +A 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.
> +
> +A transport provides a mechanism for the driver to initiate a reset of
> +the device.
> +
> +A transport provides a mechanism to reset an individual virtqueue.

Is that more of a requirement or a strong suggestion, given that not all
existing transports provide this?

> +
> +A transport provides a mechanism for the driver to read the device
> +status. A transport provides a mechanism for the driver to change the
> +device status.
> +
> +A transport provides a mechanism to implement configuration space
> +between the device and the driver.
> +
> +\section{Device Requirements}\label{sec:Creating New Transports / 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 resets itself if requested by the driver, in a transport
> +defined way, if the transport provides such a method.
> +
> +The device resets an individual virtqueue if requested by the driver,
> +in a transport defined way, if the transport provides such a method.
> +
> +\section{Driver Requirements}\label{sec:Creating New Transports / Driver Requirements}
> +
> +The driver acknowledges device notifications, as mandated by the
> +transport.
> +
> +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 asks the device to reset itself if, for example, the driver
> +times out waiting for a notification from the device for a previously
> +queued request.
> +
> +The driver asks the device to reset an individual virtqueue.

This sentence is a bit weird, like the driver would randomly decide to
reset a virtqueue :) Can we qualify this a bit?