From 1abe697341061a6c16e4a5d28b20b5efa7035a81 Mon Sep 17 00:00:00 2001 From: Thanos Makatos Date: Wed, 9 Jun 2021 11:15:34 +0100 Subject: drop mappable flag from DMA map (#553) The flags field belongs to VFIO and it's not a good idea to reuse as new VFIO flags can break things. Instead, we derive whether or not a region is mappable if a file descriptor is passed. Signed-off-by: Thanos Makatos Reviewed-by: Swapnil Ingle Reviewed-by: John Levon --- docs/vfio-user.rst | 21 +++++++++------------ include/libvfio-user.h | 3 +-- include/vfio-user.h | 1 - lib/libvfio-user.c | 13 +++++++------ samples/client.c | 2 +- test/py/libvfio_user.py | 1 - test/py/test_dirty_pages.py | 4 +--- test/unit-tests.c | 4 +--- 8 files changed, 20 insertions(+), 29 deletions(-) diff --git a/docs/vfio-user.rst b/docs/vfio-user.rst index a5d2b43..9e23fb3 100644 --- a/docs/vfio-user.rst +++ b/docs/vfio-user.rst @@ -547,8 +547,6 @@ The request payload for this message is a structure of the following format: | | +-----+------------+ | | | | 1 | writeable | | | | +-----+------------+ | -| | | 2 | mappable | | -| | +-----+------------+ | +-------------+--------+-------------+ | offset | 8 | 8 | +-------------+--------+-------------+ @@ -565,9 +563,6 @@ The request payload for this message is a structure of the following format: * *writeable* indicates that the region can be written to. - * *mappable* indicates that the region can be mapped via the mmap() system - call using the file descriptor provided in the message meta-data. - * *offset* is the file offset of the region with respect to the associated file descriptor, or zero if the region is not mappable * *address* is the base DMA address of the region. @@ -576,13 +571,15 @@ The request payload for this message is a structure of the following format: This structure is 32 bytes in size, so the message size is 16 + 32 bytes. If the DMA region being added can be directly mapped by the server, a file -descriptor must be sent as part of the message meta-data. On ``AF_UNIX`` -sockets, the file descriptor must be passed as ``SCM_RIGHTS`` type ancillary -data. Otherwise, if the DMA region cannot be directly mapped by the server, it -can be accessed by the server using ``VFIO_USER_DMA_READ`` and -``VFIO_USER_DMA_WRITE`` messages, explained in `Read and Write Operations`_. A -command to map over an existing region must be failed by the server with -``EEXIST`` set in error field in the reply. +descriptor must be sent as part of the message meta-data. The region can be +mapped via the mmap() system call. On ``AF_UNIX`` sockets, the file descriptor +must be passed as ``SCM_RIGHTS`` type ancillary data. Otherwise, if the DMA +region cannot be directly mapped by the server, no file descriptor must be sent +as part of the message meta-data and the DMA region can be accessed by the +server using ``VFIO_USER_DMA_READ`` and ``VFIO_USER_DMA_WRITE`` messages, +explained in `Read and Write Operations`_. A command to map over an existing +region must be failed by the server with ``EEXIST`` set in error field in the +reply. Reply ^^^^^ diff --git a/include/libvfio-user.h b/include/libvfio-user.h index 9c2628a..e05cacf 100644 --- a/include/libvfio-user.h +++ b/include/libvfio-user.h @@ -340,8 +340,7 @@ vfu_setup_device_reset_cb(vfu_ctx_t *vfu_ctx, vfu_reset_cb_t *reset); /* * Info for a guest DMA region. @iova is always valid; the other parameters - * will only be set if the guest DMA region is VFIO_USER_F_DMA_REGION_MAPPABLE, - * and a file descriptor has been passed across the vfio-user socket. + * will only be set if the guest DMA region is mappable. * * @iova: guest DMA range. This is the guest physical range (as we don't * support vIOMMU) that the guest registers for DMA, via a VFIO_USER_DMA_MAP diff --git a/include/vfio-user.h b/include/vfio-user.h index 43f6877..0aca789 100644 --- a/include/vfio-user.h +++ b/include/vfio-user.h @@ -121,7 +121,6 @@ struct vfio_user_dma_map { uint32_t argsz; #define VFIO_USER_F_DMA_REGION_READ (1 << 0) #define VFIO_USER_F_DMA_REGION_WRITE (1 << 1) -#define VFIO_USER_F_DMA_REGION_MAPPABLE (1 << 2) uint32_t flags; uint64_t offset; uint64_t addr; diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 62f906a..40eb010 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -508,16 +508,17 @@ handle_dma_map(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, dma_map->flags &= ~VFIO_USER_F_DMA_REGION_WRITE; } - if (dma_map->flags == VFIO_USER_F_DMA_REGION_MAPPABLE) { + if (dma_map->flags != 0) { + vfu_log(vfu_ctx, LOG_ERR, "bad flags=%#x", dma_map->flags); + return ERROR_INT(EINVAL); + } + + if (msg->nr_in_fds > 0) { 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); + vfu_log(vfu_ctx, LOG_ERR, "failed to add DMA region %s: %m", rstr); return -1; } - } else if (dma_map->flags != 0) { - vfu_log(vfu_ctx, LOG_ERR, "bad flags=%#x", dma_map->flags); } ret = dma_controller_add_region(vfu_ctx->dma, (void *)dma_map->addr, diff --git a/samples/client.c b/samples/client.c index 13880c5..ef3202c 100644 --- a/samples/client.c +++ b/samples/client.c @@ -1175,7 +1175,7 @@ int main(int argc, char *argv[]) dma_regions[i].addr = i * sysconf(_SC_PAGESIZE); dma_regions[i].size = sysconf(_SC_PAGESIZE); dma_regions[i].offset = dma_regions[i].addr; - dma_regions[i].flags = VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE | VFIO_USER_F_DMA_REGION_MAPPABLE; + dma_regions[i].flags = VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE; dma_region_fds[i] = fileno(fp); } diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index bdb812a..b276a82 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -156,7 +156,6 @@ VFU_REGION_FLAG_MEM = 4 VFIO_USER_F_DMA_REGION_READ = (1 << 0) VFIO_USER_F_DMA_REGION_WRITE = (1 << 1) -VFIO_USER_F_DMA_REGION_MAPPABLE = (1 << 2) VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP = (1 << 0) diff --git a/test/py/test_dirty_pages.py b/test/py/test_dirty_pages.py index 5d4f6db..2c9be01 100644 --- a/test/py/test_dirty_pages.py +++ b/test/py/test_dirty_pages.py @@ -74,9 +74,7 @@ def test_dirty_pages_setup(): f.truncate(0x10000) payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), - flags=(VFIO_USER_F_DMA_REGION_READ | - VFIO_USER_F_DMA_REGION_WRITE | - VFIO_USER_F_DMA_REGION_MAPPABLE), + flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10000, size=0x10000) hdr = vfio_user_header(VFIO_USER_DMA_MAP, size=len(payload)) diff --git a/test/unit-tests.c b/test/unit-tests.c index 9e3fe00..9945bac 100644 --- a/test/unit-tests.c +++ b/test/unit-tests.c @@ -123,14 +123,12 @@ test_dma_map_mappable_without_fd(void **state UNUSED) { struct vfio_user_dma_map dma_map = { .argsz = sizeof(dma_map), - .flags = VFIO_USER_F_DMA_REGION_MAPPABLE }; ret = handle_dma_map(&vfu_ctx, mkmsg(VFIO_USER_DMA_MAP, &dma_map, sizeof(dma_map)), &dma_map); - assert_int_equal(-1, ret); - assert_int_equal(errno, EINVAL); + assert_int_equal(0, ret); } static void -- cgit v1.1