@@ -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,
@@ -186,6 +186,15 @@ 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 !(virtio_guest_to_host_u16(vq, 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));
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@arm.com> --- include/kvm/virtio.h | 10 ++++++++-- virtio/core.c | 9 +++++++++ 2 files changed, 17 insertions(+), 2 deletions(-)