From fa5104150bca4182f8a38d39fa50f7e61982568e Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 20 Jan 2021 10:00:17 +0000 Subject: add whole-region mmap area for vfu_setup_region() (#225) If an fd is provided, automatically add a mmap area covering the entire region unless an mmap_areas array is provided. Signed-off-by: John Levon Reviewed-by: Swapnil Ingle --- include/libvfio-user.h | 8 ++--- lib/libvfio-user.c | 87 ++++++++++++++++++++++---------------------------- lib/private.h | 8 ++--- test/unit-tests.c | 17 +++++----- 4 files changed, 52 insertions(+), 68 deletions(-) diff --git a/include/libvfio-user.h b/include/libvfio-user.h index 8e48a7d..0b5ac02 100644 --- a/include/libvfio-user.h +++ b/include/libvfio-user.h @@ -243,11 +243,11 @@ typedef ssize_t (vfu_region_access_cb_t)(vfu_ctx_t *vfu_ctx, char *buf, * @region_idx: region index * @size: size of the region * @region_access: callback function to access region - * @flags: region flags (VFU_REGION_FLAG_) - * @mmap_areas: array of memory mappable areas; must be provided if the region - * is mappable. + * @flags: region flags (VFU_REGION_FLAG_*) + * @mmap_areas: array of memory mappable areas; if an fd is provided, but this + * is NULL, then the entire region is mappable. * @nr_mmap_areas: number of sparse areas in @mmap_areas; must be provided if - * the region is mappable. + * the @mmap_areas is non-NULL, or 0 otherwise. * @fd: file descriptor of the file backing the region if the region is * mappable; it is the server's responsibility to create a file suitable for * memory mapping by the client. diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index b56a490..8bd704a 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -78,7 +78,7 @@ vfu_log(vfu_ctx_t *vfu_ctx, int level, const char *fmt, ...) } static size_t -get_vfio_caps_size(bool is_migr_reg, struct vfu_sparse_mmap_areas *m) +get_vfio_caps_size(bool is_migr_reg, vfu_reg_info_t *reg) { size_t type_size = 0; size_t sparse_size = 0; @@ -87,9 +87,9 @@ get_vfio_caps_size(bool is_migr_reg, struct vfu_sparse_mmap_areas *m) type_size = sizeof(struct vfio_region_info_cap_type); } - if (m != NULL) { + if (reg->nr_mmap_areas != 0) { sparse_size = sizeof(struct vfio_region_info_cap_sparse_mmap) - + (m->nr_mmap_areas * sizeof(struct vfio_region_sparse_mmap_area)); + + (reg->nr_mmap_areas * sizeof(struct vfio_region_sparse_mmap_area)); } return type_size + sparse_size; @@ -107,7 +107,6 @@ dev_get_caps(vfu_ctx_t *vfu_ctx, vfu_reg_info_t *vfu_reg, bool is_migr_reg, struct vfio_info_cap_header *header; struct vfio_region_info_cap_type *type = NULL; struct vfio_region_info_cap_sparse_mmap *sparse = NULL; - struct vfu_sparse_mmap_areas *mmap_areas; assert(vfu_ctx != NULL); assert(vfio_reg != NULL); @@ -127,7 +126,7 @@ dev_get_caps(vfu_ctx_t *vfu_ctx, vfu_reg_info_t *vfu_reg, bool is_migr_reg, } if (vfu_reg->mmap_areas != NULL) { - int i, nr_mmap_areas = vfu_reg->mmap_areas->nr_mmap_areas; + int i, nr_mmap_areas = vfu_reg->nr_mmap_areas; if (type != NULL) { type->header.next = vfio_reg->cap_offset + sizeof(struct vfio_region_info_cap_type); sparse = (struct vfio_region_info_cap_sparse_mmap*)(type + 1); @@ -151,14 +150,15 @@ dev_get_caps(vfu_ctx_t *vfu_ctx, vfu_reg_info_t *vfu_reg, bool is_migr_reg, sparse->header.next = 0; sparse->nr_areas = *nr_fds = nr_mmap_areas; - mmap_areas = vfu_reg->mmap_areas; for (i = 0; i < nr_mmap_areas; i++) { + struct iovec *iov = &vfu_reg->mmap_areas[i]; + + vfu_log(vfu_ctx, LOG_DEBUG, "%s: area %d [%#llx-%#llx)", __func__, + i, iov->iov_base, iov->iov_base + iov->iov_len); + (*fds)[i] = vfu_reg->fd; - sparse->areas[i].offset = (__u64)mmap_areas->areas[i].iov_base; - sparse->areas[i].size = mmap_areas->areas[i].iov_len; - vfu_log(vfu_ctx, LOG_DEBUG, "%s: area %d %#llx-%#llx", __func__, - i, sparse->areas[i].offset, - sparse->areas[i].offset + sparse->areas[i].size); + sparse->areas[i].offset = (uintptr_t)iov->iov_base; + sparse->areas[i].size = iov->iov_len; } } return 0; @@ -375,8 +375,7 @@ dev_get_reginfo(vfu_ctx_t *vfu_ctx, uint32_t index, uint32_t argsz, if (!*vfio_reg) { return -ENOMEM; } - caps_size = get_vfio_caps_size(is_migr_reg(vfu_ctx, index), - vfu_reg->mmap_areas); + caps_size = get_vfio_caps_size(is_migr_reg(vfu_ctx, index), vfu_reg); (*vfio_reg)->argsz = caps_size + sizeof(struct vfio_region_info); (*vfio_reg)->index = index; (*vfio_reg)->offset = region_to_offset((*vfio_reg)->index); @@ -1206,49 +1205,24 @@ vfu_setup_log(vfu_ctx_t *vfu_ctx, vfu_log_fn_t *log, int log_level) } static int -copy_sparse_mmap_areas(vfu_reg_info_t *reg_info, - struct iovec *mmap_areas, uint32_t nr_mmap_areas) +copyin_mmap_areas(vfu_reg_info_t *reg_info, + struct iovec *mmap_areas, uint32_t nr_mmap_areas) { - struct vfu_sparse_mmap_areas *smmap_areas; - size_t areas_sz; + size_t size = nr_mmap_areas * sizeof (*mmap_areas); if (mmap_areas == NULL || nr_mmap_areas == 0) { return 0; } - areas_sz = nr_mmap_areas * sizeof(struct iovec); + reg_info->mmap_areas = malloc(size); - smmap_areas = calloc(1, sizeof(struct vfu_sparse_mmap_areas) + areas_sz); - if (smmap_areas == NULL) { + if (reg_info->mmap_areas == NULL) { return -ENOMEM; } - smmap_areas->nr_mmap_areas = nr_mmap_areas; - memcpy(smmap_areas->areas, mmap_areas, areas_sz); - reg_info->mmap_areas = smmap_areas; - - return 0; -} + memcpy(reg_info->mmap_areas, mmap_areas, size); + reg_info->nr_mmap_areas = nr_mmap_areas; -static int -setup_sparse_areas(vfu_reg_info_t *r, struct iovec *mmap_areas, - uint32_t nr_mmap_areas) -{ - int ret, i; - - assert(r != NULL); - - ret = copy_sparse_mmap_areas(r, mmap_areas, nr_mmap_areas); - if (ret < 0) { - return ret; - } - for (i = 0; i < r->mmap_areas->nr_mmap_areas; i++) { - struct iovec *a = &r->mmap_areas->areas[i]; - if ((unsigned long long)a->iov_base + a->iov_len > r->size) { - free(r->mmap_areas); - return -EINVAL; - } - } return 0; } @@ -1257,13 +1231,15 @@ vfu_setup_region(vfu_ctx_t *vfu_ctx, int region_idx, size_t size, vfu_region_access_cb_t *cb, int flags, struct iovec *mmap_areas, uint32_t nr_mmap_areas, int fd) { + struct iovec whole_region = { .iov_base = 0, .iov_len = size }; vfu_reg_info_t *reg; + size_t i; int ret; assert(vfu_ctx != NULL); if ((mmap_areas == NULL) != (nr_mmap_areas == 0) || - (mmap_areas == NULL) != (fd == -1)) { + (mmap_areas != NULL && fd == -1)) { vfu_log(vfu_ctx, LOG_ERR, "invalid mappable region arguments"); return ERROR(EINVAL); } @@ -1282,6 +1258,13 @@ vfu_setup_region(vfu_ctx_t *vfu_ctx, int region_idx, size_t size, return ERROR(EINVAL); } + for (i = 0; i < nr_mmap_areas; i++) { + struct iovec *iov = &mmap_areas[i]; + if ((size_t)iov->iov_base + iov->iov_len > size) { + return ERROR(EINVAL); + } + } + reg = &vfu_ctx->reg_info[region_idx]; reg->flags = flags; @@ -1289,9 +1272,15 @@ vfu_setup_region(vfu_ctx_t *vfu_ctx, int region_idx, size_t size, reg->cb = cb; reg->fd = fd; + if (mmap_areas == NULL && reg->fd != -1) { + mmap_areas = &whole_region; + nr_mmap_areas = 1; + } + if (nr_mmap_areas > 0) { - ret = setup_sparse_areas(reg, mmap_areas, nr_mmap_areas); + ret = copyin_mmap_areas(reg, mmap_areas, nr_mmap_areas); if (ret < 0) { + memset(reg, 0, sizeof (*reg)); return ERROR(-ret); } } @@ -1366,8 +1355,8 @@ vfu_setup_device_migration(vfu_ctx_t *vfu_ctx, vfu_migration_t *migration) migr_reg = &vfu_ctx->reg_info[(vfu_ctx->nr_regions - 1)]; /* FIXME: Are there sparse areas need to be setup flags accordingly */ - ret = copy_sparse_mmap_areas(migr_reg, migration->mmap_areas, - migration->nr_mmap_areas); + ret = copyin_mmap_areas(migr_reg, migration->mmap_areas, + migration->nr_mmap_areas); if (ret < 0) { return ERROR(-ret); } diff --git a/lib/private.h b/lib/private.h index 1674271..84caad5 100644 --- a/lib/private.h +++ b/lib/private.h @@ -68,11 +68,6 @@ typedef struct { struct migration; -struct vfu_sparse_mmap_areas { - int nr_mmap_areas; - struct iovec areas[]; -}; - typedef struct { /* Region flags, see VFU_REGION_FLAG_READ and friends. */ uint32_t flags; @@ -81,7 +76,8 @@ typedef struct { /* Callback that is called when the region is read or written. */ vfu_region_access_cb_t *cb; /* Sparse mmap areas if set. */ - struct vfu_sparse_mmap_areas *mmap_areas; + struct iovec *mmap_areas; + int nr_mmap_areas; /* fd for a mappable region, or -1. */ int fd; } vfu_reg_info_t; diff --git a/test/unit-tests.c b/test/unit-tests.c index c03a37a..1af604e 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -445,6 +445,7 @@ test_run_ctx(UNUSED void **state) static void test_get_region_info(UNUSED void **state) { + struct iovec iov = { .iov_base = (void*)0x8badf00, .iov_len = 0x0d15ea5e }; vfu_reg_info_t reg_info[] = { { .size = 0xcadebabe @@ -463,7 +464,6 @@ test_get_region_info(UNUSED void **state) uint32_t index = 0; uint32_t argsz = 0; struct vfio_region_info *vfio_reg; - struct vfu_sparse_mmap_areas *mmap_areas = alloca(sizeof(struct vfu_sparse_mmap_areas) + sizeof(struct iovec)); int *fds = NULL; size_t nr_fds; @@ -493,10 +493,9 @@ test_get_region_info(UNUSED void **state) assert_int_equal(0, nr_fds); /* regions caps (sparse mmap) but argsz too small */ - mmap_areas->nr_mmap_areas = 1; - mmap_areas->areas[0].iov_base = (void*)0x8badf00d; - mmap_areas->areas[0].iov_len = 0x0d15ea5e; - vfu_ctx.reg_info[1].mmap_areas = mmap_areas; + vfu_ctx.reg_info[1].mmap_areas = &iov; + vfu_ctx.reg_info[1].nr_mmap_areas = 1; + assert_int_equal(0, dev_get_reginfo(&vfu_ctx, index, argsz, &vfio_reg, &fds, &nr_fds)); @@ -798,14 +797,14 @@ test_setup_sparse_region(void **state __attribute__((unused))) assert_int_equal(EINVAL, errno); ret = vfu_setup_region(&vfu_ctx, VFU_PCI_DEV_BAR0_REGION_IDX, - 0x2000, NULL, 0, NULL, 0, 1); + 0x2000, NULL, 0, mmap_areas, 0, 1); assert_int_equal(-1, ret); assert_int_equal(EINVAL, errno); + /* default mmap area if not given */ ret = vfu_setup_region(&vfu_ctx, VFU_PCI_DEV_BAR0_REGION_IDX, - 0x2000, NULL, 0, mmap_areas, 0, 1); - assert_int_equal(-1, ret); - assert_int_equal(EINVAL, errno); + 0x2000, NULL, 0, NULL, 0, 1); + assert_int_equal(0, ret); /* sparse region exceeds region size */ mmap_areas[1].iov_len = 0x1001; -- cgit v1.1