On 5/9/24 9:13 AM, Eugenio Perez Martin wrote:
On Mon, May 6, 2024 at 5:06 PM Jonah Palmer <jonah.pal...@oracle.com> wrote:Add VIRTIO_F_IN_ORDER feature support in virtqueue_split_pop and virtqueue_packed_pop. VirtQueueElements popped from the available/descritpor ring are added to the VirtQueue's used_elems array in-order and in the same fashion as they would be added the used and descriptor rings, respectively. This will allow us to keep track of the current order, what elements have been written, as well as an element's essential data after being processed. Tested-by: Lei Yang <leiy...@redhat.com> Signed-off-by: Jonah Palmer <jonah.pal...@oracle.com> --- hw/virtio/virtio.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 893a072c9d..e6eb1bb453 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1506,7 +1506,7 @@ static void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_nu static void *virtqueue_split_pop(VirtQueue *vq, size_t sz) { - unsigned int i, head, max; + unsigned int i, j, head, max; VRingMemoryRegionCaches *caches; MemoryRegionCache indirect_desc_cache; MemoryRegionCache *desc_cache; @@ -1539,6 +1539,8 @@ static void *virtqueue_split_pop(VirtQueue *vq, size_t sz) goto done; } + j = vq->last_avail_idx; + if (!virtqueue_get_head(vq, vq->last_avail_idx++, &head)) { goto done; } @@ -1630,6 +1632,12 @@ static void *virtqueue_split_pop(VirtQueue *vq, size_t sz) elem->in_sg[i] = iov[out_num + i]; } + if (virtio_vdev_has_feature(vdev, VIRTIO_F_IN_ORDER)) { + vq->used_elems[j].index = elem->index; + vq->used_elems[j].len = elem->len; + vq->used_elems[j].ndescs = elem->ndescs; + } + vq->inuse++; trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num); @@ -1758,6 +1766,13 @@ static void *virtqueue_packed_pop(VirtQueue *vq, size_t sz) elem->index = id; elem->ndescs = (desc_cache == &indirect_desc_cache) ? 1 : elem_entries; + + if (virtio_vdev_has_feature(vdev, VIRTIO_F_IN_ORDER)) { + vq->used_elems[vq->last_avail_idx].index = elem->index; + vq->used_elems[vq->last_avail_idx].len = elem->len; + vq->used_elems[vq->last_avail_idx].ndescs = elem->ndescs; + } +I suggest using a consistent style between packed and split: Either always use vq->last_avail_idx or j. If you use j, please rename to something more related to the usage, as j is usually for iterations. In my opinion I think vq->last_avail_idx is better.
Totally agree. The reason I used a separate variable in virtqueue_split_pop was to capture the value of vq->last_avail_idx before it got incremented in the next line.
Not sure if it actually matters whether or not I use the value of last_avail_idx before or after it's incremented. I don't think it does but, in any case, I opted to use the value before it was incremented so as to be consistent with virtqueue_packed_pop, where last_avail_idx is used before it's incremented.
I'll change j to something more meaningful though. Maybe 'init_last_avail_idx'? Hmm... will need to think on it.
vq->last_avail_idx += elem->ndescs; vq->inuse += elem->ndescs; -- 2.39.3
