[virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
On 20.05.20 18:25, Alexander Duyck wrote:
> On Wed, May 20, 2020 at 2:28 AM David Hildenbrand wrote:
>>
>> On 20.05.20 04:02, Alexander Duyck wrote:
>>> From: Alexander Duyck
>>>
>>> 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
[virtio-dev] Re: [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting
On 20.05.20 04:02, Alexander Duyck wrote:
> From: Alexander Duyck
>
> 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
> ---
> 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
> /
[virtio-dev] [PATCH v4 0/3] Support virtio cross-device resources
This patchset implements the current proposal for virtio cross-device resource sharing [1]. It will be used to import virtio resources into the virtio-video driver currently under discussion [2]. The patch under consideration to add support in the virtio-video driver is [3]. It uses the APIs from v3 of this series, but the changes to update it are relatively minor. This patchset adds a new flavor of dma-bufs that supports querying the underlying virtio object UUID, as well as adding support for exporting resources from virtgpu. [1] https://markmail.org/thread/2ypjt5cfeu3m6lxu [2] https://markmail.org/thread/p5d3k566srtdtute [3] https://markmail.org/thread/j4xlqaaim266qpks v3 -> v4 changes: - Replace dma-buf hooks with virtio dma-buf from v1. - Remove virtio_attach callback, as the work that had been done in that callback is now done on dma-buf export. The documented requirement that get_uuid only be called on attached virtio dma-bufs is also removed. - Rebase and add call to virtio_gpu_notify for ASSIGN_UUID. David Stevens (3): virtio: add dma-buf support for exported objects virtio-gpu: add VIRTIO_GPU_F_RESOURCE_UUID feature drm/virtio: Support virtgpu exported resources drivers/gpu/drm/virtio/virtgpu_drv.c | 3 + drivers/gpu/drm/virtio/virtgpu_drv.h | 20 ++ drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++ drivers/gpu/drm/virtio/virtgpu_prime.c | 98 +- drivers/gpu/drm/virtio/virtgpu_vq.c| 55 +++ drivers/virtio/Makefile| 2 +- drivers/virtio/virtio.c| 6 ++ drivers/virtio/virtio_dma_buf.c| 91 include/linux/virtio.h | 1 + include/linux/virtio_dma_buf.h | 58 +++ include/uapi/linux/virtio_gpu.h| 19 + 11 files changed, 353 insertions(+), 4 deletions(-) create mode 100644 drivers/virtio/virtio_dma_buf.c create mode 100644 include/linux/virtio_dma_buf.h -- 2.27.0.rc0.183.gde8f92d652-goog - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v4 2/3] virtio-gpu: add VIRTIO_GPU_F_RESOURCE_UUID feature
This feature allows the guest to request a UUID from the host for a
particular virtio_gpu resource. The UUID can then be shared with other
virtio devices, to allow the other host devices to access the
virtio_gpu's corresponding host resource.
Signed-off-by: David Stevens
---
include/uapi/linux/virtio_gpu.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index 0c85914d9369..9721d58b4d58 100644
--- a/include/uapi/linux/virtio_gpu.h
+++ b/include/uapi/linux/virtio_gpu.h
@@ -50,6 +50,10 @@
* VIRTIO_GPU_CMD_GET_EDID
*/
#define VIRTIO_GPU_F_EDID1
+/*
+ * VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID
+ */
+#define VIRTIO_GPU_F_RESOURCE_UUID 2
enum virtio_gpu_ctrl_type {
VIRTIO_GPU_UNDEFINED = 0,
@@ -66,6 +70,7 @@ enum virtio_gpu_ctrl_type {
VIRTIO_GPU_CMD_GET_CAPSET_INFO,
VIRTIO_GPU_CMD_GET_CAPSET,
VIRTIO_GPU_CMD_GET_EDID,
+ VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID,
/* 3d commands */
VIRTIO_GPU_CMD_CTX_CREATE = 0x0200,
@@ -87,6 +92,7 @@ enum virtio_gpu_ctrl_type {
VIRTIO_GPU_RESP_OK_CAPSET_INFO,
VIRTIO_GPU_RESP_OK_CAPSET,
VIRTIO_GPU_RESP_OK_EDID,
+ VIRTIO_GPU_RESP_OK_RESOURCE_UUID,
/* error responses */
VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200,
@@ -340,4 +346,17 @@ enum virtio_gpu_formats {
VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM = 134,
};
+/* VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID */
+struct virtio_gpu_resource_assign_uuid {
+ struct virtio_gpu_ctrl_hdr hdr;
+ __le32 resource_id;
+ __le32 padding;
+};
+
+/* VIRTIO_GPU_RESP_OK_RESOURCE_UUID */
+struct virtio_gpu_resp_resource_uuid {
+ struct virtio_gpu_ctrl_hdr hdr;
+ __u8 uuid[16];
+};
+
#endif
--
2.27.0.rc0.183.gde8f92d652-goog
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v4 1/3] virtio: add dma-buf support for exported objects
This change adds a new flavor of dma-bufs that can be used by virtio
drivers to share exported objects. A virtio dma-buf can be queried by
virtio drivers to obtain the UUID which identifies the underlying
exported object.
Signed-off-by: David Stevens
---
drivers/virtio/Makefile | 2 +-
drivers/virtio/virtio.c | 6 +++
drivers/virtio/virtio_dma_buf.c | 89 +
include/linux/virtio.h | 1 +
include/linux/virtio_dma_buf.h | 58 +
5 files changed, 155 insertions(+), 1 deletion(-)
create mode 100644 drivers/virtio/virtio_dma_buf.c
create mode 100644 include/linux/virtio_dma_buf.h
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 29a1386ecc03..ecdae5b596de 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
+obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o virtio_dma_buf.o
obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index a977e32a88f2..5d46f0ded92d 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -357,6 +357,12 @@ int register_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(register_virtio_device);
+bool is_virtio_device(struct device *dev)
+{
+ return dev->bus == &virtio_bus;
+}
+EXPORT_SYMBOL_GPL(is_virtio_device);
+
void unregister_virtio_device(struct virtio_device *dev)
{
int index = dev->index; /* save for after device release */
diff --git a/drivers/virtio/virtio_dma_buf.c b/drivers/virtio/virtio_dma_buf.c
new file mode 100644
index ..23e3399b11ed
--- /dev/null
+++ b/drivers/virtio/virtio_dma_buf.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * dma-bufs for virtio exported objects
+ *
+ * Copyright (C) 2020 Google, Inc.
+ */
+
+#include
+
+/**
+ * virtio_dma_buf_export - Creates a new dma-buf for a virtio exported object
+ *
+ * This wraps dma_buf_export() to allow virtio drivers to create a dma-buf
+ * for an virtio exported object that can be queried by other virtio drivers
+ * for the object's UUID.
+ */
+struct dma_buf *virtio_dma_buf_export(
+ const struct virtio_dma_buf_export_info *virtio_exp_info)
+{
+ struct dma_buf_export_info exp_info;
+
+ if (!virtio_exp_info->ops
+ || virtio_exp_info->ops->ops.attach != &virtio_dma_buf_attach
+ || !virtio_exp_info->ops->get_uuid) {
+ return ERR_PTR(-EINVAL);
+ }
+
+ exp_info.exp_name = virtio_exp_info->exp_name;
+ exp_info.owner = virtio_exp_info->owner;
+ exp_info.ops = &virtio_exp_info->ops->ops;
+ exp_info.size = virtio_exp_info->size;
+ exp_info.flags = virtio_exp_info->flags;
+ exp_info.resv = virtio_exp_info->resv;
+ exp_info.priv = virtio_exp_info->priv;
+ BUILD_BUG_ON(sizeof(struct virtio_dma_buf_export_info)
+!= sizeof(struct dma_buf_export_info));
+
+ return dma_buf_export(&exp_info);
+}
+EXPORT_SYMBOL(virtio_dma_buf_export);
+
+/**
+ * virtio_dma_buf_attach - mandatory attach callback for virtio dma-bufs
+ */
+int virtio_dma_buf_attach(struct dma_buf *dma_buf,
+ struct dma_buf_attachment *attach)
+{
+ int ret;
+ const struct virtio_dma_buf_ops *ops = container_of(
+ dma_buf->ops, const struct virtio_dma_buf_ops, ops);
+
+ if (ops->device_attach) {
+ ret = ops->device_attach(dma_buf, attach);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+EXPORT_SYMBOL(virtio_dma_buf_attach);
+
+/**
+ * is_virtio_dma_buf - returns true if the given dma-buf is a virtio dma-buf
+ * @dma_buf: buffer to query
+ */
+bool is_virtio_dma_buf(struct dma_buf *dma_buf)
+{
+ return dma_buf->ops->attach == &virtio_dma_buf_attach;
+}
+EXPORT_SYMBOL(is_virtio_dma_buf);
+
+/**
+ * virtio_dma_buf_get_uuid - gets the uuid of the virtio dma-buf's exported
object
+ * @dma_buf: [in] buffer to query
+ * @uuid: [out] the uuid
+ *
+ * Returns: 0 on success, negative on failure.
+ */
+int virtio_dma_buf_get_uuid(struct dma_buf *dma_buf,
+ uuid_t *uuid)
+{
+ const struct virtio_dma_buf_ops *ops = container_of(
+ dma_buf->ops, const struct virtio_dma_buf_ops, ops);
+
+ if (!is_virtio_dma_buf(dma_buf))
+ return -EINVAL;
+
+ return ops->get_uuid(dma_buf, uuid);
+}
+EXPORT_SYMBOL(virtio_dma_buf_get_uuid);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 15f906e4a748..9397e25616c4 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -128,6 +128,7 @@ static inline struct virtio_device *dev_to_virtio(struct
device *_dev)
void virtio_add_status(struct
[virtio-dev] [PATCH v4 3/3] drm/virtio: Support virtgpu exported resources
Add support for UUID-based resource sharing mechanism to virtgpu. This
implements the new virtgpu commands and hooks them up to dma-buf's
get_uuid callback.
Signed-off-by: David Stevens
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 3 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 20 ++
drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++
drivers/gpu/drm/virtio/virtgpu_prime.c | 96 +-
drivers/gpu/drm/virtio/virtgpu_vq.c| 55 +++
5 files changed, 175 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index ab4bed78e656..b039f493bda9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -165,6 +165,7 @@ static unsigned int features[] = {
VIRTIO_GPU_F_VIRGL,
#endif
VIRTIO_GPU_F_EDID,
+ VIRTIO_GPU_F_RESOURCE_UUID,
};
static struct virtio_driver virtio_gpu_driver = {
.feature_table = features,
@@ -202,6 +203,8 @@ static struct drm_driver driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_mmap = drm_gem_prime_mmap,
+ .gem_prime_export = virtgpu_gem_prime_export,
+ .gem_prime_import = virtgpu_gem_prime_import,
.gem_prime_import_sg_table = virtgpu_gem_prime_import_sg_table,
.gem_create_object = virtio_gpu_create_object,
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 49bebdee6d91..39dc907aa805 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -49,6 +49,10 @@
#define DRIVER_MINOR 1
#define DRIVER_PATCHLEVEL 0
+#define UUID_INITIALIZING 0
+#define UUID_INITIALIZED 1
+#define UUID_INITIALIZATION_FAILED 2
+
struct virtio_gpu_object_params {
uint32_t format;
uint32_t width;
@@ -71,6 +75,9 @@ struct virtio_gpu_object {
uint32_t hw_res_handle;
bool dumb;
bool created;
+
+ int uuid_state;
+ uuid_t uuid;
};
#define gem_to_virtio_gpu_obj(gobj) \
container_of((gobj), struct virtio_gpu_object, base.base)
@@ -200,6 +207,7 @@ struct virtio_gpu_device {
bool has_virgl_3d;
bool has_edid;
bool has_indirect;
+ bool has_resource_assign_uuid;
struct work_struct config_changed_work;
@@ -210,6 +218,8 @@ struct virtio_gpu_device {
struct virtio_gpu_drv_capset *capsets;
uint32_t num_capsets;
struct list_head cap_cache;
+
+ spinlock_t resource_export_lock;
};
struct virtio_gpu_fpriv {
@@ -335,6 +345,10 @@ void virtio_gpu_dequeue_fence_func(struct work_struct
*work);
void virtio_gpu_notify(struct virtio_gpu_device *vgdev);
+int
+virtio_gpu_cmd_resource_assign_uuid(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object_array *objs);
+
/* virtgpu_display.c */
void virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev);
void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev);
@@ -366,6 +380,12 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
/* virtgpu_prime.c */
+struct dma_buf *virtgpu_gem_prime_export(struct drm_gem_object *obj,
+int flags);
+struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev,
+ struct dma_buf *buf);
+int virtgpu_gem_prime_get_uuid(struct drm_gem_object *obj,
+ uuid_t *uuid);
struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
struct drm_device *dev, struct dma_buf_attachment *attach,
struct sg_table *sgt);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 023a030ca7b9..7bcd0c75effa 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -125,6 +125,7 @@ int virtio_gpu_init(struct drm_device *dev)
vgdev->dev = dev->dev;
spin_lock_init(&vgdev->display_info_lock);
+ spin_lock_init(&vgdev->resource_export_lock);
ida_init(&vgdev->ctx_id_ida);
ida_init(&vgdev->resource_ida);
init_waitqueue_head(&vgdev->resp_wq);
@@ -153,6 +154,9 @@ int virtio_gpu_init(struct drm_device *dev)
if (virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC)) {
vgdev->has_indirect = true;
}
+ if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_UUID)) {
+ vgdev->has_resource_assign_uuid = true;
+ }
DRM_INFO("features: %cvirgl %cedid\n",
vgdev->has_virgl_3d ? '+' : '-',
diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c
b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 050d24c39a8f..a753bb70fcf1 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -23,12 +23,102
[virtio-dev] Re: [virtio-comment] [PATCH v3 3/3] content: Document balloon feature free page reporting
On Tue, May 26, 2020 at 2:06 AM David Hildenbrand wrote:
>
> On 20.05.20 04:02, Alexander Duyck wrote:
> > From: Alexander Duyck
> >
> > 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
> > ---
> > 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 \fie
[virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
On Tue, May 26, 2020 at 1:24 AM David Hildenbrand wrote:
>
> On 20.05.20 18:25, Alexander Duyck wrote:
> > On Wed, May 20, 2020 at 2:28 AM David Hildenbrand wrote:
> >>
> >> On 20.05.20 04:02, Alexander Duyck wrote:
> >>> From: Alexander Duyck
> >>>
> >>> 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
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH v2 1/3] content: Document balloon feature free page hints
On Wed, May 20, 2020 at 1:24 AM David Hildenbrand wrote:
>
> On 19.05.20 23:00, Alexander Duyck wrote:
> > On Tue, May 19, 2020 at 9:09 AM David Hildenbrand wrote:
[...]
>
> Let's think this through, what about this scenario:
>
> The device sets \field{free_page_hint_cmd_id} = X
> The driver starts reporting free pages (and reports all pages it has)
> 1. Sends X to start the windows
> 2. Sends all page hints (\field{free_page_hint_cmd_id} stays X)
> 3. Sends VIRTIO_BALLOON_CMD_ID_STOP to end the window
> The driver sets \field{free_page_hint_cmd_id} = DONE or STOP
>
> The guest can reuse the pages any time (triggered by the shrinker),
> especially, during 2, before the hypervisor even processed a hint
> request. It can happen that the guest reuses a page before the
> hypervisor processes the request and before
> \field{free_page_hint_cmd_id} changes.
>
> In QEMU, the double-bitmap magic makes sure that this is guaranteed to
> work IIRC.
>
> In that case, the page has to be migrated in that windows, the
> hypervisor must not modify the content.
> >>>
> >>> If by "reuse" you mean write to or reinitialize then that is correct.
> >>> All that is really happening is that any pages that are hinted have
> >>> the potential to be left behind with the original VM and not migrated
> >>> to the new one. We get the notification that the migration happened
> >>> when CMD_ID_DONE is passed to us. At that point the hinting is
> >>> complete and the device has no use for additional data.
> >>>
> >>> Instead of CMD_ID_STOP it probably would have made more sense to call
> >>> it something like CMD_ID_PAUSE or CMD_ID_HOLD as that is what it is
> >>> really doing. It is just temporarily holding the hints off while the
> >>> hypervisor synchronizes the dirty bits from the host.
> >>
> >> I think if migration fails, it will be left set to STOP. Guess we should
> >> specify that possibility somehow as well.
> >
> > Actually I think that is a bug in the QEMU implementation. We cannot
> > let that happen otherwise the guest is never going to let the memory
> > go. If we have to abort the migration we should be calling
> > virtio_balloon_free_page_done(), probably in response to something
> > like PRECOPY_NOTIFY_CLEANUP?
> >
> > The problem is I am not that familiar with the migration process
> > itself within QEMU so I am not sure which labels mean what in the
> > notifier, but we should be signaling DONE if we are either going to
> > abort the migration or if we completed it and are now on the new
> > system.
> >
>
> We are starting to see way too many issues in QEMU code that are hard to
> tackle.
>
> 1. The issue with hinted pages not being 0 or the old content when
> re-accessed, due to the way they are skipped during migration. MST
> called that a memory corruption and that this would be broken. Very hard
> to fix, maybe impossible.
>
> 2. Some cases (migration failing while we are only sending memory) not
> setting the status to DONE. The handling is just a mess.
>
> 3. Take a look at that asynchronous iothread handling.
> virtio_ballloon_get_free_page_hints(). I think you can trick it into
> doing nasty things while you are e.g., resetting the device. Also,
> virtio_ballloon_get_free_page_hints() will just busy-loop while waiting
> for requests. Just ugly. Also that block_iothread, just hard to grasp.
>
> Besides all the other issues I keep finding.
>
> At this point I'd just love to rip out the QEMU implement and let
> whoever wants to really have it try again. The Linux side seems to be in
> a better state.
>
> This feature is in an unmaintainable state in QEMU.
>
> @MST, is there something that speaks against ripping out the QEMU and
> Linux parts, documenting in the spec that this feature should not be used?
Have we mad a decision on this? I plan to resubmit my QEMU patch set
as I had a bug in patch 3 that needs to be addressed. I'm just
wondering if I should rebase on David's patches that fix free page
hinting, or if I should just replace the patch with one that removes
free page hinting?
Thanks.
- Alex
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 resubmit] virtio-balloon: Disable free page reporting if page poison reporting is not enabled
Do I need to resubmit this patch? It has been over two weeks now since
it was originally submitted, and a week and a half since I last sent
out an email following up. I'm just wondering if there is some list I
missed as I am assuming the maintainers and lists I contacted here are
correct? It looks like we are at RC7 now so I am worried this won't
make it in before LInus releases 5.7.
Thanks.
- Alex
On Fri, May 15, 2020 at 10:02 AM Alexander Duyck
wrote:
>
> Just following up. It has been a week since I submitted this. I was
> hoping we could get it in for 5.7 since this affects free page
> reporting which will be introduced with that kernel release.
>
> Thanks.
>
> - Alex
>
> On Fri, May 8, 2020 at 10:40 AM Alexander Duyck
> wrote:
> >
> > From: Alexander Duyck
> >
> > We should disable free page reporting if page poisoning is enabled but we
> > cannot report it via the balloon interface. This way we can avoid the
> > possibility of corrupting guest memory. Normally the page poisoning feature
> > should always be present when free page reporting is enabled on the
> > hypervisor, however this allows us to correctly handle a case of the
> > virtio-balloon device being possibly misconfigured.
> >
> > Fixes: 5d757c8d518d ("virtio-balloon: add support for providing free page
> > reports to host")
> > Acked-by: David Hildenbrand
> > Signed-off-by: Alexander Duyck
> > ---
> >
> > Changes since v1:
> > Originally this patch also modified free page hinting, that has been
> > removed.
> > Updated patch title and description.
> > Added a comment explaining reasoning for disabling free page reporting.
> >
> > Resbumitting v2 w/ Ack from David Hildebrand.
> >
> > drivers/virtio/virtio_balloon.c |9 -
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c
> > b/drivers/virtio/virtio_balloon.c
> > index 51086a5afdd4..1f157d2f4952 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -1107,11 +1107,18 @@ static int virtballoon_restore(struct virtio_device
> > *vdev)
> >
> > static int virtballoon_validate(struct virtio_device *vdev)
> > {
> > - /* Tell the host whether we care about poisoned pages. */
> > + /*
> > +* Inform the hypervisor that our pages are poisoned or
> > +* initialized. If we cannot do that then we should disable
> > +* page reporting as it could potentially change the contents
> > +* of our free pages.
> > +*/
> > if (!want_init_on_free() &&
> > (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) ||
> > !page_poisoning_enabled()))
> > __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_PAGE_POISON);
> > + else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON))
> > + __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING);
> >
> > __virtio_clear_bit(vdev, VIRTIO_F_IOMMU_PLATFORM);
> > return 0;
> >
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
On 26.05.20 16:50, Alexander Duyck wrote:
> On Tue, May 26, 2020 at 1:24 AM David Hildenbrand wrote:
>>
>> On 20.05.20 18:25, Alexander Duyck wrote:
>>> On Wed, May 20, 2020 at 2:28 AM David Hildenbrand wrote:
On 20.05.20 04:02, Alexander Duyck wrote:
> From: Alexander Duyck
>
> 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
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h.
[virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
On Tue, 26 May 2020 17:28:00 +0200 David Hildenbrand wrote: > On 26.05.20 16:50, Alexander Duyck wrote: > > On Tue, May 26, 2020 at 1:24 AM David Hildenbrand 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.) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
On Tue, May 26, 2020 at 8:38 AM Cornelia Huck wrote: > > On Tue, 26 May 2020 17:28:00 +0200 > David Hildenbrand wrote: > > > On 26.05.20 16:50, Alexander Duyck wrote: > > > On Tue, May 26, 2020 at 1:24 AM David Hildenbrand > > > 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 - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v4 2/3] content: Document balloon feature free page reporting
From: Alexander Duyck
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
---
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
[virtio-dev] [PATCH v4 0/3] virtio-spec: Add documentation for recently added balloon features
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(-) -- - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v4 1/3] content: Document balloon feature page poison
From: Alexander Duyck
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
---
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 Ballo
[virtio-dev] [PATCH v4 3/3] content: Document balloon feature free page hints
From: Alexander Duyck
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
---
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_P
[virtio-dev] [PATCH v25 QEMU 1/3] virtio-balloon: Implement support for page poison reporting feature
From: Alexander Duyck
We need to make certain to advertise support for page poison reporting if
we want to actually get data on if the guest will be poisoning pages.
Add a value for reporting the poison value being used if page poisoning is
enabled in the guest. With this we can determine if we will need to skip
free page reporting when it is enabled in the future.
The value currently has no impact on existing balloon interfaces. In the
case of existing balloon interfaces the onus is on the guest driver to
reapply whatever poison is in place.
When we add free page reporting the poison value is used to determine if
we can perform in-place page reporting. The expectation is that a reported
page will already contain the value specified by the poison, and the
reporting of the page should not change that value.
Acked-by: David Hildenbrand
Signed-off-by: Alexander Duyck
---
hw/core/machine.c |4 +++-
hw/virtio/virtio-balloon.c | 29 +
include/hw/virtio/virtio-balloon.h |1 +
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index bb3a7b18b193..9eca7d8c9bfe 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,7 +28,9 @@
#include "hw/mem/nvdimm.h"
#include "migration/vmstate.h"
-GlobalProperty hw_compat_5_0[] = {};
+GlobalProperty hw_compat_5_0[] = {
+{ "virtio-balloon-device", "page-poison", "false" },
+};
const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
GlobalProperty hw_compat_4_2[] = {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 065cd450f10f..26f6a7ca2e35 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -634,6 +634,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev,
uint8_t *config_data)
config.num_pages = cpu_to_le32(dev->num_pages);
config.actual = cpu_to_le32(dev->actual);
+config.poison_val = cpu_to_le32(dev->poison_val);
if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
config.free_page_report_cmd_id =
@@ -683,6 +684,14 @@ static ram_addr_t get_current_ram_size(void)
return size;
}
+static bool virtio_balloon_page_poison_support(void *opaque)
+{
+VirtIOBalloon *s = opaque;
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON);
+}
+
static void virtio_balloon_set_config(VirtIODevice *vdev,
const uint8_t *config_data)
{
@@ -697,6 +706,10 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
qapi_event_send_balloon_change(vm_ram_size -
((ram_addr_t) dev->actual <<
VIRTIO_BALLOON_PFN_SHIFT));
}
+dev->poison_val = 0;
+if (virtio_balloon_page_poison_support(dev)) {
+dev->poison_val = le32_to_cpu(config.poison_val);
+}
trace_virtio_balloon_set_config(dev->actual, oldactual);
}
@@ -755,6 +768,17 @@ static const VMStateDescription
vmstate_virtio_balloon_free_page_report = {
}
};
+static const VMStateDescription vmstate_virtio_balloon_page_poison = {
+.name = "vitio-balloon-device/page-poison",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = virtio_balloon_page_poison_support,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(poison_val, VirtIOBalloon),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_virtio_balloon_device = {
.name = "virtio-balloon-device",
.version_id = 1,
@@ -767,6 +791,7 @@ static const VMStateDescription
vmstate_virtio_balloon_device = {
},
.subsections = (const VMStateDescription * []) {
&vmstate_virtio_balloon_free_page_report,
+&vmstate_virtio_balloon_page_poison,
NULL
}
};
@@ -854,6 +879,8 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
g_free(s->stats_vq_elem);
s->stats_vq_elem = NULL;
}
+
+s->poison_val = 0;
}
static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
@@ -916,6 +943,8 @@ static Property virtio_balloon_properties[] = {
VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+DEFINE_PROP_BIT("page-poison", VirtIOBalloon, host_features,
+VIRTIO_BALLOON_F_PAGE_POISON, true),
/* QEMU 4.0 accidentally changed the config size even when free-page-hint
* is disabled, resulting in QEMU 3.1 migration incompatibility. This
* property retains this quirk for QEMU 4.1 machine types.
diff --git a/include/hw/virtio/virtio-balloon.h
b/include/hw/virtio/virtio-balloon.h
index d1c968d2376e..7fe78e5c14d7 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -70,6 +70,7 @@ typedef struct VirtIOBalloon {
uint32_t host_features;
[virtio-dev] [PATCH v25 QEMU 2/3] virtio-balloon: Provide an interface for free page reporting
From: Alexander Duyck
Add support for free page reporting. The idea is to function very similar
to how the balloon works in that we basically end up madvising the page as
not being used. However we don't really need to bother with any deflate
type logic since the page will be faulted back into the guest when it is
read or written to.
This provides a new way of letting the guest proactively report free
pages to the hypervisor, so the hypervisor can reuse them. In contrast to
inflate/deflate that is triggered via the hypervisor explicitly.
Acked-by: David Hildenbrand
Signed-off-by: Alexander Duyck
---
hw/virtio/virtio-balloon.c | 72
include/hw/virtio/virtio-balloon.h |2 +
2 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 26f6a7ca2e35..3e2ac1104b5f 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -321,6 +321,67 @@ static void balloon_stats_set_poll_interval(Object *obj,
Visitor *v,
balloon_stats_change_timer(s, 0);
}
+static void virtio_balloon_handle_report(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
+VirtQueueElement *elem;
+
+while ((elem = virtqueue_pop(vq, sizeof(VirtQueueElement {
+unsigned int i;
+
+/*
+ * When we discard the page it has the effect of removing the page
+ * from the hypervisor itself and causing it to be zeroed when it
+ * is returned to us. So we must not discard the page if it is
+ * accessible by another device or process, or if the guest is
+ * expecting it to retain a non-zero value.
+ */
+if (qemu_balloon_is_inhibited() || dev->poison_val) {
+goto skip_element;
+}
+
+for (i = 0; i < elem->in_num; i++) {
+void *addr = elem->in_sg[i].iov_base;
+size_t size = elem->in_sg[i].iov_len;
+ram_addr_t ram_offset;
+RAMBlock *rb;
+
+/*
+ * There is no need to check the memory section to see if
+ * it is ram/readonly/romd like there is for handle_output
+ * below. If the region is not meant to be written to then
+ * address_space_map will have allocated a bounce buffer
+ * and it will be freed in address_space_unmap and trigger
+ * and unassigned_mem_write before failing to copy over the
+ * buffer. If more than one bad descriptor is provided it
+ * will return NULL after the first bounce buffer and fail
+ * to map any resources.
+ */
+rb = qemu_ram_block_from_host(addr, false, &ram_offset);
+if (!rb) {
+trace_virtio_balloon_bad_addr(elem->in_addr[i]);
+continue;
+}
+
+/*
+ * For now we will simply ignore unaligned memory regions, or
+ * regions that overrun the end of the RAMBlock.
+ */
+if (!QEMU_IS_ALIGNED(ram_offset | size, qemu_ram_pagesize(rb)) ||
+(ram_offset + size) > qemu_ram_get_used_length(rb)) {
+continue;
+}
+
+ram_block_discard_range(rb, ram_offset, size);
+}
+
+skip_element:
+virtqueue_push(vq, elem, 0);
+virtio_notify(vdev, vq);
+g_free(elem);
+}
+}
+
static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
@@ -841,6 +902,12 @@ static void virtio_balloon_device_realize(DeviceState
*dev, Error **errp)
virtio_error(vdev, "iothread is missing");
}
}
+
+if (virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_REPORTING)) {
+s->reporting_vq = virtio_add_queue(vdev, 32,
+ virtio_balloon_handle_report);
+}
+
reset_stats(s);
}
@@ -863,6 +930,9 @@ static void virtio_balloon_device_unrealize(DeviceState
*dev)
if (s->free_page_vq) {
virtio_delete_queue(s->free_page_vq);
}
+if (s->reporting_vq) {
+virtio_delete_queue(s->reporting_vq);
+}
virtio_cleanup(vdev);
}
@@ -945,6 +1015,8 @@ static Property virtio_balloon_properties[] = {
VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
DEFINE_PROP_BIT("page-poison", VirtIOBalloon, host_features,
VIRTIO_BALLOON_F_PAGE_POISON, true),
+DEFINE_PROP_BIT("free-page-reporting", VirtIOBalloon, host_features,
+VIRTIO_BALLOON_F_REPORTING, false),
/* QEMU 4.0 accidentally changed the config size even when free-page-hint
* is disabled, resulting in QEMU 3.1 migration incompatibility. This
* property retains this quirk for QEMU 4.1 machine types.
diff --git a/include/hw/virtio/virtio-balloon.h
b/include/hw/virtio/virtio-balloon.h
index 7fe78e5c14d7..d49fef00cef2 100
[virtio-dev] [PATCH v25 QEMU 0/3] virtio-balloon: add support for page poison and free page reporting
This series provides an asynchronous means of reporting free guest pages
to QEMU through virtio-balloon so that the memory associated with those
pages can be dropped and reused by other processes and/or guests on the
host. Using this it is possible to avoid unnecessary I/O to disk and
greatly improve performance in the case of memory overcommit on the host.
I originally submitted this patch series back on February 11th 2020[1],
but at that time I was focused primarily on the kernel portion of this
patch set. However as of April 7th those patches are now included in
Linus's kernel tree[2] and so I am submitting the QEMU pieces for
inclusion.
[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
Changes from v17:
Fixed typo in patch 1 title
Addressed white-space issues reported via checkpatch
Added braces {} for two if statements to match expected coding style
Changes from v18:
Updated patches 2 and 3 based on input from dhildenb
Added comment to patch 2 describing what keeps us from reporting a bad page
Added patch to address issue with ROM devices being directly writable
Changes from v19:
Added std-headers change to match changes pushed for linux kernel headers
Added patch to remove "report" from page hinting code paths
Updated comment to better explain why we disable hints w/ page poisoning
Removed code that was modifying config size for poison vs hinting
Dropped x-page-poison property
Added code to bounds check the reported region vs the RAM block
Dropped patch for ROM devices as that was already pulled in by Paolo
Changes from v20:
Rearranged patches to push Linux header sync patches to front
Removed association between free page hinting and VIRTIO_BALLOON_F_PAGE_POISON
Added code to enable VIRTIO_BALLOON_F_PAGE_POISON if page reporting is enabled
Fixed possible resource leak if poison or qemu_balloon_is_inhibited return true
Changes from v21:
Added ack for patch 3
Rewrote patch description for page poison reporting feature
Made page-poison independent property and set to enabled by default
Added logic to migrate poison_val
Added several comments in code to better explain features
Switched free-page-reporting property to disabled by default
Changes from v22:
Added ack for patches 4 & 5
Added additional comment fixes in patch 3 to remove "reporting" reference
Renamed rvq in patch 5 to reporting_vq to better match linux kernel
Moved call adding reporting_vq to after free_page_vq
Changes from v23:
Rebased on latest QEMU
Dropped patches 1 & 2 as Linux kernel headers were synced
Added compat machine code for page-poison feature
Changes from v24:
Moved free page hinting rename to end of set as feature may be removed entirely
Added code to cleanup reporting_vq
---
Alexander Duyck (3):
virtio-balloon: Implement support for page poison reporting feature
virtio-balloon: Provide an interface for free page reporting
virtio-balloon: Replace free page hinting references to 'report' with
'hint'
hw/core/machine.c |4 +
hw/virtio/virtio-balloon.c | 179
include/hw/virtio/virtio-balloon.h | 23 ++---
3 files changed, 155 insertions(+), 51 deletions(-)
--
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v25 QEMU 3/3] virtio-balloon: Replace free page hinting references to 'report' with 'hint'
From: Alexander Duyck
In an upcoming patch a feature named Free Page Reporting is about to be
added. In order to avoid any confusion we should drop the use of the word
'report' when referring to Free Page Hinting. So what this patch does is go
through and replace all instances of 'report' with 'hint" when we are
referring to free page hinting.
Acked-by: David Hildenbrand
Signed-off-by: Alexander Duyck
---
hw/virtio/virtio-balloon.c | 78 ++--
include/hw/virtio/virtio-balloon.h | 20 +
2 files changed, 49 insertions(+), 49 deletions(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 3e2ac1104b5f..dc15409b0bb6 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -527,21 +527,21 @@ static bool get_free_page_hints(VirtIOBalloon *dev)
ret = false;
goto out;
}
-if (id == dev->free_page_report_cmd_id) {
-dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
+if (id == dev->free_page_hint_cmd_id) {
+dev->free_page_hint_status = FREE_PAGE_HINT_S_START;
} else {
/*
* Stop the optimization only when it has started. This
* avoids a stale stop sign for the previous command.
*/
-if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
-dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+if (dev->free_page_hint_status == FREE_PAGE_HINT_S_START) {
+dev->free_page_hint_status = FREE_PAGE_HINT_S_STOP;
}
}
}
if (elem->in_num) {
-if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) {
+if (dev->free_page_hint_status == FREE_PAGE_HINT_S_START) {
qemu_guest_free_page_hint(elem->in_sg[0].iov_base,
elem->in_sg[0].iov_len);
}
@@ -567,11 +567,11 @@ static void virtio_ballloon_get_free_page_hints(void
*opaque)
qemu_mutex_unlock(&dev->free_page_lock);
virtio_notify(vdev, vq);
/*
- * Start to poll the vq once the reporting started. Otherwise, continue
+ * Start to poll the vq once the hinting started. Otherwise, continue
* only when there are entries on the vq, which need to be given back.
*/
} while (continue_to_get_hints ||
- dev->free_page_report_status == FREE_PAGE_REPORT_S_START);
+ dev->free_page_hint_status == FREE_PAGE_HINT_S_START);
virtio_queue_set_notification(vq, 1);
}
@@ -592,14 +592,14 @@ static void virtio_balloon_free_page_start(VirtIOBalloon
*s)
return;
}
-if (s->free_page_report_cmd_id == UINT_MAX) {
-s->free_page_report_cmd_id =
- VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
+if (s->free_page_hint_cmd_id == UINT_MAX) {
+s->free_page_hint_cmd_id =
+ VIRTIO_BALLOON_FREE_PAGE_HINT_CMD_ID_MIN;
} else {
-s->free_page_report_cmd_id++;
+s->free_page_hint_cmd_id++;
}
-s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED;
+s->free_page_hint_status = FREE_PAGE_HINT_S_REQUESTED;
virtio_notify_config(vdev);
}
@@ -607,18 +607,18 @@ static void virtio_balloon_free_page_stop(VirtIOBalloon
*s)
{
VirtIODevice *vdev = VIRTIO_DEVICE(s);
-if (s->free_page_report_status != FREE_PAGE_REPORT_S_STOP) {
+if (s->free_page_hint_status != FREE_PAGE_HINT_S_STOP) {
/*
* The lock also guarantees us that the
* virtio_ballloon_get_free_page_hints exits after the
- * free_page_report_status is set to S_STOP.
+ * free_page_hint_status is set to S_STOP.
*/
qemu_mutex_lock(&s->free_page_lock);
/*
- * The guest hasn't done the reporting, so host sends a notification
- * to the guest to actively stop the reporting.
+ * The guest isn't done hinting, so send a notification
+ * to the guest to actively stop the hinting.
*/
-s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
+s->free_page_hint_status = FREE_PAGE_HINT_S_STOP;
qemu_mutex_unlock(&s->free_page_lock);
virtio_notify_config(vdev);
}
@@ -628,15 +628,15 @@ static void virtio_balloon_free_page_done(VirtIOBalloon
*s)
{
VirtIODevice *vdev = VIRTIO_DEVICE(s);
-s->free_page_report_status = FREE_PAGE_REPORT_S_DONE;
+s->free_page_hint_status = FREE_PAGE_HINT_S_DONE;
virtio_notify_config(vdev);
}
static int
-virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data)
+virtio_balloon_free_page_hint_notify(NotifierWithReturn *n, void *data)
{
VirtIOBalloon *dev = container_of(n, VirtIOBalloon,
- free_page_report_notify);
+ free_page_hint_notify);
VirtIODevice *vdev =
[virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
On Tue, 26 May 2020 20:20:21 -0700 Alexander Duyck wrote: > On Tue, May 26, 2020 at 8:38 AM Cornelia Huck wrote: > > > > On Tue, 26 May 2020 17:28:00 +0200 > > David Hildenbrand wrote: > > > > > On 26.05.20 16:50, Alexander Duyck wrote: > > > > On Tue, May 26, 2020 at 1:24 AM David Hildenbrand > > > > 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. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [virtio-comment] [PATCH v3 2/3] content: Document balloon feature page poison
On 05/26/2020 11:38 PM, Cornelia Huck wrote: On Tue, 26 May 2020 17:28:00 +0200 David Hildenbrand wrote: On 26.05.20 16:50, Alexander Duyck wrote: On Tue, May 26, 2020 at 1:24 AM David Hildenbrand 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 - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
