Bad things happen when the VIRTIO_RING_F_EVENT_IDX feature isn't
negotiated and we try to write the avail_event anyway. SeaBIOS, for
example, stores internal data where avail_event should be [1].
Technically the Virtio specification doesn't forbid the device from
writing the avail_event, and it's up to the driver to reserve space for it
("the transitional driver [...] MUST allocate the total number of bytes
for the virtqueue according to [formula containing the avail event]").
But it doesn't hurt us to avoid writing avail_event, and kvmtool needs
changes for interrupt suppression anyway, in order to comply with the
spec. Indeed Virtio 1.0 cs04 says, in 2.4.7.2 Device Requirements:
Virtqueue Interrupt Suppression:
"""
If the VIRTIO_F_EVENT_IDX feature bit is not negotiated:
* The device MUST ignore the used_event value.
* After the device writes a descriptor index into the used ring:
- If flags is 1, the device SHOULD NOT send an interrupt.
"""
So let's do that.
[1] https://patchwork.kernel.org/patch/10038931/
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx>
---
include/kvm/virtio.h | 10 ++++++++--
virtio/core.c | 8 ++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 69856d9ef4e7..ab4028ca2e2f 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -34,6 +34,7 @@ struct virt_queue {
u16 last_avail_idx;
u16 last_used_signalled;
u16 endian;
+ bool use_event_idx;
};
/*
@@ -110,11 +111,15 @@ static inline struct vring_desc *virt_queue__get_desc(struct virt_queue *queue,
static inline bool virt_queue__available(struct virt_queue *vq)
{
+ u16 last_avail_idx = virtio_host_to_guest_u16(vq, vq->last_avail_idx);
+
if (!vq->vring.avail)
return 0;
- vring_avail_event(&vq->vring) = virtio_host_to_guest_u16(vq, vq->last_avail_idx);
- return virtio_guest_to_host_u16(vq, vq->vring.avail->idx) != vq->last_avail_idx;
+ if (vq->use_event_idx)
+ vring_avail_event(&vq->vring) = last_avail_idx;
+
+ return vq->vring.avail->idx != last_avail_idx;
}
void virt_queue__used_idx_advance(struct virt_queue *queue, u16 jump);
@@ -179,6 +184,7 @@ static inline void virtio_init_device_vq(struct virtio_device *vdev,
struct virt_queue *vq)
{
vq->endian = vdev->endian;
+ vq->use_event_idx = (vdev->features & VIRTIO_RING_F_EVENT_IDX);
}
void virtio_set_guest_features(struct kvm *kvm, struct virtio_device *vdev,
diff --git a/virtio/core.c b/virtio/core.c
index e98241f04fcd..f4d531a27544 100644
--- a/virtio/core.c
+++ b/virtio/core.c
@@ -186,6 +186,14 @@ bool virtio_queue__should_signal(struct virt_queue *vq)
{
u16 old_idx, new_idx, event_idx;
+ if (!vq->use_event_idx) {
+ /*
+ * When VIRTIO_RING_F_EVENT_IDX isn't negotiated, interrupt the
+ * guest if it didn't explicitly request to be left alone.
+ */
+ return !(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT);
+ }
+
old_idx = vq->last_used_signalled;
new_idx = virtio_guest_to_host_u16(vq, vq->vring.used->idx);
event_idx = virtio_guest_to_host_u16(vq, vring_used_event(&vq->vring));
--
2.14.3
