diff options
author | Thanos Makatos <thanos.makatos@nutanix.com> | 2020-12-14 15:50:37 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-12-14 15:50:37 +0000 |
commit | f240c889171ab22e7b8b0b460792b0df94c335c1 (patch) | |
tree | c0a60b0dbd7f32ffac3b8caf8d1bbcfa8bfed491 | |
parent | d7157efb22c9fc06f85e40f0e6b6c5e16dc5e8e2 (diff) | |
download | libvfio-user-f240c889171ab22e7b8b0b460792b0df94c335c1.zip libvfio-user-f240c889171ab22e7b8b0b460792b0df94c335c1.tar.gz libvfio-user-f240c889171ab22e7b8b0b460792b0df94c335c1.tar.bz2 |
return region capabilities a la VFIO (#187)
This patch returns region capabilities the same way VFIO does: if argsz
is not large enough then it returns only region info and sets argsz to
what it should be in order to fit the capabilities, the client then
retries with a large enough argsz. The protocol specification has been
updated as well.
Plus unit tests.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
-rw-r--r-- | lib/libvfio-user.c | 44 | ||||
-rw-r--r-- | lib/private.h | 3 | ||||
-rw-r--r-- | samples/client.c | 103 | ||||
-rw-r--r-- | samples/server.c | 14 | ||||
-rw-r--r-- | test/unit-tests.c | 63 |
5 files changed, 160 insertions, 67 deletions
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index c3cd1e2..042a473 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -154,12 +154,6 @@ dev_get_caps(vfu_ctx_t *vfu_ctx, vfu_reg_info_t *vfu_reg, bool is_migr_reg, sparse->areas[i].offset + sparse->areas[i].size); } } - - /* - * FIXME VFIO_REGION_INFO_FLAG_MMAP is valid if the region is - * memory-mappable in general, not only if it supports sparse mmap. - */ - vfio_reg->flags |= VFIO_REGION_INFO_FLAG_MMAP | VFIO_REGION_INFO_FLAG_CAPS; } #define VFU_REGION_SHIFT 40 @@ -211,13 +205,12 @@ is_migr_reg(vfu_ctx_t *vfu_ctx, int index) return &vfu_ctx->reg_info[index] == vfu_ctx->migr_reg; } -static long -dev_get_reginfo(vfu_ctx_t *vfu_ctx, uint32_t index, +long +dev_get_reginfo(vfu_ctx_t *vfu_ctx, uint32_t index, uint32_t argsz, struct vfio_region_info **vfio_reg) { vfu_reg_info_t *vfu_reg; size_t caps_size; - uint32_t argsz; assert(vfu_ctx != NULL); assert(vfio_reg != NULL); @@ -225,26 +218,38 @@ dev_get_reginfo(vfu_ctx_t *vfu_ctx, uint32_t index, vfu_reg = &vfu_ctx->reg_info[index]; if (index >= vfu_ctx->nr_regions) { - vfu_log(vfu_ctx, LOG_DEBUG, "bad region index %d", index); + vfu_log(vfu_ctx, LOG_DEBUG, "bad region index %d in get region info", + index); return -EINVAL; } - caps_size = get_vfio_caps_size(is_migr_reg(vfu_ctx, index), - vfu_reg->mmap_areas); - argsz = caps_size + sizeof(struct vfio_region_info); + if (argsz < sizeof(struct vfio_region_info)) { + vfu_log(vfu_ctx, LOG_DEBUG, "bad argsz %d", argsz); + return -EINVAL; + } + + /* + * TODO We assume that the client expects to receive argsz bytes. + */ *vfio_reg = calloc(1, argsz); if (!*vfio_reg) { return -ENOMEM; } - /* FIXME document in the protocol that vfio_req->argsz is ignored */ - (*vfio_reg)->argsz = argsz; + caps_size = get_vfio_caps_size(is_migr_reg(vfu_ctx, index), + vfu_reg->mmap_areas); + (*vfio_reg)->argsz = caps_size + sizeof(struct vfio_region_info); (*vfio_reg)->flags = vfu_reg->flags; (*vfio_reg)->index = index; (*vfio_reg)->offset = region_to_offset((*vfio_reg)->index); (*vfio_reg)->size = vfu_reg->size; if (caps_size > 0) { - dev_get_caps(vfu_ctx, vfu_reg, is_migr_reg(vfu_ctx, index), *vfio_reg); + if (vfu_reg->mmap_areas != NULL) { + (*vfio_reg)->flags |= VFIO_REGION_INFO_FLAG_CAPS; + } + if (argsz >= (*vfio_reg)->argsz) { + dev_get_caps(vfu_ctx, vfu_reg, is_migr_reg(vfu_ctx, index), *vfio_reg); + } } vfu_log(vfu_ctx, LOG_DEBUG, "region_info[%d] offset %#llx flags %#x size %llu " @@ -479,11 +484,12 @@ handle_device_get_region_info(vfu_ctx_t *vfu_ctx, uint32_t size, struct vfio_region_info *reg_info_in, struct vfio_region_info **reg_info_out) { - if (size != sizeof(*reg_info_in) || size != reg_info_in->argsz) { + if (size < sizeof(*reg_info_in)) { return -EINVAL; } - return dev_get_reginfo(vfu_ctx, reg_info_in->index, reg_info_out); + return dev_get_reginfo(vfu_ctx, reg_info_in->index, reg_info_in->argsz, + reg_info_out); } int @@ -955,7 +961,7 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size, &dev_reg_info); if (ret == 0) { _iovecs[1].iov_base = dev_reg_info; - _iovecs[1].iov_len = dev_reg_info->argsz; + _iovecs[1].iov_len = hdr->msg_size; *iovecs = _iovecs; *nr_iovecs = 2; } diff --git a/lib/private.h b/lib/private.h index 5321734..7860ec2 100644 --- a/lib/private.h +++ b/lib/private.h @@ -171,6 +171,9 @@ consume_fd(int *fds, size_t nr_fds, size_t index); int handle_device_get_info(vfu_ctx_t *vfu_ctx, uint32_t size, struct vfio_device_info *dev_info); +long +dev_get_reginfo(vfu_ctx_t *vfu_ctx, uint32_t index, uint32_t argsz, + struct vfio_region_info **vfio_reg); #endif /* LIB_VFIO_USER_PRIVATE_H */ diff --git a/samples/client.c b/samples/client.c index 99289cb..3e3ba54 100644 --- a/samples/client.c +++ b/samples/client.c @@ -207,26 +207,13 @@ send_device_reset(int sock) /* returns whether a VFIO migration capability is found */ static bool -get_region_vfio_caps(int sock, size_t cap_sz) +get_region_vfio_caps(struct vfio_info_cap_header *header) { - struct vfio_info_cap_header *header, *_header; struct vfio_region_info_cap_type *type; struct vfio_region_info_cap_sparse_mmap *sparse; unsigned int i; - ssize_t ret; bool migr = false; - header = _header = calloc(cap_sz, 1); - if (header == NULL) { - err(EXIT_FAILURE, NULL); - } - - ret = recv(sock, header, cap_sz, 0); - if (ret < 0) { - err(EXIT_FAILURE, "failed to receive VFIO cap info"); - } - assert((size_t)ret == cap_sz); - while (true) { switch (header->id) { case VFIO_REGION_INFO_CAP_SPARSE_MMAP: @@ -256,47 +243,70 @@ get_region_vfio_caps(int sock, size_t cap_sz) } header = (struct vfio_info_cap_header*)((char*)header + header->next - sizeof(struct vfio_region_info)); } - free(_header); return migr; } +static void +do_get_device_region_info(int sock, struct vfio_region_info *region_info) +{ + int ret = vfu_msg(sock, 0, + VFIO_USER_DEVICE_GET_REGION_INFO, + region_info, region_info->argsz, + NULL, + region_info, region_info->argsz); + if (ret < 0) { + errx(EXIT_FAILURE, "failed to get device region info: %s", + strerror(-ret)); + } +} + +static bool +get_device_region_info(int sock, uint32_t index) +{ + struct vfio_region_info *region_info; + size_t cap_sz; + size_t size = sizeof(struct vfio_region_info); + + region_info = alloca(size); + region_info->argsz = size; + region_info->index = index; + + do_get_device_region_info(sock, region_info); + if (region_info->argsz > size) { + size = region_info->size; + region_info = alloca(size); + region_info->argsz = size; + region_info->index = index; + do_get_device_region_info(sock, region_info); + assert(region_info->size == size); + } + + cap_sz = region_info->argsz - sizeof(struct vfio_region_info); + printf("%s: region_info[%d] offset %#llx flags %#x size %llu " + "cap_sz %lu\n", __func__, index, region_info->offset, + region_info->flags, region_info->size, cap_sz); + if (cap_sz) { + if (get_region_vfio_caps((struct vfio_info_cap_header*)(region_info + 1))) { + return true; + } + } + return false; +} + /* * Returns the index of the migration region if found, -1 otherwise. */ static int -get_device_region_info(int sock, struct vfio_device_info *client_dev_info) +get_device_regions_info(int sock, struct vfio_device_info *client_dev_info) { - struct vfio_region_info region_info; - uint16_t msg_id = 1; - size_t cap_sz; - int ret, migr_reg_index = -1; + int migr_reg_index = -1; unsigned int i; for (i = 0; i < client_dev_info->num_regions; i++) { - memset(®ion_info, 0, sizeof(region_info)); - region_info.argsz = sizeof(region_info); - region_info.index = i; - msg_id++; - ret = vfu_msg(sock, msg_id, - VFIO_USER_DEVICE_GET_REGION_INFO, - ®ion_info, sizeof region_info, - NULL, - ®ion_info, sizeof(region_info)); - if (ret < 0) { - errx(EXIT_FAILURE, "failed to get device region info: %s", - strerror(-ret)); + if (get_device_region_info(sock, i)) { + assert(migr_reg_index == -1); + migr_reg_index = i; } - - cap_sz = region_info.argsz - sizeof(struct vfio_region_info); - printf("%s: region_info[%d] offset %#llx flags %#x size %llu " - "cap_sz %lu\n", __func__, i, region_info.offset, - region_info.flags, region_info.size, cap_sz); - if (cap_sz) { - if (get_region_vfio_caps(sock, cap_sz)) { - assert(migr_reg_index == -1); - migr_reg_index = i; - } - } } return migr_reg_index; } @@ -319,6 +329,11 @@ get_device_info(int sock, struct vfio_device_info *dev_info) errx(EXIT_FAILURE, "failed to get device info: %s", strerror(-ret)); } + if (dev_info->num_regions != 10) { + errx(EXIT_FAILURE, "bad number of device regions %d", + dev_info->num_regions); + } + printf("devinfo: flags %#x, num_regions %d, num_irqs %d\n", dev_info->flags, dev_info->num_regions, dev_info->num_irqs); } @@ -952,7 +967,7 @@ int main(int argc, char *argv[]) get_device_info(sock, &client_dev_info); /* XXX VFIO_USER_DEVICE_GET_REGION_INFO */ - migr_reg_index = get_device_region_info(sock, &client_dev_info); + migr_reg_index = get_device_regions_info(sock, &client_dev_info); if (migr_reg_index == -1) { errx(EXIT_FAILURE, "could not find migration region"); } diff --git a/samples/server.c b/samples/server.c index 9180591..a1bedac 100644 --- a/samples/server.c +++ b/samples/server.c @@ -123,6 +123,8 @@ bar1_access(UNUSED void *pvt, UNUSED char * const buf, UNUSED size_t count, { assert(false); + /* FIXME assert that only the 2nd page is accessed */ + return -ENOTSUP; } @@ -434,13 +436,17 @@ int main(int argc, char *argv[]) err(EXIT_FAILURE, "failed to setup BAR0 region"); } + /* + * Setup BAR1 to be 3 pages in size where only the first and the last pages + * are mappable. + */ struct iovec mmap_areas[] = { - { .iov_base = (void*)0x400, .iov_len = 0x400 }, - { .iov_base = (void*)0x2000, .iov_len = 0x400 } + { .iov_base = (void*)0, .iov_len = 0x1000 }, + { .iov_base = (void*)0x2000, .iov_len = 0x1000 } }; ret = vfu_setup_region(vfu_ctx, VFU_PCI_DEV_BAR1_REGION_IDX, - sysconf(_SC_PAGESIZE), &bar1_access, - VFU_REGION_FLAG_RW, mmap_areas, 2, map_area); + 0x3000, &bar1_access, VFU_REGION_FLAG_RW, + mmap_areas, 3, map_area); if (ret < 0) { err(EXIT_FAILURE, "failed to setup BAR1 region"); } diff --git a/test/unit-tests.c b/test/unit-tests.c index 347b7f7..c3946b0 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -396,6 +396,68 @@ test_run_ctx(UNUSED void **state) assert_int_equal(-1, vfu_run_ctx(&vfu_ctx)); } +static void +test_get_region_info(UNUSED void **state) +{ + vfu_reg_info_t reg_info[] = { + { + .size = 0xcadebabe + }, + { + .flags = VFU_REGION_FLAG_RW, + .size = 0xdeadbeef + } + }; + vfu_ctx_t vfu_ctx = { + .nr_regions = 2, + .reg_info = reg_info + }; + 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)); + + /* bad argsz */ + assert_int_equal(-EINVAL, dev_get_reginfo(&vfu_ctx, index, argsz, &vfio_reg)); + + /* bad region */ + index = vfu_ctx.nr_regions; + argsz = sizeof(struct vfio_region_info); + assert_int_equal(-EINVAL, dev_get_reginfo(&vfu_ctx, index, argsz, &vfio_reg)); + + /* no region caps */ + index = 1; + assert_int_equal(0, dev_get_reginfo(&vfu_ctx, index, argsz, &vfio_reg)); + assert_int_equal(sizeof(struct vfio_region_info), vfio_reg->argsz); + assert_int_equal(VFU_REGION_FLAG_RW, vfio_reg->flags); + assert_int_equal(1, vfio_reg->index); + assert_int_equal(0x10000000000, region_to_offset(vfio_reg->index)); + assert_int_equal(0xdeadbeef, vfio_reg->size); + + /* 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].flags |= VFIO_REGION_INFO_FLAG_MMAP; + assert_int_equal(0, dev_get_reginfo(&vfu_ctx, index, argsz, &vfio_reg)); + assert_int_equal(argsz + sizeof(struct vfio_region_info_cap_sparse_mmap) + sizeof(struct vfio_region_sparse_mmap_area), + vfio_reg->argsz); + assert_int_equal(VFU_REGION_FLAG_RW | VFIO_REGION_INFO_FLAG_MMAP | VFIO_REGION_INFO_FLAG_CAPS, + vfio_reg->flags); + + /* region caps and argsz large enough */ + argsz += sizeof(struct vfio_region_info_cap_sparse_mmap) + sizeof(struct vfio_region_sparse_mmap_area); + assert_int_equal(0, dev_get_reginfo(&vfu_ctx, index, argsz, &vfio_reg)); + struct vfio_region_info_cap_sparse_mmap *sparse = (struct vfio_region_info_cap_sparse_mmap*)(vfio_reg + 1); + assert_int_equal(VFIO_REGION_INFO_CAP_SPARSE_MMAP, sparse->header.id); + assert_int_equal(1, sparse->header.version); + assert_int_equal(0, sparse->header.next); + assert_int_equal(1, sparse->nr_areas); + + /* FIXME add check for migration region and for multiple sparse areas */ +} + /* * FIXME expand and validate */ @@ -525,6 +587,7 @@ int main(void) cmocka_unit_test_setup(test_vfu_ctx_create, setup), cmocka_unit_test_setup(test_pci_caps, setup), cmocka_unit_test_setup(test_device_get_info, setup), + cmocka_unit_test_setup(test_get_region_info, setup) }; return cmocka_run_group_tests(tests, NULL, NULL); |