add unit tests for DMA regions without file descriptor

Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>

8 files changed, 354 insertions, 23 deletions

diff --git a/lib/common.h b/lib/common.h
index e30bc2e..dbd3c9b 100644
--- a/lib/common.h
+++ b/lib/common.h
@@ -55,6 +55,9 @@
 #define ROUND_DOWN(x, a)    ((x) & ~((a)-1))
 #define ROUND_UP(x,a)       ROUND_DOWN((x)+(a)-1, a)
 
+#define UNIT_TEST_SYMBOL(x) \
+    typeof(x) __wrap_##x __attribute__((weak, alias(#x)))
+
 #endif /* LIB_MUSER_COMMON_H */
 
 /* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/lib/dma.c b/lib/dma.c
index b47652d..bcc6128 100644
--- a/lib/dma.c
+++ b/lib/dma.c
@@ -90,7 +90,7 @@ dma_controller_create(lm_ctx_t *lm_ctx, int max_regions)
     return dma;
 }
 
-static void
+void
 _dma_controller_do_remove_region(dma_controller_t *dma,
                                  dma_memory_region_t *region)
 {
@@ -111,6 +111,14 @@ _dma_controller_do_remove_region(dma_controller_t *dma,
         }
     }
 }
+UNIT_TEST_SYMBOL(_dma_controller_do_remove_region);
+
+/*
+ * FIXME super ugly, but without this functions within the same compilation
+ * unit don't call the wrapped version, making unit testing impossible.
+ * Ideally we'd like the UNIT_TEST_SYMBOL macro to solve this.
+ */
+#define _dma_controller_do_remove_region __wrap__dma_controller_do_remove_region
 
 /*
  * FIXME no longer used. Also, it doesn't work for addresses that span two
@@ -173,6 +181,7 @@ dma_controller_remove_region(dma_controller_t *dma,
     }
     return -ENOENT;
 }
+UNIT_TEST_SYMBOL(dma_controller_remove_region);
 
 static inline void
 dma_controller_remove_regions(dma_controller_t *dma)
@@ -292,6 +301,8 @@ dma_controller_add_region(dma_controller_t *dma,
             }
             goto err;
         }
+    } else {
+        region->virt_addr = NULL;
     }
     dma->nregions++;
 
@@ -300,6 +311,7 @@ dma_controller_add_region(dma_controller_t *dma,
 err:
     return -idx - 1;
 }
+UNIT_TEST_SYMBOL(dma_controller_add_region);
 
 static inline void
 mmap_round(size_t *offset, size_t *size, size_t page_size)
@@ -334,6 +346,7 @@ dma_map_region(dma_memory_region_t *region, int prot, size_t offset, size_t len)
 
     return mmap_base + (offset - mmap_offset);
 }
+UNIT_TEST_SYMBOL(dma_map_region);
 
 int
 dma_unmap_region(dma_memory_region_t *region, void *virt_addr, size_t len)
diff --git a/lib/muser_ctx.c b/lib/muser_ctx.c
index e00a55a..d80e952 100644
--- a/lib/muser_ctx.c
+++ b/lib/muser_ctx.c
@@ -519,7 +519,7 @@ handle_dma_map_or_unmap(lm_ctx_t *lm_ctx, uint32_t size, bool map,
                         struct vfio_user_dma_region *dma_regions)
 {
     int nr_dma_regions;
-    int ret, i;
+    int ret, i, fdi;
 
     assert(lm_ctx != NULL);
     assert(fds != NULL);
@@ -534,43 +534,35 @@ handle_dma_map_or_unmap(lm_ctx_t *lm_ctx, uint32_t size, bool map,
     }
 
     nr_dma_regions = (int)(size / sizeof(struct vfio_user_dma_region));
-    if (map && nr_fds > 0 && nr_dma_regions != nr_fds) {
-        lm_log(lm_ctx, LM_ERR, "expected %d fds but got %d instead",
-               nr_dma_regions, nr_fds);
-        return -EINVAL;
-    }
 
-    for (i = 0; i < nr_dma_regions; i++) {
+    for (i = 0, fdi = 0; i < nr_dma_regions; i++) {
         if (map) {
+            int fd = -1;
             if (dma_regions[i].flags == VFIO_USER_F_DMA_REGION_MAPPABLE) {
-                if (nr_fds == 0) {
-                    return -EINVAL;
-                }
-            } else {
-                if (nr_fds != 0) {
+                if (fdi == nr_fds) {
                     return -EINVAL;
                 }
-                fds[i] = -1;
+                fd = fds[fdi++];
             }
 
             ret = dma_controller_add_region(lm_ctx->dma,
                                             dma_regions[i].addr,
                                             dma_regions[i].size,
-                                            fds[i],
+                                            fd,
                                             dma_regions[i].offset);
             if (ret < 0) {
                 lm_log(lm_ctx, LM_INF,
                        "failed to add DMA region %#lx-%#lx offset=%#lx fd=%d: %s",
                        dma_regions[i].addr,
                        dma_regions[i].addr + dma_regions[i].size - 1,
-                       dma_regions[i].offset, fds[i],
+                       dma_regions[i].offset, fd,
                        strerror(-ret));
             } else {
                 lm_log(lm_ctx, LM_DBG,
                        "added DMA region %#lx-%#lx offset=%#lx fd=%d",
                        dma_regions[i].addr,
                        dma_regions[i].addr + dma_regions[i].size - 1,
-                       dma_regions[i].offset, fds[i]);
+                       dma_regions[i].offset, fd);
             }
         } else {
             ret = dma_controller_remove_region(lm_ctx->dma,
diff --git a/lib/muser_priv.h b/lib/muser_priv.h
index 2d47d1a..ea44237 100644
--- a/lib/muser_priv.h
+++ b/lib/muser_priv.h
@@ -208,6 +208,10 @@ handle_dma_map_or_unmap(lm_ctx_t *lm_ctx, uint32_t size, bool map,
                         int *fds, int nr_fds,
                         struct vfio_user_dma_region *dma_regions);
 
+void
+_dma_controller_do_remove_region(dma_controller_t *dma,
+                                 dma_memory_region_t *region);
+
 #endif /* LIB_MUSER_PRIV_H */
 
 /* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index a04e0f1..a6c8735 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -26,8 +26,20 @@
 # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
 
-add_executable(unit-tests unit-tests.c)
-target_link_libraries(unit-tests cmocka json-c muser)
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DUNIT_TEST \
+	-Wno-implicit-function-declaration \
+	-Wl,--wrap=_dma_controller_do_remove_region \
+	-Wl,--wrap=dma_controller_add_region \
+	-Wl,--wrap=dma_map_region")
 
+add_executable(unit-tests unit-tests.c mocks.c
+		../lib/muser_ctx.c
+		../lib/dma.c
+		../lib/migration.c
+		../lib/muser_pci.c
+		../lib/tran_sock.c
+		../lib/cap.c
+		../lib/irq.c)
+target_link_libraries(unit-tests cmocka json-c)
 enable_testing()
 add_test(NAME unit-tests COMMAND unit-tests)
diff --git a/test/mocks.c b/test/mocks.c
new file mode 100644
index 0000000..7852c1d
--- /dev/null
+++ b/test/mocks.c
@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2020 Nutanix Inc. All rights reserved.
+ *
+ * Authors: Thanos Makatos <thanos@nutanix.com>
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are met:
+ *      * Redistributions of source code must retain the above copyright
+ *        notice, this list of conditions and the following disclaimer.
+ *      * Redistributions in binary form must reproduce the above copyright
+ *        notice, this list of conditions and the following disclaimer in the
+ *        documentation and/or other materials provided with the distribution.
+ *      * Neither the name of Nutanix nor the names of its contributors may be
+ *        used to endorse or promote products derived from this software without
+ *        specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ *  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ *  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ *  ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
+ *  DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ *  (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ *  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ *  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ *  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ *  OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ *  DAMAGE.
+ *
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include <stdio.h>
+
+#include "mocks.h"
+#include "dma.h"
+
+struct function
+{
+    void *addr;
+    bool patched;
+};
+
+int
+__wrap_dma_controller_add_region(dma_controller_t *dma, dma_addr_t dma_addr,
+                                 size_t size, int fd, off_t offset)
+{
+    if (!is_patched(dma_controller_add_region)) {
+        return __real_dma_controller_add_region(dma, dma_addr, size, fd, offset);
+    }
+
+    check_expected_ptr(dma);
+    check_expected(dma_addr);
+    check_expected(size);
+    check_expected(fd);
+    check_expected(offset);
+    return 0;
+}
+
+void *
+__wrap_dma_map_region(dma_memory_region_t *region, int prot, size_t offset,
+                      size_t len)
+{
+    check_expected_ptr(region);
+    check_expected(prot);
+    check_expected(offset);
+    check_expected(len);
+    return 0;
+}
+
+void
+__wrap__dma_controller_do_remove_region(dma_controller_t *dma,
+                                       dma_memory_region_t *region)
+{
+    check_expected(dma);
+    check_expected(region);
+}
+
+/* FIXME should be something faster than unsorted array, look at tsearch(3). */
+static struct function funcs[] = {
+    {.addr = &__wrap_dma_controller_add_region},
+    {.addr = &__wrap_dma_map_region},
+    {.addr = &__wrap__dma_controller_do_remove_region},
+};
+
+static struct function*
+find(void *addr)
+{
+    size_t i;
+
+    for (i = 0; i < ARRAY_SIZE(funcs); i++) {
+        if (addr == funcs[i].addr) {
+            return &funcs[i];
+        }
+    }
+    assert(false);
+}
+
+void
+patch(void *addr)
+{
+    struct function *func = find(addr);
+    func->patched = true;
+}
+
+bool
+is_patched(void *addr)
+{
+    return find(addr)->patched;
+}
+
+void
+unpatch_all(void)
+{
+    size_t i;
+    for (i = 0; i < ARRAY_SIZE(funcs); i++) {
+        funcs[i].patched = false;
+    }
+}
+
+/* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/mocks.h b/test/mocks.h
new file mode 100644
index 0000000..97a23b7
--- /dev/null
+++ b/test/mocks.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2020 Nutanix Inc. All rights reserved.
+ *
+ * Authors: Thanos Makatos <thanos@nutanix.com>
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are met:
+ *      * Redistributions of source code must retain the above copyright
+ *        notice, this list of conditions and the following disclaimer.
+ *      * Redistributions in binary form must reproduce the above copyright
+ *        notice, this list of conditions and the following disclaimer in the
+ *        documentation and/or other materials provided with the distribution.
+ *      * Neither the name of Nutanix nor the names of its contributors may be
+ *        used to endorse or promote products derived from this software without
+ *        specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ *  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ *  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ *  ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
+ *  DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ *  (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ *  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ *  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ *  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ *  OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ *  DAMAGE.
+ *
+ */
+
+#include <stdbool.h>
+
+void unpatch_all(void);
+
+void patch(void *fn);
+
+bool is_patched(void *fn);
+
+/* ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/unit-tests.c b/test/unit-tests.c
index 6ad4e3b..1dc823a 100644
--- a/test/unit-tests.c
+++ b/test/unit-tests.c
@@ -36,24 +36,169 @@
 #include <errno.h>
 #include <stdio.h>
 #include <assert.h>
+#include <alloca.h>
 
 #include "muser.h"
 #include "muser_priv.h"
+#include "dma.h"
+
+static void
+test_dma_map_without_dma(void **state __attribute__((unused)))
+{
+    lm_ctx_t lm_ctx = { 0 };
+    size_t size = sizeof(struct vfio_user_dma_region);
+    struct vfio_user_dma_region dma_region = {
+        .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
+    };
+    int fd;
+
+    assert_int_equal(0, handle_dma_map_or_unmap(&lm_ctx, size, true, &fd, 1, &dma_region));
+}
+
+static void
+test_dma_map_mappable_without_fd(void **state __attribute__((unused)))
+{
+    dma_controller_t dma = { 0 };
+    lm_ctx_t lm_ctx = { .dma = &dma };
+    size_t size = sizeof(struct vfio_user_dma_region);
+    struct vfio_user_dma_region dma_region = {
+        .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
+    };
+    int fd;
+    assert_int_equal(-EINVAL, handle_dma_map_or_unmap(&lm_ctx, size, true, &fd, 0, &dma_region));
+}
 
 static void
 test_dma_map_without_fd(void **state __attribute__((unused)))
 {
+    dma_controller_t dma = { 0 };
+    lm_ctx_t lm_ctx = { .dma = &dma };
+    dma.lm_ctx = &lm_ctx;
     size_t size = sizeof(struct vfio_user_dma_region);
+    struct vfio_user_dma_region r = {
+        .addr = 0xdeadbeef,
+        .size = 0xcafebabe,
+        .offset = 0x8badf00d
+    };
+    int fd;
+
+    patch(dma_controller_add_region);
+    expect_value(__wrap_dma_controller_add_region, dma, lm_ctx.dma);
+    expect_value(__wrap_dma_controller_add_region, dma_addr, r.addr);
+    expect_value(__wrap_dma_controller_add_region, size, r.size);
+    expect_value(__wrap_dma_controller_add_region, fd, -1);
+    expect_value(__wrap_dma_controller_add_region, offset, r.offset);
+    assert_int_equal(0, handle_dma_map_or_unmap(&lm_ctx, size, true, &fd, 0, &r));
+}
+
+/*
+ * Tests that adding multiple DMA regions where not all of them are mappable
+ * results in only the mappable one being memory mapped.
+ */
+static void
+test_dma_add_regions_mixed(void **state __attribute__((unused)))
+{
+    dma_controller_t dma = { 0 };
+    lm_ctx_t lm_ctx = { .dma = &dma };
+    dma.lm_ctx = &lm_ctx;
+    struct vfio_user_dma_region r[2] = {
+        [0] = {
+            .addr = 0xdeadbeef,
+            .size = 0x1000,
+            .offset = 0
+        },
+        [1] = {
+            .addr = 0xcafebabe,
+            .size = 0x1000,
+            .offset = 0x1000,
+            .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
+        }
+    };
+    int fd = 0x8badf00d;
+
+    patch(dma_controller_add_region);
+    expect_value(__wrap_dma_controller_add_region, dma, lm_ctx.dma);
+    expect_value(__wrap_dma_controller_add_region, dma_addr, r[0].addr);
+    expect_value(__wrap_dma_controller_add_region, size, r[0].size);
+    expect_value(__wrap_dma_controller_add_region, fd, -1);
+    expect_value(__wrap_dma_controller_add_region, offset, r[0].offset);
+    expect_value(__wrap_dma_controller_add_region, dma, lm_ctx.dma);
+    expect_value(__wrap_dma_controller_add_region, dma_addr, r[1].addr);
+    expect_value(__wrap_dma_controller_add_region, size, r[1].size);
+    expect_value(__wrap_dma_controller_add_region, fd, fd);
+    expect_value(__wrap_dma_controller_add_region, offset, r[1].offset);
+
+    assert_int_equal(0, handle_dma_map_or_unmap(&lm_ctx, sizeof r, true, &fd, 1, r));
+}
+
+
+static void
+test_dma_controller_add_region_no_fd(void **state __attribute__((unused)))
+{
     lm_ctx_t lm_ctx = { 0 };
-    assert_true(0 == handle_dma_map_or_unmap(&lm_ctx, size, true, NULL, 0, NULL));
-    /* TODO verify that dma_controller_add_region wasn't called */
+    dma_controller_t dma = { .lm_ctx = &lm_ctx, .max_regions = 1 };
+    dma_addr_t dma_addr = 0xdeadbeef;
+    size_t size = 0;
+    int fd = -1;
+    off_t offset = 0;
+    dma_memory_region_t *r;
+
+    assert_int_equal(0, dma_controller_add_region(&dma, dma_addr, size, fd, offset));
+    assert_int_equal(1, dma.nregions);
+    r = &dma.regions[0];
+    assert_ptr_equal(NULL, r->virt_addr);
+    assert_ptr_equal(dma_addr, r->dma_addr);
+    assert_int_equal(size, r->size);
+    assert_int_equal(0x1000, r->page_size);
+    assert_int_equal(offset, r->offset);
+    assert_int_equal(fd, r->fd);
+    assert_int_equal(0, r->refcnt);
+}
+
+static void
+test_dma_controller_remove_region_no_fd(void **state __attribute__((unused)))
+{
+    dma_memory_region_t r = {
+        .dma_addr = 0xdeadbeef,
+        .size = 0x100,
+        .fd = -1,
+        .virt_addr = NULL
+    };
+    lm_ctx_t lm_ctx = { 0 };
+    dma_controller_t *dma = alloca(sizeof(*dma) + sizeof(r));
+    dma->lm_ctx = &lm_ctx;
+    dma->nregions = 1;
+    dma->max_regions = 1;
+    dma->regions[0] = r;
+    patch(_dma_controller_do_remove_region);
+    expect_value(__wrap__dma_controller_do_remove_region, dma, dma);
+    expect_value(__wrap__dma_controller_do_remove_region, region, &dma->regions[0]); 
+    assert_int_equal(0, dma_controller_remove_region(dma, r.dma_addr, r.size, NULL, NULL));
+}
+
+/*
+ * FIXME we shouldn't have to specify a setup function explicitly for each unit
+ * test, cmocka should provide that. E.g. cmocka_run_group_tests enables us to
+ * run a function before/after ALL unit tests have finished, we can extend it
+ * and provide a function to execute before and after each unit test.
+ */
+static int
+setup(void **state __attribute__((unused))) {
+    unpatch_all();
+    return 0;
 }
 
 int main(void)
 {
-    const struct CMUnitTest tests[] = {
-        cmocka_unit_test(test_dma_map_without_fd),
+   const struct CMUnitTest tests[] = {
+        cmocka_unit_test_setup(test_dma_map_without_dma, setup),
+        cmocka_unit_test_setup(test_dma_map_mappable_without_fd, setup),
+        cmocka_unit_test_setup(test_dma_map_without_fd, setup),
+        cmocka_unit_test_setup(test_dma_add_regions_mixed, setup),
+        cmocka_unit_test_setup(test_dma_controller_add_region_no_fd, setup),
+        cmocka_unit_test_setup(test_dma_controller_remove_region_no_fd, setup),
     };
+
     return cmocka_run_group_tests(tests, NULL, NULL);
 }