packet: Refactor vhost-user memory region handling

This patch refactors the handling of vhost-user memory regions by
introducing a new `struct vdev_memory` to encapsulate the regions
array and their count (`nregions`) within the main `vu_dev` structure.

This new `vdev_memory` structure is then passed to the packet pool by
re-using the existing `p->buf` field. A `p->buf_size` of 0 indicates
that `p->buf` holds a pointer to `struct vdev_memory` instead of a
regular packet buffer. A new helper, `get_vdev_memory()`, is added to
abstract this access pattern.

Previous implementation was using a marker at the end of the memory
regions array. We can now uses all the slots.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

8 files changed, 64 insertions, 40 deletions

diff --git a/packet.c b/packet.c
index cbc43c2..27693c5 100644
--- a/packet.c
+++ b/packet.c
@@ -23,6 +23,20 @@
 #include "log.h"
 
 /**
+ * get_vdev_memory() - Return a pointer to the memory regions of the pool
+ * @p:		Packet pool
+ *
+ * Return: Null if none, otherwise a pointer to vdev_memory structure
+ */
+static struct vdev_memory *get_vdev_memory(const struct pool *p)
+{
+	if (p->buf_size)
+		return NULL;
+
+	return (struct vdev_memory *)p->buf;
+}
+
+/**
  * packet_check_range() - Check if a memory range is valid for a pool
  * @p:		Packet pool
  * @ptr:	Start of desired data range
@@ -35,16 +49,19 @@
 static int packet_check_range(const struct pool *p, const char *ptr, size_t len,
 			      const char *func, int line)
 {
+	struct vdev_memory *memory;
+
 	if (len > PACKET_MAX_LEN) {
 		debug("packet range length %zu (max %zu), %s:%i",
 		      len, PACKET_MAX_LEN, func, line);
 		return -1;
 	}
 
-	if (p->buf_size == 0) {
+	memory = get_vdev_memory(p);
+	if (memory) {
 		int ret;
 
-		ret = vu_packet_check_range((void *)p->buf, ptr, len);
+		ret = vu_packet_check_range(memory, ptr, len);
 
 		if (ret == -1)
 			debug("cannot find region, %s:%i", func, line);
diff --git a/packet.h b/packet.h
index 43b9022..e51cbd1 100644
--- a/packet.h
+++ b/packet.h
@@ -8,6 +8,7 @@
 
 #include <stdbool.h>
 #include "iov.h"
+#include "virtio.h"
 
 /* Maximum size of a single packet stored in pool, including headers */
 #define PACKET_MAX_LEN	((size_t)UINT16_MAX)
@@ -15,7 +16,7 @@
 /**
  * struct pool - Generic pool of packets stored in a buffer
  * @buf:	Buffer storing packet descriptors,
- * 		a struct vu_dev_region array for passt vhost-user mode
+ * 		a struct vdev_region for passt vhost-user mode
  * @buf_size:	Total size of buffer,
  * 		0 for passt vhost-user mode
  * @size:	Number of usable descriptors for the pool
@@ -30,7 +31,8 @@ struct pool {
 	struct iovec pkt[];
 };
 
-int vu_packet_check_range(void *buf, const char *ptr, size_t len);
+int vu_packet_check_range(struct vdev_memory *memory,
+			  const char *ptr, size_t len);
 void packet_add_do(struct pool *p, struct iov_tail *data,
 		   const char *func, int line);
 bool packet_get_do(const struct pool *p, const size_t idx,
diff --git a/tap.c b/tap.c
index 51d79c2..5e0173b 100644
--- a/tap.c
+++ b/tap.c
@@ -1460,7 +1460,7 @@ static void tap_sock_tun_init(struct ctx *c)
  * @base:	Buffer base
  * @size	Buffer size
  */
-void tap_sock_update_pool(void *base, size_t size)
+static void tap_sock_update_pool(void *base, size_t size)
 {
 	int i;
 
@@ -1481,8 +1481,8 @@ void tap_sock_update_pool(void *base, size_t size)
 void tap_backend_init(struct ctx *c)
 {
 	if (c->mode == MODE_VU) {
-		tap_sock_update_pool(NULL, 0);
 		vu_init(c);
+		tap_sock_update_pool(&c->vdev->memory, 0);
 	} else {
 		tap_sock_update_pool(pkt_buf, sizeof(pkt_buf));
 	}
diff --git a/tap.h b/tap.h
index ce55108..21db4d2 100644
--- a/tap.h
+++ b/tap.h
@@ -115,7 +115,6 @@ void tap_handler_passt(struct ctx *c, uint32_t events,
 		       const struct timespec *now);
 int tap_sock_unix_open(char *sock_path);
 void tap_sock_reset(struct ctx *c);
-void tap_sock_update_pool(void *base, size_t size);
 void tap_backend_init(struct ctx *c);
 void tap_flush_pools(void);
 void tap_handler(struct ctx *c, const struct timespec *now);
diff --git a/vhost_user.c b/vhost_user.c
index c1522d5..f97ec60 100644
--- a/vhost_user.c
+++ b/vhost_user.c
@@ -137,8 +137,8 @@ static void *qva_to_va(struct vu_dev *dev, uint64_t qemu_addr)
 	unsigned int i;
 
 	/* Find matching memory region.  */
-	for (i = 0; i < dev->nregions; i++) {
-		const struct vu_dev_region *r = &dev->regions[i];
+	for (i = 0; i < dev->memory.nregions; i++) {
+		const struct vu_dev_region *r = &dev->memory.regions[i];
 
 		if ((qemu_addr >= r->qva) && (qemu_addr < (r->qva + r->size))) {
 			/* NOLINTNEXTLINE(performance-no-int-to-ptr) */
@@ -428,8 +428,8 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev,
 	struct vhost_user_memory m = vmsg->payload.memory, *memory = &m;
 	unsigned int i;
 
-	for (i = 0; i < vdev->nregions; i++) {
-		const struct vu_dev_region *r = &vdev->regions[i];
+	for (i = 0; i < vdev->memory.nregions; i++) {
+		const struct vu_dev_region *r = &vdev->memory.regions[i];
 
 		if (r->mmap_addr) {
 			/* NOLINTNEXTLINE(performance-no-int-to-ptr) */
@@ -437,12 +437,12 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev,
 			       r->size + r->mmap_offset);
 		}
 	}
-	vdev->nregions = memory->nregions;
+	vdev->memory.nregions = memory->nregions;
 
 	debug("vhost-user nregions: %u", memory->nregions);
-	for (i = 0; i < vdev->nregions; i++) {
+	for (i = 0; i < vdev->memory.nregions; i++) {
 		struct vhost_user_memory_region *msg_region = &memory->regions[i];
-		struct vu_dev_region *dev_region = &vdev->regions[i];
+		struct vu_dev_region *dev_region = &vdev->memory.regions[i];
 		void *mmap_addr;
 
 		debug("vhost-user region %d", i);
@@ -484,13 +484,7 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev,
 		}
 	}
 
-	/* As vu_packet_check_range() has no access to the number of
-	 * memory regions, mark the end of the array with mmap_addr = 0
-	 */
-	ASSERT(vdev->nregions < VHOST_USER_MAX_RAM_SLOTS - 1);
-	vdev->regions[vdev->nregions].mmap_addr = 0;
-
-	tap_sock_update_pool(vdev->regions, 0);
+	ASSERT(vdev->memory.nregions < VHOST_USER_MAX_RAM_SLOTS);
 
 	return false;
 }
@@ -1106,8 +1100,8 @@ void vu_cleanup(struct vu_dev *vdev)
 		vq->vring.avail = 0;
 	}
 
-	for (i = 0; i < vdev->nregions; i++) {
-		const struct vu_dev_region *r = &vdev->regions[i];
+	for (i = 0; i < vdev->memory.nregions; i++) {
+		const struct vu_dev_region *r = &vdev->memory.regions[i];
 
 		if (r->mmap_addr) {
 			/* NOLINTNEXTLINE(performance-no-int-to-ptr) */
@@ -1115,7 +1109,7 @@ void vu_cleanup(struct vu_dev *vdev)
 			       r->size + r->mmap_offset);
 		}
 	}
-	vdev->nregions = 0;
+	vdev->memory.nregions = 0;
 
 	vu_close_log(vdev);
 
diff --git a/virtio.c b/virtio.c
index ed7842b..bd388c2 100644
--- a/virtio.c
+++ b/virtio.c
@@ -102,8 +102,8 @@ static void *vu_gpa_to_va(const struct vu_dev *dev, uint64_t *plen,
 		return NULL;
 
 	/* Find matching memory region. */
-	for (i = 0; i < dev->nregions; i++) {
-		const struct vu_dev_region *r = &dev->regions[i];
+	for (i = 0; i < dev->memory.nregions; i++) {
+		const struct vu_dev_region *r = &dev->memory.regions[i];
 
 		if ((guest_addr >= r->gpa) &&
 		    (guest_addr < (r->gpa + r->size))) {
diff --git a/virtio.h b/virtio.h
index 3275745..b55cc40 100644
--- a/virtio.h
+++ b/virtio.h
@@ -97,10 +97,21 @@ struct vu_dev_region {
 #define VHOST_USER_MAX_RAM_SLOTS 32
 
 /**
+ * struct vdev_memory - Describes the shared memory regions for a vhost-user
+ * 			device
+ * @nregions:		Number of shared memory regions
+ * @regions:		Guest shared memory regions
+ */
+struct vdev_memory {
+	uint32_t nregions;
+	struct vu_dev_region regions[VHOST_USER_MAX_RAM_SLOTS];
+};
+
+/**
  * struct vu_dev - vhost-user device information
  * @context:			Execution context
- * @nregions:			Number of shared memory regions
- * @regions:			Guest shared memory regions
+ * @memory:			Shared memory regions
+ * @vq:				Virtqueues of the device
  * @features:			Vhost-user features
  * @protocol_features:		Vhost-user protocol features
  * @log_call_fd:		Eventfd to report logging update
@@ -109,8 +120,7 @@ struct vu_dev_region {
  */
 struct vu_dev {
 	struct ctx *context;
-	uint32_t nregions;
-	struct vu_dev_region regions[VHOST_USER_MAX_RAM_SLOTS];
+	struct vdev_memory memory;
 	struct vu_virtq vq[VHOST_USER_MAX_QUEUES];
 	uint64_t features;
 	uint64_t protocol_features;
diff --git a/vu_common.c b/vu_common.c
index b77b214..b716070 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -25,26 +25,28 @@
 /**
  * vu_packet_check_range() - Check if a given memory zone is contained in
  * 			     a mapped guest memory region
- * @buf:	Array of the available memory regions
+ * @memory:	Array of the available memory regions
  * @ptr:	Start of desired data range
- * @size:	Length of desired data range
+ * @len:	Length of desired data range
  *
  * Return: 0 if the zone is in a mapped memory region, -1 otherwise
  */
-int vu_packet_check_range(void *buf, const char *ptr, size_t len)
+int vu_packet_check_range(struct vdev_memory *memory,
+			  const char *ptr, size_t len)
 {
-	struct vu_dev_region *dev_region;
+	struct vu_dev_region *dev_region = memory->regions;
+	unsigned int i;
 
-	for (dev_region = buf; dev_region->mmap_addr; dev_region++) {
-		uintptr_t base_addr = dev_region->mmap_addr +
-			dev_region->mmap_offset;
+	for (i = 0; i < memory->nregions; i++) {
+		uintptr_t base_addr = dev_region[i].mmap_addr +
+			dev_region[i].mmap_offset;
 		/* NOLINTNEXTLINE(performance-no-int-to-ptr) */
 		const char *base = (const char *)base_addr;
 
-		ASSERT(base_addr >= dev_region->mmap_addr);
+		ASSERT(base_addr >= dev_region[i].mmap_addr);
 
-		if (len <= dev_region->size && base <= ptr &&
-		    (size_t)(ptr - base) <= dev_region->size - len)
+		if (len <= dev_region[i].size && base <= ptr &&
+		    (size_t)(ptr - base) <= dev_region[i].size - len)
 			return 0;
 	}