Re: [PATCH V9] virtio-transport: Add a section to define mandatory transport requirements

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

Hello,

I have some minor comments:

On Fri, Aug 09, 2024 at 03:52:28PM +0530, Viresh Kumar wrote:
> The current Virtio documentation lacks a set of generic requirements
> applicable to all transports. Defining these generic requirements could
> be beneficial when integrating support for a new transport.
> 
> This section outlines the essential requirements that any transport
> method must adhere to.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V8->V9:
> - Change order of the statements.
> - Specify DEVICE_NEEDS_RESET and FEATURES_OK bits.
> - Language improvements.
> 
> V7->V8:
> - Remove separate sections for device/driver requirements.
> - Remove optional requirements.
> - Other generic improvements.
> 
> 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 | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 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..af6c42c75a60
> --- /dev/null
> +++ b/newtransport.tex
> @@ -0,0 +1,44 @@
> +\chapter{Creating New Transports}\label{sec:Creating New Transports}
> +
> +Devices and drivers utilize various transport methods to facilitate
> +communication, such as PCI, MMIO, or Channel I/O. These transport
> +methods determine multiple aspects of the interaction between the device

Maybe remove `multiple` here. I think it does not add much but I do not
have an strong opinion about this.

> +and the driver, including device discovery, capability exchange,
> +interrupt handling, and data transfer. For instance, in a host/guest
> +architecture, the host might expose a device to the guest via a virtual
> +PCI bus, and the guest would employ a PCI device driver to interface
> +with the device.
> +

Maybe a verb different than `employ`? But I do not have an strong
opinion about this.

> +This section outlines the mandatory requirements that a transport method
> +implements.
> +
> +A transport provides a mechanism to implement configuration space for
> +the device.
> +
> +A transport provides a mechanism for the driver to identify the device
> +type.
> +
> +A transport provides a mechanism for the driver to read the device's
> +FEATURES_OK and DEVICE_NEEDS_RESET status bits.
> +
> +A transport provides a mechanism for the driver to modify the device's
> +status.
> +
> +A transport provides a mechanism for the driver to read and modify the
> +device's feature bits.
> +
> +A transport allows one or more virtqueues to be implemented by the
> +device. The number of virtqueues is device specific and not specified by
> +the transport.
> +
> +A transport provides a mechanism for the driver to communicate virtqueue
> +configuration and memory location to the device.
> +
> +A transport provides a mechanism for the device to send device
> +notifications to the driver, such as used buffer notifications.
> +
> +A transport provides a mechanism for the driver to send driver
> +notifications to the device, such as available buffer notifications.
> +
> +A transport provides a mechanism for the driver to initiate a device
> +reset.

LGTM!

Matias.