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

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

On Wed, Jul 24, 2024 at 04:15:37PM +0530, Viresh Kumar wrote:
> On 11-07-24, 13:18, 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 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.
> 
> I have tried to simplify the requirements now based on the discussion over the
> list. Do they look better now ?

I don't know do you keep talking about "pair" everywhere. Just say
"the device and the driver" or "a device and a driver" as apporpriate.
And in many cases it is actually just one of these.

> 
> diff --git a/newtransport.tex b/newtransport.tex
> new file mode 100644
> index 000000000000..8f7a21dc64e2
> --- /dev/null
> +++ b/newtransport.tex
> @@ -0,0 +1,37 @@
> +\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

so communication or interaction?

> +and the driver, like device discovery, exchanging capabilities,

"like" is informal english. do you mean "including"?

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

what is "PCI-specific"? mean "a pci device driver"?
> +
> +This section specifies the mandatory requirements that any transport
> +needs to implement.
> +
> +\section{Transport Requirements}\label{sec:Creating New Transports / Transport Requirements}
> +
> +A transport provides a mechanism to implement configuration space
> +between a pair of device-driver.

configuration space is not "between".

> +
> +A transport provides a mechanism for a pair of device-driver to
> +communicate virtqueue configurations and virtqueue memory location.


this is only communicated one way

> +
> +A transport provides a mechanism for the driver to identify the device
> +type.
> +
> +A transport allows one or more virtqueues for a given device-driver
> +pair.

no idea what does virtqueues for a pair mean.

> The number of virtqueues per pair are governed by the individual
> +device protocol.

what is device protocol?

> +
> +A transport provides a mechanism for the device and driver to notify
> +each other, for example regarding availability of a buffer on the
> +virtqueue.

notify should be more specific. about vq events?
this is only notified one way, not "each other".

> +
> +A transport provides a mechanism for the driver to initiate a reset of
> +the device.
> +
> +A transport provides a mechanism for the driver to read or modify the
> +device status.

I don't think reading status is strictly required, except for
FEATURES_OK bit.

-- 
MST