diff options
author | Swapnil Ingle <swapnil.ingle@nutanix.com> | 2021-08-19 15:57:04 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-08-19 15:57:04 +0200 |
commit | 66216488980a40d3cdde892d633a3deeb1d4786f (patch) | |
tree | 5ac67599b911a22ad7aa26b6aed0c141c22005fd | |
parent | 2944bf7402dc424d75627415ea999de99890886f (diff) | |
download | libvfio-user-66216488980a40d3cdde892d633a3deeb1d4786f.zip libvfio-user-66216488980a40d3cdde892d633a3deeb1d4786f.tar.gz libvfio-user-66216488980a40d3cdde892d633a3deeb1d4786f.tar.bz2 |
dma: cleanup dma_{map,unmap}_sg (#591)
* dma: cleanup dma_{map,unmap}_sg
Instead of using index to traverse sg and iovec, better to use it as
pointers. It's more readable and less prone from coding mistakes.
Also adding unit tests for the same.
Signed-off-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: John Levon <john.levon@nutanix.com>
-rw-r--r-- | lib/dma.h | 48 | ||||
-rw-r--r-- | test/unit-tests.c | 60 |
2 files changed, 78 insertions, 30 deletions
@@ -229,35 +229,38 @@ dma_map_sg(dma_controller_t *dma, dma_sg_t *sg, struct iovec *iov, int cnt) { dma_memory_region_t *region; - int i; assert(dma != NULL); assert(sg != NULL); assert(iov != NULL); + assert(cnt > 0); - for (i = 0; i < cnt; i++) { - if (sg[i].region >= dma->nregions) { + do { + if (sg->region >= dma->nregions) { return ERROR_INT(EINVAL); } - region = &dma->regions[sg[i].region]; + region = &dma->regions[sg->region]; if (region->info.vaddr == NULL) { return ERROR_INT(EFAULT); } - if (sg[i].writeable) { + if (sg->writeable) { if (dma->dirty_pgsize > 0) { - _dma_mark_dirty(dma, region, &sg[i]); + _dma_mark_dirty(dma, region, sg); } - LIST_INSERT_HEAD(&dma->maps, &sg[i], entry); + LIST_INSERT_HEAD(&dma->maps, sg, entry); } vfu_log(dma->vfu_ctx, LOG_DEBUG, "map %p-%p", - sg[i].dma_addr + sg[i].offset, - sg[i].dma_addr + sg[i].offset + sg[i].length); - iov[i].iov_base = region->info.vaddr + sg[i].offset; - iov[i].iov_len = sg[i].length; + sg->dma_addr + sg->offset, + sg->dma_addr + sg->offset + sg->length); + iov->iov_base = region->info.vaddr + sg->offset; + iov->iov_len = sg->length; region->refcnt++; - } + + sg++; + iov++; + } while (--cnt > 0); return 0; } @@ -266,9 +269,13 @@ static inline void dma_unmap_sg(dma_controller_t *dma, const dma_sg_t *sg, UNUSED struct iovec *iov, int cnt) { - int i; - for (i = 0; i < cnt; i++) { + assert(dma != NULL); + assert(sg != NULL); + assert(iov != NULL); + assert(cnt > 0); + + do { dma_memory_region_t *r; /* * FIXME this double loop will be removed if we replace the array with @@ -276,20 +283,21 @@ dma_unmap_sg(dma_controller_t *dma, const dma_sg_t *sg, */ for (r = dma->regions; r < dma->regions + dma->nregions && - r->info.iova.iov_base != sg[i].dma_addr; + r->info.iova.iov_base != sg->dma_addr; r++); if (r > dma->regions + dma->nregions) { /* bad region */ continue; } - if (sg[i].writeable) { - LIST_REMOVE(&sg[i], entry); + if (sg->writeable) { + LIST_REMOVE(sg, entry); } vfu_log(dma->vfu_ctx, LOG_DEBUG, "unmap %p-%p", - sg[i].dma_addr + sg[i].offset, - sg[i].dma_addr + sg[i].offset + sg[i].length); + sg->dma_addr + sg->offset, + sg->dma_addr + sg->offset + sg->length); r->refcnt--; - } + sg++; + } while (--cnt > 0); } int diff --git a/test/unit-tests.c b/test/unit-tests.c index bcfd348..b605728 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -360,28 +360,67 @@ test_dma_controller_remove_region_unmapped(void **state UNUSED) static void test_dma_map_sg(void **state UNUSED) { - dma_sg_t sg = { .region = 1 }; - struct iovec iovec = { 0 }; + dma_sg_t sg[] = { {.region = 0}, {.region = 1}, {.region = 2} }; + struct iovec iovec[] = { {0}, {0}, {0} }; /* bad region */ - assert_int_equal(-1, dma_map_sg(vfu_ctx.dma, &sg, &iovec, 1)); + assert_int_equal(-1, dma_map_sg(vfu_ctx.dma, &sg[0], &iovec[0], 1)); assert_int_equal(EINVAL, errno); vfu_ctx.dma->nregions = 1; /* w/o fd */ - sg.region = 0; - assert_int_equal(-1, dma_map_sg(vfu_ctx.dma, &sg, &iovec, 1)); + sg[0].region = 0; + assert_int_equal(-1, dma_map_sg(vfu_ctx.dma, &sg[0], &iovec[0], 1)); assert_int_equal(EFAULT, errno); /* w/ fd */ vfu_ctx.dma->regions[0].info.vaddr = (void *)0xdead0000; - sg.offset = 0x0000beef; - sg.length = 0xcafebabe; - assert_int_equal(0, dma_map_sg(vfu_ctx.dma, &sg, &iovec, 1)); - assert_int_equal(0xdeadbeef, iovec.iov_base); - assert_int_equal(0x00000000cafebabe, iovec.iov_len); + sg[0].offset = 0x0000beef; + sg[0].length = 0xcafebabe; + assert_int_equal(0, dma_map_sg(vfu_ctx.dma, &sg[0], &iovec[0], 1)); + assert_int_equal(0xdeadbeef, iovec[0].iov_base); + assert_int_equal(0x00000000cafebabe, iovec[0].iov_len); + + /* multiple sg entries */ + vfu_ctx.dma->nregions = 3; + vfu_ctx.dma->regions[0].info.vaddr = (void *)0xc0000; + sg[0].offset = 0; + sg[0].length = 0x20000; + vfu_ctx.dma->regions[1].info.vaddr = (void *)0xe0000; + sg[1].length = 0x20000; + vfu_ctx.dma->regions[2].info.vaddr = (void *)0x100000; + sg[2].length = 0x10000; + assert_int_equal(0, dma_map_sg(vfu_ctx.dma, sg, iovec, 3)); + assert_int_equal(0xc0000, iovec[0].iov_base); + assert_int_equal(0x20000, iovec[0].iov_len); + assert_int_equal(0xe0000, iovec[1].iov_base); + assert_int_equal(0x20000, iovec[1].iov_len); + assert_int_equal(0x100000, iovec[2].iov_base); + assert_int_equal(0x10000, iovec[2].iov_len); +} + +static void +test_dma_unmap_sg(void **state UNUSED) +{ + dma_sg_t sg[] = { {.region = 0}, {.region = 1}, {.region = 2} }; + struct iovec iovec[] = { {0}, {0}, {0} }; + + vfu_ctx.dma->nregions = 3; + vfu_ctx.dma->regions[0].info.iova.iov_base = (void *)0xc0000; + vfu_ctx.dma->regions[0].refcnt = 1; + sg[0].dma_addr = (void *)0xc0000; + vfu_ctx.dma->regions[1].info.iova.iov_base = (void *)0xe0000; + vfu_ctx.dma->regions[1].refcnt = 1; + sg[1].dma_addr = (void *)0xe0000; + vfu_ctx.dma->regions[2].info.iova.iov_base = (void *)0x100000; + vfu_ctx.dma->regions[2].refcnt = 1; + sg[2].dma_addr = (void *)0x100000; + dma_unmap_sg(vfu_ctx.dma, sg, iovec, 3); + assert_int_equal(0, vfu_ctx.dma->regions[0].refcnt); + assert_int_equal(0, vfu_ctx.dma->regions[1].refcnt); + assert_int_equal(0, vfu_ctx.dma->regions[2].refcnt); } static void @@ -749,6 +788,7 @@ main(void) cmocka_unit_test_setup(test_dma_controller_remove_region_mapped, setup), cmocka_unit_test_setup(test_dma_controller_remove_region_unmapped, setup), cmocka_unit_test_setup(test_dma_map_sg, setup), + cmocka_unit_test_setup(test_dma_unmap_sg, setup), cmocka_unit_test_setup(test_dma_addr_to_sg, setup), cmocka_unit_test_setup(test_vfu_setup_device_dma, setup), cmocka_unit_test_setup(test_migration_state_transitions, setup), |