diff options
| -rw-r--r-- | include/vfio-user.h | 25 | ||||
| -rw-r--r-- | lib/libvfio-user.c | 146 | ||||
| -rw-r--r-- | lib/private.h | 7 | ||||
| -rw-r--r-- | samples/client.c | 33 | ||||
| -rw-r--r-- | test/unit-tests.c | 189 |
5 files changed, 128 insertions, 272 deletions
diff --git a/include/vfio-user.h b/include/vfio-user.h index f6cd75a..3994abc 100644 --- a/include/vfio-user.h +++ b/include/vfio-user.h @@ -107,13 +107,21 @@ struct vfio_user_device_info { uint32_t num_irqs; } __attribute__((packed)); +/* based on struct vfio_bitmap */ +struct vfio_user_bitmap { + uint64_t pgsize; + uint64_t size; + char data[]; +} __attribute__((packed)); + struct vfio_user_dma_region { - uint64_t addr; - uint64_t size; - uint64_t offset; - uint32_t prot; - uint32_t flags; + uint64_t addr; + uint64_t size; + uint64_t offset; + uint32_t prot; #define VFIO_USER_F_DMA_REGION_MAPPABLE (1 << 0) + uint32_t flags; + struct vfio_user_bitmap bitmap[]; } __attribute__((packed)); struct vfio_user_region_access { @@ -133,13 +141,6 @@ struct vfio_user_irq_info { uint32_t subindex; } __attribute__((packed)); -/* based on struct vfio_bitmap */ -struct vfio_user_bitmap { - uint64_t pgsize; - uint64_t size; - char data[]; -} __attribute__((packed)); - /* based on struct vfio_iommu_type1_dirty_bitmap_get */ struct vfio_user_bitmap_range { uint64_t iova; diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 138a27b..ae044e8 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -467,90 +467,95 @@ consume_fd(int *fds, size_t nr_fds, size_t index) } int -handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg) +handle_dma_map(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, + struct vfio_user_dma_region *region) { - struct vfio_user_dma_region *dma_regions = msg->in_data; - bool is_map = (msg->hdr.cmd == VFIO_USER_DMA_MAP); - size_t nr_dma_regions; - size_t fdi; - size_t i; + char rstr[1024]; + int fd = -1; int ret; assert(vfu_ctx != NULL); + assert(msg != NULL); + assert(region != NULL); - if (msg->in_size % sizeof(struct vfio_user_dma_region) != 0) { - vfu_log(vfu_ctx, LOG_ERR, "bad size of DMA regions %zu", msg->in_size); + if (msg->in_size != sizeof(*region)) { + vfu_log(vfu_ctx, LOG_ERR, "bad size of DMA map region %zu", + msg->in_size); return ERROR_INT(EINVAL); } - if (vfu_ctx->dma == NULL) { - return 0; - } + snprintf(rstr, sizeof(rstr), + "[%#lx, %#lx) offset=%#lx prot=%#x flags=%#x", + region->addr, region->addr + region->size, region->offset, + region->prot, region->flags); - nr_dma_regions = msg->in_size / sizeof(struct vfio_user_dma_region); - if (nr_dma_regions == 0) { - return 0; + vfu_log(vfu_ctx, LOG_DEBUG, "adding DMA region %s", rstr); + + if (region->flags == VFIO_USER_F_DMA_REGION_MAPPABLE) { + fd = consume_fd(msg->in_fds, msg->nr_in_fds, 0); + if (fd < 0) { + vfu_log(vfu_ctx, LOG_ERR, + "failed to add DMA region %s: mappable but fd not provided", + rstr); + return -1; + } } - for (i = 0, fdi = 0; i < nr_dma_regions; i++) { - struct vfio_user_dma_region *region = &dma_regions[i]; - char rstr[1024]; + ret = dma_controller_add_region(vfu_ctx->dma, (void *)region->addr, + region->size, fd, region->offset, + region->prot); + if (ret < 0) { + ret = errno; + vfu_log(vfu_ctx, LOG_ERR, "failed to add DMA region %s: %m", rstr); + if (fd != -1) { + close(fd); + } + return ERROR_INT(ret); + } - snprintf(rstr, sizeof(rstr), "[%#lx, %#lx) offset=%#lx " - "prot=%#x flags=%#x", region->addr, region->addr + region->size, - region->offset, region->prot, region->flags); + if (vfu_ctx->dma_register != NULL) { + vfu_ctx->dma_register(vfu_ctx, &vfu_ctx->dma->regions[ret].info); + } + return 0; +} - vfu_log(vfu_ctx, LOG_DEBUG, "%s DMA region %s", - is_map ? "adding" : "removing", rstr); +int +handle_dma_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, + struct vfio_user_dma_region *region) +{ + int ret; + char rstr[1024]; - if (is_map) { - int fd = -1; - if (region->flags == VFIO_USER_F_DMA_REGION_MAPPABLE) { - fd = consume_fd(msg->in_fds, msg->nr_in_fds, fdi++); - if (fd < 0) { - vfu_log(vfu_ctx, LOG_ERR, "failed to add DMA region %s: " - "mappable but fd not provided", rstr); - return -1; - } - } + assert(vfu_ctx != NULL); + assert(msg != NULL); + assert(region != NULL); - ret = dma_controller_add_region(vfu_ctx->dma, (void *)region->addr, - region->size, fd, region->offset, - region->prot); - if (ret < 0) { - ret = errno; - vfu_log(vfu_ctx, LOG_ERR, "failed to add DMA region %s: %m", - rstr); - if (fd != -1) { - close(fd); - } - return ERROR_INT(ret); - } + if (msg->in_size != sizeof(*region)) { + vfu_log(vfu_ctx, LOG_ERR, "bad size of DMA unmap region %zu", + msg->in_size); + return ERROR_INT(EINVAL); + } - if (vfu_ctx->dma_register != NULL) { - vfu_ctx->dma_register(vfu_ctx, - &vfu_ctx->dma->regions[ret].info); - } + snprintf(rstr, sizeof(rstr), "[%#lx, %#lx) offset=%#lx " + "prot=%#x flags=%#x", region->addr, region->addr + region->size, + region->offset, region->prot, region->flags); - ret = 0; - } else { + vfu_log(vfu_ctx, LOG_DEBUG, "removing DMA region %s", rstr); - if (region->flags != 0) { - vfu_log(vfu_ctx, LOG_ERR, "bad flags=%#x", region->flags); - return ERROR_INT(ENOTSUP); - } - ret = dma_controller_remove_region(vfu_ctx->dma, - (void *)region->addr, - region->size, - vfu_ctx->dma_unregister, - vfu_ctx); - if (ret < 0) { - ret = errno; - vfu_log(vfu_ctx, LOG_ERR, "failed to remove DMA region %s: %m", - rstr); - return ERROR_INT(ret); - } - } + if (region->flags != 0) { + vfu_log(vfu_ctx, LOG_ERR, "bad flags=%#x", region->flags); + return ERROR_INT(ENOTSUP); + } + ret = dma_controller_remove_region(vfu_ctx->dma, + (void *)region->addr, + region->size, + vfu_ctx->dma_unregister, + vfu_ctx); + if (ret < 0) { + ret = errno; + vfu_log(vfu_ctx, LOG_WARNING, + "failed to remove DMA region %s: %m", rstr); + return ERROR_INT(ret); } return ret; } @@ -861,8 +866,15 @@ MOCK_DEFINE(exec_command)(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg) switch (msg->hdr.cmd) { case VFIO_USER_DMA_MAP: + if (vfu_ctx->dma != NULL) { + ret = handle_dma_map(vfu_ctx, msg, msg->in_data); + } + break; + case VFIO_USER_DMA_UNMAP: - ret = handle_dma_map_or_unmap(vfu_ctx, msg); + if (vfu_ctx->dma != NULL) { + ret = handle_dma_unmap(vfu_ctx, msg, msg->in_data); + } break; case VFIO_USER_DEVICE_GET_INFO: diff --git a/lib/private.h b/lib/private.h index 11e899c..5824ac4 100644 --- a/lib/private.h +++ b/lib/private.h @@ -177,7 +177,12 @@ int consume_fd(int *fds, size_t nr_fds, size_t index); int -handle_dma_map_or_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg); +handle_dma_map(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, + struct vfio_user_dma_region *dma_regions); + +int +handle_dma_unmap(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, + struct vfio_user_dma_region *dma_regions); int handle_device_get_region_info(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg); diff --git a/samples/client.c b/samples/client.c index a533d4f..0de23cf 100644 --- a/samples/client.c +++ b/samples/client.c @@ -1039,21 +1039,22 @@ migrate_to(char *old_sock_path, int *server_max_fds, } static void -map_dma_regions(int sock, int max_fds, struct vfio_user_dma_region *dma_regions, +map_dma_regions(int sock, struct vfio_user_dma_region *dma_regions, int *dma_region_fds, int nr_dma_regions) { int i, ret; - for (i = 0; i < nr_dma_regions / max_fds; i++) { - struct iovec iovecs[2] = { { 0, } }; - - /* [0] is for the header. */ - iovecs[1].iov_base = dma_regions + (i * max_fds); - iovecs[1].iov_len = sizeof(*dma_regions) * max_fds; - + for (i = 0; i < nr_dma_regions; i++) { + struct iovec iovecs[2] = { + /* [0] is for the header. */ + [1] = { + .iov_base = &dma_regions[i], + .iov_len = sizeof(*dma_regions) + } + }; ret = tran_sock_msg_iovec(sock, 0x1234 + i, VFIO_USER_DMA_MAP, iovecs, ARRAY_SIZE(iovecs), - dma_region_fds + (i * max_fds), max_fds, + &dma_region_fds[i], 1, NULL, NULL, 0, NULL, 0); if (ret < 0) { err(EXIT_FAILURE, "failed to map DMA regions"); @@ -1137,9 +1138,7 @@ int main(int argc, char *argv[]) /* * XXX VFIO_USER_DMA_MAP * - * Tell the server we have some DMA regions it can access. Each DMA region - * is accompanied by a file descriptor, so let's create more (2x) DMA - * regions that can fit in a message that can be handled by the server. + * Tell the server we have some DMA regions it can access. */ nr_dma_regions = server_max_fds << 1; @@ -1163,8 +1162,7 @@ int main(int argc, char *argv[]) dma_region_fds[i] = fileno(fp); } - map_dma_regions(sock, server_max_fds, dma_regions, dma_region_fds, - nr_dma_regions); + map_dma_regions(sock, dma_regions, dma_region_fds, nr_dma_regions); /* * XXX VFIO_USER_DEVICE_GET_IRQ_INFO and VFIO_IRQ_SET_ACTION_TRIGGER @@ -1219,9 +1217,10 @@ int main(int argc, char *argv[]) memcpy(r, dma_regions, sizeof(r)); for (i = 0; i < (int)ARRAY_SIZE(r); i++) { r[i].flags = 0; + ret = tran_sock_msg(sock, 7, VFIO_USER_DMA_UNMAP, &r[i], + sizeof(struct vfio_user_dma_region), + NULL, NULL, 0); } - ret = tran_sock_msg(sock, 7, VFIO_USER_DMA_UNMAP, r, sizeof(r), - NULL, NULL, 0); } if (ret < 0) { err(EXIT_FAILURE, "failed to unmap DMA regions"); @@ -1268,7 +1267,7 @@ int main(int argc, char *argv[]) * XXX reconfigure DMA regions, note that the first half of the has been * unmapped. */ - map_dma_regions(sock, server_max_fds, dma_regions + server_max_fds, + map_dma_regions(sock, dma_regions + server_max_fds, dma_region_fds + server_max_fds, nr_dma_regions - server_max_fds); diff --git a/test/unit-tests.c b/test/unit-tests.c index 3ae2825..e712867 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -115,19 +115,7 @@ setup(void **state UNUSED) return 0; } -static void -test_dma_map_without_dma(void **state UNUSED) -{ - struct vfio_user_dma_region dma_region = { - .flags = VFIO_USER_F_DMA_REGION_MAPPABLE - }; - - vfu_ctx.dma = NULL; - - ret = handle_dma_map_or_unmap(&vfu_ctx, mkmsg(VFIO_USER_DMA_MAP, - &dma_region, sizeof(dma_region))); - assert_int_equal(0, ret); -} +/* FIXME must replace test_dma_map_without_dma */ static void test_dma_map_mappable_without_fd(void **state UNUSED) @@ -136,8 +124,9 @@ test_dma_map_mappable_without_fd(void **state UNUSED) .flags = VFIO_USER_F_DMA_REGION_MAPPABLE }; - ret = handle_dma_map_or_unmap(&vfu_ctx, mkmsg(VFIO_USER_DMA_MAP, - &dma_region, sizeof(dma_region))); + ret = handle_dma_map(&vfu_ctx, + mkmsg(VFIO_USER_DMA_MAP, &dma_region, sizeof(dma_region)), + &dma_region); assert_int_equal(-1, ret); assert_int_equal(errno, EINVAL); } @@ -161,8 +150,7 @@ test_dma_map_without_fd(void **state UNUSED) expect_value(dma_controller_add_region, fd, -1); expect_value(dma_controller_add_region, offset, r.offset); expect_value(dma_controller_add_region, prot, r.prot); - ret = handle_dma_map_or_unmap(&vfu_ctx, mkmsg(VFIO_USER_DMA_MAP, - &r, sizeof(r))); + ret = handle_dma_map(&vfu_ctx, mkmsg(VFIO_USER_DMA_MAP, &r, sizeof(r)), &r); assert_int_equal(0, ret); } @@ -183,156 +171,7 @@ check_dma_info(const LargestIntegralType value, } /* - * Tests that adding multiple DMA regions that not all of them are mappable - * results in only the mappable one being memory mapped. - * FIXME this test no longer tests what it's supposed to be testing, since - * checking that a non-mappable DMA region doesn't get mmap'ed can only be done - * in dma_controller_add_region. We don't have such a test (we only have - * test_dma_controller_add_region_no_fd), we should add it. - */ -static void -test_dma_add_regions_mixed(void **state UNUSED) -{ - size_t count = 0; - struct vfio_user_dma_region r[2] = { - [0] = { - .addr = 0xdeadbeef, - .size = 0x1000, - .offset = 0, - .prot = PROT_NONE - }, - [1] = { - .addr = 0xcafebabe, - .size = 0x1000, - .offset = 0x1000, - .flags = VFIO_USER_F_DMA_REGION_MAPPABLE, - .prot = PROT_READ | PROT_WRITE - } - }; - - vfu_ctx.dma_register = mock_dma_register; - vfu_ctx.pvt = &count; - - fds[0] = 0x0badf00d; - nr_fds = 1; - - vfu_ctx.dma->regions[0].info.iova.iov_base = (void *)r[0].addr; - vfu_ctx.dma->regions[0].info.iova.iov_len = r[0].size; - vfu_ctx.dma->regions[0].info.prot = r[0].prot; - vfu_ctx.dma->regions[1].info.iova.iov_base = (void *)r[1].addr; - vfu_ctx.dma->regions[1].info.iova.iov_len = r[1].size; - vfu_ctx.dma->regions[1].info.prot = r[1].prot; - vfu_ctx.dma->nregions = 2; - - patch("dma_controller_add_region"); - /* 1st region */ - will_return(dma_controller_add_region, 0); - will_return(dma_controller_add_region, 0); - expect_value(dma_controller_add_region, dma, vfu_ctx.dma); - expect_value(dma_controller_add_region, dma_addr, r[0].addr); - expect_value(dma_controller_add_region, size, r[0].size); - expect_value(dma_controller_add_region, fd, -1); - expect_value(dma_controller_add_region, offset, r[0].offset); - expect_value(dma_controller_add_region, prot, r[0].prot); - expect_value(mock_dma_register, vfu_ctx, &vfu_ctx); - expect_check(mock_dma_register, info, check_dma_info, - &vfu_ctx.dma->regions[0].info); - /* 2nd region */ - will_return(dma_controller_add_region, 0); - will_return(dma_controller_add_region, 1); - expect_value(dma_controller_add_region, dma, vfu_ctx.dma); - expect_value(dma_controller_add_region, dma_addr, r[1].addr); - expect_value(dma_controller_add_region, size, r[1].size); - expect_value(dma_controller_add_region, fd, fds[0]); - expect_value(dma_controller_add_region, offset, r[1].offset); - expect_value(dma_controller_add_region, prot, r[1].prot); - expect_value(mock_dma_register, vfu_ctx, &vfu_ctx); - expect_check(mock_dma_register, info, check_dma_info, - &vfu_ctx.dma->regions[1].info); - - ret = handle_dma_map_or_unmap(&vfu_ctx, mkmsg(VFIO_USER_DMA_MAP, - r, sizeof(r))); - assert_int_equal(0, ret); - assert_int_equal(-1, fds[0]); -} - -/* - * Tests that handle_dma_map_or_unmap closes unconsumed file descriptors when - * failing halfway through. - */ -static void -test_dma_add_regions_mixed_partial_failure(void **state UNUSED) -{ - struct vfio_user_dma_region r[3] = { - [0] = { - .addr = 0xdeadbeef, - .size = 0x1000, - .offset = 0 - }, - [1] = { - .addr = 0xcafebabe, - .size = 0x1000, - .offset = 0x1000, - .flags = VFIO_USER_F_DMA_REGION_MAPPABLE, - .prot = PROT_READ - }, - [2] = { - .addr = 0xbabecafe, - .size = 0x1000, - .offset = 0x2000, - .flags = VFIO_USER_F_DMA_REGION_MAPPABLE, - .prot = PROT_READ|PROT_WRITE - } - }; - - fds[0] = 0xa; - fds[1] = 0xb; - nr_fds = 2; - - patch("dma_controller_add_region"); - - /* 1st region */ - will_return(dma_controller_add_region, 0); - will_return(dma_controller_add_region, 0); - expect_value(dma_controller_add_region, dma, vfu_ctx.dma); - expect_value(dma_controller_add_region, dma_addr, r[0].addr); - expect_value(dma_controller_add_region, size, r[0].size); - expect_value(dma_controller_add_region, fd, -1); - expect_value(dma_controller_add_region, offset, r[0].offset); - expect_value(dma_controller_add_region, prot, r[0].prot); - - /* 2nd region */ - will_return(dma_controller_add_region, 0); - will_return(dma_controller_add_region, 1); - expect_value(dma_controller_add_region, dma, vfu_ctx.dma); - expect_value(dma_controller_add_region, dma_addr, r[1].addr); - expect_value(dma_controller_add_region, size, r[1].size); - expect_value(dma_controller_add_region, fd, fds[0]); - expect_value(dma_controller_add_region, offset, r[1].offset); - expect_value(dma_controller_add_region, prot, r[1].prot); - - /* 3rd region */ - will_return(dma_controller_add_region, EREMOTEIO); - will_return(dma_controller_add_region, -1); - expect_value(dma_controller_add_region, dma, vfu_ctx.dma); - expect_value(dma_controller_add_region, dma_addr, r[2].addr); - expect_value(dma_controller_add_region, size, r[2].size); - expect_value(dma_controller_add_region, fd, fds[1]); - expect_value(dma_controller_add_region, offset, r[2].offset); - expect_value(dma_controller_add_region, prot, r[2].prot); - - patch("close"); - expect_value(close, fd, 0xb); - will_return(close, 0); - - ret = handle_dma_map_or_unmap(&vfu_ctx, mkmsg(VFIO_USER_DMA_MAP, - r, sizeof(r))); - assert_int_equal(-1, ret); - assert_int_equal(EREMOTEIO, errno); -} - -/* - * Checks that handle_dma_map_or_unmap returns 0 when dma_controller_add_region + * Checks that handle_dma_map returns 0 when dma_controller_add_region * succeeds. */ static void @@ -353,12 +192,14 @@ test_dma_map_return_value(void **state UNUSED) will_return(dma_controller_add_region, 0); will_return(dma_controller_add_region, 2); - assert_int_equal(0, handle_dma_map_or_unmap(&vfu_ctx, - mkmsg(VFIO_USER_DMA_MAP, &r, sizeof(r)))); + assert_int_equal(0, handle_dma_map(&vfu_ctx, + mkmsg(VFIO_USER_DMA_MAP, &r, + sizeof(r)), + &r)); } /* - * Tests that handle_dma_map_or_unmap correctly removes a region. + * Tests that handle_dma_unmap correctly removes a region. */ static void test_handle_dma_unmap(void **state UNUSED) @@ -385,8 +226,9 @@ test_handle_dma_unmap(void **state UNUSED) &vfu_ctx.dma->regions[0].info); will_return(mock_dma_unregister, 0); - ret = handle_dma_map_or_unmap(&vfu_ctx, mkmsg(VFIO_USER_DMA_UNMAP, - &r, sizeof(r))); + ret = handle_dma_unmap(&vfu_ctx, + mkmsg(VFIO_USER_DMA_UNMAP, &r, sizeof(r)), + &r); assert_int_equal(0, ret); assert_int_equal(2, vfu_ctx.dma->nregions); @@ -946,11 +788,8 @@ int main(void) { 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_add_regions_mixed_partial_failure, setup), cmocka_unit_test_setup(test_dma_map_return_value, setup), cmocka_unit_test_setup(test_handle_dma_unmap, setup), cmocka_unit_test_setup(test_dma_controller_add_region_no_fd, setup), |