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 <physicalmtea@gmail.com>
---
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;