[Qemu-devel] virtio: Report new guest memory statistics pertinent to memory ballooning (V3)

[Qemu-devel] virtio: Report new guest memory statistics pertinent to memory ballooning (V3)

[Adam Litke]

virtio: Report new guest memory statistics pertinent to memory ballooning (V3)

Changes since V2:
 - Use a virtqueue for communication instead of the device config space

Changes since V1:
 - In the monitor, print all stats on one line with less abbreviated names
 - Coding style changes

When using ballooning to manage overcommitted memory on a host, a system for
guests to communicate their memory usage to the host can provide information
that will minimize the impact of ballooning on the guests.  The current method
employs a daemon running in each guest that communicates memory statistics to a
host daemon at a specified time interval.  The host daemon aggregates this
information and inflates and/or deflates balloons according to the level of
host memory pressure.  This approach is effective but overly complex since a
daemon must be installed inside each guest and coordinated to communicate with
the host.  A simpler approach is to collect memory statistics in the virtio
balloon driver and communicate them directly to the hypervisor.

This patch implements the qemu side of the communication channel.  I will post
the kernel driver modifications in-reply to this message.

Signed-off-by: Adam Litke <agl@us.ibm.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Avi Kivity <avi@redhat.com>
Cc: qemu-devel@nongnu.org

diff --git a/balloon.h b/balloon.h
index 60b4a5d..def4c56 100644
--- a/balloon.h
+++ b/balloon.h
@@ -16,12 +16,12 @@
 
 #include "cpu-defs.h"
 
-typedef ram_addr_t (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
+typedef int (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
 
 void qemu_add_balloon_handler(QEMUBalloonEvent *func, void *opaque);
 
 void qemu_balloon(ram_addr_t target);
 
-ram_addr_t qemu_balloon_status(void);
+int qemu_balloon_status(void);
 
 #endif
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index cfd3b41..81dd1f3 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -19,6 +19,7 @@
 #include "balloon.h"
 #include "virtio-balloon.h"
 #include "kvm.h"
+#include "monitor.h"
 
 #if defined(__linux__)
 #include <sys/mman.h>
@@ -27,9 +28,13 @@
 typedef struct VirtIOBalloon
 {
     VirtIODevice vdev;
-    VirtQueue *ivq, *dvq;
+    VirtQueue *ivq, *dvq, *svq;
     uint32_t num_pages;
     uint32_t actual;
+    uint32_t stats[VIRTIO_BALLOON_S_NR];
+    VirtQueueElement stats_vq_elem;
+    size_t stats_vq_offset;
+    uint8_t stats_requested;
 } VirtIOBalloon;
 
 static VirtIOBalloon *to_virtio_balloon(VirtIODevice *vdev)
@@ -46,6 +51,34 @@ static void balloon_page(void *addr, int deflate)
 #endif
 }
 
+static inline void reset_stats(VirtIOBalloon *dev)
+{
+    int i;
+    for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
+}
+
+static inline void print_stat(uint32_t val, const char *label)
+{
+    if (val != -1) {
+        monitor_printf(cur_mon, ",%s=%u", label, val);
+    }
+}
+
+static void virtio_balloon_print_stats(VirtIOBalloon *dev)
+{
+    uint32_t actual = ram_size - (dev->actual << VIRTIO_BALLOON_PFN_SHIFT);
+
+    monitor_printf(cur_mon, "balloon: actual=%d", (int)(actual >> 20));
+    print_stat(dev->stats[VIRTIO_BALLOON_S_SWAP_IN], "pages_swapped_in");
+    print_stat(dev->stats[VIRTIO_BALLOON_S_SWAP_OUT], "pages_swapped_out");
+    print_stat(dev->stats[VIRTIO_BALLOON_S_ANON], "anon_pages");
+    print_stat(dev->stats[VIRTIO_BALLOON_S_MAJFLT], "major_page_faults");
+    print_stat(dev->stats[VIRTIO_BALLOON_S_MINFLT], "minor_page_faults");
+    print_stat(dev->stats[VIRTIO_BALLOON_S_MEMFREE], "free_memory");
+    print_stat(dev->stats[VIRTIO_BALLOON_S_MEMTOT], "total_memory");
+    monitor_printf(cur_mon, "\n");
+}
+
 /* FIXME: once we do a virtio refactoring, this will get subsumed into common
  * code */
 static size_t memcpy_from_iovector(void *data, size_t offset, size_t size,
@@ -104,6 +137,31 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOBalloon *s = to_virtio_balloon(vdev);
+    VirtQueueElement *elem = &s->stats_vq_elem;
+    VirtIOBalloonStat stat;
+    size_t offset = 0;
+
+    if (!virtqueue_pop(vq, elem))
+        return;
+
+    while (memcpy_from_iovector(&stat, offset, sizeof(stat), elem->out_sg,
+                                elem->out_num) == sizeof(stat)) {
+        offset += sizeof(stat);
+        if (stat.tag < VIRTIO_BALLOON_S_NR)
+            s->stats[stat.tag] = stat.val;
+    }
+    s->stats_vq_offset = offset;
+
+    if (s->stats_requested) {
+        virtio_balloon_print_stats(s);
+        monitor_resume(cur_mon);
+        s->stats_requested = 0;
+    }
+}
+
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOBalloon *dev = to_virtio_balloon(vdev);
@@ -126,10 +184,19 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev)
 {
-    return 0;
+    return 1 << VIRTIO_BALLOON_F_STATS_VQ;
 }
 
-static ram_addr_t virtio_balloon_to_target(void *opaque, ram_addr_t target)
+static void request_stats(VirtIOBalloon *vb)
+{
+    vb->stats_requested = 1;
+    reset_stats(vb);
+    monitor_suspend(cur_mon);
+    virtqueue_push(vb->svq, &vb->stats_vq_elem, vb->stats_vq_offset);
+    virtio_notify(&vb->vdev, vb->svq);
+}
+
+static int virtio_balloon_to_target(void *opaque, ram_addr_t target)
 {
     VirtIOBalloon *dev = opaque;
 
@@ -139,9 +206,14 @@ static ram_addr_t virtio_balloon_to_target(void *opaque, ram_addr_t target)
     if (target) {
         dev->num_pages = (ram_size - target) >> VIRTIO_BALLOON_PFN_SHIFT;
         virtio_notify_config(&dev->vdev);
+    } else if (dev->vdev.features & (1 << VIRTIO_BALLOON_F_STATS_VQ)) {
+        request_stats(dev);
+    } else {
+        reset_stats(dev);
+        virtio_balloon_print_stats(dev);
     }
 
-    return ram_size - (dev->actual << VIRTIO_BALLOON_PFN_SHIFT);
+    return 0;
 }
 
 static void virtio_balloon_save(QEMUFile *f, void *opaque)
@@ -152,6 +224,9 @@ static void virtio_balloon_save(QEMUFile *f, void *opaque)
 
     qemu_put_be32(f, s->num_pages);
     qemu_put_be32(f, s->actual);
+    qemu_put_buffer(f, (uint8_t *)&s->stats_vq_elem, sizeof(VirtQueueElement));
+    qemu_put_buffer(f, (uint8_t *)&s->stats_vq_offset, sizeof(size_t));
+    qemu_put_byte(f, s->stats_requested);
 }
 
 static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
@@ -165,6 +240,9 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
 
     s->num_pages = qemu_get_be32(f);
     s->actual = qemu_get_be32(f);
+    qemu_get_buffer(f, (uint8_t *)&s->stats_vq_elem, sizeof(VirtQueueElement));
+    qemu_get_buffer(f, (uint8_t *)&s->stats_vq_offset, sizeof(size_t));
+    s->stats_requested = qemu_get_byte(f);
 
     return 0;
 }
@@ -183,6 +261,7 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
 
     s->ivq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
     s->dvq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
+    s->svq = virtio_add_queue(&s->vdev, 128, virtio_balloon_receive_stats);
 
     qemu_add_balloon_handler(virtio_balloon_to_target, s);
 
diff --git a/hw/virtio-balloon.h b/hw/virtio-balloon.h
index 9a0d119..005ddeb 100644
--- a/hw/virtio-balloon.h
+++ b/hw/virtio-balloon.h
@@ -25,6 +25,7 @@
 
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
+#define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -37,4 +38,19 @@ struct virtio_balloon_config
     uint32_t actual;
 };
 
+/* Memory Statistics */
+#define VIRTIO_BALLOON_S_SWAP_IN  0   /* Number of pages swapped in */
+#define VIRTIO_BALLOON_S_SWAP_OUT 1   /* Number of pages swapped out */
+#define VIRTIO_BALLOON_S_ANON     2   /* Number of anonymous pages in use */
+#define VIRTIO_BALLOON_S_MAJFLT   3   /* Number of major faults */
+#define VIRTIO_BALLOON_S_MINFLT   4   /* Number of minor faults */
+#define VIRTIO_BALLOON_S_MEMFREE  5   /* Total amount of free memory */
+#define VIRTIO_BALLOON_S_MEMTOT   6   /* Total amount of memory */
+#define VIRTIO_BALLOON_S_NR       7
+
+typedef struct VirtIOBalloonStat {
+    uint16_t tag;
+    uint32_t val;
+} VirtIOBalloonStat;
+
 #endif
diff --git a/monitor.c b/monitor.c
index ed1ce6e..73484c8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1583,16 +1583,14 @@ static void do_balloon(Monitor *mon, int value)
 
 static void do_info_balloon(Monitor *mon)
 {
-    ram_addr_t actual;
+    int ret;
 
-    actual = qemu_balloon_status();
     if (kvm_enabled() && !kvm_has_sync_mmu())
         monitor_printf(mon, "Using KVM without synchronous MMU, "
                        "ballooning disabled\n");
-    else if (actual == 0)
+    ret = qemu_balloon_status();
+    if (ret == -1)
         monitor_printf(mon, "Ballooning not activated in VM\n");
-    else
-        monitor_printf(mon, "balloon: actual=%d\n", (int)(actual >> 20));
 }
 
 static qemu_acl *find_acl(Monitor *mon, const char *name)
diff --git a/vl.c b/vl.c
index 710d52e..aebceab 100644
--- a/vl.c
+++ b/vl.c
@@ -337,11 +337,11 @@ void qemu_balloon(ram_addr_t target)
         qemu_balloon_event(qemu_balloon_event_opaque, target);
 }
 
-ram_addr_t qemu_balloon_status(void)
+int qemu_balloon_status(void)
 {
     if (qemu_balloon_event)
         return qemu_balloon_event(qemu_balloon_event_opaque, 0);
-    return 0;
+    return -1;
 }
 
 /***********************************************************/


-- 
Thanks,
Adam

[Qemu-devel] virtio: Add memory statistics reporting to the balloon driver (V2)

[Adam Litke]

virtio: Add memory statistics reporting to the balloon driver (V2)

Changes since V1:
 - Use a virtqueue instead of the device config space

When using ballooning to manage overcommitted memory on a host, a system for
guests to communicate their memory usage to the host can provide information
that will minimize the impact of ballooning on the guests.  The current method
employs a daemon running in each guest that communicates memory statistics to a
host daemon at a specified time interval.  The host daemon aggregates this
information and inflates and/or deflates balloons according to the level of
host memory pressure.  This approach is effective but overly complex since a
daemon must be installed inside each guest and coordinated to communicate with
the host.  A simpler approach is to collect memory statistics in the virtio
balloon driver and communicate them directly to the hypervisor.

This patch enables the guest-side support by adding stats collection and
reporting to the virtio balloon driver.

Signed-off-by: Adam Litke <agl@us.ibm.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 200c22f..59b9533 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -29,7 +29,7 @@
 struct virtio_balloon
 {
 	struct virtio_device *vdev;
-	struct virtqueue *inflate_vq, *deflate_vq;
+	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
 
 	/* Where the ballooning thread waits for config to change. */
 	wait_queue_head_t config_change;
@@ -50,6 +50,9 @@ struct virtio_balloon
 	/* The array of pfns we tell the Host about. */
 	unsigned int num_pfns;
 	u32 pfns[256];
+
+	/* Memory statistics */
+	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
 };
 
 static struct virtio_device_id id_table[] = {
@@ -155,6 +158,60 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
 	}
 }
 
+static inline void update_stat(struct virtio_balloon *vb, int idx,
+				unsigned int tag, unsigned long val)
+{
+	BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
+	vb->stats[idx].tag = tag;
+	vb->stats[idx].val = cpu_to_le32(val);
+}
+
+#define K(x) ((x) << (PAGE_SHIFT - 10))
+static void update_balloon_stats(struct virtio_balloon *vb)
+{
+	unsigned long events[NR_VM_EVENT_ITEMS];
+	struct sysinfo i;
+	unsigned long anon_pages;
+	int idx = 0;
+
+	all_vm_events(events);
+	si_meminfo(&i);
+	anon_pages = K(global_page_state(NR_ANON_PAGES));
+
+	update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN, events[PSWPIN]);
+	update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT, events[PSWPOUT]);
+	update_stat(vb, idx++, VIRTIO_BALLOON_S_ANON, anon_pages);
+	update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
+	update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+	update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE, K(i.freeram));
+	update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT, K(i.totalram));
+}
+
+/*
+ * While most virtqueues communicate guest-initiated requests to the hypervisor,
+ * the stats queue operates in reverse.  The driver initializes the virtqueue
+ * with a single buffer.  From that point forward, all conversations consist of
+ * a hypervisor request (a call to this function) which directs us to refill
+ * the virtqueue with a fresh stats buffer.
+ */
+static void stats_ack(struct virtqueue *vq)
+{
+	struct virtio_balloon *vb;
+	unsigned int len;
+	struct scatterlist sg;
+
+	vb = vq->vq_ops->get_buf(vq, &len);
+	if (!vb)
+		return;
+
+	update_balloon_stats(vb);
+
+	sg_init_one(&sg, vb->stats, sizeof(vb->stats));
+	if (vq->vq_ops->add_buf(vq, &sg, 1, 0, vb) < 0)
+		BUG();
+	vq->vq_ops->kick(vq);
+}
+
 static void virtballoon_changed(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb = vdev->priv;
@@ -205,10 +262,10 @@ static int balloon(void *_vballoon)
 static int virtballoon_probe(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb;
-	struct virtqueue *vqs[2];
-	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack };
-	const char *names[] = { "inflate", "deflate" };
-	int err;
+	struct virtqueue *vqs[3];
+	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_ack };
+	const char *names[] = { "inflate", "deflate", "stats" };
+	int err, nvqs;
 
 	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
 	if (!vb) {
@@ -221,13 +278,28 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	init_waitqueue_head(&vb->config_change);
 	vb->vdev = vdev;
 
-	/* We expect two virtqueues. */
-	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks, names);
+	/* We expect two virtqueues: inflate and deflate,
+	 * and optionally stat. */
+	nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
+	err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names);
 	if (err)
 		goto out_free_vb;
 
 	vb->inflate_vq = vqs[0];
 	vb->deflate_vq = vqs[1];
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
+		struct scatterlist sg;
+		vb->stats_vq = vqs[2];
+
+		/*
+		 * Prime this virtqueue with one buffer so the hypervisor can
+		 * use it to signal us later.
+		 */
+		sg_init_one(&sg, vb->stats, sizeof(vb->stats));
+		if (vb->stats_vq->vq_ops->add_buf(vb->stats_vq, &sg, 1, 0, vb) < 0)
+			BUG();
+		vb->stats_vq->vq_ops->kick(vb->stats_vq);
+	}
 
 	vb->thread = kthread_run(balloon, vb, "vballoon");
 	if (IS_ERR(vb->thread)) {
@@ -265,7 +337,9 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	kfree(vb);
 }
 
-static unsigned int features[] = { VIRTIO_BALLOON_F_MUST_TELL_HOST };
+static unsigned int features[] = {
+	VIRTIO_BALLOON_F_MUST_TELL_HOST, VIRTIO_BALLOON_F_STATS_VQ,
+};
 
 static struct virtio_driver virtio_balloon = {
 	.feature_table = features,
diff --git a/include/linux/virtio_balloon.h b/include/linux/virtio_balloon.h
index 09d7300..3d21b9f 100644
--- a/include/linux/virtio_balloon.h
+++ b/include/linux/virtio_balloon.h
@@ -6,6 +6,7 @@
 
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
+#define VIRTIO_BALLOON_F_STATS_VQ 1 /* Memory Stats virtqueue */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -17,4 +18,20 @@ struct virtio_balloon_config
 	/* Number of pages we've actually got in balloon. */
 	__le32 actual;
 };
+
+#define VIRTIO_BALLOON_S_SWAP_IN  0   /* Number of pages swapped in */
+#define VIRTIO_BALLOON_S_SWAP_OUT 1   /* Number of pages swapped out */
+#define VIRTIO_BALLOON_S_ANON     2   /* Number of anonymous pages in use */
+#define VIRTIO_BALLOON_S_MAJFLT   3   /* Number of major faults */
+#define VIRTIO_BALLOON_S_MINFLT   4   /* Number of minor faults */
+#define VIRTIO_BALLOON_S_MEMFREE  5   /* Total amount of free memory */
+#define VIRTIO_BALLOON_S_MEMTOT   6   /* Total amount of memory */
+#define VIRTIO_BALLOON_S_NR       7
+
+struct virtio_balloon_stat
+{
+	__le16 tag;
+	__le32 val;
+};
+
 #endif /* _LINUX_VIRTIO_BALLOON_H */


-- 
Thanks,
Adam

[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V2)

[Rusty Russell]

On Wed, 18 Nov 2009 07:06:29 am Adam Litke wrote:
> virtio: Add memory statistics reporting to the balloon driver (V2)
> 
> Changes since V1:
>  - Use a virtqueue instead of the device config space

Hi Adam,

    If Anthony's happy, I'm happy with this approach.

Couple of minor points:

> +static inline void update_stat(struct virtio_balloon *vb, int idx,
> +				unsigned int tag, unsigned long val)
> +{
> +	BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
> +	vb->stats[idx].tag = tag;
> +	vb->stats[idx].val = cpu_to_le32(val);
> +}

The little-endian conversion of the balloon driver is a historical mistake
(no other driver does this).  Let's not extend it to the stats.

Here you've done one and not the other, which is even worse.  (Sparse would
have found this, I assume).

> +struct virtio_balloon_stat
> +{
> +	__le16 tag;
> +	__le32 val;
> +};

Is 32 bits sufficient?  A big machine might get over 4bn faults, and certainly
4 TB of memory isn't that far away.

Thanks,
Rusty.

Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V2)

[Anthony Liguori]

Rusty Russell wrote:
> On Wed, 18 Nov 2009 07:06:29 am Adam Litke wrote:
>   
>> virtio: Add memory statistics reporting to the balloon driver (V2)
>>
>> Changes since V1:
>>  - Use a virtqueue instead of the device config space
>>     
>
> Hi Adam,
>
>     If Anthony's happy, I'm happy with this approach.
>
> Couple of minor points:
>
>   
>> +static inline void update_stat(struct virtio_balloon *vb, int idx,
>> +				unsigned int tag, unsigned long val)
>> +{
>> +	BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
>> +	vb->stats[idx].tag = tag;
>> +	vb->stats[idx].val = cpu_to_le32(val);
>> +}
>>     
>
> The little-endian conversion of the balloon driver is a historical mistake
> (no other driver does this).  Let's not extend it to the stats.
>   

I think the mistake is that the other drivers don't do that.

We cheat in qemu and assume that the guest is always in a fixed 
endianness but this is not always the case for all architectures.

That said, since we make this mistake everywhere, I guess I understand 
the argument to have consistency and to just admit that we're broken 
here.  But this is where the endianness bits come from.

> Here you've done one and not the other, which is even worse.  (Sparse would
> have found this, I assume).
>   

Yup, that's definitely wrong.

>> +struct virtio_balloon_stat
>> +{
>> +	__le16 tag;
>> +	__le32 val;
>> +};
>>     
>
> Is 32 bits sufficient?  A big machine might get over 4bn faults, and certainly
> 4 TB of memory isn't that far away.
>   

I think making the interface u64 and byte based would be the best 
solution.  Making assumptions about page size across guest and host is 
another thing we should try to avoid.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V2)

[Jamie Lokier]

Anthony Liguori wrote:
> Rusty Russell wrote:
> >The little-endian conversion of the balloon driver is a historical mistake
> >(no other driver does this).  Let's not extend it to the stats.
> 
> I think the mistake is that the other drivers don't do that.
> 
> We cheat in qemu and assume that the guest is always in a fixed 
> endianness but this is not always the case for all architectures.

If guests can have different endianness (reasonable on some CPUs where
it's switchable - some even have more than 2 options), then I guess
the *host* on those systems have different endianness too.

Is the host's endianness signalled to the guest anywhere, so that
guest drivers can do cpu_to_qemuhost32(), when someone eventually
finds that necessary?

-- Jamie

Re: [Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V2)

[Rusty Russell]

On Thu, 19 Nov 2009 01:32:26 am Anthony Liguori wrote:
> Rusty Russell wrote:
> > The little-endian conversion of the balloon driver is a historical mistake
> > (no other driver does this).  Let's not extend it to the stats.
> 
> I think the mistake is that the other drivers don't do that.
> 
> We cheat in qemu and assume that the guest is always in a fixed 
> endianness but this is not always the case for all architectures.

Perhaps, but it's documented in the spec.  My assertion remains that to do
any virtualization you need to know what the guest endian is anyway, so
endian converts throughout the drivers just add pain for driver authors.

> I think making the interface u64 and byte based would be the best 
> solution.  Making assumptions about page size across guest and host is 
> another thing we should try to avoid.

Yep, just report the raw byte counts as u64.

Cheers,
Rusty.