Cornelia Huck cohuck@redhat.com writes:
On Tue, Dec 05 2023, 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.
Thank you for tackling this, albeit the devil's in the details :)
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
content.tex | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/content.tex b/content.tex index 0a62dce5f65f..d4d5e7d7045b 100644 --- a/content.tex +++ b/content.tex @@ -631,8 +631,52 @@ \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. +The virtio devices are exposed to the guest as if they are physical +devices using a specific transport method, like PCI, MMIO or Channel +I/O.
I'm not sure we can talk about "exposed to the guest" here, except as an example... maybe if we reword the whole paragraph (see my suggestion below.)
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.. Virtio can use +various different buses, thus the standard is split into virtio general +and bus-specific sections.
I think we should concentrate on the transport being what links device and driver together... what about (reusing parts of your writeup):
"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 is split into sections describing general virtio implementation and transport-specific sections."
+\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options / Virtio Transport Requirements}
+\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options}
I'm not sure we can introduce MUST (NOT) requirements for basic functionality after the spec has been published for quite a time already (although I'd assume every implementation is fulfilling the requirements anyway)... thoughts?
+The device MUST present each event, in a transport defined way, from the +moment it takes place until the driver acknowledges the event.
I don't believe "event" is well-defined here.
Maybe:
"A device initiated transaction can isn't considered complete until acknowledged by the driver. As such data MUST remain visible to the driver until the transaction is complete"?
+The device MUST NOT access virtqueue's contents before the driver +notifies that the queue is ready for access, in a transport defined way.
+The device MUST NOT access buffers on the virtqueue, after it has +modified them and notified the driver about their availability.
+The device MUST reset the virtqueues if requested by the driver, in a +transport defined way.
Isn't all of this already defined in one place of the spec or another?
I think the recent example is the virtio-sound driver continuing to feed data into buffers after those buffers where submitted into the virtqueue. We should be explicit that the only time both sides of a VirtIO implementation can access things at the same time is with explicitly shared memory (and you need some sort of mechanism to mediate that to avoid chaos).
+\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio Transport Options}
+The driver MUST NOT access guest memory locations outside what's made +available by the device to the driver.
I don't think that makes sense -- I'd assume most guest memory locations do not have anything to do with virtio, and we should try to avoid host/guest terminology.
I agree guest memory isn't the right terminology here. However there are discussions about how to implement secure buffers for VirtIO - so for example a buffer mediated by some sort of secure layer. In those cases the driver may not have access to it outside of the transactions.
+The driver MUST NOT write to the read-only memory area and MUST NOT read +from the write-only memory area.
Which memory areas does that refer to? Parts of the transport-specific data structures?
+The driver MUST acknowledge events presented by the device, as mandated +by the transport.
I don't think this is quite correct in the absolute -- for example, it should be fine to not acknowledge events if some overriding event comes along, or if the driver initiates a reset.
+The driver MUST NOT access virtqueue contents before the device notifies +about the readiness of the same.
+The driver MUST NOT access buffers, after it has added them to the +virtqueue and notified the device about their availability. The driver +MAY access them after the device has processed them and notified the +driver of their availability, in a transport defined way.
+The driver MAY ask the device to reset the virtqueues if, for example, +the driver times out waiting for a notification from the device for a +previously queued request.
Again, I believe this has already been covered in the generic sections -- do we instead need to specify that a transport MUST provide a method to do xy? (or SHOULD, MAY, as applicable -- it would be good to list explicitly what is mandatory for a transport to implement, and what is optional.)
Yes I think so. The s390x channel transport gets referenced because it has a nice enumerated list of operations. It would be good to codify which operations are mandatory for all transports and which are optional.