clear dirty pages bitmap after getting dirty pages but keep mapped segments dirty (#551)

Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: John Levon <john.levon@nutanix.com>

3 files changed, 95 insertions, 21 deletions

diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py
index b276a82..1105fb5 100644
--- a/test/py/libvfio_user.py
+++ b/test/py/libvfio_user.py
@@ -389,7 +389,9 @@ class dma_sg_t(Structure):
         ("region", c.c_int),
         ("length", c.c_uint64),
         ("offset", c.c_uint64),
-        ("mappable", c.c_bool)
+        ("writeable", c.c_bool),
+        ("le_next", c.c_void_p), # FIXME add struct for LIST_ENTRY 
+        ("le_prev", c.c_void_p),
     ]
 
 #
@@ -432,6 +434,10 @@ lib.vfu_setup_device_migration_callbacks.argtypes = (c.c_void_p,
     c.POINTER(vfu_migration_callbacks_t), c.c_uint64)
 lib.vfu_addr_to_sg.argtypes = (c.c_void_p, c.c_void_p, c.c_size_t,
                                c.POINTER(dma_sg_t), c.c_int, c.c_int)
+lib.vfu_map_sg.argtypes = (c.c_void_p, c.POINTER(dma_sg_t), c.POINTER(iovec_t),
+                           c.c_int, c.c_int)
+lib.vfu_unmap_sg.argtypes = (c.c_void_p, c.POINTER(dma_sg_t),
+                             c.POINTER(iovec_t), c.c_int)
 
 def to_byte(val):
     """Cast an int to a byte value."""
@@ -691,4 +697,14 @@ def vfu_addr_to_sg(ctx, dma_addr, length, max_sg=1,
 
     sg = dma_sg_t()
 
-    return lib.vfu_addr_to_sg(ctx, dma_addr, length, sg, max_sg, prot)
+    return (lib.vfu_addr_to_sg(ctx, dma_addr, length, sg, max_sg, prot), sg)
+
+
+def vfu_map_sg(ctx, sg, iovec, cnt=1, flags=0):
+    # FIXME not sure wheter cnt != 1 will work because iovec is an array
+    return lib.vfu_map_sg(ctx, sg, iovec, cnt, flags)
+
+def vfu_unmap_sg(ctx, sg, iovec, cnt=1):
+    return lib.vfu_unmap_sg(ctx, sg, iovec, cnt)
+
+# ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: #
diff --git a/test/py/test_dirty_pages.py b/test/py/test_dirty_pages.py
index 2c9be01..25a73da 100644
--- a/test/py/test_dirty_pages.py
+++ b/test/py/test_dirty_pages.py
@@ -135,7 +135,8 @@ def test_dirty_pages_start_bad_flags():
     vfu_run_ctx(ctx)
     get_reply(sock, expect=errno.EINVAL)
 
-def test_dirty_pages_start():
+
+def start_logging():
     payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
                                     flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_START)
 
@@ -144,10 +145,11 @@ def test_dirty_pages_start():
     vfu_run_ctx(ctx)
     get_reply(sock)
 
-    # should be idempotent
-    sock.send(hdr + payload)
-    vfu_run_ctx(ctx)
-    get_reply(sock)
+
+def test_dirty_pages_start():
+    start_logging()
+    start_logging() # should be idempotent
+
 
 def test_dirty_pages_get_short_read():
     payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
@@ -162,9 +164,10 @@ def test_dirty_pages_get_short_read():
 # This should in fact work; update when it does.
 #
 def test_dirty_pages_get_sub_range():
-    dirty_pages = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
+    argsz = len(vfio_user_dirty_pages()) + len(vfio_user_bitmap_range()) + 8
+    dirty_pages = vfio_user_dirty_pages(argsz=argsz,
         flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP)
-    bitmap = vfio_user_bitmap(pgsize=0x1000, size=1)
+    bitmap = vfio_user_bitmap(pgsize=0x1000, size=8)
     br = vfio_user_bitmap_range(iova=0x11000, size=0x1000, bitmap=bitmap)
 
     hdr = vfio_user_header(VFIO_USER_DIRTY_PAGES,
@@ -174,7 +177,8 @@ def test_dirty_pages_get_sub_range():
     get_reply(sock, expect=errno.ENOTSUP)
 
 def test_dirty_pages_get_bad_page_size():
-    dirty_pages = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
+    argsz = len(vfio_user_dirty_pages()) + len(vfio_user_bitmap_range()) + 8
+    dirty_pages = vfio_user_dirty_pages(argsz=argsz,
         flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP)
     bitmap = vfio_user_bitmap(pgsize=0x2000, size=8)
     br = vfio_user_bitmap_range(iova=0x10000, size=0x10000, bitmap=bitmap)
@@ -186,7 +190,8 @@ def test_dirty_pages_get_bad_page_size():
     get_reply(sock, expect=errno.EINVAL)
 
 def test_dirty_pages_get_bad_bitmap_size():
-    dirty_pages = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
+    argsz = len(vfio_user_dirty_pages()) + len(vfio_user_bitmap_range()) + 8
+    dirty_pages = vfio_user_dirty_pages(argsz=argsz,
         flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP)
     bitmap = vfio_user_bitmap(pgsize=0x1000, size=1)
     br = vfio_user_bitmap_range(iova=0x10000, size=0x10000, bitmap=bitmap)
@@ -247,14 +252,8 @@ def test_dirty_pages_get_unmodified():
     assert br.bitmap.pgsize == 0x1000
     assert br.bitmap.size == 8
 
-def test_dirty_pages_get_modified():
-    # sufficient to mark the region dirty
-    ret = vfu_addr_to_sg(ctx, dma_addr=0x10000, length=0x1000)
-    assert ret == 1
-
-    ret = vfu_addr_to_sg(ctx, dma_addr=0x14000, length=0x4000)
-    assert ret == 1
 
+def get_dirty_page_bitmap():
     argsz = len(vfio_user_dirty_pages()) + len(vfio_user_bitmap_range()) + 8
 
     dirty_pages = vfio_user_dirty_pages(argsz=argsz,
@@ -270,11 +269,46 @@ def test_dirty_pages_get_modified():
 
     dirty_pages, result = vfio_user_dirty_pages.pop_from_buffer(result)
     br, result = vfio_user_bitmap_range.pop_from_buffer(result)
-    bitmap = struct.unpack("Q", result)[0]
+    return struct.unpack("Q", result)[0]
+
+sg3 = None
+iovec3 = None
 
+def test_dirty_pages_get_modified():
+    ret, sg1 = vfu_addr_to_sg(ctx, dma_addr=0x10000, length=0x1000)
+    assert ret == 1
+    iovec1 = iovec_t()
+    ret = vfu_map_sg(ctx, sg1, iovec1)
+    assert ret == 0
+
+    ret, sg2 = vfu_addr_to_sg(ctx, dma_addr=0x11000, length=0x1000,
+                              prot=mmap.PROT_READ)
+    assert ret == 1
+    iovec2 = iovec_t()
+    ret = vfu_map_sg(ctx, sg2, iovec2)
+    assert ret == 0
+
+    global sg3, iovec3
+    ret, sg3 = vfu_addr_to_sg(ctx, dma_addr=0x14000, length=0x4000)
+    assert ret == 1
+    iovec3 = iovec_t()
+    ret = vfu_map_sg(ctx, sg3, iovec3)
+    assert ret == 0
+
+    bitmap = get_dirty_page_bitmap()
     assert bitmap == 0b11110001
 
-def test_dirty_pages_stop():
+    # unmap segment, dirty bitmap should be the same
+    vfu_unmap_sg(ctx, sg1, iovec1)
+    bitmap = get_dirty_page_bitmap() 
+    assert bitmap == 0b11110001
+
+    # check again, previously unmapped segment should be clean
+    bitmap = get_dirty_page_bitmap()
+    assert bitmap == 0b11110000
+
+
+def stop_logging():
     payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
                                     flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP)
 
@@ -291,6 +325,30 @@ def test_dirty_pages_stop():
     vfu_run_ctx(ctx)
     get_reply(sock)
 
+
+def test_dirty_pages_stop():
+    stop_logging()
+
+    # one segment is still mapped, starting logging again and bitmap should be
+    # dirty
+    start_logging()
+    assert get_dirty_page_bitmap() == 0b11110000
+
+    # unmap segment, bitmap should still be dirty
+    vfu_unmap_sg(ctx, sg3, iovec3)
+    assert get_dirty_page_bitmap() == 0b11110000
+
+    # bitmap should be clear after it was unmapped before previous reqeust for
+    # dirty pages
+    assert get_dirty_page_bitmap() == 0b00000000
+
+    # FIXME we have a memory leak as we don't free dirty bitmaps when
+    # destroying the context.
+    stop_logging()
+
 def test_dirty_pages_cleanup():
     disconnect_client(ctx, sock)
     vfu_destroy_ctx(ctx)
+
+
+# ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab:
diff --git a/test/unit-tests.c b/test/unit-tests.c
index 9945bac..8a41a76 100644
--- a/test/unit-tests.c
+++ b/test/unit-tests.c
@@ -407,7 +407,7 @@ test_dma_addr_to_sg(void **state UNUSED)
     assert_int_equal(0x2000 - (unsigned long long)r->info.iova.iov_base,
                      sg.offset);
     assert_int_equal(0x400, sg.length);
-    assert_true(sg.mappable);
+    assert_true(vfu_sg_is_mappable(&vfu_ctx, &sg));
 
     errno = 0;
     r->info.prot = PROT_WRITE;