From bf3938dec68e1c820063db4f63aa2355c5703e4b Mon Sep 17 00:00:00 2001 From: John Levon Date: Mon, 24 May 2021 10:42:42 +0100 Subject: fix region offset handling (#485) The specification states that the region offset given in the region info should be used as the "offset" when mmap()ing the region from the client side. However, the library instead implemented a fixed offset scheme similar to that of vfio - and no clients actually set up the file like that. Instead, let servers define their own offsets, and pass them through to clients as is. It's up to the server to decide how its backing file or files is organized. Signed-off-by: John Levon Reviewed-by: Thanos Makatos --- test/py/libvfio_user.py | 11 +++-------- test/py/test_device_get_region_info.py | 12 +++++++----- test/unit-tests.c | 25 +++++++++++++------------ 3 files changed, 23 insertions(+), 25 deletions(-) (limited to 'test') diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index e889d95..21f7732 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -227,7 +227,7 @@ vfu_region_access_cb_t = c.CFUNCTYPE(c.c_int, c.c_void_p, c.POINTER(c.c_char), c.c_ulong, c.c_long, c.c_bool) lib.vfu_setup_region.argtypes = (c.c_void_p, c.c_int, c.c_ulong, vfu_region_access_cb_t, c.c_int, c.c_void_p, - c.c_uint32, c.c_int) + c.c_uint32, c.c_int, c.c_ulong) lib.vfu_pci_get_config_space.argtypes = (c.c_void_p,) lib.vfu_pci_get_config_space.restype = (c.c_void_p) lib.vfu_setup_device_nr_irqs.argtypes = (c.c_void_p, c.c_int, c.c_uint32) @@ -239,8 +239,6 @@ lib.vfu_pci_find_capability.restype = (c.c_ulong) lib.vfu_pci_find_next_capability.argtypes = (c.c_void_p, c.c_bool, c.c_ulong, c.c_int) lib.vfu_pci_find_next_capability.restype = (c.c_ulong) -lib.vfu_region_to_offset.argtypes = (c.c_int,) -lib.vfu_region_to_offset.restype = (c.c_ulong) def to_byte(val): @@ -419,7 +417,7 @@ def vfu_destroy_ctx(ctx): os.remove(SOCK_PATH) def vfu_setup_region(ctx, index, size, cb=None, flags=0, - mmap_areas=None, fd=-1): + mmap_areas=None, fd=-1, offset=0): assert ctx != None nr_mmap_areas = 0 @@ -436,7 +434,7 @@ def vfu_setup_region(ctx, index, size, cb=None, flags=0, ret = lib.vfu_setup_region(ctx, index, size, c.cast(cb, vfu_region_access_cb_t), - flags, c_mmap_areas, nr_mmap_areas, fd) + flags, c_mmap_areas, nr_mmap_areas, fd, offset) return ret def vfu_setup_device_nr_irqs(ctx, irqtype, count): @@ -463,6 +461,3 @@ def vfu_pci_find_next_capability(ctx, extended, offset, cap_id): assert ctx != None return lib.vfu_pci_find_next_capability(ctx, extended, offset, cap_id) - -def vfu_region_to_offset(region): - return lib.vfu_region_to_offset(region) diff --git a/test/py/test_device_get_region_info.py b/test/py/test_device_get_region_info.py index 101a561..742e689 100644 --- a/test/py/test_device_get_region_info.py +++ b/test/py/test_device_get_region_info.py @@ -50,9 +50,9 @@ def test_device_get_region_info_setup(): mmap_areas = [ (0x2000, 0x1000), (0x4000, 0x2000) ] - ret = vfu_setup_region(ctx, index=VFU_PCI_DEV_BAR2_REGION_IDX, size=0x10000, + ret = vfu_setup_region(ctx, index=VFU_PCI_DEV_BAR2_REGION_IDX, size=0x8000, flags=(VFU_REGION_FLAG_RW | VFU_REGION_FLAG_MEM), - mmap_areas=mmap_areas, fd=f.fileno()) + mmap_areas=mmap_areas, fd=f.fileno(), offset=0x8000) assert ret == 0 f = tempfile.TemporaryFile() @@ -118,7 +118,7 @@ def test_device_get_region_info_larger_argsz(): assert info.index == VFU_PCI_DEV_BAR1_REGION_IDX assert info.cap_off == 0 assert info.size == 4096 - assert info.offset == vfu_region_to_offset(VFU_PCI_DEV_BAR1_REGION_IDX) + assert info.offset == 0 def test_device_get_region_info_small_argsz_caps(): global sock @@ -139,8 +139,8 @@ def test_device_get_region_info_small_argsz_caps(): VFIO_REGION_INFO_FLAG_CAPS) assert info.index == VFU_PCI_DEV_BAR2_REGION_IDX assert info.cap_off == 0 - assert info.size == 0x10000 - assert info.offset == vfu_region_to_offset(VFU_PCI_DEV_BAR2_REGION_IDX) + assert info.size == 0x8000 + assert info.offset == 0x8000 # skip reading the SCM_RIGHTS disconnect_client(ctx, sock) @@ -165,6 +165,8 @@ def test_device_get_region_info_caps(): assert info.argsz == 80 assert info.cap_off == 32 + assert info.size == 0x8000 + assert info.offset == 0x8000 assert cap.id == VFIO_REGION_INFO_CAP_SPARSE_MMAP assert cap.version == 1 diff --git a/test/unit-tests.c b/test/unit-tests.c index cac4445..2d42f6f 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -572,18 +572,18 @@ test_setup_sparse_region(void **state UNUSED) /* invalid mappable settings */ ret = vfu_setup_region(&vfu_ctx, VFU_PCI_DEV_BAR0_REGION_IDX, - 0x2000, NULL, 0, mmap_areas, 2, -1); + 0x2000, NULL, 0, mmap_areas, 2, -1, 0); assert_int_equal(-1, ret); assert_int_equal(EINVAL, errno); ret = vfu_setup_region(&vfu_ctx, VFU_PCI_DEV_BAR0_REGION_IDX, - 0x2000, NULL, 0, mmap_areas, 0, 1); + 0x2000, NULL, 0, mmap_areas, 0, 1, 0); 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, NULL, 0, 1); + 0x2000, NULL, 0, NULL, 0, 1, 0); assert_int_equal(0, ret); free(reg_info.mmap_areas); @@ -591,14 +591,14 @@ test_setup_sparse_region(void **state UNUSED) /* sparse region exceeds region size */ mmap_areas[1].iov_len = 0x1001; ret = vfu_setup_region(&vfu_ctx, VFU_PCI_DEV_BAR0_REGION_IDX, - 0x2000, NULL, 0, mmap_areas, 2, 0); + 0x2000, NULL, 0, mmap_areas, 2, 0, 0); assert_int_equal(-1, ret); assert_int_equal(EINVAL, errno); /* sparse region within region size */ mmap_areas[1].iov_len = 0x1000; ret = vfu_setup_region(&vfu_ctx, VFU_PCI_DEV_BAR0_REGION_IDX, - 0x2000, NULL, 0, mmap_areas, 2, 0); + 0x2000, NULL, 0, mmap_areas, 2, 0, 0); assert_int_equal(0, ret); free(reg_info.mmap_areas); @@ -960,7 +960,7 @@ test_setup_migration_region_too_small(void **state) vfu_ctx_t *v = get_vfu_ctx(state); int r = vfu_setup_region(v, VFU_PCI_DEV_MIGR_REGION_IDX, vfu_get_migr_register_area_size() - 1, NULL, - VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1); + VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1, 0); assert_int_equal(-1, r); assert_int_equal(EINVAL, errno); } @@ -971,7 +971,7 @@ test_setup_migration_region_size_ok(void **state) vfu_ctx_t *v = get_vfu_ctx(state); int r = vfu_setup_region(v, VFU_PCI_DEV_MIGR_REGION_IDX, vfu_get_migr_register_area_size(), NULL, - VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1); + VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1, 0); assert_int_equal(0, r); } @@ -981,7 +981,7 @@ test_setup_migration_region_fully_mappable(void **state) struct test_setup_migr_reg_dat *p = *state; int r = vfu_setup_region(p->v, VFU_PCI_DEV_MIGR_REGION_IDX, p->s, NULL, VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, - 0xdeadbeef); + 0xdeadbeef, 0); assert_int_equal(-1, r); assert_int_equal(EINVAL, errno); } @@ -997,7 +997,8 @@ test_setup_migration_region_sparsely_mappable_over_migration_registers(void **st } }; int r = vfu_setup_region(p->v, VFU_PCI_DEV_MIGR_REGION_IDX, p->s, NULL, - VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, mmap_areas, 1, 0xdeadbeef); + VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, mmap_areas, 1, + 0xdeadbeef, 0); assert_int_equal(-1, r); assert_int_equal(EINVAL, errno); } @@ -1014,7 +1015,7 @@ test_setup_migration_region_sparsely_mappable_valid(void **state) }; int r = vfu_setup_region(p->v, VFU_PCI_DEV_MIGR_REGION_IDX, p->s, NULL, VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, mmap_areas, 1, - 0xdeadbeef); + 0xdeadbeef, 0); assert_int_equal(0, r); } @@ -1031,7 +1032,7 @@ test_setup_migration_callbacks_bad_data_offset(void **state) { struct test_setup_migr_reg_dat *p = *state; int r = vfu_setup_region(p->v, VFU_PCI_DEV_MIGR_REGION_IDX, p->s, NULL, - VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1); + VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1, 0); assert_int_equal(0, r); r = vfu_setup_device_migration_callbacks(p->v, &p->c, vfu_get_migr_register_area_size() - 1); @@ -1043,7 +1044,7 @@ test_setup_migration_callbacks(void **state) { struct test_setup_migr_reg_dat *p = *state; int r = vfu_setup_region(p->v, VFU_PCI_DEV_MIGR_REGION_IDX, p->s, NULL, - VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1); + VFU_REGION_FLAG_READ | VFU_REGION_FLAG_WRITE, NULL, 0, -1, 0); assert_int_equal(0, r); r = vfu_setup_device_migration_callbacks(p->v, &p->c, vfu_get_migr_register_area_size()); -- cgit v1.1