* [PATCH V5] virtio-transport: Clarify requirements
@ 2024-06-11 5:35 Viresh Kumar
2024-06-11 6:10 ` Parav Pandit
0 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2024-06-11 5:35 UTC (permalink / raw)
To: virtio-comment
Cc: Viresh Kumar, Vincent Guittot, Alex Bennée, stratos-dev,
Manos Pitsidianakis, Cornelia Huck, Michael S. Tsirkin
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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
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.
content.tex | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 80 insertions(+), 2 deletions(-)
diff --git a/content.tex b/content.tex
index 0a62dce5f65f..e2a836327818 100644
--- a/content.tex
+++ b/content.tex
@@ -631,8 +631,86 @@ \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}
+
+There are some mechanisms that any transport is required to implement,
+and some requirements that devices and drivers are required to follow.
+
+\subsection{Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / 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 virtqueues and device.
+
+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.
+
+\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 itself and 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 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 and the virtqueues if, for
+example, the driver times out waiting for a notification from the device
+for a previously queued request.
+
\input{transport-pci.tex}
\input{transport-mmio.tex}
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 14+ messages in thread* RE: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 5:35 [PATCH V5] virtio-transport: Clarify requirements Viresh Kumar
@ 2024-06-11 6:10 ` Parav Pandit
2024-06-11 6:58 ` Viresh Kumar
2024-06-11 15:28 ` Cornelia Huck
0 siblings, 2 replies; 14+ messages in thread
From: Parav Pandit @ 2024-06-11 6:10 UTC (permalink / raw)
To: Viresh Kumar, virtio-comment@lists.linux.dev
Cc: Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis,
Cornelia Huck, Michael S. Tsirkin
Hi Viresh,
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Tuesday, June 11, 2024 11:06 AM
[..]
> diff --git a/content.tex b/content.tex
> index 0a62dce5f65f..e2a836327818 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -631,8 +631,86 @@ \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}
> +
> +There are some mechanisms that any transport is required to implement,
> +and some requirements that devices and drivers are required to follow.
> +
> +\subsection{Transport Requirements}\label{sec:Virtio Transport Options
> +/ Virtio Transport Requirements / Transport Requirements}
> +
> +A transport provides a mechanism for the driver to discover the device.
> +
I would like to add a normative.
A transport MAY provide a mechanism to create and destroy virtio devices.
(for example PCI transport provides this).
It isn’t blocker for this patch.
In case if you spin v6, please add, else will send this change after yours is merged.
> +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 virtqueues and device.
> +
> +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.
> +
> +\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.
> +
This needs to be corrected to only say "modify".
A device may still access (read) what is already wrote, for example, used index of the split queue.
Or packed queue flags.
> +The device resets itself and 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.
> +
This also needs fix.
A driver may access (poll) for the packed queue flags or used index of the split q without device notifying it.
> +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 and the virtqueues if, for
> +example, the driver times out waiting for a notification from the
> +device for a previously queued request.
> +
Please remove "virtqueue" from above. The "device to reset itself" is enough.
"and the virtqueues" is confusing, it sort of implies the vqs are something separate from the device reset flow, which is not true.
So device reset is good enough.
>
> \input{transport-pci.tex}
> \input{transport-mmio.tex}
> --
> 2.31.1.272.g89b43f80a514
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 6:10 ` Parav Pandit
@ 2024-06-11 6:58 ` Viresh Kumar
2024-06-11 7:41 ` Parav Pandit
2024-06-11 15:28 ` Cornelia Huck
1 sibling, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2024-06-11 6:58 UTC (permalink / raw)
To: Parav Pandit
Cc: virtio-comment@lists.linux.dev, Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis,
Cornelia Huck, Michael S. Tsirkin
Hi Parav,
On 11-06-24, 06:10, Parav Pandit wrote:
> > From: Viresh Kumar <viresh.kumar@linaro.org>
> > +\subsection{Transport Requirements}\label{sec:Virtio Transport Options
> > +/ Virtio Transport Requirements / Transport Requirements}
> > +
> > +A transport provides a mechanism for the driver to discover the device.
> > +
> I would like to add a normative.
Some background on why normative statements are removed for this
change:
https://lore.kernel.org/all/20240521073925-mutt-send-email-mst@kernel.org/
> A transport MAY provide a mechanism to create and destroy virtio devices.
>
> (for example PCI transport provides this).
>
> It isn’t blocker for this patch.
> In case if you spin v6, please add, else will send this change after yours is merged.
> > +The device doesn't access or modify buffers on a virtqueue after it has
> > +notified the driver about their availability.
> > +
> This needs to be corrected to only say "modify".
>
> A device may still access (read) what is already wrote, for example, used index of the split queue.
> Or packed queue flags.
Hmm, we are talking about "buffers on a virtqueue" here. I am not sure
if the device should access them anymore. Once the control is passed
to the driver, the buffers may get modified anytime and device
shouldn't access them. Used index or flags are configuration things,
accessing them is fine I guess.
> > +The device resets itself and 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.
> > +
> This also needs fix.
> A driver may access (poll) for the packed queue flags or used index of the split q without device notifying it.
Maybe this (in a similar way to the device specific sentence earlier)
?
The driver doesn't access buffers on the virtqueue before the device
notifies about the readiness of the same.
Configuration specific fields can of course be accessed anytime.
> > +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 and the virtqueues if, for
> > +example, the driver times out waiting for a notification from the
> > +device for a previously queued request.
> > +
> Please remove "virtqueue" from above. The "device to reset itself" is enough.
> "and the virtqueues" is confusing, it sort of implies the vqs are something separate from the device reset flow, which is not true.
> So device reset is good enough.
I wonder if it will cause some confusion, i.e. about the state of the
virtqueues on device reset. I understand what you are saying, but is
it a given that virtqueues will be reset once device is ?
I looked at: "Basic Facilities of a Virtio Device" -> "Device Reset",
and it doesn't talk about vitqueue being reset with the device.
Also there is a separate section about, "Virtqueue Reset" and it talks
about the possibility of a virtqueue getting reset alone.
Maybe something like ?
The driver asks the device to reset itself (and its virtqueues) if,
for example, the driver times out waiting for a notification from the
device for a previously queued request.
Thanks.
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 6:58 ` Viresh Kumar
@ 2024-06-11 7:41 ` Parav Pandit
2024-06-11 8:41 ` Viresh Kumar
0 siblings, 1 reply; 14+ messages in thread
From: Parav Pandit @ 2024-06-11 7:41 UTC (permalink / raw)
To: Viresh Kumar
Cc: virtio-comment@lists.linux.dev, Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis,
Cornelia Huck, Michael S. Tsirkin
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Tuesday, June 11, 2024 12:28 PM
> To: Parav Pandit <parav@nvidia.com>
> Cc: virtio-comment@lists.linux.dev; Vincent Guittot
> <vincent.guittot@linaro.org>; Alex Bennée <alex.bennee@linaro.org>; stratos-
> dev@op-lists.linaro.org; Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org>; Cornelia Huck <cohuck@redhat.com>;
> Michael S. Tsirkin <mst@redhat.com>
> Subject: Re: [PATCH V5] virtio-transport: Clarify requirements
>
> Hi Parav,
>
> On 11-06-24, 06:10, Parav Pandit wrote:
> > > From: Viresh Kumar <viresh.kumar@linaro.org>
> > > +\subsection{Transport Requirements}\label{sec:Virtio Transport
> > > +Options / Virtio Transport Requirements / Transport Requirements}
> > > +
> > > +A transport provides a mechanism for the driver to discover the device.
> > > +
> > I would like to add a normative.
>
> Some background on why normative statements are removed for this
> change:
>
> https://lore.kernel.org/all/20240521073925-mutt-send-email-mst@kernel.org/
>
Got it.
I agree to Michael's suggestion "I feel a non-normative section is enough for this."
So following his guidance, we should remove it from the requirements and have this as,
"implementation guide" or appendix section and it also looks good without the key wards too.
Otherwise, this requirement section becomes the example of how not to write normative in a normative section.
(and we want to avoid future such examples).
> > A transport MAY provide a mechanism to create and destroy virtio devices.
> >
> > (for example PCI transport provides this).
> >
> > It isn't blocker for this patch.
> > In case if you spin v6, please add, else will send this change after yours is
> merged.
>
> > > +The device doesn't access or modify buffers on a virtqueue after it
> > > +has notified the driver about their availability.
> > > +
> > This needs to be corrected to only say "modify".
> >
> > A device may still access (read) what is already wrote, for example, used
> index of the split queue.
> > Or packed queue flags.
>
> Hmm, we are talking about "buffers on a virtqueue" here. I am not sure if the
> device should access them anymore. Once the control is passed to the driver,
> the buffers may get modified anytime and device shouldn't access them. Used
> index or flags are configuration things, accessing them is fine I guess.
I missed the word "buffer".
You are correct. Above now reads good to me.
>
> > > +The device resets itself and 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.
> > > +
> > This also needs fix.
> > A driver may access (poll) for the packed queue flags or used index of the
> split q without device notifying it.
>
> Maybe this (in a similar way to the device specific sentence earlier) ?
>
> The driver doesn't access buffers on the virtqueue before the device notifies
> about the readiness of the same.
>
Right.
> Configuration specific fields can of course be accessed anytime.
>
> > > +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 and the virtqueues if,
> > > +for example, the driver times out waiting for a notification from
> > > +the device for a previously queued request.
> > > +
> > Please remove "virtqueue" from above. The "device to reset itself" is
> enough.
> > "and the virtqueues" is confusing, it sort of implies the vqs are something
> separate from the device reset flow, which is not true.
> > So device reset is good enough.
>
> I wonder if it will cause some confusion, i.e. about the state of the virtqueues
> on device reset. I understand what you are saying, but is it a given that
> virtqueues will be reset once device is ?
>
Yes, it is given.
See the spec snippet:
" A device MUST NOT send notifications or interact with the queues after indicating completion of the reset
by reinitializing device status to 0, until the driver re-initializes the device."
> I looked at: "Basic Facilities of a Virtio Device" -> "Device Reset", and it doesn't
> talk about vitqueue being reset with the device.
>
It is covered.
> Also there is a separate section about, "Virtqueue Reset" and it talks about the
> possibility of a virtqueue getting reset alone.
>
> Maybe something like ?
>
> The driver asks the device to reset itself (and its virtqueues) if, for example, the
> driver times out waiting for a notification from the device for a previously
> queued request.
A requirement should be,
a transport should provide a mechanism to reset an individual virtqueue.
(it is not must).
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 7:41 ` Parav Pandit
@ 2024-06-11 8:41 ` Viresh Kumar
2024-06-11 8:54 ` Parav Pandit
2024-06-13 9:32 ` Viresh Kumar
0 siblings, 2 replies; 14+ messages in thread
From: Viresh Kumar @ 2024-06-11 8:41 UTC (permalink / raw)
To: Parav Pandit
Cc: virtio-comment@lists.linux.dev, Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis,
Cornelia Huck, Michael S. Tsirkin
On 11-06-24, 07:41, Parav Pandit wrote:
> Got it.
> I agree to Michael's suggestion "I feel a non-normative section is enough for this."
> So following his guidance, we should remove it from the requirements and have this as,
>
> "implementation guide" or appendix section and it also looks good without the key wards too.
>
> Otherwise, this requirement section becomes the example of how not to write normative in a normative section.
> (and we want to avoid future such examples).
I am okay with whatever Cornelia and Michael finalize on this.
> > I wonder if it will cause some confusion, i.e. about the state of the virtqueues
> > on device reset. I understand what you are saying, but is it a given that
> > virtqueues will be reset once device is ?
> >
> Yes, it is given.
> See the spec snippet:
> " A device MUST NOT send notifications or interact with the queues after indicating completion of the reset
> by reinitializing device status to 0, until the driver re-initializes the device."
I read this a bit differently TBH. It only says that the device
shouldn't interact with vqs between reset and reinitialize states. It
doesn't really talk about virtqueues getting reset with the device.
> > I looked at: "Basic Facilities of a Virtio Device" -> "Device Reset", and it doesn't
> > talk about vitqueue being reset with the device.
> >
> It is covered.
I am okay to remove it, since you are sure about it :)
> > Also there is a separate section about, "Virtqueue Reset" and it talks about the
> > possibility of a virtqueue getting reset alone.
> >
> > Maybe something like ?
> >
> > The driver asks the device to reset itself (and its virtqueues) if, for example, the
> > driver times out waiting for a notification from the device for a previously
> > queued request.
>
> A requirement should be,
> a transport should provide a mechanism to reset an individual virtqueue.
> (it is not must).
Right, I can add another requirement for separate resetting of
virtqueues.
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 8:41 ` Viresh Kumar
@ 2024-06-11 8:54 ` Parav Pandit
2024-06-13 9:32 ` Viresh Kumar
1 sibling, 0 replies; 14+ messages in thread
From: Parav Pandit @ 2024-06-11 8:54 UTC (permalink / raw)
To: Viresh Kumar
Cc: virtio-comment@lists.linux.dev, Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis,
Cornelia Huck, Michael S. Tsirkin
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Tuesday, June 11, 2024 2:11 PM
[..]
> > See the spec snippet:
> > " A device MUST NOT send notifications or interact with the queues
> > after indicating completion of the reset by reinitializing device status to 0,
> until the driver re-initializes the device."
>
> I read this a bit differently TBH. It only says that the device shouldn't interact
> with vqs between reset and reinitialize states. It doesn't really talk about
> virtqueues getting reset with the device.
I don't see a difference between the two.
Do you?
>
> > > I looked at: "Basic Facilities of a Virtio Device" -> "Device
> > > Reset", and it doesn't talk about vitqueue being reset with the device.
> > >
> > It is covered.
>
> I am okay to remove it, since you are sure about it :)
>
Because of the above spec text. :)
> > > Also there is a separate section about, "Virtqueue Reset" and it
> > > talks about the possibility of a virtqueue getting reset alone.
> > >
> > > Maybe something like ?
> > >
> > > The driver asks the device to reset itself (and its virtqueues) if,
> > > for example, the driver times out waiting for a notification from
> > > the device for a previously queued request.
> >
> > A requirement should be,
> > a transport should provide a mechanism to reset an individual virtqueue.
> > (it is not must).
>
> Right, I can add another requirement for separate resetting of virtqueues.
>
Ok. thanks.
> --
> viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 8:41 ` Viresh Kumar
2024-06-11 8:54 ` Parav Pandit
@ 2024-06-13 9:32 ` Viresh Kumar
2024-06-21 8:05 ` Viresh Kumar
2024-06-21 9:39 ` Michael S. Tsirkin
1 sibling, 2 replies; 14+ messages in thread
From: Viresh Kumar @ 2024-06-13 9:32 UTC (permalink / raw)
To: Parav Pandit, Cornelia Huck, Michael S. Tsirkin
Cc: virtio-comment@lists.linux.dev, Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis
On 11-06-24, 14:11, Viresh Kumar wrote:
> On 11-06-24, 07:41, Parav Pandit wrote:
> > Got it.
> > I agree to Michael's suggestion "I feel a non-normative section is enough for this."
> > So following his guidance, we should remove it from the requirements and have this as,
> >
> > "implementation guide" or appendix section and it also looks good without the key wards too.
> >
> > Otherwise, this requirement section becomes the example of how not to write normative in a normative section.
> > (and we want to avoid future such examples).
>
> I am okay with whatever Cornelia and Michael finalize on this.
Michael, Cornelia: Any decision on this ?
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-13 9:32 ` Viresh Kumar
@ 2024-06-21 8:05 ` Viresh Kumar
2024-06-21 9:39 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2024-06-21 8:05 UTC (permalink / raw)
To: Parav Pandit, Cornelia Huck, Michael S. Tsirkin
Cc: virtio-comment@lists.linux.dev, Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis
On 13-06-24, 15:02, Viresh Kumar wrote:
> On 11-06-24, 14:11, Viresh Kumar wrote:
> > On 11-06-24, 07:41, Parav Pandit wrote:
> > > Got it.
> > > I agree to Michael's suggestion "I feel a non-normative section is enough for this."
> > > So following his guidance, we should remove it from the requirements and have this as,
> > >
> > > "implementation guide" or appendix section and it also looks good without the key wards too.
> > >
> > > Otherwise, this requirement section becomes the example of how not to write normative in a normative section.
> > > (and we want to avoid future such examples).
> >
> > I am okay with whatever Cornelia and Michael finalize on this.
>
> Michael, Cornelia: Any decision on this ?
Ping.
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-13 9:32 ` Viresh Kumar
2024-06-21 8:05 ` Viresh Kumar
@ 2024-06-21 9:39 ` Michael S. Tsirkin
2024-06-21 9:43 ` Viresh Kumar
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2024-06-21 9:39 UTC (permalink / raw)
To: Viresh Kumar
Cc: Parav Pandit, Cornelia Huck, virtio-comment@lists.linux.dev,
Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis
On Thu, Jun 13, 2024 at 03:02:03PM +0530, Viresh Kumar wrote:
> On 11-06-24, 14:11, Viresh Kumar wrote:
> > On 11-06-24, 07:41, Parav Pandit wrote:
> > > Got it.
> > > I agree to Michael's suggestion "I feel a non-normative section is enough for this."
> > > So following his guidance, we should remove it from the requirements and have this as,
> > >
> > > "implementation guide" or appendix section and it also looks good without the key wards too.
> > >
> > > Otherwise, this requirement section becomes the example of how not to write normative in a normative section.
> > > (and we want to avoid future such examples).
> >
> > I am okay with whatever Cornelia and Michael finalize on this.
>
> Michael, Cornelia: Any decision on this ?
On what?
> --
> viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-21 9:39 ` Michael S. Tsirkin
@ 2024-06-21 9:43 ` Viresh Kumar
2024-06-21 10:08 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Viresh Kumar @ 2024-06-21 9:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Parav Pandit, Cornelia Huck, virtio-comment@lists.linux.dev,
Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis
On 21-06-24, 05:39, Michael S. Tsirkin wrote:
> On Thu, Jun 13, 2024 at 03:02:03PM +0530, Viresh Kumar wrote:
> > On 11-06-24, 14:11, Viresh Kumar wrote:
> > > On 11-06-24, 07:41, Parav Pandit wrote:
> > > > Got it.
> > > > I agree to Michael's suggestion "I feel a non-normative section is enough for this."
> > > > So following his guidance, we should remove it from the requirements and have this as,
> > > >
> > > > "implementation guide" or appendix section and it also looks good without the key wards too.
> > > >
> > > > Otherwise, this requirement section becomes the example of how not to write normative in a normative section.
> > > > (and we want to avoid future such examples).
> > >
> > > I am okay with whatever Cornelia and Michael finalize on this.
> >
> > Michael, Cornelia: Any decision on this ?
>
> On what?
I am a bit confused about where exactly should we keep this section.
As the current patch or appendix or somewhere else ?
--
viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-21 9:43 ` Viresh Kumar
@ 2024-06-21 10:08 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2024-06-21 10:08 UTC (permalink / raw)
To: Viresh Kumar
Cc: Parav Pandit, Cornelia Huck, virtio-comment@lists.linux.dev,
Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis
On Fri, Jun 21, 2024 at 03:13:35PM +0530, Viresh Kumar wrote:
> On 21-06-24, 05:39, Michael S. Tsirkin wrote:
> > On Thu, Jun 13, 2024 at 03:02:03PM +0530, Viresh Kumar wrote:
> > > On 11-06-24, 14:11, Viresh Kumar wrote:
> > > > On 11-06-24, 07:41, Parav Pandit wrote:
> > > > > Got it.
> > > > > I agree to Michael's suggestion "I feel a non-normative section is enough for this."
> > > > > So following his guidance, we should remove it from the requirements and have this as,
> > > > >
> > > > > "implementation guide" or appendix section and it also looks good without the key wards too.
> > > > >
> > > > > Otherwise, this requirement section becomes the example of how not to write normative in a normative section.
> > > > > (and we want to avoid future such examples).
> > > >
> > > > I am okay with whatever Cornelia and Michael finalize on this.
> > >
> > > Michael, Cornelia: Any decision on this ?
> >
> > On what?
>
> I am a bit confused about where exactly should we keep this section.
> As the current patch or appendix or somewhere else ?
Add a new chapter: "creating new transports" alongside newdevice.tex.
There, explain how to go about developing new transports.
Your text can be helpful there.
> --
> viresh
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 6:10 ` Parav Pandit
2024-06-11 6:58 ` Viresh Kumar
@ 2024-06-11 15:28 ` Cornelia Huck
2024-06-11 17:40 ` Parav Pandit
1 sibling, 1 reply; 14+ messages in thread
From: Cornelia Huck @ 2024-06-11 15:28 UTC (permalink / raw)
To: Parav Pandit, Viresh Kumar, virtio-comment@lists.linux.dev
Cc: Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis,
Michael S. Tsirkin
On Tue, Jun 11 2024, Parav Pandit <parav@nvidia.com> wrote:
> Hi Viresh,
>
>> From: Viresh Kumar <viresh.kumar@linaro.org>
>> Sent: Tuesday, June 11, 2024 11:06 AM
>
> [..]
>> diff --git a/content.tex b/content.tex
>> index 0a62dce5f65f..e2a836327818 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -631,8 +631,86 @@ \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}
>> +
>> +There are some mechanisms that any transport is required to implement,
>> +and some requirements that devices and drivers are required to follow.
>> +
>> +\subsection{Transport Requirements}\label{sec:Virtio Transport Options
>> +/ Virtio Transport Requirements / Transport Requirements}
>> +
>> +A transport provides a mechanism for the driver to discover the device.
>> +
> I would like to add a normative.
>
> A transport MAY provide a mechanism to create and destroy virtio devices.
>
> (for example PCI transport provides this).
>
I'm not a fan of that statement: the transport already allows for the
driver to discover a device, whether that happens statically or
dynamically really is the choice of the transport, and I don't think we
should go into that much detail.
[I'll go offline now, so please don't expect further responses from me
right now...]
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 15:28 ` Cornelia Huck
@ 2024-06-11 17:40 ` Parav Pandit
2024-06-12 7:19 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Parav Pandit @ 2024-06-11 17:40 UTC (permalink / raw)
To: Cornelia Huck, Viresh Kumar, virtio-comment@lists.linux.dev
Cc: Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis,
Michael S. Tsirkin
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, June 11, 2024 8:59 PM
>
> On Tue, Jun 11 2024, Parav Pandit <parav@nvidia.com> wrote:
>
> > Hi Viresh,
> >
> >> From: Viresh Kumar <viresh.kumar@linaro.org>
> >> Sent: Tuesday, June 11, 2024 11:06 AM
> >
> > [..]
> >> diff --git a/content.tex b/content.tex index
> >> 0a62dce5f65f..e2a836327818 100644
> >> --- a/content.tex
> >> +++ b/content.tex
> >> @@ -631,8 +631,86 @@ \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}
> >> +
> >> +There are some mechanisms that any transport is required to
> >> +implement, and some requirements that devices and drivers are
> required to follow.
> >> +
> >> +\subsection{Transport Requirements}\label{sec:Virtio Transport
> >> +Options / Virtio Transport Requirements / Transport Requirements}
> >> +
> >> +A transport provides a mechanism for the driver to discover the device.
> >> +
> > I would like to add a normative.
> >
> > A transport MAY provide a mechanism to create and destroy virtio devices.
> >
> > (for example PCI transport provides this).
> >
>
> I'm not a fan of that statement: the transport already allows for the driver to
> discover a device, whether that happens statically or dynamically really is the
> choice of the transport, and I don't think we should go into that much detail.
>
Discovery != life cycle management of the device.
The whole purpose of this section to clarify the transport requirements scope and guidelines for new transport.
And if its incomplete I don't see a point of having this section at all.
Anyways, not too critical for me either.
> [I'll go offline now, so please don't expect further responses from me right
> now...]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V5] virtio-transport: Clarify requirements
2024-06-11 17:40 ` Parav Pandit
@ 2024-06-12 7:19 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2024-06-12 7:19 UTC (permalink / raw)
To: Parav Pandit
Cc: Cornelia Huck, Viresh Kumar, virtio-comment@lists.linux.dev,
Vincent Guittot, Alex Bennée,
stratos-dev@op-lists.linaro.org, Manos Pitsidianakis
On Tue, Jun 11, 2024 at 05:40:45PM +0000, Parav Pandit wrote:
>
>
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, June 11, 2024 8:59 PM
> >
> > On Tue, Jun 11 2024, Parav Pandit <parav@nvidia.com> wrote:
> >
> > > Hi Viresh,
> > >
> > >> From: Viresh Kumar <viresh.kumar@linaro.org>
> > >> Sent: Tuesday, June 11, 2024 11:06 AM
> > >
> > > [..]
> > >> diff --git a/content.tex b/content.tex index
> > >> 0a62dce5f65f..e2a836327818 100644
> > >> --- a/content.tex
> > >> +++ b/content.tex
> > >> @@ -631,8 +631,86 @@ \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}
> > >> +
> > >> +There are some mechanisms that any transport is required to
> > >> +implement, and some requirements that devices and drivers are
> > required to follow.
> > >> +
> > >> +\subsection{Transport Requirements}\label{sec:Virtio Transport
> > >> +Options / Virtio Transport Requirements / Transport Requirements}
> > >> +
> > >> +A transport provides a mechanism for the driver to discover the device.
> > >> +
> > > I would like to add a normative.
> > >
> > > A transport MAY provide a mechanism to create and destroy virtio devices.
> > >
> > > (for example PCI transport provides this).
> > >
> >
> > I'm not a fan of that statement: the transport already allows for the driver to
> > discover a device, whether that happens statically or dynamically really is the
> > choice of the transport, and I don't think we should go into that much detail.
> >
> Discovery != life cycle management of the device.
> The whole purpose of this section to clarify the transport requirements scope and guidelines for new transport.
> And if its incomplete I don't see a point of having this section at all.
I see it as complementing the section about adding new devices. So we
give some hints on how to add a new transport, basically a transport
developer is documenting his/her journey for others to follow. Yes this
does add support overhead, it will get out of sync so we better make
this clear straight away. Not enough transports are going to be added
to spend too many cycles making this complete, I think.
> Anyways, not too critical for me either.
>
> > [I'll go offline now, so please don't expect further responses from me right
> > now...]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-06-21 10:08 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-11 5:35 [PATCH V5] virtio-transport: Clarify requirements Viresh Kumar
2024-06-11 6:10 ` Parav Pandit
2024-06-11 6:58 ` Viresh Kumar
2024-06-11 7:41 ` Parav Pandit
2024-06-11 8:41 ` Viresh Kumar
2024-06-11 8:54 ` Parav Pandit
2024-06-13 9:32 ` Viresh Kumar
2024-06-21 8:05 ` Viresh Kumar
2024-06-21 9:39 ` Michael S. Tsirkin
2024-06-21 9:43 ` Viresh Kumar
2024-06-21 10:08 ` Michael S. Tsirkin
2024-06-11 15:28 ` Cornelia Huck
2024-06-11 17:40 ` Parav Pandit
2024-06-12 7:19 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox