OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[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]