From patchwork Fri Nov 17 14:59:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Philippe Brucker X-Patchwork-Id: 10062855 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A2F9760352 for ; Fri, 17 Nov 2017 14:58:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 967502A555 for ; Fri, 17 Nov 2017 14:58:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8B5222AD1B; Fri, 17 Nov 2017 14:58:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 22C5D2A555 for ; Fri, 17 Nov 2017 14:58:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934222AbdKQO6s (ORCPT ); Fri, 17 Nov 2017 09:58:48 -0500 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:35836 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757386AbdKQO6o (ORCPT ); Fri, 17 Nov 2017 09:58:44 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E4E6580D; Fri, 17 Nov 2017 06:58:43 -0800 (PST) Received: from e106794-lin.cambridge.arm.com (e106794-lin.cambridge.arm.com [10.1.210.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 28B913F246; Fri, 17 Nov 2017 06:58:43 -0800 (PST) From: Jean-Philippe Brucker To: kvm@vger.kernel.org Cc: will.deacon@arm.com, kraxel@redhat.com Subject: [PATCH v2 kvmtool 2/3] virtio: Support drivers that don't negotiate VIRTIO_RING_F_EVENT_IDX Date: Fri, 17 Nov 2017 14:59:15 +0000 Message-Id: <20171117145916.13530-3-jean-philippe.brucker@arm.com> X-Mailer: git-send-email 2.14.3 In-Reply-To: <20171117145916.13530-1-jean-philippe.brucker@arm.com> References: <20171117145916.13530-1-jean-philippe.brucker@arm.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- include/kvm/virtio.h | 10 ++++++++-- virtio/core.c | 9 +++++++++ 2 files changed, 17 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..ddce48bf6ab6 100644 --- a/virtio/core.c +++ b/virtio/core.c @@ -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));