[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] Re: [PATCH v2] balloon: transitional device support
On Mon, Apr 27, 2015 at 02:02:13PM +0200, Cornelia Huck wrote: > On Sun, 26 Apr 2015 20:40:10 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > Virtio 1.0 cs02 doesn't include a modern balloon device. At some > > point we'll likely define an incompatible interface with a > > different ID and different semantics. But for now, it's not a > > big effort to support a transitional balloon device: this has the > > advantage of supporting existing drivers, transparently, as well > > as transports that don't allow mixing virtio 0 and virtio 1 > > devices. And balloon is an easy device to test, so it's also > > useful for people to test virtio core handling of transitional > > devices. > > > > Three issues with legacy hypervisors have been identified: > > 1. Actual value is actually used, and is necessary for management > > to work. Luckily 4 byte config space writes are now atomic. > > When using old guests, hypervisors can detect access to the last byte. > > When using old hypervisors, drivers can use atomic 4-byte accesses. > > 2. Hypervisors actually didn't ignore the stats from the first > > buffer supplied. This means the values there would be > > incorrect until hypervisor resends the request. > > Add a note suggesting hypervisors ignore the 1st buffer. > > 3. QEMU simply over-writes stats from each buffer it gets. > > Thus if driver supplies a different subset of stats > > on each request, stale values will be there. > > Require drivers to supply the same subset on each > > request. This also gives us a simple way to figure out > > which stats are supported. > > > > Please note the issues are there in legacy mode only: > > we need to fix them even if we don't implement a modern > > interface for balloon. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > > > changes from v1: > > dropped stats format change > > added a bunch of normative statements > > documented work-arounds for legacy hypervisor bugs > > > > conformance.tex | 24 +++++++- > > content.tex | 173 ++++++++++++++++++++++++++++++++++++++++++++++++-------- > > 2 files changed, 170 insertions(+), 27 deletions(-) > > > > diff --git a/conformance.tex b/conformance.tex > > index 80b333f..c51287a 100644 > > --- a/conformance.tex > > +++ b/conformance.tex > > @@ -15,7 +15,7 @@ Conformance targets: > > \begin{itemize} > > \item Clause \ref{sec:Conformance / Driver Conformance}, > > \item One of clauses \ref{sec:Conformance / Driver Conformance / PCI Driver Conformance}, \ref{sec:Conformance / Driver Conformance / MMIO Driver Conformance} or \ref{sec:Conformance / Driver Conformance / Channel I/O Driver Conformance}. > > - \item One of clauses \ref{sec:Conformance / Driver Conformance / Network Driver Conformance}, \ref{sec:Conformance / Driver Conformance / Block Driver Conformance}, \ref{sec:Conformance / Driver Conformance / Console Driver Conformance}, \ref{sec:Conformance / Driver Conformance / Entropy Driver Conformance} or \ref{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}. > > + \item One of clauses \ref{sec:Conformance / Driver Conformance / Network Driver Conformance}, \ref{sec:Conformance / Driver Conformance / Block Driver Conformance}, \ref{sec:Conformance / Driver Conformance / Console Driver Conformance}, \ref{sec:Conformance / Driver Conformance / Entropy Driver Conformance} or \ref{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}. > > Unrelated whitespace change? > > > \end{itemize} > > \item[Device] A device MUST conform to three conformance clauses: > > \begin{itemize} > > (...) > > > diff --git a/content.tex b/content.tex > > index 11015a5..3f728e4 100644 > > --- a/content.tex > > +++ b/content.tex > > (...) > > > @@ -4393,6 +4392,26 @@ guest memory statistics to the host. > > memory statistics is present. > > \end{description} > > > > +\drivernormative{\subsubsection}{Feature bits}{Device Types / Memory Balloon Device / Feature bits} > > +The driver SHOULD accept the VIRTIO_BALLOON_F_MUST_TELL_HOST > > +feature if offered by the device. > > + > > +\devicenormative{\subsubsection}{Feature bits}{Device Types / Memory Balloon Device / Feature bits} > > +If the device offers the VIRTIO_BALLOON_F_MUST_TELL_HOST feature > > +bit, and if the driver did not accept this feature bit, the > > +device MAY signal failure by failing to set FEATURES_OK > > +\field{device status} bit when the driver writes FEATURES_OK into > > +\field{device status}. > > I'd drop "...when the driver writes...", as this should be clear. I'll just say "when the driver writes it", consistent with other places where we mention FEATURES_OK. > > +\subparagraph{Legacy Interface: Feature bits}\label{sec:Device > > +Types / Memory Balloon Device / Feature bits / Legacy Interface: > > +Feature bits} > > +As the legacy interface does not have a way to gracefully report feature > > +negotiation failure, when using the legacy interface, > > +transitional devices MUST support guests which do not negotiate > > +VIRTIO_BALLOON_F_MUST_TELL_HOST feature, and SHOULD > > +allow guest to use memory before notifying host if > > +VIRTIO_BALLOON_F_MUST_TELL_HOST is not negotiated. > > + > > \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon Device / Device configuration layout} > > Both fields of this configuration > > are always available. > > @@ -4404,25 +4423,32 @@ struct virtio_balloon_config { > > }; > > \end{lstlisting} > > > > -Note that these fields are always little endian, despite convention > > +\subparagraph{Legacy Interface: Device configuration layout}\label{sec:Device Types / Memory Balloon Device / Device > > +configuration layout / Legacy Interface: Device configuration layout} > > +When using the legacy interface, transitional devices and drivers > > +MUST format the fields in struct virtio_balloon_config > > Hm... is using MUST in a non-normative section ok? > > > +according to the little-endian format. > > +\begin{note} > > +This is unlike the usual convention that was used for other legacy devices > > Drop "that was used for other legacy devices"? > > > that legacy device fields are guest endian. > > +\end{note} > > > > \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Device / Device Initialization} > > > > +The device initialization process is outlined below: > > + > > \begin{enumerate} > > \item The inflate and deflate virtqueues are identified. > > > > \item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated: > > \begin{enumerate} > > \item Identify the stats virtqueue. > > - > > - \item Add one empty buffer to the stats virtqueue and notify the > > - device. > > + \item Add one empty buffer to the stats virtqueue. > > + \item DRIVER_OK is set: device operation begins. > > + \item Notify the device about the stats virtqueue buffer. > > \end{enumerate} > > \end{enumerate} > > > > -Device operation begins immediately. > > - > > \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation} > > > > The device is driven by the receipt of a > > (...) > > > @@ -4488,7 +4540,27 @@ When using the legacy interface, the driver SHOULD ignore the \field{len} value > > Historically, some devices put the total descriptor length there, > > even though no data was actually written. > > \end{note} > > +When using the legacy interface, the driver MUST write out all > > +4 bytes each time it updates the \field{actual} value in the > > +configuration space, using a single atomic operation. > > +\begin{note} > > +Historically, devices used the \field{actual} value, even though > > +the device-specific configuration space was never guaranteed > > s/never/not/ ? > > > +to be atomic. > > +\end{note} > > +When using the legacy interface, the device MUST NOT use the > > +\field{actual} value written by the driver in the configuration > > +space, until the last, most-significant byte of the value has been > > +written. > > +\begin{note} > > +Historically, drivers wrote the \field{actual} value > > +by using multiple single-byte writes in order, from the > > +least-significant to the most-significant value. > > But this is only true of _some_ drivers (virtio-pci), no? > > > +\end{note} > > > > +\footnote{As updates to device-specific configuration space are not atomic, this field > > +isn't particularly reliable, but can be used to diagnose buggy guests. > > Does this footnote still make sense for non-legacy? I guess not, as we > mandate atomic updates? > > > +} > > \subsubsection{Memory Statistics}\label{sec:Device Types / Memory Balloon Device / Device Operation / Memory Statistics} > > > > The stats virtqueue is atypical because communication is driven > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]