All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] [PATCH v4 0/3] virtio-spec: Add documentation for recently added balloon features
@ 2020-05-27  4:06 Alexander Duyck
  2020-05-27  4:06 ` [virtio-comment] [PATCH v4 1/3] content: Document balloon feature page poison Alexander Duyck
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Alexander Duyck @ 2020-05-27  4:06 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

Changes since v3:
Reordered patches to place free page hinting at end of patch set
  Moved contents out of patch to poison and free page reporting patches
  Updated patch description to document some known issues with feature
Further clean-ups based on input from David Hildenbrand

[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 page poison
      content: Document balloon feature free page reporting
      content: Document balloon feature free page hints


 conformance.tex |    6 +
 content.tex     |  269 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 269 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] 9+ messages in thread

* [virtio-comment] [PATCH v4 1/3] content: Document balloon feature page poison
  2020-05-27  4:06 [virtio-comment] [PATCH v4 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
@ 2020-05-27  4:06 ` Alexander Duyck
  2020-05-29  8:13   ` David Hildenbrand
  2020-05-27  4:06 ` [virtio-comment] [PATCH v4 2/3] content: Document balloon feature free page reporting Alexander Duyck
  2020-05-27  4:07 ` [virtio-comment] [PATCH v4 3/3] content: Document balloon feature free page hints Alexander Duyck
  2 siblings, 1 reply; 9+ messages in thread
From: Alexander Duyck @ 2020-05-27  4:06 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 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.

Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 conformance.tex |    2 ++
 content.tex     |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index b6fdec090383..4ed9d62e8088 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 / Page Poison}
 \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 / 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 91735e3eb018..4a0ab90260ff 100644
--- a/content.tex
+++ b/content.tex
@@ -5019,6 +5019,9 @@ \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_PAGE_POISON(4) ] A hint to the device, that the driver
+    will immediately write \field{poison_val} to pages after deflating them.
+    Configuration field \field{poison_val} is valid.
 
 \end{description}
 
@@ -5026,6 +5029,10 @@ \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 will
+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
@@ -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{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}
 
@@ -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_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
+  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,40 @@ \subsubsection{Memory Statistics Tags}\label{sec:Device Types / Memory Balloon D
   allocations in the guest.
 \end{description}
 
+\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
+free pages with \field{poison_val}. When the feature is enabled, pages will
+be immediately written to by the driver after deflating.
+
+If the guest is not initializing freed pages the driver should reject the
+VIRTIO_BALLOON_F_PAGE_POISON feature.
+
+If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the driver
+will place the initialization 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 initialize the deflated pages with \field{poison_val} when
+they are reused by the driver.
+
+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] 9+ messages in thread

* [virtio-comment] [PATCH v4 2/3] content: Document balloon feature free page reporting
  2020-05-27  4:06 [virtio-comment] [PATCH v4 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
  2020-05-27  4:06 ` [virtio-comment] [PATCH v4 1/3] content: Document balloon feature page poison Alexander Duyck
@ 2020-05-27  4:06 ` Alexander Duyck
  2020-05-27  4:07 ` [virtio-comment] [PATCH v4 3/3] content: Document balloon feature free page hints Alexander Duyck
  2 siblings, 0 replies; 9+ messages in thread
From: Alexander Duyck @ 2020-05-27  4:06 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     |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 4ed9d62e8088..18a5a94a72aa 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 / 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}
@@ -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 / 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 4a0ab90260ff..403651d1413b 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[4] reporting_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.
+
+  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}
@@ -5022,6 +5025,8 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
 \item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] A hint to the device, that the driver
     will immediately write \field{poison_val} to pages after deflating them.
     Configuration field \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}
 
@@ -5033,6 +5038,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
 not immediately write \field{poison_val} to deflated pages (e.g., to
 initialize them, or fill them with a poison value).
 
+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
@@ -5088,10 +5097,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 the device.
 \end{enumerate}
 
 \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
@@ -5365,8 +5380,9 @@ \subsubsection{Memory Statistics Tags}\label{sec:Device Types / Memory Balloon D
 \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
-free pages with \field{poison_val}. When the feature is enabled, pages will
-be immediately written to by the driver after deflating.
+free pages with \field{poison_val}. When the feature is enabled, pages will be
+immediately written to by the driver after deflating, and pages reported by
+free page reporting will retain the value indicated by \field{poison_val}.
 
 If the guest is not initializing freed pages the driver should reject the
 VIRTIO_BALLOON_F_PAGE_POISON feature.
@@ -5396,6 +5412,71 @@ \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. Reported free pages can
+be reused by the driver after the reporting request has been acknowledged
+without notifying the device.
+
+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 free pages.
+
+\item The driver gathers free 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 driver can reuse the
+  reported 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 free 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 initialize all free pages with \field{poison_val} before
+reporting them.
+
+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 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.
+
+If the VIRTIO_BALLOON_F_PAGE_POISON feature has not 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] 9+ messages in thread

* [virtio-comment] [PATCH v4 3/3] content: Document balloon feature free page hints
  2020-05-27  4:06 [virtio-comment] [PATCH v4 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
  2020-05-27  4:06 ` [virtio-comment] [PATCH v4 1/3] content: Document balloon feature page poison Alexander Duyck
  2020-05-27  4:06 ` [virtio-comment] [PATCH v4 2/3] content: Document balloon feature free page reporting Alexander Duyck
@ 2020-05-27  4:07 ` Alexander Duyck
  2020-06-24 18:26   ` David Hildenbrand
  2 siblings, 1 reply; 9+ messages in thread
From: Alexander Duyck @ 2020-05-27  4:07 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.

In working on this the specification as pointed out certain issues with the
Linux driver and QEMU device implementation. The issues include:
1. The Linux driver does not re-initialize pages when it reuses them
before receiving the "DONE" command, as such this can lead to possible data
corruption.
2. The QEMU device is not returning the "DONE" command if a migration
fails. This results in the guest holding onto pages until forced out by the
shrinker.

There are also additional issues that have been found not related to the
specification.

There is currently discussion on if the feature should be removed so this
patch is a place-holder for if we decide to keep the feature and fix the
issues. Otherwise this patch can be dropped and we can work on a patch to
document the need to avoid the feature.

Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 conformance.tex |    2 +
 content.tex     |  125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index 18a5a94a72aa..5496a25e93ef 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}
 \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}
@@ -333,6 +334,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}
 \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}
diff --git a/content.tex b/content.tex
index 403651d1413b..a3a9c13516fd 100644
--- a/content.tex
+++ b/content.tex
@@ -5006,11 +5006,14 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque
 \item[0] inflateq
 \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}
@@ -5022,6 +5025,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.
 \item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] A hint to the device, that the driver
     will immediately write \field{poison_val} to pages after deflating them.
     Configuration field \field{poison_val} is valid.
@@ -5060,6 +5067,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu
 \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon Device / Device configuration layout}
   \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.
+
   \field{poison_val} is available if VIRTIO_BALLOON_F_PAGE_POISON has been
     negotiated.
 
@@ -5094,6 +5105,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic
   \item Add one empty buffer to the stats virtqueue.
   \end{enumerate}
 
+\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.
 
@@ -5377,6 +5391,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 MUST report a value of VIRTIO_BALLOON_CMD_ID_DONE in
+\field{free_page_hint_cmd_id} when 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



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

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

On 27.05.20 06:06, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> 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.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
>  conformance.tex |    2 ++
>  content.tex     |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index b6fdec090383..4ed9d62e8088 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 / Page Poison}
>  \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 / 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 91735e3eb018..4a0ab90260ff 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5019,6 +5019,9 @@ \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_PAGE_POISON(4) ] A hint to the device, that the driver
> +    will immediately write \field{poison_val} to pages after deflating them.
> +    Configuration field \field{poison_val} is valid.
>  

Here we have "that the driver will immediately" ...

But we never document that in form of a normative statement (e.g., "The
driver MUST initialize pages with \field{poison_val} after deflating").

Just wondering if that is intended (I imagine it will be different with
free page reporting)?

Apart from that looks 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] 9+ messages in thread

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

On Fri, May 29, 2020 at 1:13 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 27.05.20 06:06, Alexander Duyck wrote:
> > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >
> > 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.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > ---
> >  conformance.tex |    2 ++
> >  content.tex     |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 57 insertions(+), 4 deletions(-)
> >
> > diff --git a/conformance.tex b/conformance.tex
> > index b6fdec090383..4ed9d62e8088 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 / Page Poison}
> >  \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 / 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 91735e3eb018..4a0ab90260ff 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -5019,6 +5019,9 @@ \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_PAGE_POISON(4) ] A hint to the device, that the driver
> > +    will immediately write \field{poison_val} to pages after deflating them.
> > +    Configuration field \field{poison_val} is valid.
> >
>
> Here we have "that the driver will immediately" ...
>
> But we never document that in form of a normative statement (e.g., "The
> driver MUST initialize pages with \field{poison_val} after deflating").

I'm pretty sure we did document that. In the normative statement for
the driver below we have:
+The driver MUST initialize the deflated pages with \field{poison_val} when
+they are reused by the driver.

> Just wondering if that is intended (I imagine it will be different with
> free page reporting)?

I think we add some additional items with page reporting, but I think
those are in the page reporting section if I recall correctly.

> Apart from that looks good to me!
>
> --
> Thanks,
>
> David / dhildenb
>

Thanks for taking all the time to review this.

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

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

On 29.05.20 18:57, Alexander Duyck wrote:
> On Fri, May 29, 2020 at 1:13 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 27.05.20 06:06, Alexander Duyck wrote:
>>> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>>>
>>> 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.
>>>
>>> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>>> ---
>>>  conformance.tex |    2 ++
>>>  content.tex     |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>>>  2 files changed, 57 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/conformance.tex b/conformance.tex
>>> index b6fdec090383..4ed9d62e8088 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 / Page Poison}
>>>  \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 / 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 91735e3eb018..4a0ab90260ff 100644
>>> --- a/content.tex
>>> +++ b/content.tex
>>> @@ -5019,6 +5019,9 @@ \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_PAGE_POISON(4) ] A hint to the device, that the driver
>>> +    will immediately write \field{poison_val} to pages after deflating them.
>>> +    Configuration field \field{poison_val} is valid.
>>>
>>
>> Here we have "that the driver will immediately" ...
>>
>> But we never document that in form of a normative statement (e.g., "The
>> driver MUST initialize pages with \field{poison_val} after deflating").
> 
> I'm pretty sure we did document that. In the normative statement for
> the driver below we have:
> +The driver MUST initialize the deflated pages with \field{poison_val} when
> +they are reused by the driver.
> 

Doh! I think I missed that somehow

Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!

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

* Re: [virtio-comment] [PATCH v4 3/3] content: Document balloon feature free page hints
  2020-05-27  4:07 ` [virtio-comment] [PATCH v4 3/3] content: Document balloon feature free page hints Alexander Duyck
@ 2020-06-24 18:26   ` David Hildenbrand
  2020-07-06 15:46     ` [virtio-comment] Re: [virtio-dev] " Alexander Duyck
  0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand @ 2020-06-24 18:26 UTC (permalink / raw)
  To: Alexander Duyck, mst, cohuck; +Cc: virtio-comment, virtio-dev, wei.w.wang

On 27.05.20 06:07, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> Free page hints allow the balloon driver to provide information on what
> pages are not currently in use so that we can avoid the cost of copying
> them in migration scenarios. Add a feature description for free page hints
> describing basic functioning and requirements.
> 
> In working on this the specification as pointed out certain issues with the
> Linux driver and QEMU device implementation. The issues include:
> 1. The Linux driver does not re-initialize pages when it reuses them
> before receiving the "DONE" command, as such this can lead to possible data
> corruption.
> 2. The QEMU device is not returning the "DONE" command if a migration
> fails. This results in the guest holding onto pages until forced out by the
> shrinker.
> 
> There are also additional issues that have been found not related to the
> specification.
> 
> There is currently discussion on if the feature should be removed so this
> patch is a place-holder for if we decide to keep the feature and fix the
> issues. Otherwise this patch can be dropped and we can work on a patch to
> document the need to avoid the feature.

Looks like the feature will stay, hope we can document the expected
semantics reasonably well now and fix the remaining issues. After all we
spend quite some time in reverse-engineering and fixing already ...

[...]
>  
> +\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:

Maybe mention somewhere (resulting from a discussion with Michael) that
the semantics of hinted pages are similar to MADV_FREE, except
- it's asynchronous (and the cmd_id is used to synchronize)
- when reading pages after hinted, the content is undefined (might be
  something besides the old content or zero).

Might help to understand what the semantics are.

> +
> +\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.
> +

nit: I'd avoid using the term "report" here. Maybe "specifies" or sth.
like that.

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

"or VIRTIO_BALLOON_CMD_ID_DONE" I assume.

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

Ack.

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

Hm. The "dirty pages" wording  is a little bit too (migration) specific
for my taste. Will think about this more.

> +
> +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}.

ACK, that's what you fixed.

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

Only before setting DONE, correct? But ...

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

... there it is :)

> +
> +The device MUST report a value of VIRTIO_BALLOON_CMD_ID_DONE in
> +\field{free_page_hint_cmd_id} when it no longer has need for the
> +previously hinted pages.

Right, that's still to be fixed if migration fails. I think we can reuse
the CLEANUP part.


Will we have to document that the guest must only indicate a cmd_id (in
out_buf) only once and not multiple times? Essentially what we discussed
in reply to your latest fix.

Again, thanks a lot for documenting and fixing ...

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

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] [PATCH v4 3/3] content: Document balloon feature free page hints
  2020-06-24 18:26   ` David Hildenbrand
@ 2020-07-06 15:46     ` Alexander Duyck
  0 siblings, 0 replies; 9+ messages in thread
From: Alexander Duyck @ 2020-07-06 15:46 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Michael S. Tsirkin, Cornelia Huck, virtio-comment, virtio-dev,
	Wang, Wei W

On Wed, Jun 24, 2020 at 11:26 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 27.05.20 06:07, Alexander Duyck wrote:
> > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >
> > Free page hints allow the balloon driver to provide information on what
> > pages are not currently in use so that we can avoid the cost of copying
> > them in migration scenarios. Add a feature description for free page hints
> > describing basic functioning and requirements.
> >
> > In working on this the specification as pointed out certain issues with the
> > Linux driver and QEMU device implementation. The issues include:
> > 1. The Linux driver does not re-initialize pages when it reuses them
> > before receiving the "DONE" command, as such this can lead to possible data
> > corruption.
> > 2. The QEMU device is not returning the "DONE" command if a migration
> > fails. This results in the guest holding onto pages until forced out by the
> > shrinker.
> >
> > There are also additional issues that have been found not related to the
> > specification.
> >
> > There is currently discussion on if the feature should be removed so this
> > patch is a place-holder for if we decide to keep the feature and fix the
> > issues. Otherwise this patch can be dropped and we can work on a patch to
> > document the need to avoid the feature.
>
> Looks like the feature will stay, hope we can document the expected
> semantics reasonably well now and fix the remaining issues. After all we
> spend quite some time in reverse-engineering and fixing already ...

Agreed. Might as well document as much as we can so somebody else will
not have to go back through the same process again.

> [...]
> >
> > +\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:
>
> Maybe mention somewhere (resulting from a discussion with Michael) that
> the semantics of hinted pages are similar to MADV_FREE, except
> - it's asynchronous (and the cmd_id is used to synchronize)
> - when reading pages after hinted, the content is undefined (might be
>   something besides the old content or zero).
>
> Might help to understand what the semantics are.

I'll work on writing something up. I am not sure if mentioning
MADV_FREE makes much sense though since the behavior still differs
quite a bit, but I can describe the behavior.

> > +
> > +\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.
> > +
>
> nit: I'd avoid using the term "report" here. Maybe "specifies" or sth.
> like that.

Done.

> > +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.
>
> "or VIRTIO_BALLOON_CMD_ID_DONE" I assume.

Correct, I will update.

> > +
> > +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.
>
> Ack.
>
> > +
> > +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.
>
> Hm. The "dirty pages" wording  is a little bit too (migration) specific
> for my taste. Will think about this more.

I will update it. I can probably just state that it should set STOP
any time it cannot make use of the hints provided by the driver.

> > +
> > +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}.
>
> ACK, that's what you fixed.

Right.

> > +
> > +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.
>
> Only before setting DONE, correct? But ...
>
> > +
> > +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.
>
> ... there it is :)

Yeah, I was slicing it up with two definitions. The first one defines
the bounds in which the device might be allowed to modify the memory,
and the second indicates the situations where the first rule will not
apply.

> > +
> > +The device MUST report a value of VIRTIO_BALLOON_CMD_ID_DONE in
> > +\field{free_page_hint_cmd_id} when it no longer has need for the
> > +previously hinted pages.
>
> Right, that's still to be fixed if migration fails. I think we can reuse
> the CLEANUP part.

Thanks for taking care of that.

> Will we have to document that the guest must only indicate a cmd_id (in
> out_buf) only once and not multiple times? Essentially what we discussed
> in reply to your latest fix.

Okay I will add that to the spec.

> Again, thanks a lot for documenting and fixing ...

No problem. Thanks for reviewing.

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

end of thread, other threads:[~2020-07-06 15:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27  4:06 [virtio-comment] [PATCH v4 0/3] virtio-spec: Add documentation for recently added balloon features Alexander Duyck
2020-05-27  4:06 ` [virtio-comment] [PATCH v4 1/3] content: Document balloon feature page poison Alexander Duyck
2020-05-29  8:13   ` David Hildenbrand
2020-05-29 16:57     ` Alexander Duyck
2020-05-29 17:15       ` David Hildenbrand
2020-05-27  4:06 ` [virtio-comment] [PATCH v4 2/3] content: Document balloon feature free page reporting Alexander Duyck
2020-05-27  4:07 ` [virtio-comment] [PATCH v4 3/3] content: Document balloon feature free page hints Alexander Duyck
2020-06-24 18:26   ` David Hildenbrand
2020-07-06 15:46     ` [virtio-comment] Re: [virtio-dev] " 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.