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

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

[Viresh Kumar]

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

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

[Cornelia Huck]

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?

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

[Viresh Kumar]

On 11-07-24, 10:24, Cornelia Huck wrote:
> On Thu, Jul 11 2024, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > +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?

Suggestion. How do you suggest I rewrite it as ?

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

Right. Maybe:

The driver asks the device to reset an individual virtqueue, for
example, if the driver times out waiting for a notification from the
device for a previously queued request on the specific virtqueue.

-- 
viresh

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

[Michael S. Tsirkin]

On Thu, Jul 11, 2024 at 10:24:16AM +0200, Cornelia Huck wrote:
> 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.

Hmm not really.
At the moment virtqueues are all in driver memory, what transport
provides is a mechanism for device to locate this memory.


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

I think this should be "a transport can provide" in all instances where
the feature is optional. Given it's usually tied to a feature bit,
also mention that.

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

so far so good.

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

what are transactions? how does driver acknowledge the transaction to
be complete? Sounds like something Currently spec mentions transactions in the
context of i2c (and in passing, iommu) - maybe put this inside
that device description?

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

These two sections are weird.
What is said here that isn't already said in other sections?


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

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

[Stefano Garzarella]

On Thu, Jul 11, 2024 at 01:18:18PM GMT, 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.
>
>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.

I agree that this would be desirable, but for example IIUC the MMIO 
transport does not provide this, in fact we have to specify to the 
kernel the address where to find the devices.

Should we put it more as something desirable but not required?

Having said that I don't know whether to use different words or split 
"Transport Requirements" into 2 sub sections "Required Requirements" and 
"Optional Requirements".

Thanks,
Stefano

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

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

[Viresh Kumar]

On 11-07-24, 10:41, Stefano Garzarella wrote:
> On Thu, Jul 11, 2024 at 01:18:18PM GMT, Viresh Kumar wrote:
> > +A transport provides a mechanism for the driver to discover the device.
> 
> I agree that this would be desirable, but for example IIUC the MMIO
> transport does not provide this, in fact we have to specify to the kernel
> the address where to find the devices.
> 
> Should we put it more as something desirable but not required?

What about:

A transport may provide a mechanism for the driver to discover the device.

But I thought I am required to remove all occurrences of
should/must/may and use direct speech. Maybe "may" is still fine ? :)

> Having said that I don't know whether to use different words or split
> "Transport Requirements" into 2 sub sections "Required Requirements" and
> "Optional Requirements".

-- 
viresh

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

[Michael S. Tsirkin]

On Thu, Jul 11, 2024 at 02:53:32PM +0530, Viresh Kumar wrote:
> On 11-07-24, 10:41, Stefano Garzarella wrote:
> > On Thu, Jul 11, 2024 at 01:18:18PM GMT, Viresh Kumar wrote:
> > > +A transport provides a mechanism for the driver to discover the device.
> > 
> > I agree that this would be desirable, but for example IIUC the MMIO
> > transport does not provide this, in fact we have to specify to the kernel
> > the address where to find the devices.
> > 
> > Should we put it more as something desirable but not required?
> 
> What about:
> 
> A transport may provide a mechanism for the driver to discover the device.
> 
> But I thought I am required to remove all occurrences of
> should/must/may and use direct speech. Maybe "may" is still fine ? :)

Nope, but "can" is fine.
You are not supposed to just mechanically remove text, rephrase so
it still makes sense.

> > Having said that I don't know whether to use different words or split
> > "Transport Requirements" into 2 sub sections "Required Requirements" and
> > "Optional Requirements".
> 
> -- 
> viresh

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

[Michael S. Tsirkin]

On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
> ---

I am not sure what is this supposed doing - just listing basic things
transport does or all things it can do?

If the later, this ignores several things that transports can do,
such as shared memory, data in notification, etc.






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

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

[Cornelia Huck]

On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
>> ---
>
> I am not sure what is this supposed doing - just listing basic things
> transport does or all things it can do?
>
> If the later, this ignores several things that transports can do,
> such as shared memory, data in notification, etc.

Maybe split this up?

A transport needs to implement the following:
<list of mandatory things>

A transport is encouraged to implement the following:
<list of non-mandatory, but strongly suggested things>

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

[Michael S. Tsirkin]

On Thu, Jul 11, 2024 at 01:46:19PM +0200, Cornelia Huck wrote:
> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
> >> ---
> >
> > I am not sure what is this supposed doing - just listing basic things
> > transport does or all things it can do?
> >
> > If the later, this ignores several things that transports can do,
> > such as shared memory, data in notification, etc.
> 
> Maybe split this up?
> 
> A transport needs to implement the following:
> <list of mandatory things>
> 
> A transport is encouraged to implement the following:
> <list of non-mandatory, but strongly suggested things>

Some things just depend on feature bits, and a transport
can make a feature bit mandatory if it wants to.

-- 
MST

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

[Cornelia Huck]

On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Jul 11, 2024 at 01:46:19PM +0200, Cornelia Huck wrote:
>> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
>> >> ---
>> >
>> > I am not sure what is this supposed doing - just listing basic things
>> > transport does or all things it can do?
>> >
>> > If the later, this ignores several things that transports can do,
>> > such as shared memory, data in notification, etc.
>> 
>> Maybe split this up?
>> 
>> A transport needs to implement the following:
>> <list of mandatory things>
>> 
>> A transport is encouraged to implement the following:
>> <list of non-mandatory, but strongly suggested things>
>
> Some things just depend on feature bits, and a transport
> can make a feature bit mandatory if it wants to.

I wasn't thinking about mandatory for a device/driver, but mandatory for
a transport (e.g. stuff like per-queue reset which is not mandatory for
a transport to implement.)

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

[Michael S. Tsirkin]

On Thu, Jul 11, 2024 at 01:59:34PM +0200, Cornelia Huck wrote:
> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Thu, Jul 11, 2024 at 01:46:19PM +0200, Cornelia Huck wrote:
> >> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> 
> >> > On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
> >> >> ---
> >> >
> >> > I am not sure what is this supposed doing - just listing basic things
> >> > transport does or all things it can do?
> >> >
> >> > If the later, this ignores several things that transports can do,
> >> > such as shared memory, data in notification, etc.
> >> 
> >> Maybe split this up?
> >> 
> >> A transport needs to implement the following:
> >> <list of mandatory things>
> >> 
> >> A transport is encouraged to implement the following:
> >> <list of non-mandatory, but strongly suggested things>
> >
> > Some things just depend on feature bits, and a transport
> > can make a feature bit mandatory if it wants to.
> 
> I wasn't thinking about mandatory for a device/driver, but mandatory for
> a transport (e.g. stuff like per-queue reset which is not mandatory for
> a transport to implement.)

Same thing really.
For example, queue reset is controlled by VIRTIO_F_RING_RESET


-- 
MST

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

[Cornelia Huck]

On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Jul 11, 2024 at 01:59:34PM +0200, Cornelia Huck wrote:
>> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Thu, Jul 11, 2024 at 01:46:19PM +0200, Cornelia Huck wrote:
>> >> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> 
>> >> > On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
>> >> >> ---
>> >> >
>> >> > I am not sure what is this supposed doing - just listing basic things
>> >> > transport does or all things it can do?
>> >> >
>> >> > If the later, this ignores several things that transports can do,
>> >> > such as shared memory, data in notification, etc.
>> >> 
>> >> Maybe split this up?
>> >> 
>> >> A transport needs to implement the following:
>> >> <list of mandatory things>
>> >> 
>> >> A transport is encouraged to implement the following:
>> >> <list of non-mandatory, but strongly suggested things>
>> >
>> > Some things just depend on feature bits, and a transport
>> > can make a feature bit mandatory if it wants to.
>> 
>> I wasn't thinking about mandatory for a device/driver, but mandatory for
>> a transport (e.g. stuff like per-queue reset which is not mandatory for
>> a transport to implement.)
>
> Same thing really.
> For example, queue reset is controlled by VIRTIO_F_RING_RESET

Well, it is -- but a transport can choose not to implement the feature
bit. I guess what I'd like to do is to set expectations: every transport
needs e.g. some way to do notifications, but it can skip some other
things like "device discovery", for example. I think the appendix is
intended as a reference for someone wanting to design a transport, and
some kind of laundry list would help there.

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

[Michael S. Tsirkin]

On Thu, Jul 11, 2024 at 04:44:55PM +0200, Cornelia Huck wrote:
> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Thu, Jul 11, 2024 at 01:59:34PM +0200, Cornelia Huck wrote:
> >> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> 
> >> > On Thu, Jul 11, 2024 at 01:46:19PM +0200, Cornelia Huck wrote:
> >> >> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> >> 
> >> >> > On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
> >> >> >> ---
> >> >> >
> >> >> > I am not sure what is this supposed doing - just listing basic things
> >> >> > transport does or all things it can do?
> >> >> >
> >> >> > If the later, this ignores several things that transports can do,
> >> >> > such as shared memory, data in notification, etc.
> >> >> 
> >> >> Maybe split this up?
> >> >> 
> >> >> A transport needs to implement the following:
> >> >> <list of mandatory things>
> >> >> 
> >> >> A transport is encouraged to implement the following:
> >> >> <list of non-mandatory, but strongly suggested things>
> >> >
> >> > Some things just depend on feature bits, and a transport
> >> > can make a feature bit mandatory if it wants to.
> >> 
> >> I wasn't thinking about mandatory for a device/driver, but mandatory for
> >> a transport (e.g. stuff like per-queue reset which is not mandatory for
> >> a transport to implement.)
> >
> > Same thing really.
> > For example, queue reset is controlled by VIRTIO_F_RING_RESET
> 
> Well, it is -- but a transport can choose not to implement the feature
> bit. I guess what I'd like to do is to set expectations: every transport
> needs e.g. some way to do notifications, but it can skip some other
> things like "device discovery", for example. I think the appendix is
> intended as a reference for someone wanting to design a transport, and
> some kind of laundry list would help there.

Yes. I am however a little worried that if we include optional features
we have just increased the cost of adding any new transport feature -
it now has to be added in this appendix, too.

So maybe stick to just the basics? WDYT?

-- 
MST

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

[Cornelia Huck]

On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Jul 11, 2024 at 04:44:55PM +0200, Cornelia Huck wrote:
>> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Thu, Jul 11, 2024 at 01:59:34PM +0200, Cornelia Huck wrote:
>> >> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> 
>> >> > On Thu, Jul 11, 2024 at 01:46:19PM +0200, Cornelia Huck wrote:
>> >> >> On Thu, Jul 11 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> >> 
>> >> >> > On Thu, Jul 11, 2024 at 01:18:18PM +0530, 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>
>> >> >> >> ---
>> >> >> >
>> >> >> > I am not sure what is this supposed doing - just listing basic things
>> >> >> > transport does or all things it can do?
>> >> >> >
>> >> >> > If the later, this ignores several things that transports can do,
>> >> >> > such as shared memory, data in notification, etc.
>> >> >> 
>> >> >> Maybe split this up?
>> >> >> 
>> >> >> A transport needs to implement the following:
>> >> >> <list of mandatory things>
>> >> >> 
>> >> >> A transport is encouraged to implement the following:
>> >> >> <list of non-mandatory, but strongly suggested things>
>> >> >
>> >> > Some things just depend on feature bits, and a transport
>> >> > can make a feature bit mandatory if it wants to.
>> >> 
>> >> I wasn't thinking about mandatory for a device/driver, but mandatory for
>> >> a transport (e.g. stuff like per-queue reset which is not mandatory for
>> >> a transport to implement.)
>> >
>> > Same thing really.
>> > For example, queue reset is controlled by VIRTIO_F_RING_RESET
>> 
>> Well, it is -- but a transport can choose not to implement the feature
>> bit. I guess what I'd like to do is to set expectations: every transport
>> needs e.g. some way to do notifications, but it can skip some other
>> things like "device discovery", for example. I think the appendix is
>> intended as a reference for someone wanting to design a transport, and
>> some kind of laundry list would help there.
>
> Yes. I am however a little worried that if we include optional features
> we have just increased the cost of adding any new transport feature -
> it now has to be added in this appendix, too.
>
> So maybe stick to just the basics? WDYT?

I'd be happy with the basics only -- the features are documented anyway,
and the important part is being able to figure out what is not optional.

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

[Viresh Kumar]

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 ?

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
+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.
+
+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.
+
+A transport provides a mechanism for a pair of device-driver to
+communicate virtqueue configurations and virtqueue memory location.
+
+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. The number of virtqueues per pair are governed by the individual
+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.
+
+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.

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

[Michael S. Tsirkin]

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

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

[Viresh Kumar]

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

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

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

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

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

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

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

-- 
viresh

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

[Michael S. Tsirkin]

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

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

[Viresh Kumar]

On 25-07-24, 05:28, Michael S. Tsirkin wrote:
> On Thu, Jul 25, 2024 at 02:45:44PM +0530, Viresh Kumar wrote:
> > "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.

Ah.

> and configuration is it's own plural I think.

Hmm, Wikipedia [1] and chatgpt say otherwise.

Anyway, "configuration" is fine here.

> > > > 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?

Hmm, from what I understand the individual device implementation (protocol as
specified in the virtio-specification, for example I2C, SPI, etc.) specify the
exact number of virtqueues that are required for their working. I am not sure
how a transport changes that.

Please suggest how you think this must be written then.

> pls check what we say about device and driver notifications and follow
> that terminology

"A transport provides a mechanism for the device to send the configuration
change notifications and used buffer notifications to the driver."

"A transport provides a mechanism for the driver to send the available buffer
notifications to the device."

-- 
viresh

[1] https://en.wiktionary.org/wiki/configuration

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

[Michael S. Tsirkin]

On Thu, Jul 25, 2024 at 04:25:24PM +0530, Viresh Kumar wrote:
> On 25-07-24, 05:28, Michael S. Tsirkin wrote:
> > On Thu, Jul 25, 2024 at 02:45:44PM +0530, Viresh Kumar wrote:
> > > "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.
> 
> Ah.
> 
> > and configuration is it's own plural I think.
> 
> Hmm, Wikipedia [1] and chatgpt say otherwise.
> 
> Anyway, "configuration" is fine here.

The configuration as used here is its own plural.
Here's an explanation if you want the detail - not
because a random website is an expert but just because it
happens to be right.

https://www.wordhippo.com/what-is/the-plural-of/configuration.html

> > > > > 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?
> 
> Hmm, from what I understand the individual device implementation (protocol as
> specified in the virtio-specification, for example I2C, SPI, etc.) specify the
> exact number of virtqueues that are required for their working. I am not sure
> how a transport changes that.

No it's an i2c device, not i2c protocol. The i2c, spi etc protocols are
outside of virtio spec and have nothing to do with virtqueues.

> 
> Please suggest how you think this must be written then.

I still don't know what you want to say.
Virtio spec just calls them "specific devices".
Are you trying to say:
	The number of virtqueues is device specific and not specified by the
	transport.

> > pls check what we say about device and driver notifications and follow
> > that terminology
> 
> "A transport provides a mechanism for the device to send the configuration
> change notifications and used buffer notifications to the driver."
> 
> "A transport provides a mechanism for the driver to send the available buffer
> notifications to the device."


you can just say "device notifications" and "driver notifications",
respectively, there might be more down the road.

> 
> -- 
> viresh
> 
> [1] https://en.wiktionary.org/wiki/configuration

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

[Viresh Kumar]

On 25-07-24, 08:24, Michael S. Tsirkin wrote:
> The configuration as used here is its own plural.
> Here's an explanation if you want the detail - not
> because a random website is an expert but just because it
> happens to be right.
> 
> https://www.wordhippo.com/what-is/the-plural-of/configuration.html

Ah.

> No it's an i2c device, not i2c protocol. The i2c, spi etc protocols are
> outside of virtio spec and have nothing to do with virtqueues.
> 
> > 
> > Please suggest how you think this must be written then.
> 
> I still don't know what you want to say.
> Virtio spec just calls them "specific devices".
> Are you trying to say:
> 	The number of virtqueues is device specific and not specified by the
> 	transport.

Yes.

-- 
viresh