virtio — archive

[PATCH v2] used ring: specify legacy behaviour for len field

From: Paolo Bonzini <[email protected]>
To: [email protected]
Date: Thu, 16 Apr 2015 13:09:00 -0000

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 <[email protected]> > Cc: Paolo Bonzini <[email protected]> > Signed-off-by: Michael S. Tsirkin <[email protected]> > --- > > 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. >