Re: [PATCH v2 1/3] virtio: Pass iovec arrays as separate parameters to vu_queue_pop()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6460 bytes --]

On Fri, Mar 13, 2026 at 07:26:16PM +0100, Laurent Vivier wrote:
> Currently vu_queue_pop() and vu_queue_map_desc() read the iovec arrays
> (in_sg/out_sg) and their sizes (in_num/out_num) from the vu_virtq_element
> struct.  This couples the iovec storage to the element, requiring callers
> like vu_handle_tx() to pre-initialize the element fields before calling
> vu_queue_pop().
> 
> Pass the iovec arrays and their maximum sizes as separate parameters
> instead.  vu_queue_map_desc() now writes the actual descriptor count
> and iovec pointers back into the element after mapping, rather than
> using the element as both input and output.
> 
> This decouples the iovec storage from the element, which is a
> prerequisite for multi-buffer support where a single frame can span
> multiple virtqueue elements sharing a common iovec pool.
> 
> No functional change.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  virtio.c    | 29 ++++++++++++++++++++++-------
>  virtio.h    |  4 +++-
>  vu_common.c | 14 +++++++-------
>  3 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/virtio.c b/virtio.c
> index 447137ee83dd..a671163c27a0 100644
> --- a/virtio.c
> +++ b/virtio.c
> @@ -428,12 +428,18 @@ static bool virtqueue_map_desc(const struct vu_dev *dev,
>   * @vq:		Virtqueue
>   * @idx:	First descriptor ring entry to map
>   * @elem:	Virtqueue element to store descriptor ring iov
> + * @in_sg:	Incoming iovec array for device-writable descriptors
> + * @max_in_sg:	Maximum number of entries in @in_sg
> + * @out_sg:	Outgoing iovec array for device-readable descriptors
> + * @max_out_sg:	Maximum number of entries in @out_sg
>   *
>   * Return: -1 if there is an error, 0 otherwise
>   */
>  static int vu_queue_map_desc(const struct vu_dev *dev,
>  			     struct vu_virtq *vq, unsigned int idx,
> -			     struct vu_virtq_element *elem)
> +			     struct vu_virtq_element *elem,
> +			     struct iovec *in_sg, size_t max_in_sg,
> +			     struct iovec *out_sg, size_t max_out_sg)
>  {
>  	const struct vring_desc *desc = vq->vring.desc;
>  	struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE];
> @@ -470,16 +476,16 @@ static int vu_queue_map_desc(const struct vu_dev *dev,
>  	/* Collect all the descriptors */
>  	do {
>  		if (le16toh(desc[i].flags) & VRING_DESC_F_WRITE) {
> -			if (!virtqueue_map_desc(dev, &in_num, elem->in_sg,
> -						elem->in_num,
> +			if (!virtqueue_map_desc(dev, &in_num, in_sg,
> +						max_in_sg,
>  						le64toh(desc[i].addr),
>  						le32toh(desc[i].len)))
>  				return -1;
>  		} else {
>  			if (in_num)
>  				die("Incorrect order for descriptors");
> -			if (!virtqueue_map_desc(dev, &out_num, elem->out_sg,
> -						elem->out_num,
> +			if (!virtqueue_map_desc(dev, &out_num, out_sg,
> +						max_out_sg,
>  						le64toh(desc[i].addr),
>  						le32toh(desc[i].len))) {
>  				return -1;
> @@ -496,7 +502,9 @@ static int vu_queue_map_desc(const struct vu_dev *dev,
>  		die("vhost-user: Failed to read descriptor list");
>  
>  	elem->index = idx;
> +	elem->in_sg = in_sg;
>  	elem->in_num = in_num;
> +	elem->out_sg = out_sg;
>  	elem->out_num = out_num;
>  
>  	return 0;
> @@ -507,11 +515,17 @@ static int vu_queue_map_desc(const struct vu_dev *dev,
>   * @dev:	Vhost-user device
>   * @vq:		Virtqueue
>   * @elem:	Virtqueue element to fill with the entry information
> + * @in_sg:	Incoming iovec array for device-writable descriptors
> + * @max_in_sg:	Maximum number of entries in @in_sg
> + * @out_sg:	Outgoing iovec array for device-readable descriptors
> + * @max_out_sg:	Maximum number of entries in @out_sg
>   *
>   * Return: -1 if there is an error, 0 otherwise
>   */
>  int vu_queue_pop(const struct vu_dev *dev, struct vu_virtq *vq,
> -		 struct vu_virtq_element *elem)
> +		 struct vu_virtq_element *elem,
> +		 struct iovec *in_sg, size_t max_in_sg,
> +		 struct iovec *out_sg, size_t max_out_sg)
>  {
>  	unsigned int head;
>  	int ret;
> @@ -535,7 +549,8 @@ int vu_queue_pop(const struct vu_dev *dev, struct vu_virtq *vq,
>  	if (vu_has_feature(dev, VIRTIO_RING_F_EVENT_IDX))
>  		vring_set_avail_event(vq, vq->last_avail_idx);
>  
> -	ret = vu_queue_map_desc(dev, vq, head, elem);
> +	ret = vu_queue_map_desc(dev, vq, head, elem, in_sg, max_in_sg,
> +				out_sg, max_out_sg);
>  
>  	if (ret < 0)
>  		return ret;
> diff --git a/virtio.h b/virtio.h
> index d04bbe84e5c4..c7e447d59860 100644
> --- a/virtio.h
> +++ b/virtio.h
> @@ -188,7 +188,9 @@ static inline bool vu_has_protocol_feature(const struct vu_dev *vdev,
>  
>  void vu_queue_notify(const struct vu_dev *dev, struct vu_virtq *vq);
>  int vu_queue_pop(const struct vu_dev *dev, struct vu_virtq *vq,
> -		 struct vu_virtq_element *elem);
> +		 struct vu_virtq_element *elem,
> +		 struct iovec *in_sg, size_t max_in_sg,
> +		 struct iovec *out_sg, size_t max_out_sg);
>  void vu_queue_detach_element(struct vu_virtq *vq);
>  void vu_queue_unpop(struct vu_virtq *vq);
>  bool vu_queue_rewind(struct vu_virtq *vq, unsigned int num);
> diff --git a/vu_common.c b/vu_common.c
> index 5f2ce18e5b71..4d809ac38a4b 100644
> --- a/vu_common.c
> +++ b/vu_common.c
> @@ -91,7 +91,11 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq,
>  		struct iovec *iov;
>  		int ret;
>  
> -		ret = vu_queue_pop(vdev, vq, &elem[elem_cnt]);
> +		ret = vu_queue_pop(vdev, vq, &elem[elem_cnt],
> +				   elem[elem_cnt].in_sg,
> +				   elem[elem_cnt].in_num,
> +				   elem[elem_cnt].out_sg,
> +				   elem[elem_cnt].out_num);
>  		if (ret < 0)
>  			break;
>  
> @@ -178,12 +182,8 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
>  		int ret;
>  		struct iov_tail data;
>  
> -		elem[count].out_num = VU_MAX_TX_BUFFER_NB;
> -		elem[count].out_sg = &out_sg[out_sg_count];
> -		elem[count].in_num = 0;
> -		elem[count].in_sg = NULL;
> -
> -		ret = vu_queue_pop(vdev, vq, &elem[count]);
> +		ret = vu_queue_pop(vdev, vq, &elem[count], NULL, 0,
> +				   &out_sg[out_sg_count], VU_MAX_TX_BUFFER_NB);
>  		if (ret < 0)
>  			break;
>  		out_sg_count += elem[count].out_num;
> -- 
> 2.53.0
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]