[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio] [PATCH] used ring: define the meaning and requirements of the len field.
On Tue, Apr 14, 2015 at 10:59:06AM +0930, Rusty Russell wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > On Fri, Apr 10, 2015 at 12:49:56PM +0930, Rusty Russell wrote: > >> "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 > > > > Didn't you want to mention read errors too? > > Already mentioned in previous patch. In the non-legacy case, yes. But I mean mention that legacy block (and scsi? Paolo do you remember) devices would overestimate len in case of errors. -- MST
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]