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

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

On Thu, Jul 25, 2024 at 02:45:44PM +0530, Viresh Kumar wrote:
> On 24-07-24, 07:05, Michael S. Tsirkin wrote:
> > I don't know do you keep talking about "pair" everywhere.
> 
> This came from one of the review comments and so I tried to use the same
> language 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.
> 
> That's a mistake then. Will fix it.
> 
> > > +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".
> 
> "... implement configuration space for the device and the driver."

just the device

> > > +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 device to communicate virtqueue
> configurations and memory location to the driver."

not really, it's normally the driver that communicates all this.
and configuration is it's own plural I think.

> > > +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.
> 
> "A transport allows one or more virtqueues to be implemented for a device and a
> driver."

by the device

> > > The number of virtqueues per pair are governed by the individual
> > > +device protocol.
> > 
> > what is device protocol?
> 
> "The number of virtqueues are governed by the device implementation."

what are you trying to say? that transport does not need to
specify this?
basically we can just kill this sentence then?

> > > +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 device to send a virtqueue event to
> the driver, for example regarding availability of a buffer on the virtqueue."

/facepalm
this just messes up terminology completely.
pls check what we say about device and driver notifications and follow
that terminology


> > > +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.
> 
> "A transport provides a mechanism for the driver to read the FEATURES_OK bit."

status bit

> 
> "A transport provides a mechanism for the driver to modify the device status."
> 
> -- 
> viresh