re-work SG dirty tracking (#672)

Move SG dirtying to vfu_unmap_sg(): as we don't want to track SGs
ourselves, doing this in vfu_map_sg() is no longer the right place.

Note that the lack of tracking implies that any SGs must be unmapped
before the final stop and copy phase. To avoid the need for this, add
vfu_mark_sg_dirty(): this allows a consumer to mark a region as dirty
explicitly without needing to unmap it. Currently it's the same as
vfu_unmap_sg(), but that's an implementation detail.

Note this still marks current maps after a get operation; that will
change subsequently.

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

1 files changed, 26 insertions, 29 deletions

diff --git a/test/py/test_dirty_pages.py b/test/py/test_dirty_pages.py
index 8af325a..9a0de3b 100644
--- a/test/py/test_dirty_pages.py
+++ b/test/py/test_dirty_pages.py
@@ -318,6 +318,7 @@ def test_dirty_pages_get_modified():
     ret = vfu_map_sg(ctx, sg1, iovec1)
     assert ret == 0
 
+    # read only
     ret, sg2 = vfu_addr_to_sg(ctx, dma_addr=0x11000, length=0x1000,
                               prot=mmap.PROT_READ)
     assert ret == 1
@@ -325,52 +326,47 @@ def test_dirty_pages_get_modified():
     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)
+    ret, sg3 = vfu_addr_to_sg(ctx, dma_addr=0x12000, length=0x1000)
     assert ret == 1
     iovec3 = iovec_t()
     ret = vfu_map_sg(ctx, sg3, iovec3)
     assert ret == 0
 
+    ret, sg4 = vfu_addr_to_sg(ctx, dma_addr=0x14000, length=0x4000)
+    assert ret == 1
+    iovec4 = iovec_t()
+    ret = vfu_map_sg(ctx, sg4, iovec4)
+    assert ret == 0
+
+    # not unmapped yet, dirty bitmap should be zero, but dirty maps will have
+    # been marked dirty still
     bitmap = get_dirty_page_bitmap()
-    assert bitmap == 0b11110001
+    assert bitmap == 0b00000000
 
-    # unmap segment, dirty bitmap should be the same
+    # unmap segments, dirty bitmap should be updated
     vfu_unmap_sg(ctx, sg1, iovec1)
+    vfu_unmap_sg(ctx, sg4, iovec4)
     bitmap = get_dirty_page_bitmap()
-    assert bitmap == 0b11110001
+    assert bitmap == 0b11110101
 
-    # check again, previously unmapped segment should be clean
+    # after another two unmaps, should just be one dirty page
+    vfu_unmap_sg(ctx, sg2, iovec2)
+    vfu_unmap_sg(ctx, sg3, iovec3)
     bitmap = get_dirty_page_bitmap()
-    assert bitmap == 0b11110000
-
+    assert bitmap == 0b00000100
 
-def stop_logging():
-    payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
-                                    flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP)
-
-    msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload)
+    # and should now be clear
+    bitmap = get_dirty_page_bitmap()
+    assert bitmap == 0b00000000
 
 
 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 request 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()
+    payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()),
+                                    flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP)
+
+    msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload)
 
 
 def test_dirty_pages_start_with_quiesce():
@@ -402,6 +398,7 @@ def test_dirty_pages_bitmap_with_quiesce():
     iovec1 = iovec_t()
     ret = vfu_map_sg(ctx, sg1, iovec1)
     assert ret == 0
+    vfu_unmap_sg(ctx, sg1, iovec1)
 
     send_dirty_page_bitmap(busy=True)