The common vhost code only supported a single mmap per device. vhost-user
worked around this by saving the address/length/fd of each mmap after the end
of the rte_virtio_memory struct. This only works if the vhost-user code frees
dev->mem, since the common code is unaware of the extra info. The
VHOST_USER_RESET_OWNER message is one situation where the common code frees
dev->mem and leaks the fds and mappings. This happens every time I shut down a
VM.
The new code does not keep the fds around since they aren't required for
munmap. It saves the address/length in a new structure which is read by the
common code.
The vhost-cuse changes are only compile tested.
Signed-off-by: Rich Lane <rlane at bigswitch.com>
---
lib/librte_vhost/rte_virtio_net.h | 14 +++--
lib/librte_vhost/vhost_cuse/virtio-net-cdev.c | 24 ++++---
lib/librte_vhost/vhost_user/virtio-net-user.c | 90 ++++++++-------------------
lib/librte_vhost/virtio-net.c | 24 ++++++-
lib/librte_vhost/virtio-net.h | 3 +
5 files changed, 75 insertions(+), 80 deletions(-)
diff --git a/lib/librte_vhost/rte_virtio_net.h
b/lib/librte_vhost/rte_virtio_net.h
index 10dcb90..5233879 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -144,16 +144,22 @@ struct virtio_memory_regions {
uint64_t address_offset; /**< Offset of region for
address translation. */
};
+/**
+ * Record a memory mapping so that it can be munmap'd later.
+ */
+struct virtio_memory_mapping {
+ void *addr;
+ size_t length;
+};
/**
* Memory structure includes region and mapping information.
*/
struct virtio_memory {
- uint64_t base_address; /**< Base QEMU userspace address of the
memory file. */
- uint64_t mapped_address; /**< Mapped address of memory file base
in our applications memory space. */
- uint64_t mapped_size; /**< Total size of memory file. */
uint32_t nregions; /**< Number of memory regions. */
- struct virtio_memory_regions regions[0]; /**< Memory region
information. */
+ uint32_t nmappings; /**< Number of memory mappings */
+ struct virtio_memory_regions regions[VHOST_MEMORY_MAX_NREGIONS];
/**< Memory region information. */
+ struct virtio_memory_mapping mappings[VHOST_MEMORY_MAX_NREGIONS];
/**< Memory mappings */
};
/**
diff --git a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c
b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c
index ae2c3fa..1cd0c52 100644
--- a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c
+++ b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c
@@ -278,15 +278,20 @@ cuse_set_mem_table(struct vhost_device_ctx ctx,
if (dev == NULL)
return -1;
- if (dev->mem && dev->mem->mapped_address) {
- munmap((void *)(uintptr_t)dev->mem->mapped_address,
- (size_t)dev->mem->mapped_size);
- free(dev->mem);
+ if (nregions > VHOST_MEMORY_MAX_NREGIONS) {
+ RTE_LOG(ERR, VHOST_CONFIG,
+ "(%"PRIu64") Too many memory regions (%u, max %u)\n",
+ dev->device_fh, nregions,
+ VHOST_MEMORY_MAX_NREGIONS);
+ return -1;
+ }
+
+ if (dev->mem) {
+ rte_vhost_free_mem(dev->mem);
dev->mem = NULL;
}
- dev->mem = calloc(1, sizeof(struct virtio_memory) +
- sizeof(struct virtio_memory_regions) * nregions);
+ dev->mem = calloc(1, sizeof(*dev->mem));
if (dev->mem == NULL) {
RTE_LOG(ERR, VHOST_CONFIG,
"(%"PRIu64") Failed to allocate memory for dev->mem\n",
@@ -325,9 +330,10 @@ cuse_set_mem_table(struct vhost_device_ctx ctx,
dev->mem = NULL;
return -1;
}
- dev->mem->mapped_address = mapped_address;
- dev->mem->base_address = base_address;
- dev->mem->mapped_size = mapped_size;
+
+ rte_vhost_add_mapping(dev->mem,
+ (void *)(uintptr_t)mapped_address,
+ mapped_size);
}
}
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c
b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 2934d1c..492927a 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -48,18 +48,6 @@
#include "vhost-net-user.h"
#include "vhost-net.h"
-struct orig_region_map {
- int fd;
- uint64_t mapped_address;
- uint64_t mapped_size;
- uint64_t blksz;
-};
-
-#define orig_region(ptr, nregions) \
- ((struct orig_region_map *)RTE_PTR_ADD((ptr), \
- sizeof(struct virtio_memory) + \
- sizeof(struct virtio_memory_regions) * (nregions)))
-
static uint64_t
get_blk_size(int fd)
{
@@ -69,34 +57,15 @@ get_blk_size(int fd)
return (uint64_t)stat.st_blksize;
}
-static void
-free_mem_region(struct virtio_net *dev)
-{
- struct orig_region_map *region;
- unsigned int idx;
-
- if (!dev || !dev->mem)
- return;
-
- region = orig_region(dev->mem, dev->mem->nregions);
- for (idx = 0; idx < dev->mem->nregions; idx++) {
- if (region[idx].mapped_address) {
- munmap((void *)(uintptr_t)region[idx].mapped_address,
- region[idx].mapped_size);
- close(region[idx].fd);
- }
- }
-}
-
int
user_set_mem_table(struct vhost_device_ctx ctx, struct VhostUserMsg *pmsg)
{
struct VhostUserMemory memory = pmsg->payload.memory;
struct virtio_memory_regions *pregion;
- uint64_t mapped_address, mapped_size;
+ void *mapped_address;
+ uint64_t mapped_size;
struct virtio_net *dev;
unsigned int idx = 0;
- struct orig_region_map *pregion_orig;
uint64_t alignment;
/* unmap old memory regions one by one*/
@@ -104,20 +73,24 @@ user_set_mem_table(struct vhost_device_ctx ctx, struct
VhostUserMsg *pmsg)
if (dev == NULL)
return -1;
+ if (memory.nregions > VHOST_MEMORY_MAX_NREGIONS) {
+ RTE_LOG(ERR, VHOST_CONFIG,
+ "(%"PRIu64") Too many memory regions (%u, max %u)\n",
+ dev->device_fh, memory.nregions,
+ VHOST_MEMORY_MAX_NREGIONS);
+ return -1;
+ }
+
/* Remove from the data plane. */
if (dev->flags & VIRTIO_DEV_RUNNING)
notify_ops->destroy_device(dev);
if (dev->mem) {
- free_mem_region(dev);
- free(dev->mem);
+ rte_vhost_free_mem(dev->mem);
dev->mem = NULL;
}
- dev->mem = calloc(1,
- sizeof(struct virtio_memory) +
- sizeof(struct virtio_memory_regions) * memory.nregions +
- sizeof(struct orig_region_map) * memory.nregions);
+ dev->mem = calloc(1, sizeof(*dev->mem));
if (dev->mem == NULL) {
RTE_LOG(ERR, VHOST_CONFIG,
"(%"PRIu64") Failed to allocate memory for dev->mem\n",
@@ -126,7 +99,6 @@ user_set_mem_table(struct vhost_device_ctx ctx, struct
VhostUserMsg *pmsg)
}
dev->mem->nregions = memory.nregions;
- pregion_orig = orig_region(dev->mem, memory.nregions);
for (idx = 0; idx < memory.nregions; idx++) {
pregion = &dev->mem->regions[idx];
pregion->guest_phys_address =
@@ -154,7 +126,7 @@ user_set_mem_table(struct vhost_device_ctx ctx, struct
VhostUserMsg *pmsg)
alignment = get_blk_size(pmsg->fds[idx]);
mapped_size = RTE_ALIGN_CEIL(mapped_size, alignment);
- mapped_address = (uint64_t)(uintptr_t)mmap(NULL,
+ mapped_address = mmap(NULL,
mapped_size,
PROT_READ | PROT_WRITE, MAP_SHARED,
pmsg->fds[idx],
@@ -163,33 +135,23 @@ user_set_mem_table(struct vhost_device_ctx ctx, struct
VhostUserMsg *pmsg)
RTE_LOG(INFO, VHOST_CONFIG,
"mapped region %d fd:%d to:%p sz:0x%"PRIx64" "
"off:0x%"PRIx64" align:0x%"PRIx64"\n",
- idx, pmsg->fds[idx], (void *)(uintptr_t)mapped_address,
+ idx, pmsg->fds[idx], mapped_address,
mapped_size, memory.regions[idx].mmap_offset,
alignment);
- if (mapped_address == (uint64_t)(uintptr_t)MAP_FAILED) {
+ if (mapped_address == MAP_FAILED) {
RTE_LOG(ERR, VHOST_CONFIG,
"mmap qemu guest failed.\n");
goto err_mmap;
}
- pregion_orig[idx].mapped_address = mapped_address;
- pregion_orig[idx].mapped_size = mapped_size;
- pregion_orig[idx].blksz = alignment;
- pregion_orig[idx].fd = pmsg->fds[idx];
+ rte_vhost_add_mapping(dev->mem, mapped_address, mapped_size);
- mapped_address += memory.regions[idx].mmap_offset;
-
- pregion->address_offset = mapped_address -
+ pregion->address_offset =
+ (uint64_t)(uintptr_t)mapped_address +
+ memory.regions[idx].mmap_offset -
pregion->guest_phys_address;
- if (memory.regions[idx].guest_phys_addr == 0) {
- dev->mem->base_address =
- memory.regions[idx].userspace_addr;
- dev->mem->mapped_address =
- pregion->address_offset;
- }
-
LOG_DEBUG(VHOST_CONFIG,
"REGION: %u GPA: %p QEMU VA: %p SIZE (%"PRIu64")\n",
idx,
@@ -198,15 +160,15 @@ user_set_mem_table(struct vhost_device_ctx ctx, struct
VhostUserMsg *pmsg)
pregion->memory_size);
}
+ for (idx = 0; idx < memory.nregions; idx++)
+ close(pmsg->fds[idx]);
+
return 0;
err_mmap:
- while (idx--) {
- munmap((void *)(uintptr_t)pregion_orig[idx].mapped_address,
- pregion_orig[idx].mapped_size);
- close(pregion_orig[idx].fd);
- }
- free(dev->mem);
+ for (idx = 0; idx < memory.nregions; idx++)
+ close(pmsg->fds[idx]);
+ rte_vhost_free_mem(dev->mem);
dev->mem = NULL;
return -1;
}
@@ -347,7 +309,7 @@ user_destroy_device(struct vhost_device_ctx ctx)
notify_ops->destroy_device(dev);
if (dev && dev->mem) {
- free_mem_region(dev);
+ rte_vhost_free_mem(dev->mem);
free(dev->mem);
dev->mem = NULL;
}
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index de78a0f..cd0e09e 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -201,9 +201,7 @@ cleanup_device(struct virtio_net *dev, int destroy)
/* Unmap QEMU memory file if mapped. */
if (dev->mem) {
- munmap((void *)(uintptr_t)dev->mem->mapped_address,
- (size_t)dev->mem->mapped_size);
- free(dev->mem);
+ rte_vhost_free_mem(dev->mem);
dev->mem = NULL;
}
@@ -897,3 +895,23 @@ rte_vhost_driver_callback_register(struct
virtio_net_device_ops const * const op
return 0;
}
+
+void
+rte_vhost_free_mem(struct virtio_memory *mem)
+{
+ unsigned i;
+
+ for (i = 0; i < mem->nmappings; i++)
+ munmap(mem->mappings[i].addr, mem->mappings[i].length);
+
+ free(mem);
+}
+
+void
+rte_vhost_add_mapping(struct virtio_memory *mem, void *addr, size_t length)
+{
+ struct virtio_memory_mapping *m = &mem->mappings[mem->nmappings++];
+
+ m->addr = addr;
+ m->length = length;
+}
diff --git a/lib/librte_vhost/virtio-net.h b/lib/librte_vhost/virtio-net.h
index 75fb57e..a2135b0 100644
--- a/lib/librte_vhost/virtio-net.h
+++ b/lib/librte_vhost/virtio-net.h
@@ -40,4 +40,7 @@
struct virtio_net_device_ops const *notify_ops;
struct virtio_net *get_device(struct vhost_device_ctx ctx);
+void rte_vhost_free_mem(struct virtio_memory *mem);
+void rte_vhost_add_mapping(struct virtio_memory *mem, void *addr, size_t
length);
+
#endif
--
1.9.1
