All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] [PATCH RFC 0/3] virtio-spec: Add documentation for recently added balloon features
@ 2020-04-29 18:57 Alexander Duyck
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 1/3] content: Document balloon feature free page hints Alexander Duyck
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Alexander Duyck @ 2020-04-29 18:57 UTC (permalink / raw)
  To: cohuck, david, mst; +Cc: virtio-dev, virtio-comment

This patch set is meant to add documentation for balloon features that have
been recently added to the Linux kernel[1,2] and that we are currently
working on adding to QEMU[3].

I am submitting them as an RFC as it has been a while since I have written
any requirements or specification documents, so my wording on things will
likely need some updates. In addition I have not before submitted
virtio-spec updates. As such I would appreciate information on if this is
the correct process for updating the document, or if I need to take some
other/additional steps.

[1]: https://lore.kernel.org/lkml/20200211224416.29318.44077.stgit@localhost.localdomain/
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0c504f154718904ae49349147e3b7e6ae91ffdc
[3]: https://lists.oasis-open.org/archives/virtio-dev/202004/msg00180.html

---

Alexander Duyck (3):
      content: Document balloon feature free page hints
      content: Document balloon feature page poison
      content: Document balloon feature free page reporting


 content.tex |  232 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 229 insertions(+), 3 deletions(-)

--

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [virtio-comment] [PATCH RFC 1/3] content: Document balloon feature free page hints
  2020-04-29 18:57 [virtio-comment] [PATCH RFC 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
@ 2020-04-29 18:57 ` Alexander Duyck
  2020-05-07 15:33   ` [virtio-comment] " Cornelia Huck
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 2/3] content: Document balloon feature page poison Alexander Duyck
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 3/3] content: Document balloon feature free page reporting Alexander Duyck
  2 siblings, 1 reply; 10+ messages in thread
From: Alexander Duyck @ 2020-04-29 18:57 UTC (permalink / raw)
  To: cohuck, david, mst; +Cc: virtio-dev, virtio-comment

From: Alexander Duyck <alexander.h.duyck@linux.intel.com>

Free page hints allow the balloon driver to provide information on what
pages are not currently in use so that we can avoid the cost of copying
them in migration scenarios. Add a feature description for free page hints
describing basic functioning and requirements.

Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 content.tex |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 110 insertions(+), 3 deletions(-)

diff --git a/content.tex b/content.tex
index b91a132df146..796901e83a71 100644
--- a/content.tex
+++ b/content.tex
@@ -5006,9 +5006,12 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
 \item[0] inflateq
 \item[1] deflateq
 \item[2] statsq.
+\item[3] free_page_vq.
 \end{description}
 
-  Virtqueue 2 only exists if VIRTIO_BALLOON_F_STATS_VQ set.
+  statsq only exists if VIRTIO_BALLOON_F_STATS_VQ set.
+
+  free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT set.
 
 \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Feature bits}
 \begin{description}
@@ -5019,6 +5022,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
     memory statistics is present.
 \item[VIRTIO_BALLOON_F_DEFLATE_ON_OOM (2) ] Deflate balloon on
     guest out of memory condition.
+\item[ VIRTIO_BALLOON_F_FREE_PAGE_HINT(3) ] Device has support for free
+    page hinting. A virtqueue for providing hints as to what memory is
+    currently free is present. Configuration field free_page_hint_cmd_id
+    is valid.
 
 \end{description}
 
@@ -5042,13 +5049,15 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
 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.
+  The first two fields of this configuration are always present. The
+  availability of the others all depend on various feature bits as
+  indicated above.
 
 \begin{lstlisting}
 struct virtio_balloon_config {
         le32 num_pages;
         le32 actual;
+        le32 free_page_hint_cmd_id;
 };
 \end{lstlisting}
 
@@ -5075,6 +5084,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
   \item DRIVER_OK is set: device operation begins.
   \item Notify the device about the stats virtqueue buffer.
   \end{enumerate}
+
+\item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is negotiated the
+  free_page_vq is identified.
 \end{enumerate}
 
 \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
@@ -5345,6 +5357,101 @@ \subsubsection{Memory Statistics Tags}\label{sec:Device Types / Memory Balloon D
   allocations in the guest.
 \end{description}
 
+\subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
+
+Free page hinting is used during migration to determine what pages within
+the guest are current unused so that they can be skipped over when it comes
+time for migration. The device will indicate that it is ready to start
+performing hinting by setting the \field{free_page_hint_cmd_id} to one of the
+non-reserved values that can be used as a command ID:
+
+\begin{description}
+\item[VIRTIO_BALLOON_CMD_ID_STOP (0)] Any previous command ID is invalid.
+  All hinting SHOULD halt until a new command ID is supplied.
+
+\item[VIRTIO_BALLOON_CMD_ID_DONE (1)] Any previous command ID is invalid.
+  All hinting SHOULD halt and pages returned to the guest for use.
+\end{description}
+
+A request for free page hintings proceeds as follows:
+
+\begin{enumerate}
+
+\item \field{free_page_hint_cmd_id} configuration field is examined. If it
+  contains a non-reserved value then inflation of the balloon will begin.
+
+\item To supply memory to the hinting balloon:
+  \begin{enumerate}
+  \item The driver constructs an output descriptor containing the new value
+    from \field{free_page_hint_cmd_id} configuration field and adds it to
+    the free_page_hint_vq.
+  \item The driver driver maps a series of pages and adds them to the
+    free_page_hint_vq as individual scatter-gather entries.
+  \item When the driver is no longer able to fetch additional pages to add
+    to the free_page_hint_vq it will construct an output descriptor
+    containing the command ID VIRTIO_BALLOON_CMD_ID_STOP.
+  \end{enumerate}
+
+\item A round of hinting ends either when the driver is no longer able to
+  add more pages to the balloon as described above, or when the device
+  updates \field{free_page_hint_cmd_id} configuration field contain either
+  VIRTIO_BALLOON_CMD_ID_STOP or VIRTIO_BALLOON_CMD_ID_DONE.
+
+\item The device may follow VIRTIO_BALLOON_CMD_ID_STOP with a new
+  non-reserved value for \field{free_page_hint_cmd_id} configuration field
+  in which case it will resume supplying the hinting balloon.
+
+\item Otherwise, if the device provides VIRTIO_BALLOON_CMD_ID_DONE then
+  hinting is complete and the guest may begin to re-use pages preivously
+  given to the balloon.
+
+\end{enumerate}
+
+\drivernormative{\paragraph}{Free Page Hinting}{Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
+
+Normative statements in this section apply if and only if  the
+VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.
+
+The driver SHOULD supply pages to the hinting balloon when
+\field{free_page_hint_cmd_id} reports a value of 2 or greater.
+
+The driver MUST start hinting by providing an output descriptor
+containing the current command ID for the given block of pages.
+
+The driver SHOULD stop supplying pages to the hinting balloon when
+\field{free_page_hint_cmd_id} reports a value of VIRTIO_BALLOON_CMD_ID_STOP.
+
+If the driver is unable to supply pages it MUST complete hinting by adding
+an output descriptor containing the command ID VIRTIO_BALLOON_CMD_ID_STOP.
+
+The driver MAY use pages from the balloon after adding them to the balloon,
+including when the device has not acknowledged the hinting request.
+
+The driver SHOULD return pages for use once \field{free_page_hint_cmd_id}
+reports a value of VIRTIO_BALLOON_CMD_ID_DONE.
+
+\devicenormative{\paragraph}{Free Page Hinting}{Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
+
+Normative statements in this section apply if and only if  the
+VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.
+
+The device MUST set \field{free_page_hint_cmd_id} to
+VIRTIO_BALLOON_CMD_ID_STOP any time that the host dirty bits for the given
+guest are being recorded.
+
+The device MUST guarantee that command ID is not reused until it has
+received an output descriptor containing VIRTIO_BALLOON_CMD_ID_STOP from
+the driver.
+
+The device MUST not perform hinting on pages that are provided with a
+command ID that does not match the current value in
+\field{free_page_hint_cmd_id}.
+
+The device MAY modify the contents of the page in the balloon at any time
+after detecting its physical number until it has either been written to by
+the guest or \field{free_page_hint_cmd_id} is set to
+VIRTIO_BALLOON_CMD_ID_DONE.
+
 \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
 
 The virtio SCSI host device groups together one or more virtual


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [virtio-comment] [PATCH RFC 2/3] content: Document balloon feature page poison
  2020-04-29 18:57 [virtio-comment] [PATCH RFC 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 1/3] content: Document balloon feature free page hints Alexander Duyck
@ 2020-04-29 18:57 ` Alexander Duyck
  2020-05-07 15:48   ` [virtio-comment] " Cornelia Huck
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 3/3] content: Document balloon feature free page reporting Alexander Duyck
  2 siblings, 1 reply; 10+ messages in thread
From: Alexander Duyck @ 2020-04-29 18:57 UTC (permalink / raw)
  To: cohuck, david, mst; +Cc: virtio-dev, virtio-comment

From: Alexander Duyck <alexander.h.duyck@linux.intel.com>

Page poison provides a way for the guest to notify the host of the content
expected to be found in pages when they are added back to the guest after
being discarded. The feature currently doesn't apply to the existing
balloon features, however it will apply to an upcoming feature, free page
reporting. Add documentation for the page poison feature describing the
basic functionality and requirements.

Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 content.tex |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/content.tex b/content.tex
index 796901e83a71..c98b8ea9526a 100644
--- a/content.tex
+++ b/content.tex
@@ -5026,6 +5026,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
     page hinting. A virtqueue for providing hints as to what memory is
     currently free is present. Configuration field free_page_hint_cmd_id
     is valid.
+\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] Host has to be notified if guest
+    is expecting reported pages to contain a certain value when returned.
+    Configuration field poison_val is valid.
 
 \end{description}
 
@@ -5033,6 +5036,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
 The driver SHOULD accept the VIRTIO_BALLOON_F_MUST_TELL_HOST
 feature if offered by the device.
 
+The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is not
+expecting any specific value to be stored in the page.
+
 \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
@@ -5058,6 +5064,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon
         le32 num_pages;
         le32 actual;
         le32 free_page_hint_cmd_id;
+        le32 poison_val;
 };
 \end{lstlisting}
 
@@ -5087,6 +5094,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
 
 \item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is negotiated the
   free_page_vq is identified.
+
+\item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated then
+  the driver MUST update the poison_val configuration field.
 \end{enumerate}
 
 \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
@@ -5452,6 +5462,41 @@ \subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device
 the guest or \field{free_page_hint_cmd_id} is set to
 VIRTIO_BALLOON_CMD_ID_DONE.
 
+\subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Device Operation / Page Poison}
+
+Page Poison provides a way to notify the host of the contents that are
+currently in the balloon pages, and those that are expected to be in the
+pages when they are pulled from the balloon. It is used for in-place
+reporting of pages without needing to pull them from the memory allocator
+of the guest.
+
+\begin{enumerate}
+
+\item If VIRTIO_BALLOON_F_PAGE_POISON feature is negotiated, the guest will
+  place the expected poison value in \field{poison_val} configuration data.
+
+\end{enumerate}
+
+\drivernormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
+
+Normative statements in this section apply if and only if  the
+VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
+
+The driver MUST populate the \field{poison_val} configuration data if it is
+expecting the page to contain some fixed value when free.
+
+The driver MAY opt to disable the feature if it will take care of
+re-initializing pages when first accessing them.
+
+\devicenormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
+
+Normative statements in this section apply if and only if  the
+VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
+
+The device MAY ignore the \field{poison_val} for normal balloon operations and
+free page hinting as this feature did not exist prior to those features being
+added.
+
 \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
 
 The virtio SCSI host device groups together one or more virtual


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [virtio-comment] [PATCH RFC 3/3] content: Document balloon feature free page reporting
  2020-04-29 18:57 [virtio-comment] [PATCH RFC 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 1/3] content: Document balloon feature free page hints Alexander Duyck
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 2/3] content: Document balloon feature page poison Alexander Duyck
@ 2020-04-29 18:57 ` Alexander Duyck
  2020-05-07 15:58   ` [virtio-comment] " Cornelia Huck
  2 siblings, 1 reply; 10+ messages in thread
From: Alexander Duyck @ 2020-04-29 18:57 UTC (permalink / raw)
  To: cohuck, david, mst; +Cc: virtio-dev, virtio-comment

From: Alexander Duyck <alexander.h.duyck@linux.intel.com>

Free page reporting is a feature that allows the guest to proactively
report unused pages to the host. By making use of this feature is is
possible to reduce the overall memory footprint of the guest in cases where
some significant portion of the memory is idle. Add documentation for the
free page reporting feature describing the functionality and requirements.

Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 content.tex |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/content.tex b/content.tex
index c98b8ea9526a..52955c8ff007 100644
--- a/content.tex
+++ b/content.tex
@@ -5007,12 +5007,15 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
 \item[1] deflateq
 \item[2] statsq.
 \item[3] free_page_vq.
+\item[4] reporting_vq.
 \end{description}
 
   statsq only exists if VIRTIO_BALLOON_F_STATS_VQ set.
 
   free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT set.
 
+  reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING set.
+
 \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Feature bits}
 \begin{description}
 \item[VIRTIO_BALLOON_F_MUST_TELL_HOST (0)] Host has to be told before
@@ -5029,6 +5032,8 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
 \item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] Host has to be notified if guest
     is expecting reported pages to contain a certain value when returned.
     Configuration field poison_val is valid.
+\item[ VIRTIO_BALLOON_F_PAGE_REPORTING(5) ] Device has support for free
+    page reporting. A virtqueue for reporting free guest memory is present.
 
 \end{description}
 
@@ -5039,6 +5044,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
 The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is not
 expecting any specific value to be stored in the page.
 
+The driver MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING if it is expecting
+the pages to retain some initialized value and it has not negotiated
+VIRTIO_BALLOON_F_PAGE_POISON as a feature.
+
 \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
@@ -5097,6 +5106,13 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
 
 \item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated then
   the driver MUST update the poison_val configuration field.
+
+\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated:
+  \begin{enumerate}
+  \item Identify the reporting virtqueue.
+  \item DRIVER_OK is set: device operation begins.
+  \item Begin reporting free pages to device.
+  \end{enumerate}
 \end{enumerate}
 
 \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
@@ -5497,6 +5513,64 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
 free page hinting as this feature did not exist prior to those features being
 added.
 
+\subsubsection{Free Page Reporting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
+
+Free Page Reporting provides a mechanism similar to balloon inflation,
+however it does not provide a deflation queue. The expectation is that the
+device will have a means by which it can detect the guest page access and
+fault in such pages with some initial value, likely a zero page.
+
+The driver will respond to to memory conditions and begin reporting free
+pages when some number of pages are available.
+
+\begin{enumerate}
+
+\item The driver determines it has enough pages available to begin
+  reporting pages.
+
+\item The driver gathers pages into a scatter-gather list and adds them to
+  the reporting_vq.
+
+\item The device acknowledges the reporting request.
+
+\item Once the device has acknowledged the report, the pages can be
+  returned to the location from which they were pulled.
+
+\item The driver can then continue to gather and report pages until it
+  has determined it has reported a suffcient quantity of pages.
+
+\end{enumerate}
+
+\drivernormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
+
+Normative statements in this section apply if and only if  the
+VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
+
+If the guest is expecting the free page to contain some initial value it
+MUST make use of the VIRTIO_BALLOON_F_PAGE_POISON feature to notify the
+device of this expectation via \field{poison_val}.
+
+The driver MUST NOT use the reported pages until the device has
+acknowledged the reporting request.
+
+The driver MAY report free pages any time after DRIVER_OK is set.
+
+It is RECOMMENDED that the driver avoid unecessary reads or writes to the
+page contents as this could reduce the performance for free page reporting.
+
+\devicenormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
+
+Normative statements in this section apply if and only if  the
+VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
+
+The device MAY modify the contents of a page in the report after detecting
+its physical number in an report request and before acknowledging the
+reporting request by using the reporting_vq descriptor.
+
+If the VIRTIO_BALLOON_F_PAGE_POISON feature is negotiated, the device
+SHALL NOT modify the the page if this will result in the page containing a
+value other than \field{poison_val}.
+
 \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
 
 The virtio SCSI host device groups together one or more virtual


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [virtio-comment] Re: [PATCH RFC 1/3] content: Document balloon feature free page hints
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 1/3] content: Document balloon feature free page hints Alexander Duyck
@ 2020-05-07 15:33   ` Cornelia Huck
  2020-05-07 19:58     ` Alexander Duyck
  0 siblings, 1 reply; 10+ messages in thread
From: Cornelia Huck @ 2020-05-07 15:33 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: david, mst, virtio-dev, virtio-comment

On Wed, 29 Apr 2020 11:57:45 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> Free page hints allow the balloon driver to provide information on what
> pages are not currently in use so that we can avoid the cost of copying
> them in migration scenarios. Add a feature description for free page hints
> describing basic functioning and requirements.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  content.tex |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 110 insertions(+), 3 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index b91a132df146..796901e83a71 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5006,9 +5006,12 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
>  \item[0] inflateq
>  \item[1] deflateq
>  \item[2] statsq.
> +\item[3] free_page_vq.

I'd lose the trailing period (and probably also remove it from statsq;
it just looks weird.)

>  \end{description}
>  
> -  Virtqueue 2 only exists if VIRTIO_BALLOON_F_STATS_VQ set.
> +  statsq only exists if VIRTIO_BALLOON_F_STATS_VQ set.

As you are touching this line anyway, s/set/is set/

> +
> +  free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT set.

s/set/is set/

>  
>  \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Feature bits}
>  \begin{description}
> @@ -5019,6 +5022,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>      memory statistics is present.
>  \item[VIRTIO_BALLOON_F_DEFLATE_ON_OOM (2) ] Deflate balloon on
>      guest out of memory condition.
> +\item[ VIRTIO_BALLOON_F_FREE_PAGE_HINT(3) ] Device has support for free
> +    page hinting. A virtqueue for providing hints as to what memory is
> +    currently free is present. Configuration field free_page_hint_cmd_id
> +    is valid.
>  
>  \end{description}
>  
> @@ -5042,13 +5049,15 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>  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.
> +  The first two fields of this configuration are always present. The
> +  availability of the others all depend on various feature bits as
> +  indicated above.

Maybe make this more explicit?

"
\field{num_pages} and \field{actual} are always available.

\field{free_page_hint_cmd_id} is available if
VIRTIO_BALLOON_F_FREE_PAGE_HINT has been negotiated and is read-only by
the driver.
"

>  
>  \begin{lstlisting}
>  struct virtio_balloon_config {
>          le32 num_pages;
>          le32 actual;
> +        le32 free_page_hint_cmd_id;
>  };
>  \end{lstlisting}
>  
> @@ -5075,6 +5084,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
>    \item DRIVER_OK is set: device operation begins.
>    \item Notify the device about the stats virtqueue buffer.
>    \end{enumerate}
> +
> +\item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is negotiated the
> +  free_page_vq is identified.

This startup sequence now seems wrong (identifying a vq after
DRIVER_OK). Should probably be:

- id inflate/deflate vqs
- if STATS_VQ:
  - id stats vq
  - put buffer on stats vq
- if FREE_PAGE_HINT:
  - id free page vq
- DRIVER_OK is set
- if STATS_VQ:
  - notify about buffer

>  \end{enumerate}
>  
>  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> @@ -5345,6 +5357,101 @@ \subsubsection{Memory Statistics Tags}\label{sec:Device Types / Memory Balloon D
>    allocations in the guest.
>  \end{description}
>  
> +\subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> +
> +Free page hinting is used during migration to determine what pages within

s/used/designed to be used/ ?

Or maybe

s/used during migration/can be used/ ?

> +the guest are current unused so that they can be skipped over when it comes

s/current/currently/

> +time for migration. The device will indicate that it is ready to start

s/when it comes time for migration/while migrating the guest/ ?

> +performing hinting by setting the \field{free_page_hint_cmd_id} to one of the
> +non-reserved values that can be used as a command ID:

s/command ID:/command ID. The following values are reserved:/

> +
> +\begin{description}
> +\item[VIRTIO_BALLOON_CMD_ID_STOP (0)] Any previous command ID is invalid.

"Any command ID previously supplied by the device is invalid." ?

> +  All hinting SHOULD halt until a new command ID is supplied.

"The driver should halt any hinting until..." ? Or what is actually
halting?

(I think we don't want to use 'SHOULD' outside of normative section,
although we probably haven't followed that rule everywhere.)

> +
> +\item[VIRTIO_BALLOON_CMD_ID_DONE (1)] Any previous command ID is invalid.

Same comment as above.

> +  All hinting SHOULD halt and pages returned to the guest for use.

Is this the same operation as above, only with returning all pages that
the guest previously hinted about?

(Hm. I see below that this is used by the driver on the vq. Can the
device actually set this in the config space?)

> +\end{description}
> +
> +A request for free page hintings proceeds as follows:
> +
> +\begin{enumerate}
> +
> +\item \field{free_page_hint_cmd_id} configuration field is examined. If it

"The driver examines..." ?

> +  contains a non-reserved value then inflation of the balloon will begin.
> +
> +\item To supply memory to the hinting balloon:
> +  \begin{enumerate}
> +  \item The driver constructs an output descriptor containing the new value
> +    from \field{free_page_hint_cmd_id} configuration field and adds it to
> +    the free_page_hint_vq.
> +  \item The driver driver maps a series of pages and adds them to the
> +    free_page_hint_vq as individual scatter-gather entries.
> +  \item When the driver is no longer able to fetch additional pages to add
> +    to the free_page_hint_vq it will construct an output descriptor
> +    containing the command ID VIRTIO_BALLOON_CMD_ID_STOP.
> +  \end{enumerate}

So the sequence is:

- The driver puts a descriptor containing only the id on the queue.
- The driver puts page pointers in a scatter-gather list on the queue...
- ...until it runs out of possible pages and put a descriptor
  containing only the STOP id on the queue.

> +
> +\item A round of hinting ends either when the driver is no longer able to
> +  add more pages to the balloon as described above, or when the device
> +  updates \field{free_page_hint_cmd_id} configuration field contain either
> +  VIRTIO_BALLOON_CMD_ID_STOP or VIRTIO_BALLOON_CMD_ID_DONE.

So, either the driver stops via STOP or the device stops via STOP or
DONE. The driver can't stop via DONE?

> +
> +\item The device may follow VIRTIO_BALLOON_CMD_ID_STOP with a new
> +  non-reserved value for \field{free_page_hint_cmd_id} configuration field
> +  in which case it will resume supplying the hinting balloon.

But the driver is not required to supply new pages and may instead
queue id + STOP?

> +
> +\item Otherwise, if the device provides VIRTIO_BALLOON_CMD_ID_DONE then
> +  hinting is complete and the guest may begin to re-use pages preivously

s/preivously/previously/

> +  given to the balloon.

So, there is no other way for the guest to regain use of pages it
hinted about?

(I'm not familiar with how this is implemented in Linux/QEMU, so some
of this might seem obvious to others. But a spec probably should
mention things like that as well :)

> +
> +\end{enumerate}
> +
> +\drivernormative{\paragraph}{Free Page Hinting}{Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> +
> +Normative statements in this section apply if and only if  the
> +VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.

"If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated:"

> +
> +The driver SHOULD supply pages to the hinting balloon when
> +\field{free_page_hint_cmd_id} reports a value of 2 or greater.
> +
> +The driver MUST start hinting by providing an output descriptor
> +containing the current command ID for the given block of pages.
> +
> +The driver SHOULD stop supplying pages to the hinting balloon when
> +\field{free_page_hint_cmd_id} reports a value of VIRTIO_BALLOON_CMD_ID_STOP.

What happens if it continues?

> +
> +If the driver is unable to supply pages it MUST complete hinting by adding
> +an output descriptor containing the command ID VIRTIO_BALLOON_CMD_ID_STOP.
> +
> +The driver MAY use pages from the balloon after adding them to the balloon,
> +including when the device has not acknowledged the hinting request.

How does the device acknowledge the hinting request?

> +
> +The driver SHOULD return pages for use once \field{free_page_hint_cmd_id}
> +reports a value of VIRTIO_BALLOON_CMD_ID_DONE.
> +
> +\devicenormative{\paragraph}{Free Page Hinting}{Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> +
> +Normative statements in this section apply if and only if  the
> +VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.

"If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated:"

> +
> +The device MUST set \field{free_page_hint_cmd_id} to
> +VIRTIO_BALLOON_CMD_ID_STOP any time that the host dirty bits for the given
> +guest are being recorded.

What does that mean? Is 'host dirty bits' really a generic term?

> +
> +The device MUST guarantee that command ID is not reused until it has
> +received an output descriptor containing VIRTIO_BALLOON_CMD_ID_STOP from
> +the driver.

"The device MUST NOT reuse a command ID until..."

> +
> +The device MUST not perform hinting on pages that are provided with a
> +command ID that does not match the current value in
> +\field{free_page_hint_cmd_id}.

So, what does it do if the driver supplies such pages? Ignore them? It
can't signal an error to the driver, can it?

> +
> +The device MAY modify the contents of the page in the balloon at any time
> +after detecting its physical number until it has either been written to by

What is the 'physical number'?

> +the guest or \field{free_page_hint_cmd_id} is set to
> +VIRTIO_BALLOON_CMD_ID_DONE.
> +
>  \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
>  
>  The virtio SCSI host device groups together one or more virtual
> 


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [virtio-comment] Re: [PATCH RFC 2/3] content: Document balloon feature page poison
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 2/3] content: Document balloon feature page poison Alexander Duyck
@ 2020-05-07 15:48   ` Cornelia Huck
  2020-05-07 20:16     ` Alexander Duyck
  0 siblings, 1 reply; 10+ messages in thread
From: Cornelia Huck @ 2020-05-07 15:48 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: david, mst, virtio-dev, virtio-comment

On Wed, 29 Apr 2020 11:57:51 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> Page poison provides a way for the guest to notify the host of the content
> expected to be found in pages when they are added back to the guest after
> being discarded. The feature currently doesn't apply to the existing
> balloon features, however it will apply to an upcoming feature, free page
> reporting. Add documentation for the page poison feature describing the
> basic functionality and requirements.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  content.tex |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 796901e83a71..c98b8ea9526a 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5026,6 +5026,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>      page hinting. A virtqueue for providing hints as to what memory is
>      currently free is present. Configuration field free_page_hint_cmd_id
>      is valid.
> +\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] Host has to be notified if guest
> +    is expecting reported pages to contain a certain value when returned.
> +    Configuration field poison_val is valid.
>  
>  \end{description}
>  
> @@ -5033,6 +5036,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>  The driver SHOULD accept the VIRTIO_BALLOON_F_MUST_TELL_HOST
>  feature if offered by the device.
>  
> +The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is not
> +expecting any specific value to be stored in the page.
> +
>  \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
> @@ -5058,6 +5064,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon
>          le32 num_pages;
>          le32 actual;
>          le32 free_page_hint_cmd_id;
> +        le32 poison_val;

Add

"\field{poison_val} is available if VIRTIO_BALLOON_F_PAGE_POISON has
been negotiated."

above.

>  };
>  \end{lstlisting}
>  
> @@ -5087,6 +5094,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
>  
>  \item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is negotiated the
>    free_page_vq is identified.
> +
> +\item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated then
> +  the driver MUST update the poison_val configuration field.

When does the driver set the field? Before or after DRIVER_OK?

Can it change it later?

>  \end{enumerate}
>  
>  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> @@ -5452,6 +5462,41 @@ \subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device
>  the guest or \field{free_page_hint_cmd_id} is set to
>  VIRTIO_BALLOON_CMD_ID_DONE.
>  
> +\subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +
> +Page Poison provides a way to notify the host of the contents that are
> +currently in the balloon pages, and those that are expected to be in the
> +pages when they are pulled from the balloon. It is used for in-place
> +reporting of pages without needing to pull them from the memory allocator
> +of the guest.
> +
> +\begin{enumerate}
> +
> +\item If VIRTIO_BALLOON_F_PAGE_POISON feature is negotiated, the guest will

s/is/has been/

s/the guest/the driver/

> +  place the expected poison value in \field{poison_val} configuration data.
> +
> +\end{enumerate}

Do you really need a list with one item?

> +
> +\drivernormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +
> +Normative statements in this section apply if and only if  the
> +VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.

"If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated:"

> +
> +The driver MUST populate the \field{poison_val} configuration data if it is
> +expecting the page to contain some fixed value when free.

Are there any restrictions of which values may be used here? E.g., is 0
a valid value?

> +
> +The driver MAY opt to disable the feature if it will take care of
> +re-initializing pages when first accessing them.
> +
> +\devicenormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +
> +Normative statements in this section apply if and only if  the
> +VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.

"If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated:"

> +
> +The device MAY ignore the \field{poison_val} for normal balloon operations and
> +free page hinting as this feature did not exist prior to those features being
> +added.

So, for which operations is the device required to do something here?
If it does not want to honour the field, it can simply elect to not
offer the feature? Or is there a dependency between features?

> +
>  \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
>  
>  The virtio SCSI host device groups together one or more virtual
> 


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [virtio-comment] Re: [PATCH RFC 3/3] content: Document balloon feature free page reporting
  2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 3/3] content: Document balloon feature free page reporting Alexander Duyck
@ 2020-05-07 15:58   ` Cornelia Huck
  2020-05-07 20:35     ` Alexander Duyck
  0 siblings, 1 reply; 10+ messages in thread
From: Cornelia Huck @ 2020-05-07 15:58 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: david, mst, virtio-dev, virtio-comment

On Wed, 29 Apr 2020 11:57:57 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> Free page reporting is a feature that allows the guest to proactively
> report unused pages to the host. By making use of this feature is is
> possible to reduce the overall memory footprint of the guest in cases where
> some significant portion of the memory is idle. Add documentation for the
> free page reporting feature describing the functionality and requirements.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  content.tex |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index c98b8ea9526a..52955c8ff007 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5007,12 +5007,15 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
>  \item[1] deflateq
>  \item[2] statsq.
>  \item[3] free_page_vq.
> +\item[4] reporting_vq.

Also lose the trailing period here.

>  \end{description}
>  
>    statsq only exists if VIRTIO_BALLOON_F_STATS_VQ set.
>  
>    free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT set.
>  
> +  reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING set.

s/set/is set/

> +
>  \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Feature bits}
>  \begin{description}
>  \item[VIRTIO_BALLOON_F_MUST_TELL_HOST (0)] Host has to be told before
> @@ -5029,6 +5032,8 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>  \item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] Host has to be notified if guest
>      is expecting reported pages to contain a certain value when returned.
>      Configuration field poison_val is valid.
> +\item[ VIRTIO_BALLOON_F_PAGE_REPORTING(5) ] Device has support for free
> +    page reporting. A virtqueue for reporting free guest memory is present.
>  
>  \end{description}
>  
> @@ -5039,6 +5044,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
>  The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is not
>  expecting any specific value to be stored in the page.
>  
> +The driver MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING if it is expecting
> +the pages to retain some initialized value and it has not negotiated
> +VIRTIO_BALLOON_F_PAGE_POISON as a feature.
> +
>  \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
> @@ -5097,6 +5106,13 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
>  
>  \item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated then
>    the driver MUST update the poison_val configuration field.
> +
> +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated:
> +  \begin{enumerate}
> +  \item Identify the reporting virtqueue.
> +  \item DRIVER_OK is set: device operation begins.
> +  \item Begin reporting free pages to device.
> +  \end{enumerate}
>  \end{enumerate}

See my previous comments regarding reordering this.

>  
>  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> @@ -5497,6 +5513,64 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
>  free page hinting as this feature did not exist prior to those features being
>  added.
>  
> +\subsubsection{Free Page Reporting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> +
> +Free Page Reporting provides a mechanism similar to balloon inflation,
> +however it does not provide a deflation queue. The expectation is that the
> +device will have a means by which it can detect the guest page access and
> +fault in such pages with some initial value, likely a zero page.
> +
> +The driver will respond to to memory conditions and begin reporting free
> +pages when some number of pages are available.
> +
> +\begin{enumerate}
> +
> +\item The driver determines it has enough pages available to begin
> +  reporting pages.
> +
> +\item The driver gathers pages into a scatter-gather list and adds them to
> +  the reporting_vq.
> +
> +\item The device acknowledges the reporting request.

How?

> +
> +\item Once the device has acknowledged the report, the pages can be
> +  returned to the location from which they were pulled.
> +
> +\item The driver can then continue to gather and report pages until it
> +  has determined it has reported a suffcient quantity of pages.
> +
> +\end{enumerate}
> +
> +\drivernormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> +
> +Normative statements in this section apply if and only if  the
> +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.

"If the VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated:"

> +
> +If the guest is expecting the free page to contain some initial value it

s/guest/driver/

> +MUST make use of the VIRTIO_BALLOON_F_PAGE_POISON feature to notify the
> +device of this expectation via \field{poison_val}.

"..., it MUST NOT negotiate this feature without negotiating the
VIRTIO_BALLOON_F_PAGE_POISON feature as well and supply this value via
\field{poison_val}."

> +
> +The driver MUST NOT use the reported pages until the device has
> +acknowledged the reporting request.
> +
> +The driver MAY report free pages any time after DRIVER_OK is set.
> +
> +It is RECOMMENDED that the driver avoid unecessary reads or writes to the
> +page contents as this could reduce the performance for free page reporting.
> +
> +\devicenormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> +
> +Normative statements in this section apply if and only if  the
> +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.

"If the VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated:"

> +
> +The device MAY modify the contents of a page in the report after detecting
> +its physical number in an report request and before acknowledging the

What is a 'physical number'?

> +reporting request by using the reporting_vq descriptor.

What is the 'reporting_vq descriptor'?

> +
> +If the VIRTIO_BALLOON_F_PAGE_POISON feature is negotiated, the device

s/is negotiated/has been negotiated as well/

> +SHALL NOT modify the the page if this will result in the page containing a
> +value other than \field{poison_val}.
> +
>  \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
>  
>  The virtio SCSI host device groups together one or more virtual
> 


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [virtio-comment] Re: [PATCH RFC 1/3] content: Document balloon feature free page hints
  2020-05-07 15:33   ` [virtio-comment] " Cornelia Huck
@ 2020-05-07 19:58     ` Alexander Duyck
  0 siblings, 0 replies; 10+ messages in thread
From: Alexander Duyck @ 2020-05-07 19:58 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: David Hildenbrand, Michael S. Tsirkin, virtio-dev, virtio-comment

Thanks for the review. For anything below where I didn't comment I am
just going with your suggestion.

On Thu, May 7, 2020 at 8:35 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Wed, 29 Apr 2020 11:57:45 -0700
> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>
> > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >
> > Free page hints allow the balloon driver to provide information on what
> > pages are not currently in use so that we can avoid the cost of copying
> > them in migration scenarios. Add a feature description for free page hints
> > describing basic functioning and requirements.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > ---
> >  content.tex |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 110 insertions(+), 3 deletions(-)
> >
> > diff --git a/content.tex b/content.tex
> > index b91a132df146..796901e83a71 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -5006,9 +5006,12 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
> >  \item[0] inflateq
> >  \item[1] deflateq
> >  \item[2] statsq.
> > +\item[3] free_page_vq.
>
> I'd lose the trailing period (and probably also remove it from statsq;
> it just looks weird.)
>
> >  \end{description}
> >
> > -  Virtqueue 2 only exists if VIRTIO_BALLOON_F_STATS_VQ set.
> > +  statsq only exists if VIRTIO_BALLOON_F_STATS_VQ set.
>
> As you are touching this line anyway, s/set/is set/
>
> > +
> > +  free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT set.
>
> s/set/is set/
>
> >
> >  \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Feature bits}
> >  \begin{description}
> > @@ -5019,6 +5022,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
> >      memory statistics is present.
> >  \item[VIRTIO_BALLOON_F_DEFLATE_ON_OOM (2) ] Deflate balloon on
> >      guest out of memory condition.
> > +\item[ VIRTIO_BALLOON_F_FREE_PAGE_HINT(3) ] Device has support for free
> > +    page hinting. A virtqueue for providing hints as to what memory is
> > +    currently free is present. Configuration field free_page_hint_cmd_id
> > +    is valid.
> >
> >  \end{description}
> >
> > @@ -5042,13 +5049,15 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
> >  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.
> > +  The first two fields of this configuration are always present. The
> > +  availability of the others all depend on various feature bits as
> > +  indicated above.
>
> Maybe make this more explicit?
>
> "
> \field{num_pages} and \field{actual} are always available.
>
> \field{free_page_hint_cmd_id} is available if
> VIRTIO_BALLOON_F_FREE_PAGE_HINT has been negotiated and is read-only by
> the driver.
> "
>
> >
> >  \begin{lstlisting}
> >  struct virtio_balloon_config {
> >          le32 num_pages;
> >          le32 actual;
> > +        le32 free_page_hint_cmd_id;
> >  };
> >  \end{lstlisting}
> >
> > @@ -5075,6 +5084,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
> >    \item DRIVER_OK is set: device operation begins.
> >    \item Notify the device about the stats virtqueue buffer.
> >    \end{enumerate}
> > +
> > +\item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is negotiated the
> > +  free_page_vq is identified.
>
> This startup sequence now seems wrong (identifying a vq after
> DRIVER_OK). Should probably be:
>
> - id inflate/deflate vqs
> - if STATS_VQ:
>   - id stats vq
>   - put buffer on stats vq
> - if FREE_PAGE_HINT:
>   - id free page vq
> - DRIVER_OK is set
> - if STATS_VQ:
>   - notify about buffer
>
> >  \end{enumerate}
> >
> >  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> > @@ -5345,6 +5357,101 @@ \subsubsection{Memory Statistics Tags}\label{sec:Device Types / Memory Balloon D
> >    allocations in the guest.
> >  \end{description}
> >
> > +\subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> > +
> > +Free page hinting is used during migration to determine what pages within
>
> s/used/designed to be used/ ?
>
> Or maybe
>
> s/used during migration/can be used/ ?

I'll go with the "designed to be used". Part of the issue is that free
page hinting seems to have assumptions all over the place about how it
is expected to be used so I want to make sure that people understand
that this is the primary use case.

> > +the guest are current unused so that they can be skipped over when it comes
>
> s/current/currently/
>
> > +time for migration. The device will indicate that it is ready to start
>
> s/when it comes time for migration/while migrating the guest/ ?
>
> > +performing hinting by setting the \field{free_page_hint_cmd_id} to one of the
> > +non-reserved values that can be used as a command ID:
>
> s/command ID:/command ID. The following values are reserved:/
>
> > +
> > +\begin{description}
> > +\item[VIRTIO_BALLOON_CMD_ID_STOP (0)] Any previous command ID is invalid.
>
> "Any command ID previously supplied by the device is invalid." ?
>
> > +  All hinting SHOULD halt until a new command ID is supplied.
>
> "The driver should halt any hinting until..." ? Or what is actually
> halting?
>
> (I think we don't want to use 'SHOULD' outside of normative section,
> although we probably haven't followed that rule everywhere.)

I'll go through and try to fix this and any other spots where I might
have done that.

> > +
> > +\item[VIRTIO_BALLOON_CMD_ID_DONE (1)] Any previous command ID is invalid.
>
> Same comment as above.
>
> > +  All hinting SHOULD halt and pages returned to the guest for use.
>
> Is this the same operation as above, only with returning all pages that
> the guest previously hinted about?

Yes. Basically it is the final STOP for a given hinting session so the
balloon can be deflated returning the contents to the guest.

> (Hm. I see below that this is used by the driver on the vq. Can the
> device actually set this in the config space?)

CMD_ID_STOP is used by the driver on the vq, I do not believe
CMD_ID_DONE is. The device cannot write to this piece in the config
space if I recall correctly.

> > +\end{description}
> > +
> > +A request for free page hintings proceeds as follows:
> > +
> > +\begin{enumerate}
> > +
> > +\item \field{free_page_hint_cmd_id} configuration field is examined. If it
>
> "The driver examines..." ?
>
> > +  contains a non-reserved value then inflation of the balloon will begin.
> > +
> > +\item To supply memory to the hinting balloon:
> > +  \begin{enumerate}
> > +  \item The driver constructs an output descriptor containing the new value
> > +    from \field{free_page_hint_cmd_id} configuration field and adds it to
> > +    the free_page_hint_vq.
> > +  \item The driver driver maps a series of pages and adds them to the
> > +    free_page_hint_vq as individual scatter-gather entries.
> > +  \item When the driver is no longer able to fetch additional pages to add
> > +    to the free_page_hint_vq it will construct an output descriptor
> > +    containing the command ID VIRTIO_BALLOON_CMD_ID_STOP.
> > +  \end{enumerate}
>
> So the sequence is:
>
> - The driver puts a descriptor containing only the id on the queue.
> - The driver puts page pointers in a scatter-gather list on the queue...
> - ...until it runs out of possible pages and put a descriptor
>   containing only the STOP id on the queue.

Correct.

> > +
> > +\item A round of hinting ends either when the driver is no longer able to
> > +  add more pages to the balloon as described above, or when the device
> > +  updates \field{free_page_hint_cmd_id} configuration field contain either
> > +  VIRTIO_BALLOON_CMD_ID_STOP or VIRTIO_BALLOON_CMD_ID_DONE.
>
> So, either the driver stops via STOP or the device stops via STOP or
> DONE. The driver can't stop via DONE?

Correct. Basically once the hinting starts the driver shouldn't
attempt to free the entire balloon until it receives DONE.

> > +
> > +\item The device may follow VIRTIO_BALLOON_CMD_ID_STOP with a new
> > +  non-reserved value for \field{free_page_hint_cmd_id} configuration field
> > +  in which case it will resume supplying the hinting balloon.
>
> But the driver is not required to supply new pages and may instead
> queue id + STOP?

Correct.

> > +
> > +\item Otherwise, if the device provides VIRTIO_BALLOON_CMD_ID_DONE then
> > +  hinting is complete and the guest may begin to re-use pages preivously
>
> s/preivously/previously/
>
> > +  given to the balloon.
>
> So, there is no other way for the guest to regain use of pages it
> hinted about?

There is. If there is memory pressure on the guest the hinting balloon
will automatically shrink and give up some pages. Really I don't even
conceptually think of the hinting as using a balloon so much as
something like a migration airbag. Basically the balloon will inflate
very quickly, but is extremely leaky in that it gives back pages at
the slightest bit of pressure and putting pages in the hinting balloon
doesn't guarantee that there will be any effect as they might be
ignored and migrated anyway.

> (I'm not familiar with how this is implemented in Linux/QEMU, so some
> of this might seem obvious to others. But a spec probably should
> mention things like that as well :)

It is good to point this out. Actually looking over the code I think
there is a bug in terms of that as the shrinker can kick in and start
deflating the hitning balloon and I don't see anything to prevent it
from continuing to try and inflate it as well. It seems like if we are
having to pull pages out because the shrinker is triggered we should
probably stop hinting at that point.

> > +
> > +\end{enumerate}
> > +
> > +\drivernormative{\paragraph}{Free Page Hinting}{Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> > +
> > +Normative statements in this section apply if and only if  the
> > +VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.
>
> "If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated:"
>
> > +
> > +The driver SHOULD supply pages to the hinting balloon when
> > +\field{free_page_hint_cmd_id} reports a value of 2 or greater.
> > +
> > +The driver MUST start hinting by providing an output descriptor
> > +containing the current command ID for the given block of pages.
> > +
> > +The driver SHOULD stop supplying pages to the hinting balloon when
> > +\field{free_page_hint_cmd_id} reports a value of VIRTIO_BALLOON_CMD_ID_STOP.
>
> What happens if it continues?

If I recall the values it reports will be processed to clear the ring,
but ignored in terms of hinting. So if it just kept inflating without
stopping it would be providing useless data.

> > +
> > +If the driver is unable to supply pages it MUST complete hinting by adding
> > +an output descriptor containing the command ID VIRTIO_BALLOON_CMD_ID_STOP.
> > +
> > +The driver MAY use pages from the balloon after adding them to the balloon,
> > +including when the device has not acknowledged the hinting request.
>
> How does the device acknowledge the hinting request?

The device acknowledges it by using the descriptor. Once it is
consumed it is considered acknowledged.

> > +
> > +The driver SHOULD return pages for use once \field{free_page_hint_cmd_id}
> > +reports a value of VIRTIO_BALLOON_CMD_ID_DONE.
> > +
> > +\devicenormative{\paragraph}{Free Page Hinting}{Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> > +
> > +Normative statements in this section apply if and only if  the
> > +VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.
>
> "If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated:"
>
> > +
> > +The device MUST set \field{free_page_hint_cmd_id} to
> > +VIRTIO_BALLOON_CMD_ID_STOP any time that the host dirty bits for the given
> > +guest are being recorded.
>
> What does that mean? Is 'host dirty bits' really a generic term?

I'm not sure how generic it is. Basically most pages have a dirty bit
to indicate that they contain contents that are supposed to be written
back, and in the QEMU/KVM case I believe we make use of a soft-dirty.
I can probably just replace it with "dirty pages" instead of referring
to the host dirty bits?

> > +
> > +The device MUST guarantee that command ID is not reused until it has
> > +received an output descriptor containing VIRTIO_BALLOON_CMD_ID_STOP from
> > +the driver.
>
> "The device MUST NOT reuse a command ID until..."
>
> > +
> > +The device MUST not perform hinting on pages that are provided with a
> > +command ID that does not match the current value in
> > +\field{free_page_hint_cmd_id}.
>
> So, what does it do if the driver supplies such pages? Ignore them? It
> can't signal an error to the driver, can it?

It will ignore them. As far as I know there is no way to signal that
the pages were provided outside the hinting window. I'll update it to
state that it must ignore the pages.

> > +
> > +The device MAY modify the contents of the page in the balloon at any time
> > +after detecting its physical number until it has either been written to by
>
> What is the 'physical number'?

That is a good question. I had assumed it meant something like page
frame number, and I copied the line from an earlier section in the
balloon spec. However I suppose that isn't really applicable here
since that would imply details about the hypervisor implementation.
Now that I think about it this whole paragraph is much more
complicated than I had originally thought.

Basically the device can modify the page so long as it is not written
to after the command ID associated with the given hint was issued. So
in theory you could write to the page after the command ID was issued,
but before it is added to the balloon and as a result the page would
go into the balloon but could not be modified. I think what I will do
is break it up as follows:

  The device MAY modify the contents of the page in the balloon if the page
  has not been modified since the \field{free_page_hint_cmd_id} associated
  with the hint was issued by the device.

  The device MAY NOT modify the contents of the balloon after
  \field{free_page_hint_cmd_id} is set to VIRTIO_BALLOON_CMD_ID_DONE.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [virtio-comment] Re: [PATCH RFC 2/3] content: Document balloon feature page poison
  2020-05-07 15:48   ` [virtio-comment] " Cornelia Huck
@ 2020-05-07 20:16     ` Alexander Duyck
  0 siblings, 0 replies; 10+ messages in thread
From: Alexander Duyck @ 2020-05-07 20:16 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: David Hildenbrand, Michael S. Tsirkin, virtio-dev, virtio-comment

On Thu, May 7, 2020 at 8:48 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Wed, 29 Apr 2020 11:57:51 -0700
> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>
> > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >
> > Page poison provides a way for the guest to notify the host of the content
> > expected to be found in pages when they are added back to the guest after
> > being discarded. The feature currently doesn't apply to the existing
> > balloon features, however it will apply to an upcoming feature, free page
> > reporting. Add documentation for the page poison feature describing the
> > basic functionality and requirements.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > ---
> >  content.tex |   45 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git a/content.tex b/content.tex
> > index 796901e83a71..c98b8ea9526a 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -5026,6 +5026,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
> >      page hinting. A virtqueue for providing hints as to what memory is
> >      currently free is present. Configuration field free_page_hint_cmd_id
> >      is valid.
> > +\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] Host has to be notified if guest
> > +    is expecting reported pages to contain a certain value when returned.
> > +    Configuration field poison_val is valid.
> >
> >  \end{description}
> >
> > @@ -5033,6 +5036,9 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
> >  The driver SHOULD accept the VIRTIO_BALLOON_F_MUST_TELL_HOST
> >  feature if offered by the device.
> >
> > +The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is not
> > +expecting any specific value to be stored in the page.
> > +
> >  \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
> > @@ -5058,6 +5064,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon
> >          le32 num_pages;
> >          le32 actual;
> >          le32 free_page_hint_cmd_id;
> > +        le32 poison_val;
>
> Add
>
> "\field{poison_val} is available if VIRTIO_BALLOON_F_PAGE_POISON has
> been negotiated."
>
> above.
>
> >  };
> >  \end{lstlisting}
> >
> > @@ -5087,6 +5094,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
> >
> >  \item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is negotiated the
> >    free_page_vq is identified.
> > +
> > +\item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated then
> > +  the driver MUST update the poison_val configuration field.
>
> When does the driver set the field? Before or after DRIVER_OK?
>
> Can it change it later?

It should be before DRIVER_OK, and it should not change. The expected
use case is that this provides information that should be static about
behaviors expected of the balloon driver so if the value is changed we
would likely need to unload and reload the driver, or at least deflate
the balloon and reset reporting.

> >  \end{enumerate}
> >
> >  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> > @@ -5452,6 +5462,41 @@ \subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device
> >  the guest or \field{free_page_hint_cmd_id} is set to
> >  VIRTIO_BALLOON_CMD_ID_DONE.
> >
> > +\subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> > +
> > +Page Poison provides a way to notify the host of the contents that are
> > +currently in the balloon pages, and those that are expected to be in the
> > +pages when they are pulled from the balloon. It is used for in-place
> > +reporting of pages without needing to pull them from the memory allocator
> > +of the guest.
> > +
> > +\begin{enumerate}
> > +
> > +\item If VIRTIO_BALLOON_F_PAGE_POISON feature is negotiated, the guest will
>
> s/is/has been/
>
> s/the guest/the driver/
>
> > +  place the expected poison value in \field{poison_val} configuration data.
> > +
> > +\end{enumerate}
>
> Do you really need a list with one item?

Probably not, what I was trying to do is just define this as a
separate part from the explanation of what it is in the paragraph
above. I will take a look at what options I have as this is
essentially just a statement that if the feature has been negotiated
then the guest will populate the data.

> > +
> > +\drivernormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
> > +
> > +Normative statements in this section apply if and only if  the
> > +VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
>
> "If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated:"
>
> > +
> > +The driver MUST populate the \field{poison_val} configuration data if it is
> > +expecting the page to contain some fixed value when free.
>
> Are there any restrictions of which values may be used here? E.g., is 0
> a valid value?

There are no restrictions other than that it is an unsigned 32b value.
So 0 is a valid value and actually I suspect it won't be that
uncommon.

> > +
> > +The driver MAY opt to disable the feature if it will take care of
> > +re-initializing pages when first accessing them.
> > +
> > +\devicenormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
> > +
> > +Normative statements in this section apply if and only if  the
> > +VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
>
> "If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated:"
>
> > +
> > +The device MAY ignore the \field{poison_val} for normal balloon operations and
> > +free page hinting as this feature did not exist prior to those features being
> > +added.
>
> So, for which operations is the device required to do something here?
> If it does not want to honour the field, it can simply elect to not
> offer the feature? Or is there a dependency between features?

So this is laying the groundwork for free page reporting. Currently
the driver and the device both have the option of not enabling the
page poison feature. The expectation is that in most cases the driver
will just disable it when there is no poison value to report, and the
way the current balloon drivers are implemented make it unnecessary
for existing features.

It is an optional feature that will become a requirement for new
features under certain circumstances. I still have patches outstanding
for it as the Linux kernel driver implemented it, but it never was
added to QEMU. What will happen in the future is that the guest will
disable free page reporting if it has to report a poison value and
cannot due to the poison feature not being negotiated.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [virtio-comment] Re: [PATCH RFC 3/3] content: Document balloon feature free page reporting
  2020-05-07 15:58   ` [virtio-comment] " Cornelia Huck
@ 2020-05-07 20:35     ` Alexander Duyck
  0 siblings, 0 replies; 10+ messages in thread
From: Alexander Duyck @ 2020-05-07 20:35 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: David Hildenbrand, Michael S. Tsirkin, virtio-dev, virtio-comment

On Thu, May 7, 2020 at 8:59 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Wed, 29 Apr 2020 11:57:57 -0700
> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>
> > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >
> > Free page reporting is a feature that allows the guest to proactively
> > report unused pages to the host. By making use of this feature is is
> > possible to reduce the overall memory footprint of the guest in cases where
> > some significant portion of the memory is idle. Add documentation for the
> > free page reporting feature describing the functionality and requirements.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > ---
> >  content.tex |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 74 insertions(+)
> >
> > diff --git a/content.tex b/content.tex
> > index c98b8ea9526a..52955c8ff007 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -5007,12 +5007,15 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
> >  \item[1] deflateq
> >  \item[2] statsq.
> >  \item[3] free_page_vq.
> > +\item[4] reporting_vq.
>
> Also lose the trailing period here.
>
> >  \end{description}
> >
> >    statsq only exists if VIRTIO_BALLOON_F_STATS_VQ set.
> >
> >    free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT set.
> >
> > +  reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING set.
>
> s/set/is set/
>
> > +
> >  \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Feature bits}
> >  \begin{description}
> >  \item[VIRTIO_BALLOON_F_MUST_TELL_HOST (0)] Host has to be told before
> > @@ -5029,6 +5032,8 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
> >  \item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] Host has to be notified if guest
> >      is expecting reported pages to contain a certain value when returned.
> >      Configuration field poison_val is valid.
> > +\item[ VIRTIO_BALLOON_F_PAGE_REPORTING(5) ] Device has support for free
> > +    page reporting. A virtqueue for reporting free guest memory is present.
> >
> >  \end{description}
> >
> > @@ -5039,6 +5044,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
> >  The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is not
> >  expecting any specific value to be stored in the page.
> >
> > +The driver MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING if it is expecting
> > +the pages to retain some initialized value and it has not negotiated
> > +VIRTIO_BALLOON_F_PAGE_POISON as a feature.
> > +
> >  \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
> > @@ -5097,6 +5106,13 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
> >
> >  \item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated then
> >    the driver MUST update the poison_val configuration field.
> > +
> > +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated:
> > +  \begin{enumerate}
> > +  \item Identify the reporting virtqueue.
> > +  \item DRIVER_OK is set: device operation begins.
> > +  \item Begin reporting free pages to device.
> > +  \end{enumerate}
> >  \end{enumerate}
>
> See my previous comments regarding reordering this.

I'll work on restructuring all this. With things being split up I can
drop some of the enumerated lists.

> >
> >  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> > @@ -5497,6 +5513,64 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
> >  free page hinting as this feature did not exist prior to those features being
> >  added.
> >
> > +\subsubsection{Free Page Reporting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> > +
> > +Free Page Reporting provides a mechanism similar to balloon inflation,
> > +however it does not provide a deflation queue. The expectation is that the
> > +device will have a means by which it can detect the guest page access and
> > +fault in such pages with some initial value, likely a zero page.
> > +
> > +The driver will respond to to memory conditions and begin reporting free
> > +pages when some number of pages are available.
> > +
> > +\begin{enumerate}
> > +
> > +\item The driver determines it has enough pages available to begin
> > +  reporting pages.
> > +
> > +\item The driver gathers pages into a scatter-gather list and adds them to
> > +  the reporting_vq.
> > +
> > +\item The device acknowledges the reporting request.
>
> How?

I will update this. Basically it is acknowledged by "using the the
reporting_vq descriptor".

> > +
> > +\item Once the device has acknowledged the report, the pages can be
> > +  returned to the location from which they were pulled.
> > +
> > +\item The driver can then continue to gather and report pages until it
> > +  has determined it has reported a suffcient quantity of pages.
> > +
> > +\end{enumerate}
> > +
> > +\drivernormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> > +
> > +Normative statements in this section apply if and only if  the
> > +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
>
> "If the VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated:"
>
> > +
> > +If the guest is expecting the free page to contain some initial value it
>
> s/guest/driver/
>
> > +MUST make use of the VIRTIO_BALLOON_F_PAGE_POISON feature to notify the
> > +device of this expectation via \field{poison_val}.
>
> "..., it MUST NOT negotiate this feature without negotiating the
> VIRTIO_BALLOON_F_PAGE_POISON feature as well and supply this value via
> \field{poison_val}."
>
> > +
> > +The driver MUST NOT use the reported pages until the device has
> > +acknowledged the reporting request.
> > +
> > +The driver MAY report free pages any time after DRIVER_OK is set.
> > +
> > +It is RECOMMENDED that the driver avoid unecessary reads or writes to the
> > +page contents as this could reduce the performance for free page reporting.
> > +
> > +\devicenormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> > +
> > +Normative statements in this section apply if and only if  the
> > +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
>
> "If the VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated:"
>
> > +
> > +The device MAY modify the contents of a page in the report after detecting
> > +its physical number in an report request and before acknowledging the
>
> What is a 'physical number'?

I assume here it is referring to the physical page number, aka PFN. I
had done some copy/paste from the balloon description since the start
would be the same for when the page should be inaccessible. I could
probably reword this to be from the time the device is notified of the
descriptor containing the page physical address and size.

> > +reporting request by using the reporting_vq descriptor.
>
> What is the 'reporting_vq descriptor'?

So which piece of this do I need to define? I believe I called out the
reporting_vq in the section above. The descriptor on the reporting_vq
is used to describe the scatterlist to the devices as multiple inbuf
on a single vring descriptor.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-07 20:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 18:57 [virtio-comment] [PATCH RFC 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 1/3] content: Document balloon feature free page hints Alexander Duyck
2020-05-07 15:33   ` [virtio-comment] " Cornelia Huck
2020-05-07 19:58     ` Alexander Duyck
2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 2/3] content: Document balloon feature page poison Alexander Duyck
2020-05-07 15:48   ` [virtio-comment] " Cornelia Huck
2020-05-07 20:16     ` Alexander Duyck
2020-04-29 18:57 ` [virtio-comment] [PATCH RFC 3/3] content: Document balloon feature free page reporting Alexander Duyck
2020-05-07 15:58   ` [virtio-comment] " Cornelia Huck
2020-05-07 20:35     ` Alexander Duyck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.