Re: [virtio] [PATCH] used ring: define the meaning and requirements of the len field.

[Rusty Russell <rusty@au1.ibm.com>]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Tue, Apr 07, 2015 at 10:54:49AM +0930, Rusty Russell wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > On Fri, Mar 20, 2015 at 11:48:55AM +1030, Rusty Russell wrote:
>> >> +\devicenormative{\subsubsection}{Virtqueue Notification Suppression}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
>> >> +
>> >> +The device MUST set \field{len} prior to updating the used \field{idx}.
>> >> +
>> >> +The device MUST write at least \field{len} bytes to descriptor,
>> >> +beginning at the first device-writable buffer,
>> >> +prior to updating the used \field{idx}.
>> >> +
>> >> +The device MAY write more than \field{len} bytes to descriptor.
>> >> +
>> >> +\begin{note}
>> >> +There are potential error cases where a device might not know what
>> >> +parts of the buffers have been written.  This is why \field{len} is
>> >> +permitted to be an underestimate: that's preferable to the driver believing
>> >> +that uninitialized memory has been overwritten when it has not.
>> >> +\end{note}
>> >> +
>> >> +\drivernormative{\subsubsection}{Virtqueue Notification Suppression}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
>> >> +
>> >> +The driver MUST NOT make assumptions about data in device-writable buffers
>> >> +beyond the first \field{len} bytes, and SHOULD ignore this data.
>> >> +
>> >>  \subsection{Virtqueue Notification Suppression}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Notification Suppression}
>> >>  
>> >>  The device can suppress notifications in a manner analogous to the way
>> >
>> > We know legacy devices don't follow this, so we also need some text in
>> > the legacy sections to document the differences.
>> 
>> Indeed, but it was specific to block devices.
>> 
>> It may be worth noting that older block devices would set erroneously
>> set a length corresponding to the entire buffer length, and thus legacy
>> drivers SHOULD rely on the status byte and ignore used.len?
>> 
>> Cheers,
>> Rusty.
>
> QEMU before 1.3 also set used len in tx descriptor to packet length.

Yech.  How's this on top then?  (Untested, uprade seems to have broken
tex here):

diff --git a/conformance.tex b/conformance.tex
index 619859b..fae899a 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -39,6 +39,7 @@ A driver MUST conform to the following normative statements:
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Interrupt Suppression}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Notification Suppression}
+\item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
 \item \ref{drivernormative:General Initialization And Device Operation / Device Initialization}
 \item \ref{drivernormative:General Initialization And Device Operation / Device Operation / Supplying Buffers to The Device / Updating idx}
 \item \ref{drivernormative:General Initialization And Device Operation / Device Operation / Supplying Buffers to The Device / Notifying The Device}
@@ -143,6 +144,7 @@ A device MUST conform to the following normative statements:
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Interrupt Suppression}
+\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Notification Suppression}
 \item \ref{devicenormative:Reserved Feature Bits}
 \end{itemize}
@@ -264,6 +266,7 @@ Feature Bits / Legacy Interface: A Note on Feature Bits}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Endianness}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
+\item Section \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring / Legacy Interface: The len Field}
 \item Section \ref{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery / Legacy Interfaces: A Note on PCI Device Discovery}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
diff --git a/content.tex b/content.tex
index 8e8765d..a23148d 100644
--- a/content.tex
+++ b/content.tex
@@ -643,6 +643,19 @@ that uninitialized memory has been overwritten when it has not.
 The driver MUST NOT make assumptions about data in device-writable buffers
 beyond the first \field{len} bytes, and SHOULD ignore this data.
 
+\drivernormative{\subsubsection}{Legacy Interface: The len Field}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring / Legacy Interface: The len Field}
+
+When using the legacy interface, a driver SHOULD treat a \field{len}
+which exceeds the total length of device-writable buffers as if it
+were equal to that length.
+
+\begin{note}
+Some legacy device implementations incorrectly set \field{len} to the
+total descriptor length, not the written length; in particular on
+network device transmit packets (QEMU prior to version 1.3) and block
+device writes (QEMU including version 2.2).
+\end{note}
+
 \subsection{Virtqueue Notification Suppression}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Notification Suppression}
 
 The device can suppress notifications in a manner analogous to the way