On 16/04/2015 14:57, Michael S. Tsirkin wrote:
> many hypervisors implemented len field incorrectly.
> Document existing bugs in the legacy sections.
>
> VIRTIO-141
>
> Signed-off-by: Rusty Russell <
rusty@rustcorp.com.au>
> Cc: Paolo Bonzini <
pbonzini@redhat.com>
> Signed-off-by: Michael S. Tsirkin <
mst@redhat.com>
> ---
>
> Based on patch by Rusty Russell, added documentation
> for errors for virtio blk.
>
> Paolo - could you please confirm that virtio scsi
> does not have a similar issue?
virtio-scsi has the same bug.
I would just treat len as rubbish for legacy implementations.
Paolo
> conformance.tex | 3 +++
> content.tex | 19 +++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/conformance.tex b/conformance.tex
> index 0f601d7..0945723 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}
> @@ -144,6 +145,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}
> @@ -267,6 +269,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 344fd5f..054d332 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -636,6 +636,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
> @@ -3936,6 +3949,12 @@ MUST format the fields in struct virtio_blk_req
> according to the native endian of the guest rather than
> (necessarily when not using the legacy interface) little-endian.
>
> +When \field{status} value differs from VIRTIO_BLK_S_OK,
> +the driver SHOULD treat a \field{len} which exceeds 1
> +as if it were 1. Historically, devices put the total descriptor length,
> +or the total length of device-writable buffers there,
> +even when only the status byte was actually written.
> +
> The \field{reserved} field was previously called \field{ioprio}. \field{ioprio}
> is a hint about the relative priorities of requests to the device:
> higher numbers indicate more important requests.
>