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

[Matias Ezequiel Vara Larsen <mvaralar@redhat.com>]

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