Skip to content

Commit 22e1992

Browse files
Gavin Shanmstsirkin
authored andcommitted
vhost: Add smp_rmb() in vhost_vq_avail_empty()
A smp_rmb() has been missed in vhost_vq_avail_empty(), spotted by Will. Otherwise, it's not ensured the available ring entries pushed by guest can be observed by vhost in time, leading to stale available ring entries fetched by vhost in vhost_get_vq_desc(), as reported by Yihuang Yu on NVidia's grace-hopper (ARM64) platform. /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \ -accel kvm -machine virt,gic-version=host -cpu host \ -smp maxcpus=1,cpus=1,sockets=1,clusters=1,cores=1,threads=1 \ -m 4096M,slots=16,maxmem=64G \ -object memory-backend-ram,id=mem0,size=4096M \ : \ -netdev tap,id=vnet0,vhost=true \ -device virtio-net-pci,bus=pcie.8,netdev=vnet0,mac=52:54:00:f1:26:b0 : guest# netperf -H 10.26.1.81 -l 60 -C -c -t UDP_STREAM virtio_net virtio0: output.0:id 100 is not a head! Add the missed smp_rmb() in vhost_vq_avail_empty(). When tx_can_batch() returns true, it means there's still pending tx buffers. Since it might read indices, so it still can bypass the smp_rmb() in vhost_get_vq_desc(). Note that it should be safe until vq->avail_idx is changed by commit 275bf96 ("vhost: better detection of available buffers"). Fixes: 275bf96 ("vhost: better detection of available buffers") Cc: <[email protected]> # v4.11+ Reported-by: Yihuang Yu <[email protected]> Suggested-by: Will Deacon <[email protected]> Signed-off-by: Gavin Shan <[email protected]> Acked-by: Jason Wang <[email protected]> Message-Id: <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]> Reviewed-by: Stefano Garzarella <[email protected]>
1 parent fec50db commit 22e1992

File tree

1 file changed

+11
-1
lines changed

1 file changed

+11
-1
lines changed

drivers/vhost/vhost.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2799,9 +2799,19 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
27992799
r = vhost_get_avail_idx(vq, &avail_idx);
28002800
if (unlikely(r))
28012801
return false;
2802+
28022803
vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
2804+
if (vq->avail_idx != vq->last_avail_idx) {
2805+
/* Since we have updated avail_idx, the following
2806+
* call to vhost_get_vq_desc() will read available
2807+
* ring entries. Make sure that read happens after
2808+
* the avail_idx read.
2809+
*/
2810+
smp_rmb();
2811+
return false;
2812+
}
28032813

2804-
return vq->avail_idx == vq->last_avail_idx;
2814+
return true;
28052815
}
28062816
EXPORT_SYMBOL_GPL(vhost_vq_avail_empty);
28072817

0 commit comments

Comments
 (0)