On Fri, Jul 10, 2026 at 03:35:17PM +0800, Jia Jia wrote: > virtqueue_split_pop() reads the next split avail ring entry after > checking that the ring is configured. Call virtqueue_num_heads() before > consuming that entry, so an avail index distance larger than the queue > size is rejected. > > virtqueue_num_heads() also reports an empty queue and keeps the read > barrier needed before the avail ring entry is read. Drop the now unused > virtio_queue_empty_rcu() helper. > > This prevents an invalid split queue state from being expanded into > repeated device command processing.
We can summarize this as: virtio: reduce code duplication for split ring virtio_queue_empty_rcu duplicates virtqueue_num_heads for no good reason, let's not do it. As a nice side effect, we gain better handling for misbehaving guests. > Link: https://gitlab.com/qemu-project/qemu/-/issues/3930 not sure this is very relevant. > Signed-off-by: Jia Jia <[email protected]> > --- > v2: > - Drop virtio_queue_empty_rcu() from the split pop path. > - Keep only the existing !vq->vring.avail early exit before > virtqueue_num_heads(). > - Remove the now unused virtio_queue_empty_rcu() helper. > > Tested with qemu-system-x86_64 11.0.50, built from origin/master > f893c46c3931 plus this patch, configured with: > --target-list=x86_64-softmmu --enable-kvm --disable-tcg > > The original virtio-iommu live-vring qtest reproducer used for the > report completed successfully on the patched build without host OOM, > confirming that this reproducer is fixed by the patch. > > hw/virtio/virtio.c | 30 ++++++------------------------ > 1 file changed, 6 insertions(+), 24 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index f4d86a3655..68ec3f0751 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -716,26 +716,6 @@ static inline bool is_desc_avail(uint16_t flags, bool > wrap_counter) > return (avail != used) && (avail == wrap_counter); > } > > -/* Fetch avail_idx from VQ memory only when we really need to know if > - * guest has added some buffers. > - * Called within rcu_read_lock(). */ > -static int virtio_queue_empty_rcu(VirtQueue *vq) > -{ > - if (virtio_device_disabled(vq->vdev)) { > - return 1; > - } This check is no longer there. intentional? > - > - if (unlikely(!vq->vring.avail)) { > - return 1; > - } > - > - if (vq->shadow_avail_idx != vq->last_avail_idx) { > - return 0; > - } > - > - return vring_avail_idx(vq) == vq->last_avail_idx; > -} > - > static int virtio_queue_split_empty(VirtQueue *vq) > { > bool empty; > @@ -1748,12 +1728,14 @@ static void *virtqueue_split_pop(VirtQueue *vq, > size_t sz) > address_space_cache_init_empty(&indirect_desc_cache); > > RCU_READ_LOCK_GUARD(); > - if (virtio_queue_empty_rcu(vq)) { > + if (unlikely(!vq->vring.avail)) { > + goto done; > + } > + > + rc = virtqueue_num_heads(vq, vq->last_avail_idx); > + if (rc <= 0) { > goto done; > } > - /* Needed after virtio_queue_empty(), see comment in > - * virtqueue_num_heads(). */ > - smp_rmb(); > > /* When we start there are none of either input nor output. */ > out_num = in_num = elem_entries = 0; > -- > 2.34.1
