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.
The virtio_device_disabled() check in virtio_queue_empty_rcu is redundant because virtqueue_split_pop() is only called through virtqueue_pop(), which already performs the check. Signed-off-by: Jia Jia <[email protected]> --- v3: - Rework the commit message to describe split-ring code deduplication. - Explain why the virtio_device_disabled() check is removed. - Drop the issue link because this patch addresses the generic split-ring path rather than the virtio-iommu command-processing path. 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; - } - - 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;
