From 538d6063c9f8d395e1d38285ddfe405c3fcd7619 Mon Sep 17 00:00:00 2001
From: John Levon <john.levon@nutanix.com>
Date: Fri, 27 May 2022 17:29:32 +0100
Subject: remove maps list from DMA controller (#674)

->maps existed so that if a consumer does vfu_map_sg() and then we are asked to
enable dirty page tracking, we won't mark those pages as dirty, and will hence
potentially lose data.

Now that we require quiesce and the use of either vfu_unmap_sg() or
vfu_sg_mark_dirty(), there's no need to have this list any more.

Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
 lib/dma.c                   | 21 ---------------------
 lib/dma.h                   | 28 +++++++---------------------
 test/py/libvfio_user.py     |  2 --
 test/py/test_dirty_pages.py |  2 +-
 4 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/lib/dma.c b/lib/dma.c
index 2719639..daa1b58 100644
--- a/lib/dma.c
+++ b/lib/dma.c
@@ -99,7 +99,6 @@ dma_controller_create(vfu_ctx_t *vfu_ctx, size_t max_regions, size_t max_size)
     dma->nregions = 0;
     memset(dma->regions, 0, max_regions * sizeof(dma->regions[0]));
     dma->dirty_pgsize = 0;
-    LIST_INIT(&dma->maps);
 
     return dma;
 }
@@ -468,22 +467,6 @@ out:
     return cnt;
 }
 
-static void
-dma_mark_dirty_sgs(dma_controller_t *dma)
-{
-    struct dma_sg *sg;
-
-    if (dma->dirty_pgsize == 0) {
-        return;
-    }
-
-    LIST_FOREACH(sg, &dma->maps, entry) {
-        if (sg->writeable) {
-            _dma_mark_dirty(dma, &dma->regions[sg->region], sg);
-        }
-    }
-}
-
 int
 dma_controller_dirty_page_logging_start(dma_controller_t *dma, size_t pgsize)
 {
@@ -523,8 +506,6 @@ dma_controller_dirty_page_logging_start(dma_controller_t *dma, size_t pgsize)
     }
     dma->dirty_pgsize = pgsize;
 
-    dma_mark_dirty_sgs(dma);
-
     vfu_log(dma->vfu_ctx, LOG_DEBUG, "dirty pages: started logging");
 
     return 0;
@@ -628,8 +609,6 @@ dma_controller_dirty_page_get(dma_controller_t *dma, vfu_dma_addr_t addr,
 #endif
     memset(region->dirty_bitmap, 0, size);
 
-    dma_mark_dirty_sgs(dma);
-
     return 0;
 }
 
diff --git a/lib/dma.h b/lib/dma.h
index 6e31f15..aad3b9c 100644
--- a/lib/dma.h
+++ b/lib/dma.h
@@ -89,7 +89,6 @@ struct dma_sg {
     uint64_t length;
     uint64_t offset;
     bool writeable;
-    LIST_ENTRY(dma_sg) entry;
 };
 
 typedef struct {
@@ -105,7 +104,6 @@ typedef struct dma_controller {
     int nregions;
     struct vfu_ctx *vfu_ctx;
     size_t dirty_pgsize;        // Dirty page granularity
-    LIST_HEAD(, dma_sg) maps;
     dma_memory_region_t regions[0];
 } dma_controller_t;
 
@@ -245,10 +243,6 @@ dma_map_sg(dma_controller_t *dma, dma_sg_t *sg, struct iovec *iov,
             return ERROR_INT(EFAULT);
         }
 
-        if (sg->writeable) {
-            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);
@@ -294,30 +288,22 @@ dma_mark_sg_dirty(dma_controller_t *dma, dma_sg_t *sg, int cnt)
 static inline void
 dma_unmap_sg(dma_controller_t *dma, dma_sg_t *sg, int cnt)
 {
+    dma_memory_region_t *region;
+
     assert(dma != NULL);
     assert(sg != NULL);
     assert(cnt > 0);
 
     do {
-        dma_memory_region_t *r;
-        /*
-         * FIXME this double loop will be removed if we replace the array with
-         * tfind(3)
-         */
-        for (r = dma->regions;
-             r < dma->regions + dma->nregions &&
-             r->info.iova.iov_base != sg->dma_addr;
-             r++);
-        if (r > dma->regions + dma->nregions) {
-            /* bad region */
-            continue;
+        if (sg->region >= dma->nregions) {
+            return;
         }
 
-        if (sg->writeable) {
-            LIST_REMOVE(sg, entry);
+        region = &dma->regions[sg->region];
 
+        if (sg->writeable) {
             if (dma->dirty_pgsize > 0) {
-                _dma_mark_dirty(dma, r, sg);
+                _dma_mark_dirty(dma, region, sg);
             }
         }
 
diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py
index ec9da7b..aeaefa5 100644
--- a/test/py/libvfio_user.py
+++ b/test/py/libvfio_user.py
@@ -547,8 +547,6 @@ class dma_sg_t(Structure):
         ("length", c.c_uint64),
         ("offset", c.c_uint64),
         ("writeable", c.c_bool),
-        ("le_next", c.c_void_p),
-        ("le_prev", c.c_void_p),
     ]
 
     def __str__(self):
diff --git a/test/py/test_dirty_pages.py b/test/py/test_dirty_pages.py
index 9a0de3b..9f892bd 100644
--- a/test/py/test_dirty_pages.py
+++ b/test/py/test_dirty_pages.py
@@ -347,7 +347,7 @@ def test_dirty_pages_get_modified():
     vfu_unmap_sg(ctx, sg1, iovec1)
     vfu_unmap_sg(ctx, sg4, iovec4)
     bitmap = get_dirty_page_bitmap()
-    assert bitmap == 0b11110101
+    assert bitmap == 0b11110001
 
     # after another two unmaps, should just be one dirty page
     vfu_unmap_sg(ctx, sg2, iovec2)
-- 
cgit v1.1