On Tue, Dec 16, 2025 at 2:57 AM Wafer Xie <[email protected]> wrote:
>
> From: wafer Xie <[email protected]>
>
> For the used ring, based on the state of the SVQ descriptor,
> if it is indirect, retrieve the corresponding indirect buffer by index
> and then increment the freeing counter.
> Once all descriptors in this buffer have been released,
> update the current buffer state to SVQ_INDIRECT_BUF_FREED.
>
> Signed-off-by: wafer Xie <[email protected]>
> ---
> hw/virtio/vhost-shadow-virtqueue.c | 43 +++++++++++++++++++++++++++---
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> b/hw/virtio/vhost-shadow-virtqueue.c
> index 85eff1d841..adee52f50b 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -469,12 +469,47 @@ static VirtQueueElement
> *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> return NULL;
> }
>
> + /* Check if indirect descriptors were used */
> + int indirect_buf_idx = svq->desc_state[used_elem.id].indirect_buf_idx;
> + bool used_indirect = (indirect_buf_idx >= 0);
> +
> + /* Update indirect buffer state if it was used */
> + if (used_indirect) {
> + SVQIndirectDescBuf *buf = &svq->indirect_bufs[indirect_buf_idx];
> + unsigned int ndescs = svq->desc_state[used_elem.id].ndescs;
Nitpick, we could just populate num = and reuse here.
> +
> + /* Increment freeing_descs for descriptors returned from used ring */
> + buf->freeing_descs += ndescs;
> +
> + /* Check if all descs are accounted for (freed + freeing == num) */
> + if (buf->freed_descs + buf->freeing_descs >= buf->num_descs) {
> + /* Reset buffer to freed state */
> + buf->state = SVQ_INDIRECT_BUF_FREED;
> + buf->freed_descs = buf->num_descs;
> + buf->freeing_descs = 0;
> + buf->freed_head = 0;
> + }
> +
> + svq->desc_state[used_elem.id].indirect_buf_idx = -1;
Why not continue using the buffer in a circular manner?
> + }
> +
> num = svq->desc_state[used_elem.id].ndescs;
> svq->desc_state[used_elem.id].ndescs = 0;
> - last_used_chain = vhost_svq_last_desc_of_chain(svq, num, used_elem.id);
> - svq->desc_next[last_used_chain] = svq->free_head;
> - svq->free_head = used_elem.id;
> - svq->num_free += num;
> +
> + /*
> + * If using indirect descriptors, only 1 main descriptor is used.
> + * Otherwise, we used 'num' descriptors in a chain.
> + */
> + if (used_indirect) {
> + svq->desc_next[used_elem.id] = svq->free_head;
> + svq->free_head = used_elem.id;
> + svq->num_free += 1;
> + } else {
> + last_used_chain = vhost_svq_last_desc_of_chain(svq, num,
> used_elem.id);
> + svq->desc_next[last_used_chain] = svq->free_head;
> + svq->free_head = used_elem.id;
> + svq->num_free += num;
> + }
I think the two paths are equivalent if the descriptor chain length is
1 except for the num_free += indirect ? num : 1, can we merge both
paths and just make it conditional?
>
> *len = used_elem.len;
> return g_steal_pointer(&svq->desc_state[used_elem.id].elem);
> --
> 2.48.1
>
