@@ -9,6 +9,7 @@
#include <linux/types.h>
#include <sys/uio.h>
+#include "kvm/barrier.h"
#include "kvm/kvm.h"
#define VIRTIO_IRQ_LOW 0
@@ -100,6 +101,13 @@ static inline u16 virt_queue__pop(struct virt_queue *queue)
{
__u16 guest_idx;
+ /*
+ * The guest updates the avail index after writing the ring entry.
+ * Ensure that we read the updated entry once virt_queue__available()
+ * observes the new index.
+ */
+ rmb();
+
guest_idx = queue->vring.avail->ring[queue->last_avail_idx++ % queue->vring.num];
return virtio_guest_to_host_u16(queue, guest_idx);
}
One barrier seems to be missing from kvmtool's virtio implementation, between virt_queue__available() and virt_queue__pop(). In the following scenario "avail" represents the shared "available" structure in the virtio queue: Guest | Host | avail.ring[shadow] = desc_idx | while (avail.idx != shadow) smp_wmb() | /* missing smp_rmb() */ avail.idx = ++shadow | desc_idx = avail.ring[shadow++] If the host observes the avail.idx write before the avail.ring update, then it will fetch the wrong desc_idx. Add the missing barrier. This seems to fix the horrible bug I'm often seeing when running netperf in a guest (virtio-net + tap) on AMD Seattle. The TX thread reads the wrong descriptor index and either faults when accessing the TX buffer, or pushes the wrong index to the used ring. In that case the guest complains that "id %u is not a head!" and stops the queue. Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> --- include/kvm/virtio.h | 8 ++++++++ 1 file changed, 8 insertions(+)