On Mon, May 20, 2024 at 3:01 PM Jonah Palmer <jonah.pal...@oracle.com> wrote:
>
> Add VIRTIO_F_IN_ORDER feature support for the virtqueue_fill operation.
>
> The goal of the virtqueue_ordered_fill operation when the
> VIRTIO_F_IN_ORDER feature has been negotiated is to search for this
> now-used element, set its length, and mark the element as filled in
> the VirtQueue's used_elems array.
>
> By marking the element as filled, it will indicate that this element has
> been processed and is ready to be flushed, so long as the element is
> in-order.
>
> Signed-off-by: Jonah Palmer <jonah.pal...@oracle.com>
> ---
> hw/virtio/virtio.c | 36 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 7456d61bc8..01b6b32460 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -873,6 +873,38 @@ static void virtqueue_packed_fill(VirtQueue *vq, const
> VirtQueueElement *elem,
> vq->used_elems[idx].ndescs = elem->ndescs;
> }
>
> +static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement
> *elem,
> + unsigned int len)
> +{
> + unsigned int i, steps, max_steps;
> +
> + i = vq->used_idx;
> + steps = 0;
> + /*
> + * We shouldn't need to increase 'i' by more than the distance
> + * between used_idx and last_avail_idx.
> + */
> + max_steps = (vq->last_avail_idx + vq->vring.num - vq->used_idx)
> + % vq->vring.num;
I may be missing something, but (+vq->vring.num) is redundant if we (%
vq->vring.num), isn't it?
> +
> + /* Search for element in vq->used_elems */
> + while (steps <= max_steps) {
> + /* Found element, set length and mark as filled */
> + if (vq->used_elems[i].index == elem->index) {
> + vq->used_elems[i].len = len;
> + vq->used_elems[i].in_order_filled = true;
> + break;
> + }
> +
> + i += vq->used_elems[i].ndescs;
> + steps += vq->used_elems[i].ndescs;
> +
> + if (i >= vq->vring.num) {
> + i -= vq->vring.num;
> + }
> + }
> +}
> +
Let's report an error if we finish the loop. I think:
qemu_log_mask(LOG_GUEST_ERROR,
"%s: %s cannot fill buffer id %u\n",
__func__, vdev->name, elem->index);
(or similar) should do.
apart form that,
Reviewed-by: Eugenio Pérez <epere...@redhat.com>
> static void virtqueue_packed_fill_desc(VirtQueue *vq,
> const VirtQueueElement *elem,
> unsigned int idx,
> @@ -923,7 +955,9 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement
> *elem,
> return;
> }
>
> - if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_IN_ORDER)) {
> + virtqueue_ordered_fill(vq, elem, len);
> + } else if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> virtqueue_packed_fill(vq, elem, len, idx);
> } else {
> virtqueue_split_fill(vq, elem, len, idx);
> --
> 2.39.3
>
