[PATCH kvmtool 2/3] virtio: Support drivers that don't negotiate VIRTIO_RING_F_EVENT_IDX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux