| From 706cad7d9b9a97a23e442d8b55b2cdfb4104a094 Mon Sep 17 00:00:00 2001 |
| From: Marvin Liu <yong.liu@intel.com> |
| Date: Tue, 22 Oct 2019 01:10:04 +0800 |
| Subject: [PATCH] virtio_ring: fix stalls for packed rings |
| |
| commit 40ce7919d8730f5936da2bc8a21b46bd07db6411 upstream. |
| |
| When VIRTIO_F_RING_EVENT_IDX is negotiated, virtio devices can |
| use virtqueue_enable_cb_delayed_packed to reduce the number of device |
| interrupts. At the moment, this is the case for virtio-net when the |
| napi_tx module parameter is set to false. |
| |
| In this case, the virtio driver selects an event offset and expects that |
| the device will send a notification when rolling over the event offset |
| in the ring. However, if this roll-over happens before the event |
| suppression structure update, the notification won't be sent. To address |
| this race condition the driver needs to check wether the device rolled |
| over the offset after updating the event suppression structure. |
| |
| With VIRTIO_F_RING_PACKED, the virtio driver did this by reading the |
| flags field of the descriptor at the specified offset. |
| |
| Unfortunately, checking at the event offset isn't reliable: if |
| descriptors are chained (e.g. when INDIRECT is off) not all descriptors |
| are overwritten by the device, so it's possible that the device skipped |
| the specific descriptor driver is checking when writing out used |
| descriptors. If this happens, the driver won't detect the race condition |
| and will incorrectly expect the device to send a notification. |
| |
| For virtio-net, the result will be a TX queue stall, with the |
| transmission getting blocked forever. |
| |
| With the packed ring, it isn't easy to find a location which is |
| guaranteed to change upon the roll-over, except the next device |
| descriptor, as described in the spec: |
| |
| Writes of device and driver descriptors can generally be |
| reordered, but each side (driver and device) are only required to |
| poll (or test) a single location in memory: the next device descriptor after |
| the one they processed previously, in circular order. |
| |
| while this might be sub-optimal, let's do exactly this for now. |
| |
| Cc: stable@vger.kernel.org |
| Cc: Jason Wang <jasowang@redhat.com> |
| Fixes: f51f982682e2a ("virtio_ring: leverage event idx in packed ring") |
| Signed-off-by: Marvin Liu <yong.liu@intel.com> |
| Signed-off-by: Michael S. Tsirkin <mst@redhat.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c |
| index c8be1c4f5b55..b11ddb64e2f9 100644 |
| --- a/drivers/virtio/virtio_ring.c |
| +++ b/drivers/virtio/virtio_ring.c |
| @@ -1495,9 +1495,6 @@ static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq) |
| * counter first before updating event flags. |
| */ |
| virtio_wmb(vq->weak_barriers); |
| - } else { |
| - used_idx = vq->last_used_idx; |
| - wrap_counter = vq->packed.used_wrap_counter; |
| } |
| |
| if (vq->packed.event_flags_shadow == VRING_PACKED_EVENT_FLAG_DISABLE) { |
| @@ -1514,7 +1511,9 @@ static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq) |
| */ |
| virtio_mb(vq->weak_barriers); |
| |
| - if (is_used_desc_packed(vq, used_idx, wrap_counter)) { |
| + if (is_used_desc_packed(vq, |
| + vq->last_used_idx, |
| + vq->packed.used_wrap_counter)) { |
| END_USE(vq); |
| return false; |
| } |
| -- |
| 2.7.4 |
| |