From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202508 header.b=u1TydcPo; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 233475A028C for ; Wed, 13 Aug 2025 05:27:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1755055626; bh=si/Jh5Z55ThQGEDNqEy+ycMHFPnvVOXNRTtANQL2zXo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=u1TydcPozy+dQhIqTOeBXxIhnkqDGXfIbLvvr+zYVY7LHM2MGhkdTyJNSajr9oU7w jkqdNXFUx5I0eWG3HCMz9xpoCn9KKQZOGYd/AxGh48Qn8oodPyQu+JUy+mAOgSZIrR kVMkg38Fm4uxYErSY1s3JiWvxhedDSTb0CwpPVhDrcMxPj0bwp6pCVgMJ6i3ioFtEq SoeLo9RyL7r1MNC4v2NrNTzBX0Vc0nZxwCnMKTah+mG0o+diVikUuthwhX5MVsyuCN aA0gqG38v0xzj7Bt/7CNrEHHfqjH2609C+TV4K+Hp4une9fR2gQ9v7FHM4yBI8dHwX YU/AQjDOqiOig== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4c1v1k5jBRz4xdg; Wed, 13 Aug 2025 13:27:06 +1000 (AEST) Date: Wed, 13 Aug 2025 13:25:26 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v9 29/30] packet: Refactor vhost-user memory region handling Message-ID: References: <20250808140142.3404325-1-lvivier@redhat.com> <20250808140142.3404325-30-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="mAWnD04/oRVusn/T" Content-Disposition: inline In-Reply-To: <20250808140142.3404325-30-lvivier@redhat.com> Message-ID-Hash: DNU63OPJDX7U57PLZ4UATCVQM6ID32VY X-Message-ID-Hash: DNU63OPJDX7U57PLZ4UATCVQM6ID32VY X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --mAWnD04/oRVusn/T Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 08, 2025 at 04:01:41PM +0200, Laurent Vivier wrote: > 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. >=20 > 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. >=20 > Previous implementation was using a marker at the end of the memory > regions array. We can now uses all the slots. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson > --- > packet.c | 21 +++++++++++++++++++-- > packet.h | 6 ++++-- > tap.c | 4 ++-- > tap.h | 1 - > vhost_user.c | 28 +++++++++++----------------- > virtio.c | 4 ++-- > virtio.h | 18 ++++++++++++++---- > vu_common.c | 22 ++++++++++++---------- > 8 files changed, 64 insertions(+), 40 deletions(-) >=20 > diff --git a/packet.c b/packet.c > index cbc43c2fc22d..27693c55a138 100644 > --- a/packet.c > +++ b/packet.c > @@ -22,6 +22,20 @@ > #include "util.h" > #include "log.h" > =20 > +/** > + * 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 > @@ -35,16 +49,19 @@ > static int packet_check_range(const struct pool *p, const char *ptr, siz= e_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; > } > =20 > - if (p->buf_size =3D=3D 0) { > + memory =3D get_vdev_memory(p); > + if (memory) { > int ret; > =20 > - ret =3D vu_packet_check_range((void *)p->buf, ptr, len); > + ret =3D vu_packet_check_range(memory, ptr, len); > =20 > if (ret =3D=3D -1) > debug("cannot find region, %s:%i", func, line); > diff --git a/packet.h b/packet.h > index 43b9022075d1..e51cbd19fdc4 100644 > --- a/packet.h > +++ b/packet.h > @@ -8,6 +8,7 @@ > =20 > #include > #include "iov.h" > +#include "virtio.h" > =20 > /* 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[]; > }; > =20 > -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 bbc786468455..9fd00915bb01 100644 > --- a/tap.c > +++ b/tap.c > @@ -1458,7 +1458,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; > =20 > @@ -1479,8 +1479,8 @@ void tap_sock_update_pool(void *base, size_t size) > void tap_backend_init(struct ctx *c) > { > if (c->mode =3D=3D 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 ce5510882d5d..21db4d219ecb 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 c1522d549f00..f97ec6064cac 100644 > --- a/vhost_user.c > +++ b/vhost_user.c > @@ -137,8 +137,8 @@ static void *qva_to_va(struct vu_dev *dev, uint64_t q= emu_addr) > unsigned int i; > =20 > /* Find matching memory region. */ > - for (i =3D 0; i < dev->nregions; i++) { > - const struct vu_dev_region *r =3D &dev->regions[i]; > + for (i =3D 0; i < dev->memory.nregions; i++) { > + const struct vu_dev_region *r =3D &dev->memory.regions[i]; > =20 > if ((qemu_addr >=3D 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 =3D vmsg->payload.memory, *memory =3D &m; > unsigned int i; > =20 > - for (i =3D 0; i < vdev->nregions; i++) { > - const struct vu_dev_region *r =3D &vdev->regions[i]; > + for (i =3D 0; i < vdev->memory.nregions; i++) { > + const struct vu_dev_region *r =3D &vdev->memory.regions[i]; > =20 > if (r->mmap_addr) { > /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ > @@ -437,12 +437,12 @@ static bool vu_set_mem_table_exec(struct vu_dev *vd= ev, > r->size + r->mmap_offset); > } > } > - vdev->nregions =3D memory->nregions; > + vdev->memory.nregions =3D memory->nregions; > =20 > debug("vhost-user nregions: %u", memory->nregions); > - for (i =3D 0; i < vdev->nregions; i++) { > + for (i =3D 0; i < vdev->memory.nregions; i++) { > struct vhost_user_memory_region *msg_region =3D &memory->regions[i]; > - struct vu_dev_region *dev_region =3D &vdev->regions[i]; > + struct vu_dev_region *dev_region =3D &vdev->memory.regions[i]; > void *mmap_addr; > =20 > debug("vhost-user region %d", i); > @@ -484,13 +484,7 @@ static bool vu_set_mem_table_exec(struct vu_dev *vde= v, > } > } > =20 > - /* As vu_packet_check_range() has no access to the number of > - * memory regions, mark the end of the array with mmap_addr =3D 0 > - */ > - ASSERT(vdev->nregions < VHOST_USER_MAX_RAM_SLOTS - 1); > - vdev->regions[vdev->nregions].mmap_addr =3D 0; > - > - tap_sock_update_pool(vdev->regions, 0); > + ASSERT(vdev->memory.nregions < VHOST_USER_MAX_RAM_SLOTS); > =20 > return false; > } > @@ -1106,8 +1100,8 @@ void vu_cleanup(struct vu_dev *vdev) > vq->vring.avail =3D 0; > } > =20 > - for (i =3D 0; i < vdev->nregions; i++) { > - const struct vu_dev_region *r =3D &vdev->regions[i]; > + for (i =3D 0; i < vdev->memory.nregions; i++) { > + const struct vu_dev_region *r =3D &vdev->memory.regions[i]; > =20 > 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 =3D 0; > + vdev->memory.nregions =3D 0; > =20 > vu_close_log(vdev); > =20 > diff --git a/virtio.c b/virtio.c > index ed7842b4c78a..bd388c2dfc7f 100644 > --- a/virtio.c > +++ b/virtio.c > @@ -102,8 +102,8 @@ static void *vu_gpa_to_va(const struct vu_dev *dev, u= int64_t *plen, > return NULL; > =20 > /* Find matching memory region. */ > - for (i =3D 0; i < dev->nregions; i++) { > - const struct vu_dev_region *r =3D &dev->regions[i]; > + for (i =3D 0; i < dev->memory.nregions; i++) { > + const struct vu_dev_region *r =3D &dev->memory.regions[i]; > =20 > if ((guest_addr >=3D r->gpa) && > (guest_addr < (r->gpa + r->size))) { > diff --git a/virtio.h b/virtio.h > index 32757458ea95..b55cc4042521 100644 > --- a/virtio.h > +++ b/virtio.h > @@ -96,11 +96,22 @@ struct vu_dev_region { > */ > #define VHOST_USER_MAX_RAM_SLOTS 32 > =20 > +/** > + * 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 b77b21420c57..b716070ea3c3 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 =3D memory->regions; > + unsigned int i; > =20 > - for (dev_region =3D buf; dev_region->mmap_addr; dev_region++) { > - uintptr_t base_addr =3D dev_region->mmap_addr + > - dev_region->mmap_offset; > + for (i =3D 0; i < memory->nregions; i++) { > + uintptr_t base_addr =3D dev_region[i].mmap_addr + > + dev_region[i].mmap_offset; > /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ > const char *base =3D (const char *)base_addr; > =20 > - ASSERT(base_addr >=3D dev_region->mmap_addr); > + ASSERT(base_addr >=3D dev_region[i].mmap_addr); > =20 > - if (len <=3D dev_region->size && base <=3D ptr && > - (size_t)(ptr - base) <=3D dev_region->size - len) > + if (len <=3D dev_region[i].size && base <=3D ptr && > + (size_t)(ptr - base) <=3D dev_region[i].size - len) > return 0; > } > =20 --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --mAWnD04/oRVusn/T Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmicBaYACgkQzQJF27ox 2GcfAg//eN20rfTjYgs9oPzLxzgyA2pIeyq/dMOTkxMCiRYRR7T/VBTt8rxamAo/ +2fqVO7HKmq1YH96Ffr/eDLslsDPTIuTsaxmwYu19z0RHnXnxHwRUZlK8hMyaHlC QE4BJ+0wiUe7B5o0NvQ2MDxjpccKQwspDWmo+6U5gCO3oeCpn01gqIj3Hv9fdXpM fcdUtgBnFMKh8/IqNb3SihNtmCzSVmxNYKl5cv3b1F67c80GNqFK2aGkrEfmLjLK LdQpv0HzT+14MGrqkxLp92krN4bunn5CitNqbBqmbyCnIPK/8W/NYUI/sr4mj/AT jYoS8VWBfsUHgpJgCquPmM/SjcyKSvMQQNmPImKF5UsG6XD+CUQSCKqrJyYx/kZX Eb2tNRmunvEJY+oHezlP7tWS1uMwOPBvBeUsXOlVwF1DNs4Fa8p6D2OOv1Trs1mM MrUDvQ++95BqYuLc3FXNlqC02iaQ/d9pRDVzAPn/exL2QyUuSeCRUTSAaK9R8x1K NK9nLUgP48VkznGG9vOoxUV0qRXnPTODfVGaastbv4tgUf5dbuuo133MQSM93YRf 5AqVZE49mhS4AK8vAzhT/CNRuttUsG0Ia3C5KYr0Iha+pERgiyptqPJjzvsEnSon /EZvcUVqumhx7sOkcj9/feokBzayBhXWMMDDIsjRRJKLpuvgVIA= =ncis -----END PGP SIGNATURE----- --mAWnD04/oRVusn/T--