From patchwork Sun Jan 22 07:38:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 9530813 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 5FDC16049F for ; Sun, 22 Jan 2017 07:39:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 50B0927FAD for ; Sun, 22 Jan 2017 07:39:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3A1FB2811C; Sun, 22 Jan 2017 07:39:31 +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=unavailable 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 AEF2D27FC0 for ; Sun, 22 Jan 2017 07:39:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751261AbdAVHjI (ORCPT ); Sun, 22 Jan 2017 02:39:08 -0500 Received: from mail.kernel.org ([198.145.29.136]:56324 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750943AbdAVHjG (ORCPT ); Sun, 22 Jan 2017 02:39:06 -0500 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 33CDF202B8; Sun, 22 Jan 2017 07:38:42 +0000 (UTC) Received: from redhat.com (pool-96-237-166-50.bstnma.fios.verizon.net [96.237.166.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7BBCF20274; Sun, 22 Jan 2017 07:38:40 +0000 (UTC) Date: Sun, 22 Jan 2017 09:38:39 +0200 From: "Michael S. Tsirkin" To: linux-kernel@vger.kernel.org Cc: Alex Williamson , Jason Wang , kvm@vger.kernel.org, virtualization@lists.linux-foundation.org Subject: [PATCH v3] vfio error recovery: kernel support Message-ID: <1485070594-27713-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Disposition: inline X-Mailer: git-send-email 2.8.0.287.g0deeb61 X-Mutt-Fcc: =sent X-Virus-Scanned: ClamAV using ClamSMTP Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This is a design and an initial patch for kernel side for AER support in VFIO. 0. What happens now (PCIE AER only) Fatal errors cause a link reset. Non fatal errors don't. All errors stop the VM eventually, but not immediately because it's detected and reported asynchronously. Interrupts are forwarded as usual. Correctable errors are not reported to guest at all. Note: PPC EEH is different. This focuses on AER. 1. Correctable errors I don't see a need to report these to guest. So let's not. 2. Fatal errors It's not easy to handle them gracefully since link reset is needed. As a first step, let's use the existing mechanism in that case. 2. Non-fatal errors Here we could make progress by reporting them to guest and have guest handle them. Issues: a. this behaviour should only be enabled with new userspace old userspace should work without changes Suggestion: One way to address this would be to add a new eventfd non_fatal_err_trigger. If not set, invoke err_trigger. b. drivers are supposed to stop MMIO when error is reported if vm keeps going, we will keep doing MMIO/config Suggestion 1: ignore this. vm stop happens much later when userspace runs anyway, so we are not making things much worse Suggestion 2: try to stop MMIO/config, resume on resume call Patch below implements Suggestion 1. Note that although this is really against the documentation, which states error_detected() is the point at which the driver should quiesce the device and not touch it further (until diagnostic poking at mmio_enabled or full access at resume callback). Fixing this won't be easy. However, this is not a regression. Also note this does nothing about interrupts, documentation suggests returning IRQ_NONE until reset. Again, not a regression. c. PF driver might detect that function is completely broken, if vm keeps going, we will keep doing MMIO/config Suggestion 1: ignore this. vm stop happens much later when userspace runs anyway, so we are not making things much worse Suggestion 2: detect this and invoke err_trigger to stop VM Patch below implements Suggestion 2. Aside: what should we return if driver is e.g. being unbound? For slot_reset, this implements Alex's suggestion to return PCI_ERS_RESULT_NONE. It's worth considering whether same should happen in error_report. Signed-off-by: Michael S. Tsirkin --- Completely untested by myself. Will be build-tested by 0-day :) changes from v2: - fix build error reported by 0 day testing changes from v1: - more detailed commit log - fixed a couple of bugs Alex, pls review but don't apply until we get - userspace updated to use the new interface - testing reports Cao jin, could you pls look into integrating this with your userspace patches? drivers/vfio/pci/vfio_pci.c | 34 +++++++++++++++++++++++++++++++++- drivers/vfio/pci/vfio_pci_intrs.c | 19 +++++++++++++++++++ drivers/vfio/pci/vfio_pci_private.h | 1 + include/linux/virtio_config.h | 12 +++++++++++- include/uapi/linux/vfio.h | 1 + 5 files changed, 65 insertions(+), 2 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 0e33278..7d58b8f 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -1296,7 +1296,9 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev, mutex_lock(&vdev->igate); - if (vdev->err_trigger) + if (state == pci_channel_io_normal && vdev->non_fatal_err_trigger) + eventfd_signal(vdev->non_fatal_err_trigger, 1); + else if (vdev->err_trigger) eventfd_signal(vdev->err_trigger, 1); mutex_unlock(&vdev->igate); @@ -1306,8 +1308,38 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev, return PCI_ERS_RESULT_CAN_RECOVER; } +static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev) +{ + struct vfio_pci_device *vdev; + struct vfio_device *device; + static pci_ers_result_t err = PCI_ERS_RESULT_NONE; + + device = vfio_device_get_from_dev(&pdev->dev); + if (!device) + goto err_dev; + + vdev = vfio_device_data(device); + if (!vdev) + goto err_data; + + mutex_lock(&vdev->igate); + + if (vdev->err_trigger) + eventfd_signal(vdev->err_trigger, 1); + + mutex_unlock(&vdev->igate); + + err = PCI_ERS_RESULT_RECOVERED; + +err_data: + vfio_device_put(device); +err_dev: + return err; +} + static const struct pci_error_handlers vfio_err_handlers = { .error_detected = vfio_pci_aer_err_detected, + .slot_reset = vfio_pci_aer_slot_reset, }; static struct pci_driver vfio_pci_driver = { diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 1c46045..e883db5 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -611,6 +611,17 @@ static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev, count, flags, data); } +static int vfio_pci_set_non_fatal_err_trigger(struct vfio_pci_device *vdev, + unsigned index, unsigned start, + unsigned count, uint32_t flags, void *data) +{ + if (index != VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX || start != 0 || count > 1) + return -EINVAL; + + return vfio_pci_set_ctx_trigger_single(&vdev->non_fatal_err_trigger, + count, flags, data); +} + static int vfio_pci_set_req_trigger(struct vfio_pci_device *vdev, unsigned index, unsigned start, unsigned count, uint32_t flags, void *data) @@ -664,6 +675,14 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags, break; } break; + case VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX: + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) { + case VFIO_IRQ_SET_ACTION_TRIGGER: + if (pci_is_pcie(vdev->pdev)) + func = vfio_pci_set_err_trigger; + break; + } + break; case VFIO_PCI_REQ_IRQ_INDEX: switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) { case VFIO_IRQ_SET_ACTION_TRIGGER: diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h index f37c73b..c27a507 100644 --- a/drivers/vfio/pci/vfio_pci_private.h +++ b/drivers/vfio/pci/vfio_pci_private.h @@ -93,6 +93,7 @@ struct vfio_pci_device { struct pci_saved_state *pci_saved_state; int refcnt; struct eventfd_ctx *err_trigger; + struct eventfd_ctx *non_fatal_err_trigger; struct eventfd_ctx *req_trigger; struct list_head dummy_resources_list; }; diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index 26c155b..6b0f416 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -159,7 +159,17 @@ static inline bool virtio_has_iommu_quirk(const struct virtio_device *vdev) * Note the reverse polarity of the quirk feature (compared to most * other features), this is for compatibility with legacy systems. */ - return !virtio_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM); + if (virtio_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) + return false; + + /* + * fastboot emulator for ARM puts virtio devices behind an SMMU + * and never bypasses it for legacy devices. + */ + if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64)) + return virtio_has_feature(vdev, VIRTIO_F_VERSION_1); + + return true; } static inline diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 519eff3..affa973 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -443,6 +443,7 @@ enum { VFIO_PCI_MSIX_IRQ_INDEX, VFIO_PCI_ERR_IRQ_INDEX, VFIO_PCI_REQ_IRQ_INDEX, + VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX, VFIO_PCI_NUM_IRQS };