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

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

Changes since RFC:
Incorporated suggestions from Cornelia Huck
Fixed a few additional spelling errors

Changes since v1:
Incorporated additional suggestions from Cornelia Huck
Dropped documentation referring to free page reporting from page poison patch

Changes since v2:
Rewrote multiple statements based on input from David Hildenbrand
  Dropped use of balloon and deflate from page hinting description
  Dropped use of free page reporting from page poison description
  Cleaned up several spots that didn't match RFC2119 style comments
  Added conformance links.
  Various other clean-ups.
Updated balloon command IDs based on input from Cornelia Huck

[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


 conformance.tex |    6 +
 content.tex     |  264 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 264 insertions(+), 6 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] 16+ messages in thread

* [virtio-comment] [PATCH v3 1/3] content: Document balloon feature free page hints
  2020-05-20  2:02 [virtio-comment] [PATCH v3 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
@ 2020-05-20  2:02 ` Alexander Duyck
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison Alexander Duyck
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting Alexander Duyck
  2 siblings, 0 replies; 16+ messages in thread
From: Alexander Duyck @ 2020-05-20  2:02 UTC (permalink / raw)
  To: mst, david, cohuck; +Cc: virtio-comment, virtio-dev, wei.w.wang

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>
---
 conformance.tex |    2 +
 content.tex     |  140 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index b6fdec090383..a14e26edfcb2 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Feature bits}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
+\item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
 \end{itemize}
 
 \conformance{\subsection}{SCSI Host Driver Conformance}\label{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}
@@ -331,6 +332,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Feature bits}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
+\item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
 \end{itemize}
 
 \conformance{\subsection}{SCSI Host Device Conformance}\label{sec:Conformance / Device Conformance / SCSI Host Device Conformance}
diff --git a/content.tex b/content.tex
index 91735e3eb018..816b6c1b052e 100644
--- a/content.tex
+++ b/content.tex
@@ -5005,10 +5005,13 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
 \begin{description}
 \item[0] inflateq
 \item[1] deflateq
-\item[2] statsq.
+\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 is set.
+
+  free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT 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) ] The device has support for free
+    page hinting. A virtqueue for providing hints as to what memory is
+    currently free is present. Configuration field \field{free_page_hint_cmd_id}
+    is valid.
 
 \end{description}
 
@@ -5042,13 +5049,17 @@ \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.
+  \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}
 
@@ -5072,9 +5083,15 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
   \begin{enumerate}
   \item Identify the stats virtqueue.
   \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}
+
+\item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is negotiated, the
+  free_page_vq is identified.
+
+\item DRIVER_OK is set: device operation begins.
+
+\item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated, then
+  notify the device about the stats virtqueue buffer.
 \end{enumerate}
 
 \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
@@ -5345,6 +5362,117 @@ \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 designed to be used during migration to determine what
+pages within the guest are currently unused so that they can be skipped over
+while migrating the guest. 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. The following values
+are reserved:
+
+\begin{description}
+\item[VIRTIO_BALLOON_CMD_ID_STOP (0)] Any command ID previously supplied by
+  the device is invalid. The driver should stop hinting free pages until a
+  new command ID is supplied, but should not release any hinted pages for
+  use by the guest.
+
+\item[VIRTIO_BALLOON_CMD_ID_DONE (1)] Any command ID previously supplied by
+  the device is invalid. The driver should stop hinting free pages, and 
+  should release all hinted pages for use by the guest.
+\end{description}
+
+A request for free page hinting proceeds as follows:
+
+\begin{enumerate}
+
+\item The driver examines the \field{free_page_hint_cmd_id} configuration field.
+  If it contains a non-reserved value then free page hinting will begin.
+
+\item To supply free page hints:
+  \begin{enumerate}
+  \item The driver constructs an output descriptor containing the new value
+    from the \field{free_page_hint_cmd_id} configuration field and adds it to
+    the free_page_hint_vq.
+  \item The driver maps a series of pages and adds them to the
+    free_page_hint_vq as individual scatter-gather input descriptor 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
+  supply more pages for hinting as described above, or when the device
+  updates \field{free_page_hint_cmd_id} configuration field to 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 the \field{free_page_hint_cmd_id} configuration
+  field in which case it will resume supplying free page hints.
+
+\item Otherwise, if the device provides VIRTIO_BALLOON_CMD_ID_DONE then
+  hinting is complete and the driver may release all previously hinted
+  pages for use by the guest.
+
+\end{enumerate}
+
+\drivernormative{\paragraph}{Free Page Hinting}{Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
+
+Normative statements in this section apply if the
+VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.
+
+The driver SHOULD supply pages to the free page hints 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 for hinting 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 release hinted pages for use by the guest including when the
+device has not yet used the descriptor containing the hinting request.
+
+The driver MUST treat the content of all hinted pages as uninitialized
+memory.
+
+The driver MUST initialize the contents of any previously hinted page
+released before \field{free_page_hint_cmd_id} reports a value of
+VIRTIO_BALLOON_CMD_ID_DONE.
+
+The driver SHOULD release all previously hinted pages for use by the guest
+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 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 dirty pages for the given
+guest are being recorded.
+
+The device MUST NOT reuse a command ID until it has received an output
+descriptor containing VIRTIO_BALLOON_CMD_ID_STOP from the driver.
+
+The device MUST ignore pages that are provided with a command ID that does
+not match the current value in \field{free_page_hint_cmd_id}.
+
+If the content of a previously hinted page has not been modified by the
+guest since the device issued the \field{free_page_hint_cmd_id} associated
+with the hint, the device MAY modify the contents of the page.
+
+The device MUST NOT modify the content of a previously hinted page after
+\field{free_page_hint_cmd_id} is set to VIRTIO_BALLOON_CMD_ID_DONE.
+
+The device SHOULD report a value of VIRTIO_BALLOON_CMD_ID_DONE in
+\field{free_page_hint_cmd_id} once it no longer has need for the
+previously hinted pages.
+
 \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] 16+ messages in thread

* [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-20  2:02 [virtio-comment] [PATCH v3 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 1/3] content: Document balloon feature free page hints Alexander Duyck
@ 2020-05-20  2:02 ` Alexander Duyck
  2020-05-20  9:27   ` David Hildenbrand
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting Alexander Duyck
  2 siblings, 1 reply; 16+ messages in thread
From: Alexander Duyck @ 2020-05-20  2:02 UTC (permalink / raw)
  To: mst, david, cohuck; +Cc: virtio-comment, virtio-dev, wei.w.wang

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>
---
 conformance.tex |    2 ++
 content.tex     |   44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index a14e26edfcb2..5038b36324ac 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -150,6 +150,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
+\item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
 \end{itemize}
 
 \conformance{\subsection}{SCSI Host Driver Conformance}\label{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}
@@ -333,6 +334,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
+\item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
 \end{itemize}
 
 \conformance{\subsection}{SCSI Host Device Conformance}\label{sec:Conformance / Device Conformance / SCSI Host Device Conformance}
diff --git a/content.tex b/content.tex
index 816b6c1b052e..89e9948b7399 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 \field{free_page_hint_cmd_id}
     is valid.
+\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] The device has to be notified if
+    the driver is expecting balloon 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
@@ -5055,11 +5061,15 @@ \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon
     VIRTIO_BALLOON_F_FREE_PAGE_HINT has been negotiated and is read-only by
     the driver.
 
+  \field{poison_val} is available if VIRTIO_BALLOON_F_PAGE_POISON has been
+    negotiated.
+
 \begin{lstlisting}
 struct virtio_balloon_config {
         le32 num_pages;
         le32 actual;
         le32 free_page_hint_cmd_id;
+        le32 poison_val;
 };
 \end{lstlisting}
 
@@ -5088,6 +5098,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, the
+  driver updates the \field{poison_val} configuration field.
+
 \item DRIVER_OK is set: device operation begins.
 
 \item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated, then
@@ -5473,6 +5486,37 @@ \subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device
 \field{free_page_hint_cmd_id} once it no longer has need for the
 previously hinted pages.
 
+\subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Device Operation / Page Poison}
+
+Page Poison provides a way to notify the host that the guest is initializing
+and/or poisoning free pages. When the feature is enabled pages that are
+deflated might be immediately written to by the guest.
+
+If the guest is not initializing or poisoning freed pages it should reject
+the VIRTIO_BALLOON_F_PAGE_POISON feature.
+
+If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest
+will place the expected poison value into the \field{poison_val}
+configuration field data.
+
+\drivernormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
+
+Normative statements in this section apply if the
+VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
+
+The driver MUST populate the \field{poison_val} configuration data before
+setting the DRIVER_OK bit.
+
+The driver MUST NOT modify \field{poison_val} while the DRIVER_OK bit is set.
+
+\devicenormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
+
+Normative statements in this section apply if the
+VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
+
+The device MAY use the content of \field{poison_val} as a hint to guest
+behavior.
+
 \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] 16+ messages in thread

* [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting
  2020-05-20  2:02 [virtio-comment] [PATCH v3 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 1/3] content: Document balloon feature free page hints Alexander Duyck
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison Alexander Duyck
@ 2020-05-20  2:02 ` Alexander Duyck
  2020-05-26  9:05   ` David Hildenbrand
  2 siblings, 1 reply; 16+ messages in thread
From: Alexander Duyck @ 2020-05-20  2:02 UTC (permalink / raw)
  To: mst, david, cohuck; +Cc: virtio-comment, virtio-dev, wei.w.wang

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>
---
 conformance.tex |    2 +
 content.tex     |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/conformance.tex b/conformance.tex
index 5038b36324ac..5496a25e93ef 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -151,6 +151,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
 \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
+\item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
 \end{itemize}
 
 \conformance{\subsection}{SCSI Host Driver Conformance}\label{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}
@@ -335,6 +336,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
 \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
+\item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
 \end{itemize}
 
 \conformance{\subsection}{SCSI Host Device Conformance}\label{sec:Conformance / Device Conformance / SCSI Host Device Conformance}
diff --git a/content.tex b/content.tex
index 89e9948b7399..acdbcfc81538 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 is set.
 
   free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT is set.
 
+  reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING 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) ] The device has to be notified if
     the driver is expecting balloon pages to contain a certain value when
     returned. Configuration field poison_val is valid.
+\item[ VIRTIO_BALLOON_F_PAGE_REPORTING(5) ] The 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.
 
+If the driver is expecting the pages to retain some initialized value,
+it MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING unless it also
+negotiates VIRTIO_BALLOON_F_PAGE_POISON.
+
 \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
@@ -5101,10 +5110,16 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
 \item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated, the
   driver updates the \field{poison_val} configuration field.
 
+\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated the
+  reporting_vq is identified.
+
 \item DRIVER_OK is set: device operation begins.
 
 \item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated, then
   notify the device about the stats virtqueue buffer.
+
+\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated then
+  begin reporting free pages to device.
 \end{enumerate}
 
 \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
@@ -5490,7 +5505,8 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
 
 Page Poison provides a way to notify the host that the guest is initializing
 and/or poisoning free pages. When the feature is enabled pages that are
-deflated might be immediately written to by the guest.
+deflated might be immediately written to by the guest, and pages reported by
+free page reporting will contain the value indicated by \field{poison_val}.
 
 If the guest is not initializing or poisoning freed pages it should reject
 the VIRTIO_BALLOON_F_PAGE_POISON feature.
@@ -5517,6 +5533,70 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
 The device MAY use the content of \field{poison_val} as a hint to guest
 behavior.
 
+\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 begin reporting free pages. When exactly and which free
+pages are reported is up to the driver.
+
+\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 by using 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 sufficient 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 the
+VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
+
+If the VIRTIO_BALLOON_F_PAGE_POISON feature has not been negotiated, then
+the driver MUST treat all reported pages as uninitialized memory.
+
+If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
+MUST guarantee that the page has been filled with no value other than
+\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.
+
+The driver SHOULD attempt to report large pages rather than smaller ones.
+
+The driver SHOULD avoid unnecessary reads or writes to the reported page
+contents.
+
+\devicenormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
+
+Normative statements in this section apply if the
+VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
+
+The device MAY modify the contents of any page supplied in a report
+request before acknowledging that request by using the reporting_vq
+descriptor.
+
+If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the device
+MUST NOT modify the the content of a reported page to 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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison Alexander Duyck
@ 2020-05-20  9:27   ` David Hildenbrand
  2020-05-20 16:25     ` Alexander Duyck
  0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2020-05-20  9:27 UTC (permalink / raw)
  To: Alexander Duyck, mst, cohuck; +Cc: virtio-comment, virtio-dev, wei.w.wang

On 20.05.20 04:02, Alexander Duyck 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.
> 

I would rephrase this, starting what it does *without* free page
reporting (which is not "provides a way for the guest to notify ..."),
and then eventually how this feature will also be used in the future as
well with free page reporting.

> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  conformance.tex |    2 ++
>  content.tex     |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/conformance.tex b/conformance.tex
> index a14e26edfcb2..5038b36324ac 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -150,6 +150,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation}
>  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
>  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> +\item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
>  \end{itemize}
>  
>  \conformance{\subsection}{SCSI Host Driver Conformance}\label{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}
> @@ -333,6 +334,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation}
>  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
>  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> +\item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
>  \end{itemize}
>  
>  \conformance{\subsection}{SCSI Host Device Conformance}\label{sec:Conformance / Device Conformance / SCSI Host Device Conformance}
> diff --git a/content.tex b/content.tex
> index 816b6c1b052e..89e9948b7399 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 \field{free_page_hint_cmd_id}
>      is valid.
> +\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] The device has to be notified if
> +    the driver is expecting balloon pages to contain a certain value when
> +    returned. Configuration field poison_val is valid.

That's not what it does in the context of this feature only, no?

"A hint to the device, that the driver might immediately write
\field{poison_val} to pages after deflating them. Configuration field
\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.

That's not what it does in the context of this feature only, no?

"The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is
not immediately write \field{poison_val} to deflated pages (e.g., to
initialize them, or fill them with a poison value)." ?

> +
>  \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
> @@ -5055,11 +5061,15 @@ \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon
>      VIRTIO_BALLOON_F_FREE_PAGE_HINT has been negotiated and is read-only by
>      the driver.
>  
> +  \field{poison_val} is available if VIRTIO_BALLOON_F_PAGE_POISON has been
> +    negotiated.
> +
>  \begin{lstlisting}
>  struct virtio_balloon_config {
>          le32 num_pages;
>          le32 actual;
>          le32 free_page_hint_cmd_id;
> +        le32 poison_val;
>  };
>  \end{lstlisting}
>  
> @@ -5088,6 +5098,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, the
> +  driver updates the \field{poison_val} configuration field.
> +
>  \item DRIVER_OK is set: device operation begins.
>  
>  \item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated, then
> @@ -5473,6 +5486,37 @@ \subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device
>  \field{free_page_hint_cmd_id} once it no longer has need for the
>  previously hinted pages.
>  
> +\subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +
> +Page Poison provides a way to notify the host that the guest is initializing
> +and/or poisoning free pages. When the feature is enabled pages that are

s/enabled/enabled, /

> +deflated might be immediately written to by the guest.

"pages might immediately get written to by the driver after deflating." ?

> +
> +If the guest is not initializing or poisoning freed pages it should reject

Sometimes you use "write to pages after deflating", here you use "freed
pages"

> +the VIRTIO_BALLOON_F_PAGE_POISON feature.
> +
> +If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest
> +will place the expected poison value into the \field{poison_val}

again, "expected" is misleading in the context of this patch only.

> +configuration field data.
> +
> +\drivernormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +
> +Normative statements in this section apply if the
> +VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
> +
> +The driver MUST populate the \field{poison_val} configuration data before
> +setting the DRIVER_OK bit.
> +
> +The driver MUST NOT modify \field{poison_val} while the DRIVER_OK bit is set.
> +
> +\devicenormative{\paragraph}{Page Poison}{Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +
> +Normative statements in this section apply if the
> +VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated.
> +
> +The device MAY use the content of \field{poison_val} as a hint to guest
> +behavior.


-- 
Thanks,

David / dhildenb


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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-20  9:27   ` David Hildenbrand
@ 2020-05-20 16:25     ` Alexander Duyck
  2020-05-26  8:24       ` David Hildenbrand
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Duyck @ 2020-05-20 16:25 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Michael S. Tsirkin, Cornelia Huck, virtio-comment, virtio-dev,
	Wang, Wei W

On Wed, May 20, 2020 at 2:28 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 20.05.20 04:02, Alexander Duyck 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.
> >
>
> I would rephrase this, starting what it does *without* free page
> reporting (which is not "provides a way for the guest to notify ..."),
> and then eventually how this feature will also be used in the future as
> well with free page reporting.

Below is a rewrite on this description. I'm thinking that we can
probably call out the advantage to free page reporting in a different
way. Basically with the page poison feature we know a few things about
the behavior and I have called them out in the new patch description:

Page poison provides a way for the guest to notify the host that it is
initializing or poisoning freed pages with some specific poison value. As a
result of this we can infer a couple traits about the guest:

1. Free pages will contain a specific pattern within the guest.
2. Modifying free pages from this value may cause an error in the guest.
3. Pages will be immediately written to by the driver when deflated.

There are currently no existing features that make use of this data. In the
upcoming feature free page reporting we will need to make use of this to
identify if we can evict pages from the guest without causing data
corruption.

Add documentation for the page poison feature describing the basic
functionality and requirements.

[...]

> > diff --git a/content.tex b/content.tex
> > index 816b6c1b052e..89e9948b7399 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 \field{free_page_hint_cmd_id}
> >      is valid.
> > +\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] The device has to be notified if
> > +    the driver is expecting balloon pages to contain a certain value when
> > +    returned. Configuration field poison_val is valid.
>
> That's not what it does in the context of this feature only, no?
>
> "A hint to the device, that the driver might immediately write
> \field{poison_val} to pages after deflating them. Configuration field
> \field{poison_val} is valid."

I'll probably just use this wording with a few slight tweaks. Thinking
about it though I will get rid of "might" and replace it with "will".
In the case of the page poison feature we expect the driver must be
poisoning the pages, it shouldn't be an optional thing that "might"
happen.

> >
> >  \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.
>
> That's not what it does in the context of this feature only, no?
>
> "The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is
> not immediately write \field{poison_val} to deflated pages (e.g., to
> initialize them, or fill them with a poison value)." ?

Again I will probably reuse this but with a slight tweak to "if it
will not immediately write" instead of using the "is".

[...]

> > @@ -5473,6 +5486,37 @@ \subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon Device
> >  \field{free_page_hint_cmd_id} once it no longer has need for the
> >  previously hinted pages.
> >
> > +\subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> > +
> > +Page Poison provides a way to notify the host that the guest is initializing
> > +and/or poisoning free pages. When the feature is enabled pages that are
>
> s/enabled/enabled, /
>
> > +deflated might be immediately written to by the guest.
>
> "pages might immediately get written to by the driver after deflating." ?

Again I will probably use this wording, but replace "might" with "will".

> > +
> > +If the guest is not initializing or poisoning freed pages it should reject
>
> Sometimes you use "write to pages after deflating", here you use "freed
> pages"

So when I am referencing "freed pages" I am talking about all free
memory, while when I refer to "pages after deflating" I am talking
about pages coming out of the balloon.

My thought is that there maybe be additional uses for "poison_val" be
to feed it into some future use other than just the balloon portion of
the deflation. Basically what this is telling us is that we could look
for a pattern of pages containing nothing but poison_val if we wanted
to do some sort of same page merging, or maybe define something to
optimize migration by defining a poison page similar to a zero page
that could be used to reduce migration overhead in the future.

> > +the VIRTIO_BALLOON_F_PAGE_POISON feature.
> > +
> > +If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest
> > +will place the expected poison value into the \field{poison_val}
>
> again, "expected" is misleading in the context of this patch only.

I will rewrite this statement at follows:
  If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
  will place the initialization and/or poison value into the \field{poison_val}
  configuration field data.

I think I might strengthen things a bit as well. In the driver
normative section I think I might add the following:
  The driver MUST initialize and/or poison the deflated pages with
  \field{poison_val} when they are reused by the driver.

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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-20 16:25     ` Alexander Duyck
@ 2020-05-26  8:24       ` David Hildenbrand
  2020-05-26 14:50         ` Alexander Duyck
  0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2020-05-26  8:24 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Michael S. Tsirkin, Cornelia Huck, virtio-comment, virtio-dev,
	Wang, Wei W

On 20.05.20 18:25, Alexander Duyck wrote:
> On Wed, May 20, 2020 at 2:28 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 20.05.20 04:02, Alexander Duyck 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.
>>>
>>
>> I would rephrase this, starting what it does *without* free page
>> reporting (which is not "provides a way for the guest to notify ..."),
>> and then eventually how this feature will also be used in the future as
>> well with free page reporting.
> 
> Below is a rewrite on this description. I'm thinking that we can
> probably call out the advantage to free page reporting in a different
> way. Basically with the page poison feature we know a few things about
> the behavior and I have called them out in the new patch description:
> 
> Page poison provides a way for the guest to notify the host that it is
> initializing or poisoning freed pages with some specific poison value. As a
> result of this we can infer a couple traits about the guest:
> 
> 1. Free pages will contain a specific pattern within the guest.
> 2. Modifying free pages from this value may cause an error in the guest.
> 3. Pages will be immediately written to by the driver when deflated.
> 
> There are currently no existing features that make use of this data. In the
> upcoming feature free page reporting we will need to make use of this to
> identify if we can evict pages from the guest without causing data
> corruption.
> 
> Add documentation for the page poison feature describing the basic
> functionality and requirements.
> 
> [...]
> 
>>> diff --git a/content.tex b/content.tex
>>> index 816b6c1b052e..89e9948b7399 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 \field{free_page_hint_cmd_id}
>>>      is valid.
>>> +\item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] The device has to be notified if
>>> +    the driver is expecting balloon pages to contain a certain value when
>>> +    returned. Configuration field poison_val is valid.
>>
>> That's not what it does in the context of this feature only, no?
>>
>> "A hint to the device, that the driver might immediately write
>> \field{poison_val} to pages after deflating them. Configuration field
>> \field{poison_val} is valid."
> 
> I'll probably just use this wording with a few slight tweaks. Thinking
> about it though I will get rid of "might" and replace it with "will".

I think that's always guaranteed by Linux as of now, so "will" makes sense.

[...]

>>> +
>>> +If the guest is not initializing or poisoning freed pages it should reject
>>
>> Sometimes you use "write to pages after deflating", here you use "freed
>> pages"
> 
> So when I am referencing "freed pages" I am talking about all free
> memory, while when I refer to "pages after deflating" I am talking
> about pages coming out of the balloon.
> 
> My thought is that there maybe be additional uses for "poison_val" be
> to feed it into some future use other than just the balloon portion of
> the deflation. Basically what this is telling us is that we could look
> for a pattern of pages containing nothing but poison_val if we wanted
> to do some sort of same page merging, or maybe define something to
> optimize migration by defining a poison page similar to a zero page
> that could be used to reduce migration overhead in the future.
> 
>>> +the VIRTIO_BALLOON_F_PAGE_POISON feature.
>>> +
>>> +If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest
>>> +will place the expected poison value into the \field{poison_val}
>>
>> again, "expected" is misleading in the context of this patch only.
> 
> I will rewrite this statement at follows:
>   If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
>   will place the initialization and/or poison value into the \field{poison_val}
>   configuration field data.
> 
> I think I might strengthen things a bit as well. In the driver
> normative section I think I might add the following:
>   The driver MUST initialize and/or poison the deflated pages with
>   \field{poison_val} when they are reused by the driver.
> 

Maybe simplify that whole "initialize and/or poison " handling across
this patch to "initialize with \field{poison_val}" - if the
initialization is used for poisoning or initialization doesn't matter
from spec POV.

In general looks good to me, I'll have another look at the full result.

Still wondering what to do with free page hinting ... in the meantime
I'll have a look at free page reporting :)

-- 
Thanks,

David / dhildenb


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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting
  2020-05-20  2:02 ` [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting Alexander Duyck
@ 2020-05-26  9:05   ` David Hildenbrand
  2020-05-26 14:42     ` Alexander Duyck
  0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2020-05-26  9:05 UTC (permalink / raw)
  To: Alexander Duyck, mst, cohuck; +Cc: virtio-comment, virtio-dev, wei.w.wang

On 20.05.20 04:02, Alexander Duyck 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>
> ---
>  conformance.tex |    2 +
>  content.tex     |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index 5038b36324ac..5496a25e93ef 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -151,6 +151,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
>  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
>  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +\item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
>  \end{itemize}
>  
>  \conformance{\subsection}{SCSI Host Driver Conformance}\label{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}
> @@ -335,6 +336,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
>  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
>  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> +\item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
>  \end{itemize}
>  
>  \conformance{\subsection}{SCSI Host Device Conformance}\label{sec:Conformance / Device Conformance / SCSI Host Device Conformance}
> diff --git a/content.tex b/content.tex
> index 89e9948b7399..acdbcfc81538 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 is set.
>  
>    free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT is set.
>  
> +  reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING 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) ] The device has to be notified if
>      the driver is expecting balloon pages to contain a certain value when
>      returned. Configuration field poison_val is valid.
> +\item[ VIRTIO_BALLOON_F_PAGE_REPORTING(5) ] The 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.
>  
> +If the driver is expecting the pages to retain some initialized value,

"some" -> the value communicated via poison_val?

> +it MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING unless it also
> +negotiates VIRTIO_BALLOON_F_PAGE_POISON.
> +
>  \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
> @@ -5101,10 +5110,16 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
>  \item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated, the
>    driver updates the \field{poison_val} configuration field.
>  
> +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated the
> +  reporting_vq is identified.
> +
>  \item DRIVER_OK is set: device operation begins.
>  
>  \item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated, then
>    notify the device about the stats virtqueue buffer.
> +
> +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated then
> +  begin reporting free pages to device.

s/to device/to the device/

>  \end{enumerate}
>  
>  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> @@ -5490,7 +5505,8 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
>  
>  Page Poison provides a way to notify the host that the guest is initializing
>  and/or poisoning free pages. When the feature is enabled pages that are
> -deflated might be immediately written to by the guest.
> +deflated might be immediately written to by the guest, and pages reported by
> +free page reporting will contain the value indicated by \field{poison_val}.

maybe mention that this value will be retained by the device.

>  
>  If the guest is not initializing or poisoning freed pages it should reject
>  the VIRTIO_BALLOON_F_PAGE_POISON feature.
> @@ -5517,6 +5533,70 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
>  The device MAY use the content of \field{poison_val} as a hint to guest
>  behavior.
>  
> +\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.

Too many unnecessary details IMHO. I'd simplify to

"Free Page Reporting provides a mechanism similar to balloon inflation,
however, it does not provide a deflation queue. Reported free pages can
be reused by the guest once the reporting requested without notifying
the device."

But maybe s/guest/driver/

> +
> +The driver will begin reporting free pages. When exactly and which free
> +pages are reported is up to the driver.
> +
> +\begin{enumerate}
> +
> +\item The driver determines it has enough pages available to begin
> +  reporting pages.

"free pages", "reporting free pages" ?

> +
> +\item The driver gathers pages into a scatter-gather list and adds them to
> +  the reporting_vq.

"free pages" ?

> +
> +\item The device acknowledges the reporting request by using 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.

I'd suggest something like

"Once the device has acknowledged the report, the driver can reuse the
free pages when needed (e.g., by putting them back to free page lists in
the guest operating system)."

> +
> +\item The driver can then continue to gather and report pages until it
> +  has determined it has reported a sufficient quantity of pages.

"free 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 the
> +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
> +
> +If the VIRTIO_BALLOON_F_PAGE_POISON feature has not been negotiated, then
> +the driver MUST treat all reported pages as uninitialized memory.
> +
> +If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
> +MUST guarantee that the page has been filled with no value other than
> +\field{poison_val}.

"the page" is unclear

"If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the
driver MUST initialize all free pages with \field{poison_val} before
reporting them." ?

Again, the "MUST guarantee" part is irrelevant from spec POV, because
for us, driver == guest.

> +
> +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.
> +
> +The driver SHOULD attempt to report large pages rather than smaller ones.
> +
> +The driver SHOULD avoid unnecessary reads or writes to the reported page
> +contents.

maybe simply "The driver SHOULD avoid reading/writing reported pages if
not strictly necessary."

> +
> +\devicenormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> +
> +Normative statements in this section apply if the
> +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
> +
> +The device MAY modify the contents of any page supplied in a report
> +request before acknowledging that request by using the reporting_vq
> +descriptor.

Maybe start that with

"If the VIRTIO_BALLOON_F_PAGE_POISON feature has not been negotiated,
the device ..."

> +
> +If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the device
> +MUST NOT modify the the content of a reported page to a value other than
> +\field{poison_val}.
> +
>  \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}

That part sounds good to me.


-- 
Thanks,

David / dhildenb


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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting
  2020-05-26  9:05   ` David Hildenbrand
@ 2020-05-26 14:42     ` Alexander Duyck
  0 siblings, 0 replies; 16+ messages in thread
From: Alexander Duyck @ 2020-05-26 14:42 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Michael S. Tsirkin, Cornelia Huck, virtio-comment, virtio-dev,
	Wang, Wei W

On Tue, May 26, 2020 at 2:06 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 20.05.20 04:02, Alexander Duyck 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>
> > ---
> >  conformance.tex |    2 +
> >  content.tex     |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 83 insertions(+), 1 deletion(-)
> >
> > diff --git a/conformance.tex b/conformance.tex
> > index 5038b36324ac..5496a25e93ef 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -151,6 +151,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
> >  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> >  \item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> > +\item \ref{drivernormative:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> >  \end{itemize}
> >
> >  \conformance{\subsection}{SCSI Host Driver Conformance}\label{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance}
> > @@ -335,6 +336,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Memory Statistics}
> >  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Hinting}
> >  \item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Page Poison}
> > +\item \ref{devicenormative:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> >  \end{itemize}
> >
> >  \conformance{\subsection}{SCSI Host Device Conformance}\label{sec:Conformance / Device Conformance / SCSI Host Device Conformance}
> > diff --git a/content.tex b/content.tex
> > index 89e9948b7399..acdbcfc81538 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 is set.
> >
> >    free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT is set.
> >
> > +  reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING 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) ] The device has to be notified if
> >      the driver is expecting balloon pages to contain a certain value when
> >      returned. Configuration field poison_val is valid.
> > +\item[ VIRTIO_BALLOON_F_PAGE_REPORTING(5) ] The 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.
> >
> > +If the driver is expecting the pages to retain some initialized value,
>
> "some" -> the value communicated via poison_val?

So I left this somewhat vague on purpose. What this is referring to is
that if the driver/guest is expecting any given pattern to be retained
by the driver and the VIRTIO_BALLOON_F_PAGE_POISON value is not set
then we must not accept the reporting feature. As such in this case
there is no value in poison_val as VIRTIO_BALLOON_F_PAGE_POISON is not
negotiated.

> > +it MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING unless it also
> > +negotiates VIRTIO_BALLOON_F_PAGE_POISON.
> > +
> >  \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
> > @@ -5101,10 +5110,16 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
> >  \item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated, the
> >    driver updates the \field{poison_val} configuration field.
> >
> > +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated the
> > +  reporting_vq is identified.
> > +
> >  \item DRIVER_OK is set: device operation begins.
> >
> >  \item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated, then
> >    notify the device about the stats virtqueue buffer.
> > +
> > +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated then
> > +  begin reporting free pages to device.
>
> s/to device/to the device/

I will fix it.

> >  \end{enumerate}
> >
> >  \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
> > @@ -5490,7 +5505,8 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
> >
> >  Page Poison provides a way to notify the host that the guest is initializing
> >  and/or poisoning free pages. When the feature is enabled pages that are
> > -deflated might be immediately written to by the guest.
> > +deflated might be immediately written to by the guest, and pages reported by
> > +free page reporting will contain the value indicated by \field{poison_val}.
>
> maybe mention that this value will be retained by the device.

I could just replace "contain" with "retain" if that works for you.

> >
> >  If the guest is not initializing or poisoning freed pages it should reject
> >  the VIRTIO_BALLOON_F_PAGE_POISON feature.
> > @@ -5517,6 +5533,70 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev
> >  The device MAY use the content of \field{poison_val} as a hint to guest
> >  behavior.
> >
> > +\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.
>
> Too many unnecessary details IMHO. I'd simplify to
>
> "Free Page Reporting provides a mechanism similar to balloon inflation,
> however, it does not provide a deflation queue. Reported free pages can
> be reused by the guest once the reporting requested without notifying
> the device."
>
> But maybe s/guest/driver/

I can work with that wording.

> > +
> > +The driver will begin reporting free pages. When exactly and which free
> > +pages are reported is up to the driver.
> > +
> > +\begin{enumerate}
> > +
> > +\item The driver determines it has enough pages available to begin
> > +  reporting pages.
>
> "free pages", "reporting free pages" ?

Will update.

> > +
> > +\item The driver gathers pages into a scatter-gather list and adds them to
> > +  the reporting_vq.
>
> "free pages" ?

Will fix.

> > +
> > +\item The device acknowledges the reporting request by using 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.
>
> I'd suggest something like
>
> "Once the device has acknowledged the report, the driver can reuse the
> free pages when needed (e.g., by putting them back to free page lists in
> the guest operating system)."

Works for me.

> > +
> > +\item The driver can then continue to gather and report pages until it
> > +  has determined it has reported a sufficient quantity of pages.
>
> "free pages"

I'll update that.

> > +
> > +\end{enumerate}
> > +
> > +\drivernormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> > +
> > +Normative statements in this section apply if the
> > +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
> > +
> > +If the VIRTIO_BALLOON_F_PAGE_POISON feature has not been negotiated, then
> > +the driver MUST treat all reported pages as uninitialized memory.
> > +
> > +If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
> > +MUST guarantee that the page has been filled with no value other than
> > +\field{poison_val}.
>
> "the page" is unclear
>
> "If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the
> driver MUST initialize all free pages with \field{poison_val} before
> reporting them." ?
>
> Again, the "MUST guarantee" part is irrelevant from spec POV, because
> for us, driver == guest.

Okay, I will use your wording.

> > +
> > +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.
> > +
> > +The driver SHOULD attempt to report large pages rather than smaller ones.
> > +
> > +The driver SHOULD avoid unnecessary reads or writes to the reported page
> > +contents.
>
> maybe simply "The driver SHOULD avoid reading/writing reported pages if
> not strictly necessary."

Okay, I will update.

> > +
> > +\devicenormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> > +
> > +Normative statements in this section apply if the
> > +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated.
> > +
> > +The device MAY modify the contents of any page supplied in a report
> > +request before acknowledging that request by using the reporting_vq
> > +descriptor.
>
> Maybe start that with
>
> "If the VIRTIO_BALLOON_F_PAGE_POISON feature has not been negotiated,
> the device ..."

Done.

> > +
> > +If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the device
> > +MUST NOT modify the the content of a reported page to a value other than
> > +\field{poison_val}.
> > +
> >  \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
>
> That part sounds good to me.

Thanks for the review.

- Alex

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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-26  8:24       ` David Hildenbrand
@ 2020-05-26 14:50         ` Alexander Duyck
  2020-05-26 15:28           ` David Hildenbrand
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Duyck @ 2020-05-26 14:50 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Michael S. Tsirkin, Cornelia Huck, virtio-comment, virtio-dev,
	Wang, Wei W

On Tue, May 26, 2020 at 1:24 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 20.05.20 18:25, Alexander Duyck wrote:
> > On Wed, May 20, 2020 at 2:28 AM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 20.05.20 04:02, Alexander Duyck 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.
> >>>
> >>
> >> I would rephrase this, starting what it does *without* free page
> >> reporting (which is not "provides a way for the guest to notify ..."),
> >> and then eventually how this feature will also be used in the future as
> >> well with free page reporting.
> >
> > Below is a rewrite on this description. I'm thinking that we can
> > probably call out the advantage to free page reporting in a different
> > way. Basically with the page poison feature we know a few things about
> > the behavior and I have called them out in the new patch description:
> >
> > Page poison provides a way for the guest to notify the host that it is
> > initializing or poisoning freed pages with some specific poison value. As a
> > result of this we can infer a couple traits about the guest:
> >
> > 1. Free pages will contain a specific pattern within the guest.
> > 2. Modifying free pages from this value may cause an error in the guest.
> > 3. Pages will be immediately written to by the driver when deflated.
> >
> > There are currently no existing features that make use of this data. In the
> > upcoming feature free page reporting we will need to make use of this to
> > identify if we can evict pages from the guest without causing data
> > corruption.
> >
> > Add documentation for the page poison feature describing the basic
> > functionality and requirements.
> >

[...]

> >>> +
> >>> +If the guest is not initializing or poisoning freed pages it should reject
> >>
> >> Sometimes you use "write to pages after deflating", here you use "freed
> >> pages"
> >
> > So when I am referencing "freed pages" I am talking about all free
> > memory, while when I refer to "pages after deflating" I am talking
> > about pages coming out of the balloon.
> >
> > My thought is that there maybe be additional uses for "poison_val" be
> > to feed it into some future use other than just the balloon portion of
> > the deflation. Basically what this is telling us is that we could look
> > for a pattern of pages containing nothing but poison_val if we wanted
> > to do some sort of same page merging, or maybe define something to
> > optimize migration by defining a poison page similar to a zero page
> > that could be used to reduce migration overhead in the future.
> >
> >>> +the VIRTIO_BALLOON_F_PAGE_POISON feature.
> >>> +
> >>> +If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest
> >>> +will place the expected poison value into the \field{poison_val}
> >>
> >> again, "expected" is misleading in the context of this patch only.
> >
> > I will rewrite this statement at follows:
> >   If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
> >   will place the initialization and/or poison value into the \field{poison_val}
> >   configuration field data.
> >
> > I think I might strengthen things a bit as well. In the driver
> > normative section I think I might add the following:
> >   The driver MUST initialize and/or poison the deflated pages with
> >   \field{poison_val} when they are reused by the driver.
> >
>
> Maybe simplify that whole "initialize and/or poison " handling across
> this patch to "initialize with \field{poison_val}" - if the
> initialization is used for poisoning or initialization doesn't matter
> from spec POV.
>
> In general looks good to me, I'll have another look at the full result.

Okay, I will go through and try to flush out the "and/or poison" in
favor of just calling out the initialization.

> Still wondering what to do with free page hinting ... in the meantime
> I'll have a look at free page reporting :)

The problem is it is already out there so I worry we wouldn't ever be
able to get rid of it. At most we could deprecate it, but we are still
stuck with it consuming bit resources and such.

Anyway I'll try to get another iteration of these patches out later today.

Thanks.

- Alex

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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-26 14:50         ` Alexander Duyck
@ 2020-05-26 15:28           ` David Hildenbrand
  2020-05-26 15:38             ` Cornelia Huck
  0 siblings, 1 reply; 16+ messages in thread
From: David Hildenbrand @ 2020-05-26 15:28 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Michael S. Tsirkin, Cornelia Huck, virtio-comment, virtio-dev,
	Wang, Wei W

On 26.05.20 16:50, Alexander Duyck wrote:
> On Tue, May 26, 2020 at 1:24 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 20.05.20 18:25, Alexander Duyck wrote:
>>> On Wed, May 20, 2020 at 2:28 AM David Hildenbrand <david@redhat.com> wrote:
>>>>
>>>> On 20.05.20 04:02, Alexander Duyck 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.
>>>>>
>>>>
>>>> I would rephrase this, starting what it does *without* free page
>>>> reporting (which is not "provides a way for the guest to notify ..."),
>>>> and then eventually how this feature will also be used in the future as
>>>> well with free page reporting.
>>>
>>> Below is a rewrite on this description. I'm thinking that we can
>>> probably call out the advantage to free page reporting in a different
>>> way. Basically with the page poison feature we know a few things about
>>> the behavior and I have called them out in the new patch description:
>>>
>>> Page poison provides a way for the guest to notify the host that it is
>>> initializing or poisoning freed pages with some specific poison value. As a
>>> result of this we can infer a couple traits about the guest:
>>>
>>> 1. Free pages will contain a specific pattern within the guest.
>>> 2. Modifying free pages from this value may cause an error in the guest.
>>> 3. Pages will be immediately written to by the driver when deflated.
>>>
>>> There are currently no existing features that make use of this data. In the
>>> upcoming feature free page reporting we will need to make use of this to
>>> identify if we can evict pages from the guest without causing data
>>> corruption.
>>>
>>> Add documentation for the page poison feature describing the basic
>>> functionality and requirements.
>>>
> 
> [...]
> 
>>>>> +
>>>>> +If the guest is not initializing or poisoning freed pages it should reject
>>>>
>>>> Sometimes you use "write to pages after deflating", here you use "freed
>>>> pages"
>>>
>>> So when I am referencing "freed pages" I am talking about all free
>>> memory, while when I refer to "pages after deflating" I am talking
>>> about pages coming out of the balloon.
>>>
>>> My thought is that there maybe be additional uses for "poison_val" be
>>> to feed it into some future use other than just the balloon portion of
>>> the deflation. Basically what this is telling us is that we could look
>>> for a pattern of pages containing nothing but poison_val if we wanted
>>> to do some sort of same page merging, or maybe define something to
>>> optimize migration by defining a poison page similar to a zero page
>>> that could be used to reduce migration overhead in the future.
>>>
>>>>> +the VIRTIO_BALLOON_F_PAGE_POISON feature.
>>>>> +
>>>>> +If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest
>>>>> +will place the expected poison value into the \field{poison_val}
>>>>
>>>> again, "expected" is misleading in the context of this patch only.
>>>
>>> I will rewrite this statement at follows:
>>>   If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
>>>   will place the initialization and/or poison value into the \field{poison_val}
>>>   configuration field data.
>>>
>>> I think I might strengthen things a bit as well. In the driver
>>> normative section I think I might add the following:
>>>   The driver MUST initialize and/or poison the deflated pages with
>>>   \field{poison_val} when they are reused by the driver.
>>>
>>
>> Maybe simplify that whole "initialize and/or poison " handling across
>> this patch to "initialize with \field{poison_val}" - if the
>> initialization is used for poisoning or initialization doesn't matter
>> from spec POV.
>>
>> In general looks good to me, I'll have another look at the full result.
> 
> Okay, I will go through and try to flush out the "and/or poison" in
> favor of just calling out the initialization.
> 
>> Still wondering what to do with free page hinting ... in the meantime
>> I'll have a look at free page reporting :)
> 
> The problem is it is already out there so I worry we wouldn't ever be
> able to get rid of it. At most we could deprecate it, but we are still
> stuck with it consuming bit resources and such.

Yeah, that's not an issue, they will simply turn to dead bits with
minimal documentation. I just don't see us fixing/supporting that
feature, really. Let's see what @MST things when he has time to look
into this.

-- 
Thanks,

David / dhildenb


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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-26 15:28           ` David Hildenbrand
@ 2020-05-26 15:38             ` Cornelia Huck
  2020-05-27  3:20               ` Alexander Duyck
  2020-05-27  6:14               ` [virtio-comment] Re: [virtio-dev] " Wei Wang
  0 siblings, 2 replies; 16+ messages in thread
From: Cornelia Huck @ 2020-05-26 15:38 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Alexander Duyck, Michael S. Tsirkin, virtio-comment, virtio-dev,
	Wang, Wei W

On Tue, 26 May 2020 17:28:00 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 26.05.20 16:50, Alexander Duyck wrote:
> > On Tue, May 26, 2020 at 1:24 AM David Hildenbrand <david@redhat.com> wrote:  

> >> Still wondering what to do with free page hinting ... in the meantime
> >> I'll have a look at free page reporting :)  
> > 
> > The problem is it is already out there so I worry we wouldn't ever be
> > able to get rid of it. At most we could deprecate it, but we are still
> > stuck with it consuming bit resources and such.  
> 
> Yeah, that's not an issue, they will simply turn to dead bits with
> minimal documentation. I just don't see us fixing/supporting that
> feature, really. Let's see what @MST things when he has time to look
> into this.
> 

If free page hinting is broken enough that we don't want anybody to try
implementing it, we maybe could:

- reserve the feature bit,
- say that the device SHOULD NOT offer it,
- say that the driver SHOULD NOT accept it?

Would avoid conflicts, and tell implementors that they should not
bother. (We can then proceed to start deprecating the Linux/QEMU
implementations.)


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] 16+ messages in thread

* Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-26 15:38             ` Cornelia Huck
@ 2020-05-27  3:20               ` Alexander Duyck
  2020-05-27  6:06                 ` Cornelia Huck
  2020-05-27  6:14               ` [virtio-comment] Re: [virtio-dev] " Wei Wang
  1 sibling, 1 reply; 16+ messages in thread
From: Alexander Duyck @ 2020-05-27  3:20 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: David Hildenbrand, Michael S. Tsirkin, virtio-comment,
	virtio-dev, Wang, Wei W

On Tue, May 26, 2020 at 8:38 AM Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Tue, 26 May 2020 17:28:00 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
> > On 26.05.20 16:50, Alexander Duyck wrote:
> > > On Tue, May 26, 2020 at 1:24 AM David Hildenbrand <david@redhat.com> wrote:
>
> > >> Still wondering what to do with free page hinting ... in the meantime
> > >> I'll have a look at free page reporting :)
> > >
> > > The problem is it is already out there so I worry we wouldn't ever be
> > > able to get rid of it. At most we could deprecate it, but we are still
> > > stuck with it consuming bit resources and such.
> >
> > Yeah, that's not an issue, they will simply turn to dead bits with
> > minimal documentation. I just don't see us fixing/supporting that
> > feature, really. Let's see what @MST things when he has time to look
> > into this.
> >
>
> If free page hinting is broken enough that we don't want anybody to try
> implementing it, we maybe could:
>
> - reserve the feature bit,
> - say that the device SHOULD NOT offer it,
> - say that the driver SHOULD NOT accept it?
>
> Would avoid conflicts, and tell implementors that they should not
> bother. (We can then proceed to start deprecating the Linux/QEMU
> implementations.)

What I might do is just try to work around it for now. I might
re-order my patches so that I can push the page poison and free page
reporting bits with the free page hinting changes at the end of the
set. That way if we decide to take a different route then we can
always go back and change that later and it won't have an impact on
the earlier changes.

Thanks.

- Alex

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] 16+ messages in thread

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

On Tue, 26 May 2020 20:20:21 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> On Tue, May 26, 2020 at 8:38 AM Cornelia Huck <cohuck@redhat.com> wrote:
> >
> > On Tue, 26 May 2020 17:28:00 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >  
> > > On 26.05.20 16:50, Alexander Duyck wrote:  
> > > > On Tue, May 26, 2020 at 1:24 AM David Hildenbrand <david@redhat.com> wrote:  
> >  
> > > >> Still wondering what to do with free page hinting ... in the meantime
> > > >> I'll have a look at free page reporting :)  
> > > >
> > > > The problem is it is already out there so I worry we wouldn't ever be
> > > > able to get rid of it. At most we could deprecate it, but we are still
> > > > stuck with it consuming bit resources and such.  
> > >
> > > Yeah, that's not an issue, they will simply turn to dead bits with
> > > minimal documentation. I just don't see us fixing/supporting that
> > > feature, really. Let's see what @MST things when he has time to look
> > > into this.
> > >  
> >
> > If free page hinting is broken enough that we don't want anybody to try
> > implementing it, we maybe could:
> >
> > - reserve the feature bit,
> > - say that the device SHOULD NOT offer it,
> > - say that the driver SHOULD NOT accept it?
> >
> > Would avoid conflicts, and tell implementors that they should not
> > bother. (We can then proceed to start deprecating the Linux/QEMU
> > implementations.)  
> 
> What I might do is just try to work around it for now. I might
> re-order my patches so that I can push the page poison and free page
> reporting bits with the free page hinting changes at the end of the
> set. That way if we decide to take a different route then we can
> always go back and change that later and it won't have an impact on
> the earlier changes.

Maybe:
- only add the hinting feature bit as reserved,
- document page poison and free page reporting,
- and then think about what to do with the hinting?

Just to make it clear that there is another feature bit out there; but
maybe that is overkill and we should just go ahead with patches 2/3 for
now.


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] 16+ messages in thread

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-26 15:38             ` Cornelia Huck
  2020-05-27  3:20               ` Alexander Duyck
@ 2020-05-27  6:14               ` Wei Wang
  2020-05-27 12:08                 ` David Hildenbrand
  1 sibling, 1 reply; 16+ messages in thread
From: Wei Wang @ 2020-05-27  6:14 UTC (permalink / raw)
  To: Cornelia Huck, David Hildenbrand
  Cc: Alexander Duyck, Michael S. Tsirkin, virtio-comment, virtio-dev

On 05/26/2020 11:38 PM, Cornelia Huck wrote:
> On Tue, 26 May 2020 17:28:00 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> On 26.05.20 16:50, Alexander Duyck wrote:
>>> On Tue, May 26, 2020 at 1:24 AM David Hildenbrand <david@redhat.com> wrote:
>>>> Still wondering what to do with free page hinting ... in the meantime
>>>> I'll have a look at free page reporting :)
>>> The problem is it is already out there so I worry we wouldn't ever be
>>> able to get rid of it. At most we could deprecate it, but we are still
>>> stuck with it consuming bit resources and such.
>> Yeah, that's not an issue, they will simply turn to dead bits with
>> minimal documentation. I just don't see us fixing/supporting that
>> feature, really. Let's see what @MST things when he has time to look
>> into this.
>>
> If free page hinting is broken enough that we don't want anybody to try
> implementing it, we maybe could:

May I know the issues that you got with FREE_PAGE_HINT?

I thought the discussion was about the PAGE_POISON bit (not related to 
FREE_PAGE_HINT).
It just enables a way for guest to tell host about the poison value.
I think currently we don't have a usage in QEMU to use the poison value 
(not sure about other userspace),
so it's disabled by default.

Best,
Wei


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] 16+ messages in thread

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
  2020-05-27  6:14               ` [virtio-comment] Re: [virtio-dev] " Wei Wang
@ 2020-05-27 12:08                 ` David Hildenbrand
  0 siblings, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2020-05-27 12:08 UTC (permalink / raw)
  To: Wei Wang, Cornelia Huck
  Cc: Alexander Duyck, Michael S. Tsirkin, virtio-comment, virtio-dev

On 27.05.20 08:14, Wei Wang wrote:
> On 05/26/2020 11:38 PM, Cornelia Huck wrote:
>> On Tue, 26 May 2020 17:28:00 +0200 David Hildenbrand
>> <david@redhat.com> wrote:
>> 
>>> On 26.05.20 16:50, Alexander Duyck wrote:
>>>> On Tue, May 26, 2020 at 1:24 AM David Hildenbrand
>>>> <david@redhat.com> wrote:
>>>>> Still wondering what to do with free page hinting ... in the
>>>>> meantime I'll have a look at free page reporting :)
>>>> The problem is it is already out there so I worry we wouldn't
>>>> ever be able to get rid of it. At most we could deprecate it,
>>>> but we are still stuck with it consuming bit resources and
>>>> such.
>>> Yeah, that's not an issue, they will simply turn to dead bits
>>> with minimal documentation. I just don't see us fixing/supporting
>>> that feature, really. Let's see what @MST things when he has time
>>> to look into this.
>>> 
>> If free page hinting is broken enough that we don't want anybody to
>> try implementing it, we maybe could:
> 
> May I know the issues that you got with FREE_PAGE_HINT?

Did you follow the discussion on the spec updates proposed by Alexander?
We might have identified a couple of issues in the QEMU side trying to
document the semantics of free page hinting.

For example:

1. When migration fails in the live stage, before stopping the VM, the
guest will not receive a VIRTIO_BALLOON_CMD_ID_DONE.

2. The semantics about what could happen to hinted pages are unclear
(and it is unclear if the current QEMU behavior is a BUG or expected).
While writing to a hinted page will result in the page to get migrated
and not change the value, the guest might suddenly observe a change in
the value when only reading the page.

Imagine (just as an example) something in a guest like

/* page was previously hinted and is now getting reused by the guest */
if (!page_filled_with(page, X)) {
	fill_page_with(page, X);
}
/* migration finished, value of page changed */

And Alexander pointed out, that the change the guest might observe might
not be the change to a zero page. Semantics unclear.

There seems to be more related to the async iothread/reset handling
+ the other fixes I just recently sent.


It would be good if you could have a look at the matter.

-- 
Thanks,

David / dhildenb


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] 16+ messages in thread

end of thread, other threads:[~2020-05-27 12:08 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20  2:02 [virtio-comment] [PATCH v3 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
2020-05-20  2:02 ` [virtio-comment] [PATCH v3 1/3] content: Document balloon feature free page hints Alexander Duyck
2020-05-20  2:02 ` [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison Alexander Duyck
2020-05-20  9:27   ` David Hildenbrand
2020-05-20 16:25     ` Alexander Duyck
2020-05-26  8:24       ` David Hildenbrand
2020-05-26 14:50         ` Alexander Duyck
2020-05-26 15:28           ` David Hildenbrand
2020-05-26 15:38             ` Cornelia Huck
2020-05-27  3:20               ` Alexander Duyck
2020-05-27  6:06                 ` Cornelia Huck
2020-05-27  6:14               ` [virtio-comment] Re: [virtio-dev] " Wei Wang
2020-05-27 12:08                 ` David Hildenbrand
2020-05-20  2:02 ` [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting Alexander Duyck
2020-05-26  9:05   ` David Hildenbrand
2020-05-26 14:42     ` 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.