From: Jia Jia <[email protected]> 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]> Reviewed-by: Michael S. Tsirkin <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]> Message-ID: <[email protected]> --- 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 0b2fbbed3f..c9f9e5b511 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -717,26 +717,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; @@ -1773,12 +1753,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; -- MST
