"T.J. Alumbaugh" <talum...@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 <talum...@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.
