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>

2 files changed, 52 insertions, 8 deletions

diff --git a/lib/dma.h b/lib/dma.h
index 190a016..afd7345 100644
--- a/lib/dma.h
+++ b/lib/dma.h
@@ -156,6 +156,7 @@ _dma_mark_dirty(const dma_controller_t *dma, const dma_memory_region_t *region,
 
     start = sg->offset / dma->dirty_pgsize;
     end = start + (sg->length / dma->dirty_pgsize) + (sg->length % dma->dirty_pgsize != 0) - 1;
+
     for (i = start; i <= end; i++) {
         region->dirty_bitmap[i / CHAR_BIT] |= 1 << (i % CHAR_BIT);
     }
@@ -246,11 +247,9 @@ dma_map_sg(dma_controller_t *dma, dma_sg_t *sg, struct iovec *iov,
         }
 
         if (sg->writeable) {
-            if (dma->dirty_pgsize > 0) {
-                _dma_mark_dirty(dma, region, sg);
-            }
             LIST_INSERT_HEAD(&dma->maps, sg, entry);
         }
+
         vfu_log(dma->vfu_ctx, LOG_DEBUG, "map %p-%p",
                 sg->dma_addr + sg->offset,
                 sg->dma_addr + sg->offset + sg->length);
@@ -266,13 +265,39 @@ dma_map_sg(dma_controller_t *dma, dma_sg_t *sg, struct iovec *iov,
 }
 
 static inline void
-dma_unmap_sg(dma_controller_t *dma, const dma_sg_t *sg,
-	     UNUSED struct iovec *iov, int cnt)
+dma_mark_sg_dirty(dma_controller_t *dma, dma_sg_t *sg, int cnt)
 {
+    dma_memory_region_t *region;
 
     assert(dma != NULL);
     assert(sg != NULL);
-    assert(iov != NULL);
+    assert(cnt > 0);
+
+    do {
+        if (sg->region >= dma->nregions) {
+            return;
+        }
+
+        region = &dma->regions[sg->region];
+
+        if (sg->writeable) {
+            if (dma->dirty_pgsize > 0) {
+                _dma_mark_dirty(dma, region, sg);
+            }
+        }
+
+        vfu_log(dma->vfu_ctx, LOG_DEBUG, "mark dirty %p-%p",
+                sg->dma_addr + sg->offset,
+                sg->dma_addr + sg->offset + sg->length);
+        sg++;
+    } while (--cnt > 0);
+}
+
+static inline void
+dma_unmap_sg(dma_controller_t *dma, dma_sg_t *sg, int cnt)
+{
+    assert(dma != NULL);
+    assert(sg != NULL);
     assert(cnt > 0);
 
     do {
@@ -289,9 +314,15 @@ dma_unmap_sg(dma_controller_t *dma, const dma_sg_t *sg,
             /* bad region */
             continue;
         }
+
         if (sg->writeable) {
             LIST_REMOVE(sg, entry);
+
+            if (dma->dirty_pgsize > 0) {
+                _dma_mark_dirty(dma, r, sg);
+            }
         }
+
         vfu_log(dma->vfu_ctx, LOG_DEBUG, "unmap %p-%p",
                 sg->dma_addr + sg->offset,
                 sg->dma_addr + sg->offset + sg->length);
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index 924d109..7d324aa 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -2038,7 +2038,7 @@ vfu_map_sg(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, struct iovec *iov, int cnt,
 }
 
 EXPORT void
-vfu_unmap_sg(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, struct iovec *iov, int cnt)
+vfu_mark_sg_dirty(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, int cnt)
 {
     if (unlikely(vfu_ctx->dma_unregister == NULL)) {
         return;
@@ -2046,7 +2046,20 @@ vfu_unmap_sg(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, struct iovec *iov, int cnt)
 
     quiesce_check_allowed(vfu_ctx);
 
-    return dma_unmap_sg(vfu_ctx->dma, sg, iov, cnt);
+    return dma_mark_sg_dirty(vfu_ctx->dma, sg, cnt);
+}
+
+EXPORT void
+vfu_unmap_sg(vfu_ctx_t *vfu_ctx, dma_sg_t *sg,
+             struct iovec *iov UNUSED, int cnt)
+{
+    if (unlikely(vfu_ctx->dma_unregister == NULL)) {
+        return;
+    }
+
+    quiesce_check_allowed(vfu_ctx);
+
+    return dma_unmap_sg(vfu_ctx->dma, sg, cnt);
 }
 
 static int