On Tue, Apr 9, 2024 at 7:40 PM Michael Tokarev <m...@tls.msk.ru> wrote:
>
> 09.04.2024 10:32, Michael S. Tsirkin wrote:
> > From: Wafer <wa...@jaguarmicro.com>
> >
> > In the event of writing many chains of descriptors, the device must
> > write just the id of the last buffer in the descriptor chain, skip
> > forward the number of descriptors in the chain, and then repeat the
> > operations for the rest of chains.
> >
> > Current QEMU code writes all the buffer ids consecutively, and then
> > skips all the buffers altogether. This is a bug, and can be reproduced
> > with a VirtIONet device with _F_MRG_RXBUB and without
> > _F_INDIRECT_DESC:
> >
> > If a virtio-net device has the VIRTIO_NET_F_MRG_RXBUF feature
> > but not the VIRTIO_RING_F_INDIRECT_DESC feature,
> > 'VirtIONetQueue->rx_vq' will use the merge feature
> > to store data in multiple 'elems'.
> > The 'num_buffers' in the virtio header indicates how many elements are
> > merged.
> > If the value of 'num_buffers' is greater than 1,
> > all the merged elements will be filled into the descriptor ring.
> > The 'idx' of the elements should be the value of 'vq->used_idx' plus
> > 'ndescs'.
> >
> > Fixes: 86044b24e8 ("virtio: basic packed virtqueue support")
> > Acked-by: Eugenio Pérez <epere...@redhat.com>
> > Signed-off-by: Wafer <wa...@jaguarmicro.com>
> > Message-Id: <20240407015451.5228-2-wa...@jaguarmicro.com>
> > Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > hw/virtio/virtio.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
>
> Is this a -stable material?
>
Hi Michael,
Yes it is. It should be easy to backport but let me know if you need any help.
Thanks!
> Thanks,
>
> /mjt
>
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index d229755eae..c5bedca848 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -957,12 +957,20 @@ static void virtqueue_packed_flush(VirtQueue *vq,
> > unsigned int count)
> > return;
> > }
> >
> > + /*
> > + * For indirect element's 'ndescs' is 1.
> > + * For all other elemment's 'ndescs' is the
> > + * number of descriptors chained by NEXT (as set in
> > virtqueue_packed_pop).
> > + * So When the 'elem' be filled into the descriptor ring,
> > + * The 'idx' of this 'elem' shall be
> > + * the value of 'vq->used_idx' plus the 'ndescs'.
> > + */
> > + ndescs += vq->used_elems[0].ndescs;
> > for (i = 1; i < count; i++) {
> > - virtqueue_packed_fill_desc(vq, &vq->used_elems[i], i, false);
> > + virtqueue_packed_fill_desc(vq, &vq->used_elems[i], ndescs, false);
> > ndescs += vq->used_elems[i].ndescs;
> > }
> > virtqueue_packed_fill_desc(vq, &vq->used_elems[0], 0, true);
> > - ndescs += vq->used_elems[0].ndescs;
> >
> > vq->inuse -= ndescs;
> > vq->used_idx += ndescs;
>
