From patchwork Tue Mar 29 14:17:07 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cornelia Huck X-Patchwork-Id: 8687541 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 39E589F44D for ; Tue, 29 Mar 2016 14:18:56 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 64219202F8 for ; Tue, 29 Mar 2016 14:18:55 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 61C4E201E4 for ; Tue, 29 Mar 2016 14:18:54 +0000 (UTC) Received: from localhost ([::1]:47306 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akuTt-000442-Nr for patchwork-qemu-devel@patchwork.kernel.org; Tue, 29 Mar 2016 10:18:53 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48074) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akuSV-0002AA-PT for qemu-devel@nongnu.org; Tue, 29 Mar 2016 10:17:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1akuSQ-00060V-ME for qemu-devel@nongnu.org; Tue, 29 Mar 2016 10:17:27 -0400 Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:52735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akuSQ-00060G-9P for qemu-devel@nongnu.org; Tue, 29 Mar 2016 10:17:22 -0400 Received: from localhost by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 29 Mar 2016 15:17:21 +0100 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp11.uk.ibm.com (192.168.101.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 29 Mar 2016 15:17:11 +0100 X-IBM-Helo: d06dlp03.portsmouth.uk.ibm.com X-IBM-MailFrom: cornelia.huck@de.ibm.com X-IBM-RcptTo: qemu-devel@nongnu.org Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id E1B211B08069 for ; Tue, 29 Mar 2016 15:17:45 +0100 (BST) Received: from d06av05.portsmouth.uk.ibm.com (d06av05.portsmouth.uk.ibm.com [9.149.37.229]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u2TEHBNZ7012630 for ; Tue, 29 Mar 2016 14:17:11 GMT Received: from d06av05.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u2TEHAmf021703 for ; Tue, 29 Mar 2016 08:17:11 -0600 Received: from gondolin.boeblingen.de.ibm.com (dyn-9-152-224-197.boeblingen.de.ibm.com [9.152.224.197]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u2TEH9Xj021637 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 29 Mar 2016 08:17:10 -0600 From: Cornelia Huck To: qemu-devel@nongnu.org Date: Tue, 29 Mar 2016 16:17:07 +0200 Message-Id: <1459261027-4398-2-git-send-email-cornelia.huck@de.ibm.com> X-Mailer: git-send-email 2.8.0 In-Reply-To: <1459261027-4398-1-git-send-email-cornelia.huck@de.ibm.com> References: <1459261027-4398-1-git-send-email-cornelia.huck@de.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16032914-0041-0000-0000-00000E5F13B2 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 195.75.94.107 Cc: famz@redhat.com, borntraeger@de.ibm.com, mst@redhat.com, tubo@linux.vnet.ibm.com, stefanha@redhat.com, Cornelia Huck , pbonzini@redhat.com Subject: [Qemu-devel] [PATCH 1/1] virtio: fix ioeventfd assignment race X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The ->set_host_notifier() callback is invoked whenever we want to switch from or to the generic ioeventfd handler. Currently, all transports deregister the ioeventfd backing and then re-register it. This opens a race window where we are without ioeventfd backing for a time period: In the virtio-blk dataplane case, we observed notifications coming in from both the vcpu thread and the iothread. Let's change pci, mmio and ccw to keep the ioeventfd during ->set_host_notifier() and only switch the ioeventfd handler. Signed-off-by: Cornelia Huck Tested-by: Paolo Bonzini --- hw/s390x/virtio-ccw.c | 22 +++++++++++++++++----- hw/virtio/virtio-mmio.c | 27 +++++++++++++++++---------- hw/virtio/virtio-pci.c | 28 ++++++++++++++++++---------- include/hw/virtio/virtio-bus.h | 4 ++++ 4 files changed, 56 insertions(+), 25 deletions(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index cb887ba..7b1088e 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -1196,14 +1196,26 @@ static bool virtio_ccw_query_guest_notifiers(DeviceState *d) static int virtio_ccw_set_host_notifier(DeviceState *d, int n, bool assign) { VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); + VirtQueue *vq = virtio_get_queue(vdev, n); - /* Stop using the generic ioeventfd, we are doing eventfd handling - * ourselves below */ - dev->ioeventfd_disabled = assign; if (assign) { - virtio_ccw_stop_ioeventfd(dev); + /* Stop using the generic ioeventfd, we are doing eventfd handling + * ourselves below */ + dev->ioeventfd_disabled = true; + }; + /* + * Just switch the handler, don't deassign the ioeventfd. + * Otherwise, there's a window where we don't have an + * ioeventfd and we may end up with a notification where + * we don't expect one. + */ + virtio_queue_set_host_notifier_fd_handler(vq, assign, !assign); + if (!assign) { + /* Use generic ioeventfd handler again. */ + dev->ioeventfd_disabled = false; } - return virtio_ccw_set_guest2host_notifier(dev, n, assign, false); + return 0; } static int virtio_ccw_get_mappings(VirtioCcwDevice *dev) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index d4cd91f..aafebdf 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -502,19 +502,26 @@ static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); + VirtQueue *vq = virtio_get_queue(vdev, n); - /* Stop using ioeventfd for virtqueue kick if the device starts using host - * notifiers. This makes it easy to avoid stepping on each others' toes. - */ - proxy->ioeventfd_disabled = assign; if (assign) { - virtio_mmio_stop_ioeventfd(proxy); + /* Stop using the generic ioeventfd, we are doing eventfd handling + * ourselves below */ + proxy->ioeventfd_disabled = true; + }; + /* + * Just switch the handler, don't deassign the ioeventfd. + * Otherwise, there's a window where we don't have an + * ioeventfd and we may end up with a notification where + * we don't expect one. + */ + virtio_queue_set_host_notifier_fd_handler(vq, assign, !assign); + if (!assign) { + /* Use generic ioeventfd handler again. */ + proxy->ioeventfd_disabled = false; } - /* We don't need to start here: it's not needed because backend - * currently only stops on status change away from ok, - * reset, vmstop and such. If we do add code to start here, - * need to check vmstate, device state etc. */ - return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false); + return 0; } /* virtio-mmio device */ diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 0dadb66..a91c1e8 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1115,18 +1115,26 @@ static int virtio_pci_set_host_notifier(DeviceState *d, int n, bool assign) { VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); - /* Stop using ioeventfd for virtqueue kick if the device starts using host - * notifiers. This makes it easy to avoid stepping on each others' toes. - */ - proxy->ioeventfd_disabled = assign; + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); + VirtQueue *vq = virtio_get_queue(vdev, n); + if (assign) { - virtio_pci_stop_ioeventfd(proxy); + /* Stop using the generic ioeventfd, we are doing eventfd handling + * ourselves below */ + proxy->ioeventfd_disabled = true; + }; + /* + * Just switch the handler, don't deassign the ioeventfd. + * Otherwise, there's a window where we don't have an + * ioeventfd and we may end up with a notification where + * we don't expect one. + */ + virtio_queue_set_host_notifier_fd_handler(vq, assign, !assign); + if (!assign) { + /* Use generic ioeventfd handler again. */ + proxy->ioeventfd_disabled = false; } - /* We don't need to start here: it's not needed because backend - * currently only stops on status change away from ok, - * reset, vmstop and such. If we do add code to start here, - * need to check vmstate, device state etc. */ - return virtio_pci_set_host_notifier_internal(proxy, n, assign, false); + return 0; } static void virtio_pci_vmstate_change(DeviceState *d, bool running) diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index 3f2c136..98c660a 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -52,6 +52,10 @@ typedef struct VirtioBusClass { bool (*has_extra_state)(DeviceState *d); bool (*query_guest_notifiers)(DeviceState *d); int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign); + /* + * Switch from/to the generic ioeventfd handler. + * assigned==false means 'use generic ioeventfd handler'. + */ int (*set_host_notifier)(DeviceState *d, int n, bool assigned); void (*vmstate_change)(DeviceState *d, bool running); /*