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
On Tue, Jun 11, 2024 at 05:40:45PM +0000, Parav Pandit wrote:
>
>
> > From: Cornelia Huck <cohuck(a)redhat.com>
> > Sent: Tuesday, June 11, 2024 8:59 PM
> >
> > On Tue, Jun 11 2024, Parav Pandit <parav(a)nvidia.com> wrote:
> >
> > > Hi Viresh,
> > >
> > >> From: Viresh Kumar <viresh.kumar(a)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...]
On Tue, Jun 11 2024, Parav Pandit <parav(a)nvidia.com> wrote:
> Hi Viresh,
>
>> From: Viresh Kumar <viresh.kumar(a)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...]
Hi Parav,
On 11-06-24, 06:10, Parav Pandit wrote:
> > From: Viresh Kumar <viresh.kumar(a)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
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(a)linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar(a)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
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(a)linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
---
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 | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/content.tex b/content.tex
index 0a62dce5f65f..23eea890d9b7 100644
--- a/content.tex
+++ b/content.tex
@@ -631,8 +631,84 @@ \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}
+
+\subsection{Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements / Transport Requirements}
+
+The transport provides a mechanism for the driver to discover the
+device.
+
+The transport provides a mechanism for the driver to identify the device
+type.
+
+The transport provides a mechanism for communicating virtqueue
+configurations between the device and the driver.
+
+The transport allows multiple virtqueues per device. The number of
+virtqueues for a pair of device-driver are governed by the individual
+device protocol.
+
+The transport provides a mechanism that the device and the driver use to
+access memory for implementing virtqueues.
+
+The 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.
+
+The transport provides a mechanism for the driver to initiate a reset
+of the virtqueues and device.
+
+The transport provides a mechanism for the driver to read the device
+status. The transport MUST provide a mechanism for the driver to change
+the device status.
+
+The 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
