[Qemu-devel] [PATCH v1 3/3] virtio-balloon: Rework pbp tracking data

[David Hildenbrand]

Using the address of a RAMBlock to test for a matching pbp is not really
safe. Instead, let's use the guest physical address of the base page
along with the page size (via the number of subpages).

While at it, move "struct PartiallyBalloonedPage" to virtio-balloon.h
now (previously most probably avoided to te the ramblock), renaming it.

Also, let's only dynamically allocating the bitmap. This makes the code
easier to read and maintain - we can reuse bitmap_new().

Signed-off-by: David Hildenbrand <address@hidden>
---
 hw/virtio/virtio-balloon.c         | 52 +++++++++++++++++-------------
 include/hw/virtio/virtio-balloon.h | 15 +++++++--
 2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 29cee344b2..8e5f9459c2 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -34,23 +34,31 @@
 
 #define BALLOON_PAGE_SIZE  (1 << VIRTIO_BALLOON_PFN_SHIFT)
 
-struct PartiallyBalloonedPage {
-    RAMBlock *rb;
-    ram_addr_t base;
-    unsigned long bitmap[];
-};
-
 static void virtio_balloon_reset_pbp(VirtIOBalloon *balloon)
 {
-    g_free(balloon->pbp);
-    balloon->pbp = NULL;
+    balloon->pbp.base_gpa = 0;
+    balloon->pbp.subpages = 0;
+    g_free(balloon->pbp.bitmap);
+    balloon->pbp.bitmap = NULL;
+}
+
+static bool virtio_balloon_pbp_valid(VirtIOBalloon *balloon)
+{
+    return balloon->pbp.bitmap;
+}
+
+static bool virtio_balloon_pbp_matches(VirtIOBalloon *balloon,
+                                       ram_addr_t base_gpa, long subpages)
+{
+    return balloon->pbp.subpages == subpages &&
+           balloon->pbp.base_gpa == base_gpa;
 }
 
 static void balloon_inflate_page(VirtIOBalloon *balloon,
                                  MemoryRegion *mr, hwaddr mr_offset)
 {
     void *addr = memory_region_get_ram_ptr(mr) + mr_offset;
-    ram_addr_t rb_offset, rb_aligned_offset;
+    ram_addr_t rb_offset, rb_aligned_offset, base_gpa;
     RAMBlock *rb;
     size_t rb_page_size;
     int subpages;
@@ -81,32 +89,32 @@ static void balloon_inflate_page(VirtIOBalloon *balloon,
 
     rb_aligned_offset = QEMU_ALIGN_DOWN(rb_offset, rb_page_size);
     subpages = rb_page_size / BALLOON_PAGE_SIZE;
+    base_gpa = memory_region_get_ram_addr(mr) + mr_offset -
+               (rb_offset - rb_aligned_offset);
 
-    if (balloon->pbp
-        && (rb != balloon->pbp->rb
-            || rb_aligned_offset != balloon->pbp->base)) {
+    if (virtio_balloon_pbp_valid(balloon) &&
+        !virtio_balloon_pbp_matches(balloon, base_gpa, subpages)) {
         /* We've partially ballooned part of a host page, but now
          * we're trying to balloon part of a different one.  Too hard,
          * give up on the old partial page */
         virtio_balloon_reset_pbp(balloon);
     }
 
-    if (!balloon->pbp) {
+    if (!virtio_balloon_pbp_valid(balloon)) {
         /* Starting on a new host page */
-        size_t bitlen = BITS_TO_LONGS(subpages) * sizeof(unsigned long);
-        balloon->pbp = g_malloc0(sizeof(PartiallyBalloonedPage) + bitlen);
-        balloon->pbp->rb = rb;
-        balloon->pbp->base = rb_aligned_offset;
+        balloon->pbp.base_gpa = base_gpa;
+        balloon->pbp.subpages = subpages;
+        balloon->pbp.bitmap = bitmap_new(subpages);
     }
 
-    set_bit((rb_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE,
-            balloon->pbp->bitmap);
+    set_bit((rb_offset - rb_aligned_offset) / BALLOON_PAGE_SIZE,
+            balloon->pbp.bitmap);
 
-    if (bitmap_full(balloon->pbp->bitmap, subpages)) {
+    if (bitmap_full(balloon->pbp.bitmap, subpages)) {
         /* We've accumulated a full host page, we can actually discard
          * it now */
 
-        ram_block_discard_range(rb, balloon->pbp->base, rb_page_size);
+        ram_block_discard_range(rb, rb_aligned_offset, rb_page_size);
         /* We ignore errors from ram_block_discard_range(), because it
          * has already reported them, and failing to discard a balloon
          * page is not fatal */
@@ -130,7 +138,7 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
     rb = qemu_ram_block_from_host(addr, false, &rb_offset);
     rb_page_size = qemu_ram_pagesize(rb);
 
-    if (balloon->pbp) {
+    if (virtio_balloon_pbp_valid(balloon)) {
         /* Let's play safe and always reset the pbp on deflation requests. */
         virtio_balloon_reset_pbp(balloon);
     }
diff --git a/include/hw/virtio/virtio-balloon.h 
b/include/hw/virtio/virtio-balloon.h
index 5a99293a45..0190d78d71 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -33,8 +33,6 @@ typedef struct virtio_balloon_stat_modern {
        uint64_t val;
 } VirtIOBalloonStatModern;
 
-typedef struct PartiallyBalloonedPage PartiallyBalloonedPage;
-
 enum virtio_balloon_free_page_report_status {
     FREE_PAGE_REPORT_S_STOP = 0,
     FREE_PAGE_REPORT_S_REQUESTED = 1,
@@ -42,6 +40,12 @@ enum virtio_balloon_free_page_report_status {
     FREE_PAGE_REPORT_S_DONE = 3,
 };
 
+typedef struct VirtIOPartiallyBalloonedPage {
+    ram_addr_t base_gpa;
+    long subpages;
+    unsigned long *bitmap;
+} VirtIOPartiallyBalloonedPage;
+
 typedef struct VirtIOBalloon {
     VirtIODevice parent_obj;
     VirtQueue *ivq, *dvq, *svq, *free_page_vq;
@@ -70,7 +74,12 @@ typedef struct VirtIOBalloon {
     int64_t stats_last_update;
     int64_t stats_poll_interval;
     uint32_t host_features;
-    PartiallyBalloonedPage *pbp;
+
+    /*
+     * Information about a partially ballooned page - does not have to be
+     * migrated and has to be reset on every device reset.
+     */
+    VirtIOPartiallyBalloonedPage pbp;
 
     bool qemu_4_0_config_size;
 } VirtIOBalloon;
-- 
2.21.0