[RFC PATCH v2 0/5] virtio-balloon: Working Set Reporting

[RFC PATCH v2 0/5] virtio-balloon: Working Set Reporting

[T.J. Alumbaugh]

This is the device implementation for the proposed expanded balloon feature
described here:

https://lore.kernel.org/linux-mm/20230509185419.1088297-1-yuanchu@google.com/

This series has a fixed number of "bins" for the working set report, but this is
not a constraint of the system. The bin number is fixed at device realization
time (in other implementations it is specified as a command line argument). Once
that number is fixed, this determines the correct number of bin intervals to
pass to the QMP/HMP function 'working_set_config'. Any feedback on how to
properly construct that function for this use case (passing a variable length
list?) would be appreciated.

New in V2:
=========

- Patch series is now: header file changes, device changes, QMP changes, HMP
chagnes, and migration changes.

- Exmaple usages of QMP and HMP interface are in their respective commit
messages.

- "ws" -> "working_set" throughout

Motivation
==========
As mentioned in the above message, the use case is a host with overcommitted
memory and 1 or more VMs. The goal is to get both timely and accurate
information on overall memory utilization in order to drive appropriate
reclaim activities, since in some client device use cases a VM might need a
significant fraction of the overall memory for a period of time, but then
enter a quiet period that results in a large number of cold pages in the
guest.

The balloon device now has a number of features to assist in sharing memory
resources amongst the guests and host (e.g free page hinting, stats, free page
reporting). As mentioned in slide 12 in [1], the balloon doesn't have a good
mechanism to drive the reclaim of guest cache. Our use case includes both
typical page cache as well as "application caches" with memory that should be
discarded in times of system-wide memory pressure. In some cases, virtio-pmem
can be a method for host control of guest cache but there are undesirable
security implications.

Working Set Reporting
=====================
The patch series here includes:

 - Actual device implementation for VIRTIO_F_WS_REPORTING to standardize the
   configuration and communication of Working Set reports from the guest. This
   includes a notification virtqueue for receiving config information and
   requests for a report (a feature which could be expanded for additional use
   cases) and a virtqueue for the actual report from the driver.

 - QMP changes so that a controller program can use the existing QEMU socket
   mechanism to configure and request WS reports and then read the reports as
   a JSON property on the balloon.

Working Set reporting in the balloon provides:

 - an accurate picture of current memory utilization in the guest
 - event driven reporting (with configurable rate limiting) to deliver reports
   during times of memory pressure.

The reporting mechanism can be combined with a domain-specific balloon policy
to drive the separate reclaim activities in a coordinated fashion.

TODOs:
======
 -  A synchronization mechanism must be added to the functions that send WS
    Config and WS Request, otherwise concurrent callers (through QMP) can mix
    messages on the virtqueue sending the data to the driver.

 - The device currently has a hard-coded setting of 4 'bins' for a Working Set
   report, whereas the specification calls for anywhere between 2 and 16.

 - A WS_EVENT notification through QMP should include the actual report,
   whereas right now we query for that information right after a WS_EVENT is
   received.

References:

[1] https://kvmforum2020.sched.com/event/eE4U/virtio-balloonpmemmem-managing-guest-memory-david-hildenbrand-michael-s-tsirkin-red-hat

T.J. Alumbaugh (5):
  virtio-balloon: Add Working Set Reporting feature
  virtio-balloon: device has Working Set Reporting
  virtio-balloon: Add QMP functions for Working Set
  virtio-balloon: Add HMP functions for Working Set
  virtio-balloon: Migration of working set config

 hmp-commands.hx                               |  26 ++
 hw/core/machine-hmp-cmds.c                    |  21 ++
 hw/virtio/virtio-balloon-pci.c                |   2 +
 hw/virtio/virtio-balloon.c                    | 239 +++++++++++++++++-
 include/hw/virtio/virtio-balloon.h            |  13 +-
 include/monitor/hmp.h                         |   2 +
 .../standard-headers/linux/virtio_balloon.h   |  20 ++
 include/sysemu/balloon.h                      |   9 +-
 monitor/monitor.c                             |   1 +
 qapi/machine.json                             |  66 +++++
 qapi/misc.json                                |  26 ++
 softmmu/balloon.c                             |  31 ++-
 12 files changed, 449 insertions(+), 7 deletions(-)

-- 
2.41.0.rc0.172.g3f132b7071-goog

[RFC PATCH v2 1/5] virtio-balloon: Add Working Set Reporting feature

[T.J. Alumbaugh]

Balloon header includes:
 - feature bit for Working Set Reporting
 - number of Working Set bins member in balloon config
 - types for communicating Working Set information

Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
---
 .../standard-headers/linux/virtio_balloon.h   | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
index f343bfefd8..df61eaceee 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -37,6 +37,7 @@
 #define VIRTIO_BALLOON_F_FREE_PAGE_HINT	3 /* VQ to report free pages */
 #define VIRTIO_BALLOON_F_PAGE_POISON	4 /* Guest is using page poisoning */
 #define VIRTIO_BALLOON_F_REPORTING	5 /* Page reporting virtqueue */
+#define VIRTIO_BALLOON_F_WS_REPORTING	6 /* Working set report virtqueues */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -59,6 +60,9 @@ struct virtio_balloon_config {
 	};
 	/* Stores PAGE_POISON if page poisoning is in use */
 	uint32_t poison_val;
+	/* Stores the number of histogram bins if WS reporting in use */
+	uint8_t working_set_num_bins;
+	uint8_t padding[3];
 };
 
 #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
@@ -116,4 +120,20 @@ struct virtio_balloon_stat {
 	__virtio64 val;
 } QEMU_PACKED;
 
+enum virtio_balloon_working_set_op {
+    VIRTIO_BALLOON_WS_REQUEST = 1, /* a Working Set request from the host */
+    VIRTIO_BALLOON_WS_CONFIG = 2,  /* a Working Set config from the host */
+};
+
+struct virtio_balloon_working_set {
+	/* A tag for additional metadata */
+	__virtio16 tag;
+	/* The NUMA node for this report. */
+	__virtio16 node_id;
+	uint8_t reserved[4];
+	__virtio64 idle_age_ms;
+	/* A bin each for anonymous and file-backed memory. */
+	__virtio64 memory_size_bytes[2];
+} QEMU_PACKED;
+
 #endif /* _LINUX_VIRTIO_BALLOON_H */
-- 
2.41.0.rc0.172.g3f132b7071-goog

[RFC PATCH v2 2/5] virtio-balloon: device has Working Set Reporting

[T.J. Alumbaugh]

 - working_set_vq to receive Working Set reports from guest
 - notification_vq to send config or request to guest
 - add working set as object property on device

Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
---
 hw/virtio/virtio-balloon.c         | 164 ++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-balloon.h |  13 ++-
 qapi/misc.json                     |  26 +++++
 3 files changed, 200 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index d004cf29d2..23481e51b8 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -27,6 +27,7 @@
 #include "exec/address-spaces.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-machine.h"
+#include "qapi/qapi-visit-misc.h"
 #include "qapi/visitor.h"
 #include "trace.h"
 #include "qemu/error-report.h"
@@ -169,6 +170,119 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
     }
 }
 
+/*
+ * reset_working_set - Mark all items in the array as unset
+ *
+ * This function needs to be called at device initialization and
+ * whenever a new Working Set config is specified.
+ */
+static inline void reset_working_set(VirtIOBalloon *dev)
+{
+    int i;
+    for (i = 0; i < VIRTIO_BALLOON_WS_NR_BINS; i++) {
+        dev->ws[i].idle_age = 0;
+        if (dev->ws[i].memory_size_bytes) {
+            dev->ws[i].memory_size_bytes->anon = 0;
+            dev->ws[i].memory_size_bytes->file = 0;
+        } else {
+            dev->ws[i].memory_size_bytes = g_malloc0(sizeof(MemoryBin));
+        }
+    }
+}
+
+static void virtio_balloon_receive_working_set(VirtIODevice *vdev,
+                                               VirtQueue *vq)
+{
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    VirtQueueElement *elem;
+    VirtIOBalloonWorkingSet ws;
+    size_t offset = 0;
+    int count = 0;
+
+    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+    if (!elem) {
+        return;
+    }
+
+    if (s->working_set_vq_elem != NULL) {
+        /* This should never happen if the driver follows the spec. */
+        virtqueue_push(vq, s->working_set_vq_elem, 0);
+        virtio_notify(vdev, vq);
+        g_free(s->working_set_vq_elem);
+    }
+
+    s->working_set_vq_elem = elem;
+
+    /* Initialize the Working Set to get rid of any stale values. */
+    reset_working_set(s);
+
+    while (iov_to_buf(elem->out_sg, elem->out_num, offset, &ws,
+                      sizeof(ws)) == sizeof(ws)) {
+        uint64_t idle_age_ms = virtio_tswap64(vdev, ws.idle_age_ms);
+        uint64_t bytes_anon = virtio_tswap64(vdev, ws.memory_size_bytes[0]);
+        uint64_t bytes_file = virtio_tswap64(vdev, ws.memory_size_bytes[1]);
+        s->ws[count].idle_age = idle_age_ms;
+        s->ws[count].memory_size_bytes->anon = bytes_anon;
+        s->ws[count].memory_size_bytes->file = bytes_file;
+        offset += sizeof(ws);
+        count++;
+    }
+}
+
+static __attribute__((unused)) void virtio_balloon_send_working_set_request(
+    VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtQueueElement *elem;
+    size_t sz = 0;
+    uint16_t tag = 0;
+
+    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+    if (!elem) {
+        return;
+    }
+    tag = VIRTIO_BALLOON_WS_REQUEST;
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &tag, sizeof(tag));
+    assert(sz == sizeof(tag));
+    virtqueue_push(vq, elem, sz);
+    virtio_notify(vdev, vq);
+    g_free(elem);
+}
+
+static __attribute__((unused)) void virtio_balloon_send_working_set_config(
+    VirtIODevice *vdev, VirtQueue *vq,
+    uint64_t i0, uint64_t i1, uint64_t i2,
+    uint64_t refresh, uint64_t report)
+{
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    VirtQueueElement *elem;
+    uint16_t tag = 0;
+    size_t sz = 0;
+    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+    if (!elem) {
+        return;
+    }
+
+    tag = VIRTIO_BALLOON_WS_CONFIG;
+    s->ws_intervals[0] = i0;
+    s->ws_intervals[1] = i1;
+    s->ws_intervals[2] = i2;
+    s->ws_refresh_threshold = refresh;
+    s->ws_report_threshold = report;
+
+    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &tag, sizeof(tag));
+    assert(sz == sizeof(uint16_t));
+    sz += iov_from_buf(elem->in_sg, elem->in_num, sz, s->ws_intervals,
+                       (VIRTIO_BALLOON_WS_NR_BINS - 1) * \
+                       sizeof(s->ws_intervals[0]));
+    sz += iov_from_buf(elem->in_sg, elem->in_num, sz, &s->ws_refresh_threshold,
+                       sizeof(uint64_t));
+    sz += iov_from_buf(elem->in_sg, elem->in_num, sz, &s->ws_report_threshold,
+                       sizeof(uint64_t));
+    virtqueue_push(vq, elem, sz);
+    virtio_notify(vdev, vq);
+    g_free(elem);
+}
+
 static const char *balloon_stat_names[] = {
    [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
    [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
@@ -697,8 +811,11 @@ static size_t virtio_balloon_config_size(VirtIOBalloon *s)
     if (s->qemu_4_0_config_size) {
         return sizeof(struct virtio_balloon_config);
     }
+    if (virtio_has_feature(features, VIRTIO_BALLOON_F_WS_REPORTING)) {
+        return sizeof(struct virtio_balloon_config);
+   }
     if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
-        return sizeof(struct virtio_balloon_config);
+        return offsetof(struct virtio_balloon_config, working_set_num_bins);
     }
     if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
         return offsetof(struct virtio_balloon_config, poison_val);
@@ -714,6 +831,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);
+    config.working_set_num_bins = (uint8_t) VIRTIO_BALLOON_WS_NR_BINS;
 
     if (dev->free_page_hint_status == FREE_PAGE_HINT_S_REQUESTED) {
         config.free_page_hint_cmd_id =
@@ -748,6 +866,14 @@ static bool virtio_balloon_page_poison_support(void *opaque)
     return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON);
 }
 
+static bool virtio_balloon_working_set_reporting_support(void *opaque)
+{
+    VirtIOBalloon *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_WS_REPORTING);
+}
+
 static void virtio_balloon_set_config(VirtIODevice *vdev,
                                       const uint8_t *config_data)
 {
@@ -766,6 +892,10 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
     if (virtio_balloon_page_poison_support(dev)) {
         dev->poison_val = le32_to_cpu(config.poison_val);
     }
+    dev->working_set_num_bins = 0;
+    if (virtio_balloon_working_set_reporting_support(dev)) {
+        dev->working_set_num_bins = config.working_set_num_bins;
+    }
     trace_virtio_balloon_set_config(dev->actual, oldactual);
 }
 
@@ -775,6 +905,7 @@ static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
     VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
     f |= dev->host_features;
     virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
+    virtio_add_feature(&f, VIRTIO_BALLOON_F_WS_REPORTING);
 
     return f;
 }
@@ -896,6 +1027,13 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
                                            virtio_balloon_handle_report);
     }
 
+    if (virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_WS_REPORTING)) {
+        s->working_set_vq = virtio_add_queue(vdev, 32,
+            virtio_balloon_receive_working_set);
+        s->notification_vq = virtio_add_queue(vdev, 32, NULL);
+    }
+
+    reset_working_set(s);
     reset_stats(s);
 }
 
@@ -922,6 +1060,12 @@ static void virtio_balloon_device_unrealize(DeviceState *dev)
     if (s->reporting_vq) {
         virtio_delete_queue(s->reporting_vq);
     }
+    if (s->working_set_vq) {
+        virtio_delete_queue(s->working_set_vq);
+    }
+    if (s->notification_vq) {
+        virtio_delete_queue(s->notification_vq);
+    }
     virtio_cleanup(vdev);
 }
 
@@ -938,7 +1082,11 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
         g_free(s->stats_vq_elem);
         s->stats_vq_elem = NULL;
     }
-
+    if (s->working_set_vq_elem != NULL) {
+        virtqueue_unpop(s->working_set_vq, s->working_set_vq_elem, 0);
+        g_free(s->working_set_vq_elem);
+        s->working_set_vq_elem = NULL;
+    }
     s->poison_val = 0;
 }
 
@@ -953,6 +1101,16 @@ static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
         virtio_balloon_receive_stats(vdev, s->svq);
     }
 
+    if (!s->working_set_vq_elem && vdev->vm_running &&
+        (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+         virtqueue_rewind(s->working_set_vq, 1)) {
+        /*
+         * poll working set queue for the element we have discarded when the VM
+         * was stopped
+         */
+        virtio_balloon_receive_working_set(vdev, s->working_set_vq);
+    }
+
     if (virtio_balloon_free_page_support(s)) {
         /*
          * The VM is woken up and the iothread was blocked, so signal it to
@@ -1011,6 +1169,8 @@ static Property virtio_balloon_properties[] = {
                     VIRTIO_BALLOON_F_PAGE_POISON, true),
     DEFINE_PROP_BIT("free-page-reporting", VirtIOBalloon, host_features,
                     VIRTIO_BALLOON_F_REPORTING, false),
+    DEFINE_PROP_BIT("working-set", VirtIOBalloon, host_features,
+                    VIRTIO_BALLOON_F_WS_REPORTING, 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 5139cf8ab6..a7a6655f50 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -17,6 +17,7 @@
 
 #include "standard-headers/linux/virtio_balloon.h"
 #include "hw/virtio/virtio.h"
+#include "qapi/qapi-types-misc.h"
 #include "sysemu/iothread.h"
 #include "qom/object.h"
 
@@ -25,7 +26,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOBalloon, VIRTIO_BALLOON)
 
 #define VIRTIO_BALLOON_FREE_PAGE_HINT_CMD_ID_MIN 0x80000000
 
+#define VIRTIO_BALLOON_WS_NR_BINS 4  /* Number of bins in Working Set report */
+
 typedef struct virtio_balloon_stat VirtIOBalloonStat;
+typedef struct virtio_balloon_working_set VirtIOBalloonWorkingSet;
 
 typedef struct virtio_balloon_stat_modern {
        uint16_t tag;
@@ -42,13 +46,19 @@ enum virtio_balloon_free_page_hint_status {
 
 struct VirtIOBalloon {
     VirtIODevice parent_obj;
-    VirtQueue *ivq, *dvq, *svq, *free_page_vq, *reporting_vq;
+    VirtQueue *ivq, *dvq, *svq, *free_page_vq, *reporting_vq, *working_set_vq,
+              *notification_vq;
     uint32_t free_page_hint_status;
     uint32_t num_pages;
     uint32_t actual;
     uint32_t free_page_hint_cmd_id;
     uint64_t stats[VIRTIO_BALLOON_S_NR];
+    WorkingSetInfo ws[VIRTIO_BALLOON_WS_NR_BINS];
+    uint64_t ws_intervals[VIRTIO_BALLOON_WS_NR_BINS - 1];
+    uint64_t ws_refresh_threshold;
+    uint64_t ws_report_threshold;
     VirtQueueElement *stats_vq_elem;
+    VirtQueueElement *working_set_vq_elem;
     size_t stats_vq_offset;
     QEMUTimer *stats_timer;
     IOThread *iothread;
@@ -71,6 +81,7 @@ struct VirtIOBalloon {
 
     bool qemu_4_0_config_size;
     uint32_t poison_val;
+    uint8_t working_set_num_bins;
 };
 
 #endif
diff --git a/qapi/misc.json b/qapi/misc.json
index ff070ec828..4ba8c74b64 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -550,6 +550,32 @@
  'returns': ['CommandLineOptionInfo'],
  'allow-preconfig': true}
 
+##
+# @MemoryBin:
+#
+# A bin of memory with a size in bytes. File-backed and
+# anonymous memory are tracked separately.
+#
+# @anon: number of bytes of anonymous memory
+# @file: number of bytes of file-backed memory
+##
+{ 'struct': 'MemoryBin',
+  'data': { 'anon': 'uint64',
+            'file': 'uint64' } }
+
+##
+# @WorkingSetInfo:
+#
+# A bin of memory of the given size that has been idle at most `idle-age` ms
+#
+# @idle-age: guest-relative time (in milliseconds)
+#
+# @memory-size-bytes: A MemoryBin with file and anon info.
+##
+{ 'struct': 'WorkingSetInfo',
+  'data': { 'idle-age': 'uint64',
+            'memory-size-bytes': 'MemoryBin' } }
+
 ##
 # @RTC_CHANGE:
 #
-- 
2.41.0.rc0.172.g3f132b7071-goog

[RFC PATCH v2 3/5] virtio-balloon: Add QMP functions for Working Set

[T.J. Alumbaugh]

  - Adds QMP function 'working-set-config'
  - Adds QMP function 'working-set-request'
  - Retrieve working set via 'guest-working-set' property on balloon

>> cat script.py

NAME = "name"
SOCKET = 'vm.sock'
BALLOON =  "/machine/peripheral/balloon0"

import json
import asyncio
from qemu.qmp import QMPClient

async def main():
    client = QMPClient(NAME)
    await client.connect(SOCKET)
    config = { "i0": 200, "i1": 800, "i2": 3000, "refresh": 750, "report": 1000 }
    await client.execute('working-set-config', config)
    await client.execute('working-set-request')
    property = {"path":BALLOON, "property":"guest-working-set"}
    res = await client.execute('qom-get', property)
    return res

if __name__ == "__main__":
    ret = asyncio.run(main())
    print(json.dumps(ret, indent=2))

>> (Execute qemu with flag '-qmp unix:path=vm.sock,server=on,wait=off'
>> (Perform normal activities on VM to exercise MM code)

>> python3 script.py
{
  "working_set": {
    "ws3": {
      "memory-size-bytes": {
        "anon": 890478592,
        "file": 1285832704
      },
      "idle-age": 4294967292
    },
    "ws2": {
      "memory-size-bytes": {
        "anon": 173465600,
        "file": 83353600
      },
      "idle-age": 3000
    },
    "ws1": {
      "memory-size-bytes": {
        "anon": 44236800,
        "file": 20889600
      },
      "idle-age": 800
    },
    "ws0": {
      "memory-size-bytes": {
        "anon": 14540800,
        "file": 6963200
      },
      "idle-age": 200
    }
  }
}

Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
---
 hw/virtio/virtio-balloon-pci.c |  2 +
 hw/virtio/virtio-balloon.c     | 67 ++++++++++++++++++++++++++++++++--
 include/sysemu/balloon.h       |  9 ++++-
 monitor/monitor.c              |  1 +
 qapi/machine.json              | 66 +++++++++++++++++++++++++++++++++
 softmmu/balloon.c              | 31 +++++++++++++++-
 6 files changed, 170 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
index ce2645ba71..7b781c8bab 100644
--- a/hw/virtio/virtio-balloon-pci.c
+++ b/hw/virtio/virtio-balloon-pci.c
@@ -68,6 +68,8 @@ static void virtio_balloon_pci_instance_init(Object *obj)
     object_property_add_alias(obj, "guest-stats-polling-interval",
                               OBJECT(&dev->vdev),
                               "guest-stats-polling-interval");
+    object_property_add_alias(obj, "guest-working-set", OBJECT(&dev->vdev),
+                              "guest-working-set");
 }
 
 static const VirtioPCIDeviceTypeInfo virtio_balloon_pci_info = {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 23481e51b8..a124d95534 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -229,7 +229,7 @@ static void virtio_balloon_receive_working_set(VirtIODevice *vdev,
     }
 }
 
-static __attribute__((unused)) void virtio_balloon_send_working_set_request(
+static void virtio_balloon_send_working_set_request(
     VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtQueueElement *elem;
@@ -248,7 +248,7 @@ static __attribute__((unused)) void virtio_balloon_send_working_set_request(
     g_free(elem);
 }
 
-static __attribute__((unused)) void virtio_balloon_send_working_set_config(
+static void virtio_balloon_send_working_set_config(
     VirtIODevice *vdev, VirtQueue *vq,
     uint64_t i0, uint64_t i1, uint64_t i2,
     uint64_t refresh, uint64_t report)
@@ -353,6 +353,43 @@ static void balloon_stats_poll_cb(void *opaque)
     s->stats_vq_elem = NULL;
 }
 
+static void balloon_working_set_get_all(Object *obj, Visitor *v,
+                                        const char *name, void *opaque,
+                                        Error **errp)
+{
+    Error *err = NULL;
+    VirtIOBalloon *s = VIRTIO_BALLOON(obj);
+    char ws_buf[4];
+    WorkingSetInfo *wsinfo;
+    int i;
+
+    if (!visit_start_struct(v, name, NULL, 0, &err)) {
+        goto out;
+    }
+
+    if (!visit_start_struct(v, "working_set", NULL, 0, &err)) {
+        goto out_end;
+    }
+    for (i = 0; i < VIRTIO_BALLOON_WS_NR_BINS; i++) {
+        wsinfo = s->ws + i;
+        sprintf(ws_buf, "ws%d", i);
+        if (!visit_type_WorkingSetInfo(v, ws_buf, &wsinfo, &err)) {
+            goto out_nested;
+        }
+    }
+    visit_check_struct(v, &err);
+out_nested:
+    visit_end_struct(v, NULL);
+
+    if (!err) {
+        visit_check_struct(v, &err);
+    }
+out_end:
+    visit_end_struct(v, NULL);
+out:
+    error_propagate(errp, err);
+}
+
 static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
 {
@@ -917,6 +954,25 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
                                              VIRTIO_BALLOON_PFN_SHIFT);
 }
 
+static void virtio_balloon_working_set_request(void *opaque)
+{
+    VirtIOBalloon *dev = VIRTIO_BALLOON(opaque);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+    virtio_balloon_send_working_set_request(vdev, dev->notification_vq);
+}
+
+static void virtio_balloon_working_set_config(void *opaque, uint64_t i0,
+                                              uint64_t i1, uint64_t i2,
+                                              uint64_t refresh, uint64_t report)
+{
+    VirtIOBalloon *dev = VIRTIO_BALLOON(opaque);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+    virtio_balloon_send_working_set_config(vdev, dev->notification_vq, i0, i1,
+                                           i2, refresh, report);
+}
+
 static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
 {
     VirtIOBalloon *dev = VIRTIO_BALLOON(opaque);
@@ -992,7 +1048,9 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
     virtio_init(vdev, VIRTIO_ID_BALLOON, virtio_balloon_config_size(s));
 
     ret = qemu_add_balloon_handler(virtio_balloon_to_target,
-                                   virtio_balloon_stat, s);
+                                   virtio_balloon_stat,
+                                   virtio_balloon_working_set_request,
+                                   virtio_balloon_working_set_config, s);
 
     if (ret < 0) {
         error_setg(errp, "Only one balloon device is supported");
@@ -1148,6 +1206,9 @@ static void virtio_balloon_instance_init(Object *obj)
                         balloon_stats_get_poll_interval,
                         balloon_stats_set_poll_interval,
                         NULL, NULL);
+
+    object_property_add(obj, "guest-working-set", "guest working set",
+                        balloon_working_set_get_all, NULL, NULL, NULL);
 }
 
 static const VMStateDescription vmstate_virtio_balloon = {
diff --git a/include/sysemu/balloon.h b/include/sysemu/balloon.h
index 867687b73a..1f504d1a31 100644
--- a/include/sysemu/balloon.h
+++ b/include/sysemu/balloon.h
@@ -18,10 +18,17 @@
 #include "qapi/qapi-types-machine.h"
 
 typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
+typedef void (QEMUBalloonWorkingSetRequest)(void *opaque);
+typedef void (QEMUBalloonWorkingSetConfig)(void *opaque, uint64_t i0,
+                                  uint64_t i1, uint64_t i2, uint64_t refresh,
+                                  uint64_t report);
 typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info);
 
 int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-                             QEMUBalloonStatus *stat_func, void *opaque);
+                             QEMUBalloonStatus *stat_func,
+                             QEMUBalloonWorkingSetRequest *ws_func,
+                             QEMUBalloonWorkingSetConfig *config_func,
+                             void *opaque);
 void qemu_remove_balloon_handler(void *opaque);
 
 #endif
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 602535696c..fad1b4aed5 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -333,6 +333,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
     [QAPI_EVENT_QUORUM_FAILURE]    = { 1000 * SCALE_MS },
     [QAPI_EVENT_VSERPORT_CHANGE]   = { 1000 * SCALE_MS },
     [QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE] = { 1000 * SCALE_MS },
+    [QAPI_EVENT_WORKING_SET_EVENT] = { 1000 * SCALE_MS },
 };
 
 /*
diff --git a/qapi/machine.json b/qapi/machine.json
index 37660d8f2a..5e03ff21e2 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1055,6 +1055,57 @@
 ##
 { 'command': 'balloon', 'data': {'value': 'int'} }
 
+##
+# @working-set-config:
+#
+# Specify the config parameters for Working Set reporting.
+#
+# @i0: the endpoint of the first interval (in ms)
+#
+# @i1: the endpoint of the second interval (in ms)
+#
+# @i2: the endpoint of the third interval (in ms)
+#
+# @refresh: the refresh threshold (in ms) for Working Set reporting
+#
+# @report: the report threshold (in ms) for Working Set reporting
+#
+# Returns: - Nothing on success
+#          - If no balloon device is present, DeviceNotActive
+#
+# Example:
+#
+# -> { "execute": "working-set-config",
+#                 "arguments": { "i0": 100,
+#                                "i1": 500,
+#                                "i2": 2000,
+#                                "refresh": 750,
+#                                "report": 1000 } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'working-set-config', 'data': {'i0': 'uint64',
+                                            'i1': 'uint64',
+                                            'i2': 'uint64',
+                                            'refresh': 'uint64',
+                                            'report': 'uint64'} }
+##
+# @working-set-request:
+#
+# Request the Working Set report from the guest.
+#
+# Returns: - Nothing on success
+#          - If no balloon device is present, DeviceNotActive
+#
+# Example:
+#
+# -> { "execute": "working-set-request", "arguments": {} }
+# <- { "return": {} }
+#
+##
+{ 'command': 'working-set-request', 'data': {} }
+
+
 ##
 # @BalloonInfo:
 #
@@ -1113,6 +1164,21 @@
 { 'event': 'BALLOON_CHANGE',
   'data': { 'actual': 'int' } }
 
+##
+# @WORKING_SET_EVENT:
+#
+# Emitted when the guest sends a new Working Set report.
+#
+# Note: this event is rate-limited.
+#
+# Example:
+#
+# <- { "event": "WORKING_SET_EVENT",
+#      "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
+#
+##
+{ 'event': 'WORKING_SET_EVENT' }
+
 ##
 # @MemoryInfo:
 #
diff --git a/softmmu/balloon.c b/softmmu/balloon.c
index e0e8969a4b..f27852949a 100644
--- a/softmmu/balloon.c
+++ b/softmmu/balloon.c
@@ -35,6 +35,8 @@
 
 static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
+static QEMUBalloonWorkingSetRequest *balloon_ws_request_fn;
+static QEMUBalloonWorkingSetConfig *balloon_ws_config_fn;
 static void *balloon_opaque;
 
 static bool have_balloon(Error **errp)
@@ -53,9 +55,13 @@ static bool have_balloon(Error **errp)
 }
 
 int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-                             QEMUBalloonStatus *stat_func, void *opaque)
+                             QEMUBalloonStatus *stat_func,
+                             QEMUBalloonWorkingSetRequest *ws_request_func,
+                             QEMUBalloonWorkingSetConfig *ws_config_func,
+                             void *opaque)
 {
-    if (balloon_event_fn || balloon_stat_fn || balloon_opaque) {
+    if (balloon_event_fn || balloon_stat_fn || balloon_ws_request_fn \
+        || balloon_ws_config_fn || balloon_opaque) {
         /* We're already registered one balloon handler.  How many can
          * a guest really have?
          */
@@ -63,6 +69,8 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
     }
     balloon_event_fn = event_func;
     balloon_stat_fn = stat_func;
+    balloon_ws_request_fn = ws_request_func;
+    balloon_ws_config_fn = ws_config_func;
     balloon_opaque = opaque;
     return 0;
 }
@@ -104,3 +112,22 @@ void qmp_balloon(int64_t target, Error **errp)
     trace_balloon_event(balloon_opaque, target);
     balloon_event_fn(balloon_opaque, target);
 }
+
+void qmp_working_set_request(Error **errp)
+{
+    if (!have_balloon(errp)) {
+        return;
+    }
+
+    balloon_ws_request_fn(balloon_opaque);
+}
+
+void qmp_working_set_config(uint64_t i0, uint64_t i1, uint64_t i2,
+                            uint64_t refresh, uint64_t report, Error **errp)
+{
+    if (!have_balloon(errp)) {
+        return;
+    }
+
+    balloon_ws_config_fn(balloon_opaque, i0, i1, i2, refresh, report);
+}
-- 
2.41.0.rc0.172.g3f132b7071-goog

[RFC PATCH v2 4/5] virtio-balloon: Add HMP functions for Working Set

[T.J. Alumbaugh]

- Adds 'working_set_request', 'working_set_config' HMP functions

Start qemu with '-monitor telnet:127.0.0.1:4444,server=on,wait=off'

>> telnet localhost 4444

(qemu) working_set_config 200 800 3000 750 1000
(qemu) working_set_request
(qemu) qom-get /machine/peripheral/balloon0 guest-working-set
{
    "working_set": {
        "ws3": {
            "memory-size-bytes": {
                "anon": 298287104,
                "file": 647041024
            },
            "idle-age": 4294967292
        },
        "ws2": {
            "memory-size-bytes": {
                "anon": 4505600,
                "file": 2252800
            },
            "idle-age": 3000
        },
        "ws1": {
            "memory-size-bytes": {
                "anon": 1228800,
                "file": 614400
            },
            "idle-age": 800
        },
        "ws0": {
            "memory-size-bytes": {
                "anon": 409600,
                "file": 204800
            },
            "idle-age": 200
        }
    }
}

Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
---
 hmp-commands.hx            | 26 ++++++++++++++++++++++++++
 hw/core/machine-hmp-cmds.c | 21 +++++++++++++++++++++
 include/monitor/hmp.h      |  2 ++
 3 files changed, 49 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2cbd0f77a0..8ed044b23f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1398,6 +1398,32 @@ SRST
   Request VM to change its memory allocation to *value* (in MB).
 ERST
 
+    {
+        .name       = "working_set_config",
+        .args_type  = "i0:i,i1:i,i2:i,refresh:i,report:i",
+        .params     = "bin intervals 0-2, refresh and report thresholds",
+        .help       = "Working Set intervals, refresh/report thresholds (ms)",
+        .cmd        = hmp_working_set_config,
+    },
+
+SRST
+``working_set_config``
+  Set the intervals (in ms), refresh, report thresholds for Working Set reporting
+ERST
+
+    {
+        .name       = "working_set_request",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Request the Working Set of the guest.",
+        .cmd        = hmp_working_set_request,
+    },
+
+SRST
+``working_set_request``
+  Request the Working Set from the guest.
+ERST
+
     {
         .name       = "set_link",
         .args_type  = "name:s,up:b",
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index c3e55ef9e9..3c0a7694a2 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -237,6 +237,27 @@ void hmp_balloon(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);
 }
 
+void hmp_working_set_request(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+
+    qmp_working_set_request(&err);
+    hmp_handle_error(mon, err);
+}
+
+void hmp_working_set_config(Monitor *mon, const QDict *qdict)
+{
+    uint64_t i0 = qdict_get_int(qdict, "i0");
+    uint64_t i1 = qdict_get_int(qdict, "i1");
+    uint64_t i2 = qdict_get_int(qdict, "i2");
+    uint64_t refresh = qdict_get_int(qdict, "refresh");
+    uint64_t report = qdict_get_int(qdict, "report");
+    Error *err = NULL;
+
+    qmp_working_set_config(i0, i1, i2, refresh, report, &err);
+    hmp_handle_error(mon, err);
+}
+
 void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 13f9a2dedb..a1e6c5e92a 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -59,6 +59,8 @@ void hmp_nmi(Monitor *mon, const QDict *qdict);
 void hmp_info_network(Monitor *mon, const QDict *qdict);
 void hmp_set_link(Monitor *mon, const QDict *qdict);
 void hmp_balloon(Monitor *mon, const QDict *qdict);
+void hmp_working_set_config(Monitor *mon, const QDict *qdict);
+void hmp_working_set_request(Monitor *mon, const QDict *qdict);
 void hmp_loadvm(Monitor *mon, const QDict *qdict);
 void hmp_savevm(Monitor *mon, const QDict *qdict);
 void hmp_delvm(Monitor *mon, const QDict *qdict);
-- 
2.41.0.rc0.172.g3f132b7071-goog

[RFC PATCH v2 5/5] virtio-balloon: Migration of working set config

[T.J. Alumbaugh]

Migrate working_set_num_bins through VMStateDescription.

Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
---
 hw/virtio/virtio-balloon.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index a124d95534..6e1646abfd 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -1022,6 +1022,17 @@ static const VMStateDescription vmstate_virtio_balloon_page_poison = {
     }
 };
 
+static const VMStateDescription vmstate_virtio_balloon_working_set_reporting = {
+    .name = "virtio-balloon-device/working-set-report",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = virtio_balloon_working_set_reporting_support,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(working_set_num_bins, VirtIOBalloon),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_virtio_balloon_device = {
     .name = "virtio-balloon-device",
     .version_id = 1,
@@ -1035,6 +1046,7 @@ static const VMStateDescription vmstate_virtio_balloon_device = {
     .subsections = (const VMStateDescription * []) {
         &vmstate_virtio_balloon_free_page_hint,
         &vmstate_virtio_balloon_page_poison,
+        &vmstate_virtio_balloon_working_set_reporting,
         NULL
     }
 };
-- 
2.41.0.rc0.172.g3f132b7071-goog

Re: [RFC PATCH v2 2/5] virtio-balloon: device has Working Set Reporting

[Markus Armbruster]

"T.J. Alumbaugh" <talumbau@google.com> writes:

>  - working_set_vq to receive Working Set reports from guest
>  - notification_vq to send config or request to guest
>  - add working set as object property on device
>
> Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
> ---
>  hw/virtio/virtio-balloon.c         | 164 ++++++++++++++++++++++++++++-
>  include/hw/virtio/virtio-balloon.h |  13 ++-
>  qapi/misc.json                     |  26 +++++
>  3 files changed, 200 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index d004cf29d2..23481e51b8 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -27,6 +27,7 @@
>  #include "exec/address-spaces.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-machine.h"
> +#include "qapi/qapi-visit-misc.h"

Superfluous include.

>  #include "qapi/visitor.h"
>  #include "trace.h"
>  #include "qemu/error-report.h"
> @@ -169,6 +170,119 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
>      }
>  }
>  
> +/*
> + * reset_working_set - Mark all items in the array as unset
> + *
> + * This function needs to be called at device initialization and
> + * whenever a new Working Set config is specified.
> + */
> +static inline void reset_working_set(VirtIOBalloon *dev)

Why inline?

> +{
> +    int i;
> +    for (i = 0; i < VIRTIO_BALLOON_WS_NR_BINS; i++) {
> +        dev->ws[i].idle_age = 0;
> +        if (dev->ws[i].memory_size_bytes) {
> +            dev->ws[i].memory_size_bytes->anon = 0;
> +            dev->ws[i].memory_size_bytes->file = 0;
> +        } else {
> +            dev->ws[i].memory_size_bytes = g_malloc0(sizeof(MemoryBin));
> +        }
> +    }
> +}
> +
> +static void virtio_balloon_receive_working_set(VirtIODevice *vdev,
> +                                               VirtQueue *vq)
> +{
> +    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
> +    VirtQueueElement *elem;
> +    VirtIOBalloonWorkingSet ws;
> +    size_t offset = 0;
> +    int count = 0;
> +
> +    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> +    if (!elem) {
> +        return;
> +    }
> +
> +    if (s->working_set_vq_elem != NULL) {
> +        /* This should never happen if the driver follows the spec. */
> +        virtqueue_push(vq, s->working_set_vq_elem, 0);
> +        virtio_notify(vdev, vq);
> +        g_free(s->working_set_vq_elem);
> +    }
> +
> +    s->working_set_vq_elem = elem;
> +
> +    /* Initialize the Working Set to get rid of any stale values. */
> +    reset_working_set(s);
> +
> +    while (iov_to_buf(elem->out_sg, elem->out_num, offset, &ws,
> +                      sizeof(ws)) == sizeof(ws)) {
> +        uint64_t idle_age_ms = virtio_tswap64(vdev, ws.idle_age_ms);
> +        uint64_t bytes_anon = virtio_tswap64(vdev, ws.memory_size_bytes[0]);
> +        uint64_t bytes_file = virtio_tswap64(vdev, ws.memory_size_bytes[1]);
> +        s->ws[count].idle_age = idle_age_ms;
> +        s->ws[count].memory_size_bytes->anon = bytes_anon;
> +        s->ws[count].memory_size_bytes->file = bytes_file;
> +        offset += sizeof(ws);
> +        count++;
> +    }
> +}
> +
> +static __attribute__((unused)) void virtio_balloon_send_working_set_request(
> +    VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    VirtQueueElement *elem;
> +    size_t sz = 0;
> +    uint16_t tag = 0;
> +
> +    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> +    if (!elem) {
> +        return;
> +    }
> +    tag = VIRTIO_BALLOON_WS_REQUEST;
> +    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &tag, sizeof(tag));
> +    assert(sz == sizeof(tag));
> +    virtqueue_push(vq, elem, sz);
> +    virtio_notify(vdev, vq);
> +    g_free(elem);
> +}
> +
> +static __attribute__((unused)) void virtio_balloon_send_working_set_config(
> +    VirtIODevice *vdev, VirtQueue *vq,
> +    uint64_t i0, uint64_t i1, uint64_t i2,
> +    uint64_t refresh, uint64_t report)
> +{
> +    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
> +    VirtQueueElement *elem;
> +    uint16_t tag = 0;
> +    size_t sz = 0;
> +    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> +    if (!elem) {
> +        return;
> +    }
> +
> +    tag = VIRTIO_BALLOON_WS_CONFIG;
> +    s->ws_intervals[0] = i0;
> +    s->ws_intervals[1] = i1;
> +    s->ws_intervals[2] = i2;
> +    s->ws_refresh_threshold = refresh;
> +    s->ws_report_threshold = report;
> +
> +    sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &tag, sizeof(tag));
> +    assert(sz == sizeof(uint16_t));
> +    sz += iov_from_buf(elem->in_sg, elem->in_num, sz, s->ws_intervals,
> +                       (VIRTIO_BALLOON_WS_NR_BINS - 1) * \
> +                       sizeof(s->ws_intervals[0]));
> +    sz += iov_from_buf(elem->in_sg, elem->in_num, sz, &s->ws_refresh_threshold,
> +                       sizeof(uint64_t));
> +    sz += iov_from_buf(elem->in_sg, elem->in_num, sz, &s->ws_report_threshold,
> +                       sizeof(uint64_t));
> +    virtqueue_push(vq, elem, sz);
> +    virtio_notify(vdev, vq);
> +    g_free(elem);
> +}
> +
>  static const char *balloon_stat_names[] = {
>     [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
>     [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
> @@ -697,8 +811,11 @@ static size_t virtio_balloon_config_size(VirtIOBalloon *s)
>      if (s->qemu_4_0_config_size) {
>          return sizeof(struct virtio_balloon_config);
>      }
> +    if (virtio_has_feature(features, VIRTIO_BALLOON_F_WS_REPORTING)) {
> +        return sizeof(struct virtio_balloon_config);
> +   }
>      if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
> -        return sizeof(struct virtio_balloon_config);
> +        return offsetof(struct virtio_balloon_config, working_set_num_bins);
>      }
>      if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
>          return offsetof(struct virtio_balloon_config, poison_val);
> @@ -714,6 +831,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);
> +    config.working_set_num_bins = (uint8_t) VIRTIO_BALLOON_WS_NR_BINS;
>  
>      if (dev->free_page_hint_status == FREE_PAGE_HINT_S_REQUESTED) {
>          config.free_page_hint_cmd_id =
> @@ -748,6 +866,14 @@ static bool virtio_balloon_page_poison_support(void *opaque)
>      return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON);
>  }
>  
> +static bool virtio_balloon_working_set_reporting_support(void *opaque)
> +{
> +    VirtIOBalloon *s = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> +    return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_WS_REPORTING);
> +}
> +
>  static void virtio_balloon_set_config(VirtIODevice *vdev,
>                                        const uint8_t *config_data)
>  {
> @@ -766,6 +892,10 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>      if (virtio_balloon_page_poison_support(dev)) {
>          dev->poison_val = le32_to_cpu(config.poison_val);
>      }
> +    dev->working_set_num_bins = 0;
> +    if (virtio_balloon_working_set_reporting_support(dev)) {
> +        dev->working_set_num_bins = config.working_set_num_bins;
> +    }
>      trace_virtio_balloon_set_config(dev->actual, oldactual);
>  }
>  
> @@ -775,6 +905,7 @@ static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
>      VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
>      f |= dev->host_features;
>      virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
> +    virtio_add_feature(&f, VIRTIO_BALLOON_F_WS_REPORTING);
>  
>      return f;
>  }
> @@ -896,6 +1027,13 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>                                             virtio_balloon_handle_report);
>      }
>  
> +    if (virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_WS_REPORTING)) {
> +        s->working_set_vq = virtio_add_queue(vdev, 32,
> +            virtio_balloon_receive_working_set);
> +        s->notification_vq = virtio_add_queue(vdev, 32, NULL);
> +    }
> +
> +    reset_working_set(s);
>      reset_stats(s);
>  }
>  
> @@ -922,6 +1060,12 @@ static void virtio_balloon_device_unrealize(DeviceState *dev)
>      if (s->reporting_vq) {
>          virtio_delete_queue(s->reporting_vq);
>      }
> +    if (s->working_set_vq) {
> +        virtio_delete_queue(s->working_set_vq);
> +    }
> +    if (s->notification_vq) {
> +        virtio_delete_queue(s->notification_vq);
> +    }
>      virtio_cleanup(vdev);
>  }
>  
> @@ -938,7 +1082,11 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
>          g_free(s->stats_vq_elem);
>          s->stats_vq_elem = NULL;
>      }
> -
> +    if (s->working_set_vq_elem != NULL) {
> +        virtqueue_unpop(s->working_set_vq, s->working_set_vq_elem, 0);
> +        g_free(s->working_set_vq_elem);
> +        s->working_set_vq_elem = NULL;
> +    }
>      s->poison_val = 0;
>  }
>  
> @@ -953,6 +1101,16 @@ static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
>          virtio_balloon_receive_stats(vdev, s->svq);
>      }
>  
> +    if (!s->working_set_vq_elem && vdev->vm_running &&
> +        (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> +         virtqueue_rewind(s->working_set_vq, 1)) {
> +        /*
> +         * poll working set queue for the element we have discarded when the VM
> +         * was stopped
> +         */
> +        virtio_balloon_receive_working_set(vdev, s->working_set_vq);
> +    }
> +
>      if (virtio_balloon_free_page_support(s)) {
>          /*
>           * The VM is woken up and the iothread was blocked, so signal it to
> @@ -1011,6 +1169,8 @@ static Property virtio_balloon_properties[] = {
>                      VIRTIO_BALLOON_F_PAGE_POISON, true),
>      DEFINE_PROP_BIT("free-page-reporting", VirtIOBalloon, host_features,
>                      VIRTIO_BALLOON_F_REPORTING, false),
> +    DEFINE_PROP_BIT("working-set", VirtIOBalloon, host_features,
> +                    VIRTIO_BALLOON_F_WS_REPORTING, 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 5139cf8ab6..a7a6655f50 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -17,6 +17,7 @@
>  
>  #include "standard-headers/linux/virtio_balloon.h"
>  #include "hw/virtio/virtio.h"
> +#include "qapi/qapi-types-misc.h"
>  #include "sysemu/iothread.h"
>  #include "qom/object.h"
>  
> @@ -25,7 +26,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOBalloon, VIRTIO_BALLOON)
>  
>  #define VIRTIO_BALLOON_FREE_PAGE_HINT_CMD_ID_MIN 0x80000000
>  
> +#define VIRTIO_BALLOON_WS_NR_BINS 4  /* Number of bins in Working Set report */
> +
>  typedef struct virtio_balloon_stat VirtIOBalloonStat;
> +typedef struct virtio_balloon_working_set VirtIOBalloonWorkingSet;
>  
>  typedef struct virtio_balloon_stat_modern {
>         uint16_t tag;
> @@ -42,13 +46,19 @@ enum virtio_balloon_free_page_hint_status {
>  
>  struct VirtIOBalloon {
>      VirtIODevice parent_obj;
> -    VirtQueue *ivq, *dvq, *svq, *free_page_vq, *reporting_vq;
> +    VirtQueue *ivq, *dvq, *svq, *free_page_vq, *reporting_vq, *working_set_vq,
> +              *notification_vq;
>      uint32_t free_page_hint_status;
>      uint32_t num_pages;
>      uint32_t actual;
>      uint32_t free_page_hint_cmd_id;
>      uint64_t stats[VIRTIO_BALLOON_S_NR];
> +    WorkingSetInfo ws[VIRTIO_BALLOON_WS_NR_BINS];
> +    uint64_t ws_intervals[VIRTIO_BALLOON_WS_NR_BINS - 1];
> +    uint64_t ws_refresh_threshold;
> +    uint64_t ws_report_threshold;
>      VirtQueueElement *stats_vq_elem;
> +    VirtQueueElement *working_set_vq_elem;
>      size_t stats_vq_offset;
>      QEMUTimer *stats_timer;
>      IOThread *iothread;
> @@ -71,6 +81,7 @@ struct VirtIOBalloon {
>  
>      bool qemu_4_0_config_size;
>      uint32_t poison_val;
> +    uint8_t working_set_num_bins;
>  };
>  
>  #endif
> diff --git a/qapi/misc.json b/qapi/misc.json
> index ff070ec828..4ba8c74b64 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -550,6 +550,32 @@
>   'returns': ['CommandLineOptionInfo'],
>   'allow-preconfig': true}
>  
> +##
> +# @MemoryBin:
> +#
> +# A bin of memory with a size in bytes. File-backed and
> +# anonymous memory are tracked separately.
> +#
> +# @anon: number of bytes of anonymous memory
> +# @file: number of bytes of file-backed memory
> +##
> +{ 'struct': 'MemoryBin',
> +  'data': { 'anon': 'uint64',
> +            'file': 'uint64' } }
> +
> +##
> +# @WorkingSetInfo:
> +#
> +# A bin of memory of the given size that has been idle at most `idle-age` ms

sphinx-build warns "'any' reference target not found: idle-age".  That's
because `idle-age` is rST "interpreted text" with an inferred role.
It's okay not to understand that sentence (I barely understand it
myself).  My point is it's not what you want.  Fix:

   # A bin of memory of the given size that has been idle at most
   #       @idle-age milliseconds

> +#
> +# @idle-age: guest-relative time (in milliseconds)
> +#
> +# @memory-size-bytes: A MemoryBin with file and anon info.
> +##
> +{ 'struct': 'WorkingSetInfo',
> +  'data': { 'idle-age': 'uint64',
> +            'memory-size-bytes': 'MemoryBin' } }
> +
>  ##
>  # @RTC_CHANGE:
>  #

Any particular reason for putting this into misc.json?  I'm asking
because the next commit adds related stuff to machine.json.

Re: [RFC PATCH v2 2/5] virtio-balloon: device has Working Set Reporting

[Markus Armbruster]

"T.J. Alumbaugh" <talumbau@google.com> writes:

>  - working_set_vq to receive Working Set reports from guest
>  - notification_vq to send config or request to guest
>  - add working set as object property on device
>
> Signed-off-by: T.J. Alumbaugh <talumbau@google.com>

[...]

> diff --git a/qapi/misc.json b/qapi/misc.json
> index ff070ec828..4ba8c74b64 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -550,6 +550,32 @@
>   'returns': ['CommandLineOptionInfo'],
>   'allow-preconfig': true}
>  
> +##
> +# @MemoryBin:
> +#
> +# A bin of memory with a size in bytes. File-backed and
> +# anonymous memory are tracked separately.
> +#
> +# @anon: number of bytes of anonymous memory
> +# @file: number of bytes of file-backed memory

Please format like

   # A bin of memory with a size in bytes.  File-backed and anonymous
   # memory are tracked separately.
   #
   # @anon: number of bytes of anonymous memory
   #
   # @file: number of bytes of file-backed memory

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

Separating member sections with blank lines helps with catching certain
errors.  rST loves to surprise...

> +##
> +{ 'struct': 'MemoryBin',
> +  'data': { 'anon': 'uint64',
> +            'file': 'uint64' } }
> +

[...]

Re: [RFC PATCH v2 3/5] virtio-balloon: Add QMP functions for Working Set

[Markus Armbruster]

"T.J. Alumbaugh" <talumbau@google.com> writes:

>   - Adds QMP function 'working-set-config'
>   - Adds QMP function 'working-set-request'
>   - Retrieve working set via 'guest-working-set' property on balloon
>
>>> cat script.py
>
> NAME = "name"
> SOCKET = 'vm.sock'
> BALLOON =  "/machine/peripheral/balloon0"
>
> import json
> import asyncio
> from qemu.qmp import QMPClient
>
> async def main():
>     client = QMPClient(NAME)
>     await client.connect(SOCKET)
>     config = { "i0": 200, "i1": 800, "i2": 3000, "refresh": 750, "report": 1000 }
>     await client.execute('working-set-config', config)
>     await client.execute('working-set-request')
>     property = {"path":BALLOON, "property":"guest-working-set"}
>     res = await client.execute('qom-get', property)
>     return res
>
> if __name__ == "__main__":
>     ret = asyncio.run(main())
>     print(json.dumps(ret, indent=2))
>
>>> (Execute qemu with flag '-qmp unix:path=vm.sock,server=on,wait=off'
>>> (Perform normal activities on VM to exercise MM code)
>
>>> python3 script.py
> {
>   "working_set": {
>     "ws3": {
>       "memory-size-bytes": {
>         "anon": 890478592,
>         "file": 1285832704
>       },
>       "idle-age": 4294967292
>     },
>     "ws2": {
>       "memory-size-bytes": {
>         "anon": 173465600,
>         "file": 83353600
>       },
>       "idle-age": 3000
>     },
>     "ws1": {
>       "memory-size-bytes": {
>         "anon": 44236800,
>         "file": 20889600
>       },
>       "idle-age": 800
>     },
>     "ws0": {
>       "memory-size-bytes": {
>         "anon": 14540800,
>         "file": 6963200
>       },
>       "idle-age": 200
>     }
>   }
> }
>
> Signed-off-by: T.J. Alumbaugh <talumbau@google.com>

[...]

> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 602535696c..fad1b4aed5 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -333,6 +333,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>      [QAPI_EVENT_QUORUM_FAILURE]    = { 1000 * SCALE_MS },
>      [QAPI_EVENT_VSERPORT_CHANGE]   = { 1000 * SCALE_MS },
>      [QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE] = { 1000 * SCALE_MS },
> +    [QAPI_EVENT_WORKING_SET_EVENT] = { 1000 * SCALE_MS },
>  };
>  
>  /*
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 37660d8f2a..5e03ff21e2 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1055,6 +1055,57 @@
>  ##
>  { 'command': 'balloon', 'data': {'value': 'int'} }
>  
> +##
> +# @working-set-config:
> +#
> +# Specify the config parameters for Working Set reporting.

Don't capitalize "Working Set" unless it is a proper noun.  Is it?

What about "Configure working set reporting"?

> +#
> +# @i0: the endpoint of the first interval (in ms)
> +#
> +# @i1: the endpoint of the second interval (in ms)
> +#
> +# @i2: the endpoint of the third interval (in ms)

An "endpoint" doesn't define an interval.  Also, a "in milliseconds"
suggests relative time.  Relative to what?

What do these intervals do?

> +#
> +# @refresh: the refresh threshold (in ms) for Working Set reporting
> +#
> +# @report: the report threshold (in ms) for Working Set reporting

What do these do?

> +#
> +# Returns: - Nothing on success
> +#          - If no balloon device is present, DeviceNotActive

We use errors other than GenericError only when we have a compelling
reason.  I figure the reason is consistency with other commands
operating on a ballon device.  Correct?

Please format like

   # Returns:
   #     - Nothing on success
   #     - If no balloon device is present, DeviceNotActive

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

> +#
> +# Example:
> +#
> +# -> { "execute": "working-set-config",
> +#                 "arguments": { "i0": 100,
> +#                                "i1": 500,
> +#                                "i2": 2000,
> +#                                "refresh": 750,
> +#                                "report": 1000 } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'working-set-config', 'data': {'i0': 'uint64',
> +                                            'i1': 'uint64',
> +                                            'i2': 'uint64',
> +                                            'refresh': 'uint64',
> +                                            'report': 'uint64'} }
> +##
> +# @working-set-request:
> +#
> +# Request the Working Set report from the guest.

"Request a"

> +#
> +# Returns: - Nothing on success
> +#          - If no balloon device is present, DeviceNotActive
> +#
> +# Example:
> +#
> +# -> { "execute": "working-set-request", "arguments": {} }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'working-set-request', 'data': {} }
> +
> +
>  ##
>  # @BalloonInfo:
>  #
> @@ -1113,6 +1164,21 @@
>  { 'event': 'BALLOON_CHANGE',
>    'data': { 'actual': 'int' } }
>  
> +##
> +# @WORKING_SET_EVENT:
> +#
> +# Emitted when the guest sends a new Working Set report.

Where does it send the report to?

> +#
> +# Note: this event is rate-limited.
> +#
> +# Example:
> +#
> +# <- { "event": "WORKING_SET_EVENT",
> +#      "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
> +#
> +##
> +{ 'event': 'WORKING_SET_EVENT' }
> +
>  ##
>  # @MemoryInfo:
>  #

Documentation needs to answer:

1. What is working set reporting about, and why would I want to use it?

2. How are the reports to be interpreted?

3. What do the configuration parameters do?

[...]

Re: [RFC PATCH v2 1/5] virtio-balloon: Add Working Set Reporting feature

[David Hildenbrand]

On 26.05.23 00:20, T.J. Alumbaugh wrote:

Hi,

please try writing a comprehensive patch description: the goal should be 
that one can understand what's happening in the single patch without all 
of the following patches at hand. [ that's how I am reading them, and 
ahve to ask many stupid questions :P ]

> Balloon header includes:
>   - feature bit for Working Set Reporting
>   - number of Working Set bins member in balloon config
>   - types for communicating Working Set information
> 

Can you briefly summarize how all the bits here interact?

I assume, once VIRTIO_BALLOON_F_WS_REPORTING has been negotiated

(1) There is a new virtqueue for sending WS-related requests from the
     device (host) to the driver (guest).

-> How does a request look like?
-> How does a response look like?
-> Error cases?

(2) There is a new config space option.

-> Who's supposed to read this, who's supposed to write it?
-> Can it be changed dynamically?
-> What's the meaning / implication of that value.

> Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
> ---
>   .../standard-headers/linux/virtio_balloon.h   | 20 +++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
> index f343bfefd8..df61eaceee 100644
> --- a/include/standard-headers/linux/virtio_balloon.h
> +++ b/include/standard-headers/linux/virtio_balloon.h
> @@ -37,6 +37,7 @@
>   #define VIRTIO_BALLOON_F_FREE_PAGE_HINT	3 /* VQ to report free pages */
>   #define VIRTIO_BALLOON_F_PAGE_POISON	4 /* Guest is using page poisoning */
>   #define VIRTIO_BALLOON_F_REPORTING	5 /* Page reporting virtqueue */
> +#define VIRTIO_BALLOON_F_WS_REPORTING	6 /* Working set report virtqueues */

... are there multiple virtqueues? How many?

>   
>   /* Size of a PFN in the balloon interface. */
>   #define VIRTIO_BALLOON_PFN_SHIFT 12
> @@ -59,6 +60,9 @@ struct virtio_balloon_config {
>   	};
>   	/* Stores PAGE_POISON if page poisoning is in use */
>   	uint32_t poison_val;
> +	/* Stores the number of histogram bins if WS reporting in use */
> +	uint8_t working_set_num_bins;
> +	uint8_t padding[3];
>   };
>   
>   #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
> @@ -116,4 +120,20 @@ struct virtio_balloon_stat {
>   	__virtio64 val;
>   } QEMU_PACKED;
>   
> +enum virtio_balloon_working_set_op {
> +    VIRTIO_BALLOON_WS_REQUEST = 1, /* a Working Set request from the host */
> +    VIRTIO_BALLOON_WS_CONFIG = 2,  /* a Working Set config from the host */
> +};
> +
> +struct virtio_balloon_working_set {
> +	/* A tag for additional metadata */
> +	__virtio16 tag;
> +	/* The NUMA node for this report. */
> +	__virtio16 node_id;

How will we handle the case when the guest decides to use a different 
NUMA layout (e.g., numa disabled, fake numa, ...).

Is the guest supposed to detect that and *not* indicate a NUMA ID then?


Also, I wonder

> +	uint8_t reserved[4];
> +	__virtio64 idle_age_ms;
> +	/* A bin each for anonymous and file-backed memory. */

Why not have them separately, and properly named?

I'm not sure if it's a good idea to distinguish them based on anon vs. 
file-backed.

What would you do with shmem? It can be swapped like anon memory, ... if 
swap is enabled.

What's the main motivation for splitting this up? Is the "file-backed" 
part supposed to give some idea about the pagecache size? But what about 
mlock or page pinning?


Now I should take a step back and read the cover letter :)

-- 
Thanks,

David / dhildenb

Re: [RFC PATCH v2 3/5] virtio-balloon: Add QMP functions for Working Set

[David Hildenbrand]

On 26.05.23 00:20, T.J. Alumbaugh wrote:
>    - Adds QMP function 'working-set-config'
>    - Adds QMP function 'working-set-request'
>    - Retrieve working set via 'guest-working-set' property on balloon
> 
>>> cat script.py
> 
> NAME = "name"
> SOCKET = 'vm.sock'
> BALLOON =  "/machine/peripheral/balloon0"
> 
> import json
> import asyncio
> from qemu.qmp import QMPClient
> 
> async def main():
>      client = QMPClient(NAME)
>      await client.connect(SOCKET)
>      config = { "i0": 200, "i1": 800, "i2": 3000, "refresh": 750, "report": 1000 }
>      await client.execute('working-set-config', config)
>      await client.execute('working-set-request')
>      property = {"path":BALLOON, "property":"guest-working-set"}
>      res = await client.execute('qom-get', property)
>      return res
> 
> if __name__ == "__main__":
>      ret = asyncio.run(main())
>      print(json.dumps(ret, indent=2))
> 
>>> (Execute qemu with flag '-qmp unix:path=vm.sock,server=on,wait=off'
>>> (Perform normal activities on VM to exercise MM code)
> 
>>> python3 script.py
> {
>    "working_set": {
>      "ws3": {
>        "memory-size-bytes": {
>          "anon": 890478592,
>          "file": 1285832704
>        },
>        "idle-age": 4294967292
>      },
>      "ws2": {
>        "memory-size-bytes": {
>          "anon": 173465600,
>          "file": 83353600
>        },
>        "idle-age": 3000
>      },
>      "ws1": {
>        "memory-size-bytes": {
>          "anon": 44236800,
>          "file": 20889600
>        },
>        "idle-age": 800
>      },
>      "ws0": {
>        "memory-size-bytes": {
>          "anon": 14540800,
>          "file": 6963200
>        },
>        "idle-age": 200
>      }
>    }
> }

Would it be possible to model that as QOM property instead? Then, we 
could use qom-get/qom-set on the device instead of having to craft new 
QMP/HMP functions.

-- 
Thanks,

David / dhildenb

Re: [RFC PATCH v2 0/5] virtio-balloon: Working Set Reporting

[David Hildenbrand]

On 26.05.23 00:20, T.J. Alumbaugh wrote:
> This is the device implementation for the proposed expanded balloon feature
> described here:
> 
> https://lore.kernel.org/linux-mm/20230509185419.1088297-1-yuanchu@google.com/
> 
> This series has a fixed number of "bins" for the working set report, but this is
> not a constraint of the system. The bin number is fixed at device realization
> time (in other implementations it is specified as a command line argument). Once
> that number is fixed, this determines the correct number of bin intervals to
> pass to the QMP/HMP function 'working_set_config'. Any feedback on how to
> properly construct that function for this use case (passing a variable length
> list?) would be appreciated.
> 
> New in V2:
> =========
> 
> - Patch series is now: header file changes, device changes, QMP changes, HMP
> chagnes, and migration changes.
> 
> - Exmaple usages of QMP and HMP interface are in their respective commit
> messages.
> 
> - "ws" -> "working_set" throughout
> 
> Motivation
> ==========
> As mentioned in the above message, the use case is a host with overcommitted
> memory and 1 or more VMs. The goal is to get both timely and accurate
> information on overall memory utilization in order to drive appropriate
> reclaim activities, since in some client device use cases a VM might need a
> significant fraction of the overall memory for a period of time, but then
> enter a quiet period that results in a large number of cold pages in the
> guest.
> 
> The balloon device now has a number of features to assist in sharing memory
> resources amongst the guests and host (e.g free page hinting, stats, free page
> reporting). As mentioned in slide 12 in [1], the balloon doesn't have a good
> mechanism to drive the reclaim of guest cache. Our use case includes both
> typical page cache as well as "application caches" with memory that should be
> discarded in times of system-wide memory pressure. In some cases, virtio-pmem
> can be a method for host control of guest cache but there are undesirable
> security implications.
> 
> Working Set Reporting
> =====================
> The patch series here includes:
> 
>   - Actual device implementation for VIRTIO_F_WS_REPORTING to standardize the
>     configuration and communication of Working Set reports from the guest. This
>     includes a notification virtqueue for receiving config information and
>     requests for a report (a feature which could be expanded for additional use
>     cases) and a virtqueue for the actual report from the driver.

Could the config update be modeled using the config space instead?

Is the report asynchronous to the request, or how exactly do requests 
and reports interact?

> 
>   - QMP changes so that a controller program can use the existing QEMU socket
>     mechanism to configure and request WS reports and then read the reports as
>     a JSON property on the balloon.
> 
> Working Set reporting in the balloon provides:
> 
>   - an accurate picture of current memory utilization in the guest
>   - event driven reporting (with configurable rate limiting) to deliver reports
>     during times of memory pressure.
> 
> The reporting mechanism can be combined with a domain-specific balloon policy
> to drive the separate reclaim activities in a coordinated fashion.
> 
> TODOs:
> ======
>   -  A synchronization mechanism must be added to the functions that send WS
>      Config and WS Request, otherwise concurrent callers (through QMP) can mix
>      messages on the virtqueue sending the data to the driver.
> 
>   - The device currently has a hard-coded setting of 4 'bins' for a Working Set
>     report, whereas the specification calls for anywhere between 2 and 16.

Can you briefly summarize what a bin is, how one would decide how many 
bins one wants and what the whole purpose of a bin is?

Thanks!

-- 
Thanks,

David / dhildenb

Re: [RFC PATCH v2 1/5] virtio-balloon: Add Working Set Reporting feature

[Michael S. Tsirkin]

On Wed, May 31, 2023 at 10:12:26AM +0200, David Hildenbrand wrote:
> On 26.05.23 00:20, T.J. Alumbaugh wrote:
> 
> Hi,
> 
> please try writing a comprehensive patch description: the goal should be
> that one can understand what's happening in the single patch without all of
> the following patches at hand. [ that's how I am reading them, and ahve to
> ask many stupid questions :P ]
> 
> > Balloon header includes:
> >   - feature bit for Working Set Reporting
> >   - number of Working Set bins member in balloon config
> >   - types for communicating Working Set information
> > 
> 
> Can you briefly summarize how all the bits here interact?
> 
> I assume, once VIRTIO_BALLOON_F_WS_REPORTING has been negotiated
> 
> (1) There is a new virtqueue for sending WS-related requests from the
>     device (host) to the driver (guest).

this belongs in driver description, indeed.

> -> How does a request look like?
> -> How does a response look like?
> -> Error cases?

These last 3 are best in the spec, not in driver.

> (2) There is a new config space option.
> 
> -> Who's supposed to read this, who's supposed to write it?
> -> Can it be changed dynamically?
> -> What's the meaning / implication of that value.

same. best in spec.

> > Signed-off-by: T.J. Alumbaugh <talumbau@google.com>
> > ---
> >   .../standard-headers/linux/virtio_balloon.h   | 20 +++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> > 
> > diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
> > index f343bfefd8..df61eaceee 100644
> > --- a/include/standard-headers/linux/virtio_balloon.h
> > +++ b/include/standard-headers/linux/virtio_balloon.h
> > @@ -37,6 +37,7 @@
> >   #define VIRTIO_BALLOON_F_FREE_PAGE_HINT	3 /* VQ to report free pages */
> >   #define VIRTIO_BALLOON_F_PAGE_POISON	4 /* Guest is using page poisoning */
> >   #define VIRTIO_BALLOON_F_REPORTING	5 /* Page reporting virtqueue */
> > +#define VIRTIO_BALLOON_F_WS_REPORTING	6 /* Working set report virtqueues */
> 
> ... are there multiple virtqueues? How many?
> 
> >   /* Size of a PFN in the balloon interface. */
> >   #define VIRTIO_BALLOON_PFN_SHIFT 12
> > @@ -59,6 +60,9 @@ struct virtio_balloon_config {
> >   	};
> >   	/* Stores PAGE_POISON if page poisoning is in use */
> >   	uint32_t poison_val;
> > +	/* Stores the number of histogram bins if WS reporting in use */
> > +	uint8_t working_set_num_bins;
> > +	uint8_t padding[3];
> >   };
> >   #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
> > @@ -116,4 +120,20 @@ struct virtio_balloon_stat {
> >   	__virtio64 val;
> >   } QEMU_PACKED;
> > +enum virtio_balloon_working_set_op {
> > +    VIRTIO_BALLOON_WS_REQUEST = 1, /* a Working Set request from the host */
> > +    VIRTIO_BALLOON_WS_CONFIG = 2,  /* a Working Set config from the host */
> > +};
> > +
> > +struct virtio_balloon_working_set {
> > +	/* A tag for additional metadata */
> > +	__virtio16 tag;
> > +	/* The NUMA node for this report. */
> > +	__virtio16 node_id;
> 
> How will we handle the case when the guest decides to use a different NUMA
> layout (e.g., numa disabled, fake numa, ...).
> 
> Is the guest supposed to detect that and *not* indicate a NUMA ID then?
> 
> 
> Also, I wonder
> 
> > +	uint8_t reserved[4];
> > +	__virtio64 idle_age_ms;
> > +	/* A bin each for anonymous and file-backed memory. */
> 
> Why not have them separately, and properly named?
> 
> I'm not sure if it's a good idea to distinguish them based on anon vs.
> file-backed.
> 
> What would you do with shmem? It can be swapped like anon memory, ... if
> swap is enabled.
> 
> What's the main motivation for splitting this up? Is the "file-backed" part
> supposed to give some idea about the pagecache size? But what about mlock or
> page pinning?
> 
> 
> Now I should take a step back and read the cover letter :)
> 
> -- 
> Thanks,
> 
> David / dhildenb

Re: [RFC PATCH v2 0/5] virtio-balloon: Working Set Reporting

[Michael S. Tsirkin]

On Thu, May 25, 2023 at 10:20:11PM +0000, T.J. Alumbaugh wrote:
> This is the device implementation for the proposed expanded balloon feature
> described here:
> 
> https://lore.kernel.org/linux-mm/20230509185419.1088297-1-yuanchu@google.com/
> 
> This series has a fixed number of "bins" for the working set report, but this is
> not a constraint of the system. The bin number is fixed at device realization
> time (in other implementations it is specified as a command line argument). Once
> that number is fixed, this determines the correct number of bin intervals to
> pass to the QMP/HMP function 'working_set_config'. Any feedback on how to
> properly construct that function for this use case (passing a variable length
> list?) would be appreciated.

It's been a while. Is there interest is reviving this? I also note that
reserving a feature bit is very much recommended to avoid a complete
mess.


> New in V2:
> =========
> 
> - Patch series is now: header file changes, device changes, QMP changes, HMP
> chagnes, and migration changes.
> 
> - Exmaple usages of QMP and HMP interface are in their respective commit
> messages.
> 
> - "ws" -> "working_set" throughout
> 
> Motivation
> ==========
> As mentioned in the above message, the use case is a host with overcommitted
> memory and 1 or more VMs. The goal is to get both timely and accurate
> information on overall memory utilization in order to drive appropriate
> reclaim activities, since in some client device use cases a VM might need a
> significant fraction of the overall memory for a period of time, but then
> enter a quiet period that results in a large number of cold pages in the
> guest.
> 
> The balloon device now has a number of features to assist in sharing memory
> resources amongst the guests and host (e.g free page hinting, stats, free page
> reporting). As mentioned in slide 12 in [1], the balloon doesn't have a good
> mechanism to drive the reclaim of guest cache. Our use case includes both
> typical page cache as well as "application caches" with memory that should be
> discarded in times of system-wide memory pressure. In some cases, virtio-pmem
> can be a method for host control of guest cache but there are undesirable
> security implications.
> 
> Working Set Reporting
> =====================
> The patch series here includes:
> 
>  - Actual device implementation for VIRTIO_F_WS_REPORTING to standardize the
>    configuration and communication of Working Set reports from the guest. This
>    includes a notification virtqueue for receiving config information and
>    requests for a report (a feature which could be expanded for additional use
>    cases) and a virtqueue for the actual report from the driver.
> 
>  - QMP changes so that a controller program can use the existing QEMU socket
>    mechanism to configure and request WS reports and then read the reports as
>    a JSON property on the balloon.
> 
> Working Set reporting in the balloon provides:
> 
>  - an accurate picture of current memory utilization in the guest
>  - event driven reporting (with configurable rate limiting) to deliver reports
>    during times of memory pressure.
> 
> The reporting mechanism can be combined with a domain-specific balloon policy
> to drive the separate reclaim activities in a coordinated fashion.
> 
> TODOs:
> ======
>  -  A synchronization mechanism must be added to the functions that send WS
>     Config and WS Request, otherwise concurrent callers (through QMP) can mix
>     messages on the virtqueue sending the data to the driver.
> 
>  - The device currently has a hard-coded setting of 4 'bins' for a Working Set
>    report, whereas the specification calls for anywhere between 2 and 16.
> 
>  - A WS_EVENT notification through QMP should include the actual report,
>    whereas right now we query for that information right after a WS_EVENT is
>    received.
> 
> References:
> 
> [1] https://kvmforum2020.sched.com/event/eE4U/virtio-balloonpmemmem-managing-guest-memory-david-hildenbrand-michael-s-tsirkin-red-hat
> 
> T.J. Alumbaugh (5):
>   virtio-balloon: Add Working Set Reporting feature
>   virtio-balloon: device has Working Set Reporting
>   virtio-balloon: Add QMP functions for Working Set
>   virtio-balloon: Add HMP functions for Working Set
>   virtio-balloon: Migration of working set config
> 
>  hmp-commands.hx                               |  26 ++
>  hw/core/machine-hmp-cmds.c                    |  21 ++
>  hw/virtio/virtio-balloon-pci.c                |   2 +
>  hw/virtio/virtio-balloon.c                    | 239 +++++++++++++++++-
>  include/hw/virtio/virtio-balloon.h            |  13 +-
>  include/monitor/hmp.h                         |   2 +
>  .../standard-headers/linux/virtio_balloon.h   |  20 ++
>  include/sysemu/balloon.h                      |   9 +-
>  monitor/monitor.c                             |   1 +
>  qapi/machine.json                             |  66 +++++
>  qapi/misc.json                                |  26 ++
>  softmmu/balloon.c                             |  31 ++-
>  12 files changed, 449 insertions(+), 7 deletions(-)
> 
> -- 
> 2.41.0.rc0.172.g3f132b7071-goog

Re: [RFC PATCH v2 0/5] virtio-balloon: Working Set Reporting

[Yuanchu Xie]

On Mon, Apr 7, 2025 at 2:39 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, May 25, 2023 at 10:20:11PM +0000, T.J. Alumbaugh wrote:
> > This is the device implementation for the proposed expanded balloon feature
> > described here:
> >
> > https://lore.kernel.org/linux-mm/20230509185419.1088297-1-yuanchu@google.com/
> >
> > This series has a fixed number of "bins" for the working set report, but this is
> > not a constraint of the system. The bin number is fixed at device realization
> > time (in other implementations it is specified as a command line argument). Once
> > that number is fixed, this determines the correct number of bin intervals to
> > pass to the QMP/HMP function 'working_set_config'. Any feedback on how to
> > properly construct that function for this use case (passing a variable length
> > list?) would be appreciated.
>
> It's been a while. Is there interest is reviving this? I also note that
> reserving a feature bit is very much recommended to avoid a complete
> mess.
Thanks for the reminder Michael! I've been informed [1] that this
should be brought up in VIRTIO TC, but I haven't gotten around to this
yet. Should I send a patch to this mailing list or a proposal to
virtio-comment@lists.oasis-open.org to reserve a feature bit?

[1] https://lore.kernel.org/linux-mm/20241127025728.3689245-10-yuanchu@google.com/

Re: [RFC PATCH v2 0/5] virtio-balloon: Working Set Reporting

[Michael S. Tsirkin]

On Wed, Apr 09, 2025 at 09:52:12AM -0700, Yuanchu Xie wrote:
> On Mon, Apr 7, 2025 at 2:39 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, May 25, 2023 at 10:20:11PM +0000, T.J. Alumbaugh wrote:
> > > This is the device implementation for the proposed expanded balloon feature
> > > described here:
> > >
> > > https://lore.kernel.org/linux-mm/20230509185419.1088297-1-yuanchu@google.com/
> > >
> > > This series has a fixed number of "bins" for the working set report, but this is
> > > not a constraint of the system. The bin number is fixed at device realization
> > > time (in other implementations it is specified as a command line argument). Once
> > > that number is fixed, this determines the correct number of bin intervals to
> > > pass to the QMP/HMP function 'working_set_config'. Any feedback on how to
> > > properly construct that function for this use case (passing a variable length
> > > list?) would be appreciated.
> >
> > It's been a while. Is there interest is reviving this? I also note that
> > reserving a feature bit is very much recommended to avoid a complete
> > mess.
> Thanks for the reminder Michael! I've been informed [1] that this
> should be brought up in VIRTIO TC, but I haven't gotten around to this
> yet. Should I send a patch to this mailing list or a proposal to
> virtio-comment@lists.oasis-open.org to reserve a feature bit?
> 
> [1] https://lore.kernel.org/linux-mm/20241127025728.3689245-10-yuanchu@google.com/


You can do this in any order.

-- 
MST