From patchwork Thu Oct 4 22:17:58 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 189364 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E70FB2C0317 for ; Fri, 5 Oct 2012 08:54:45 +1000 (EST) Received: from localhost ([::1]:38131 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TJuJY-0001Zl-5n for incoming@patchwork.ozlabs.org; Thu, 04 Oct 2012 18:54:44 -0400 Received: from eggs.gnu.org ([208.118.235.92]:38954) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TJuJQ-0001PC-87 for qemu-devel@nongnu.org; Thu, 04 Oct 2012 18:54:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TJuJO-00016J-Ue for qemu-devel@nongnu.org; Thu, 04 Oct 2012 18:54:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3717) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TJuJO-000167-MC for qemu-devel@nongnu.org; Thu, 04 Oct 2012 18:54:34 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q94MsXRQ003063 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 4 Oct 2012 18:54:33 -0400 Received: from bling.home (ovpn-113-153.phx2.redhat.com [10.3.113.153]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q94MHx9Q007586; Thu, 4 Oct 2012 18:17:59 -0400 From: Alex Williamson To: qemu-devel@nongnu.org Date: Thu, 04 Oct 2012 16:17:58 -0600 Message-ID: <20121004221757.3189.92167.stgit@bling.home> In-Reply-To: <20121004220545.3189.52569.stgit@bling.home> References: <20121004220545.3189.52569.stgit@bling.home> User-Agent: StGIT/0.14.3 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: alex.williamson@redhat.com Subject: [Qemu-devel] [PATCH 04/11] vfio-pci: Rework MSIX setup/teardown 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org We try to do lazy initialization of MSIX since we don't actually need to setup anything until MSIX vectors start getting used. This leads to problems if MSIX is enabled, but never used (we can end up trying to re-enable INTx while it's still enabled). We also run into problems trying to expand our reset function to tear down interrupts as we can then get vector release notifications after we've released data structures. By making explicit initialization and teardown we can avoid both of these problems and behave more similar to bare metal. Signed-off-by: Alex Williamson --- hw/vfio_pci.c | 108 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c index 7413f2d..89e00ba 100644 --- a/hw/vfio_pci.c +++ b/hw/vfio_pci.c @@ -72,8 +72,6 @@ static void vfio_disable_irqindex(VFIODevice *vdev, int index) }; ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, &irq_set); - - vdev->interrupt = VFIO_INT_NONE; } /* @@ -278,10 +276,6 @@ static int vfio_enable_vectors(VFIODevice *vdev, bool msix) g_free(irq_set); - if (!ret) { - vdev->interrupt = msix ? VFIO_INT_MSIX : VFIO_INT_MSI; - } - return ret; } @@ -296,15 +290,6 @@ static int vfio_msix_vector_use(PCIDevice *pdev, vdev->host.domain, vdev->host.bus, vdev->host.slot, vdev->host.function, nr); - if (vdev->interrupt != VFIO_INT_MSIX) { - vfio_disable_interrupts(vdev); - } - - if (!vdev->msi_vectors) { - vdev->msi_vectors = g_malloc0(vdev->msix->entries * - sizeof(VFIOMSIVector)); - } - vector = &vdev->msi_vectors[nr]; vector->vdev = vdev; vector->use = true; @@ -457,6 +442,23 @@ static void msi_set_qsize(PCIDevice *pdev, uint8_t size) pci_set_word(config + PCI_MSI_FLAGS, flags); } +static void vfio_enable_msix(VFIODevice *vdev) +{ + vfio_disable_interrupts(vdev); + + vdev->msi_vectors = g_malloc0(vdev->msix->entries * sizeof(VFIOMSIVector)); + + vdev->interrupt = VFIO_INT_MSIX; + + if (msix_set_vector_notifiers(&vdev->pdev, vfio_msix_vector_use, + vfio_msix_vector_release)) { + error_report("vfio: msix_set_vector_notifiers failed\n"); + } + + DPRINTF("%s(%04x:%02x:%02x.%x)\n", __func__, vdev->host.domain, + vdev->host.bus, vdev->host.slot, vdev->host.function); +} + static void vfio_enable_msi(VFIODevice *vdev) { int ret, i; @@ -529,17 +531,43 @@ retry: msi_set_qsize(&vdev->pdev, vdev->nr_vectors); + vdev->interrupt = VFIO_INT_MSI; + DPRINTF("%s(%04x:%02x:%02x.%x) Enabled %d MSI vectors\n", __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot, vdev->host.function, vdev->nr_vectors); } -static void vfio_disable_msi_x(VFIODevice *vdev, bool msix) +static void vfio_disable_msi_common(VFIODevice *vdev) +{ + g_free(vdev->msi_vectors); + vdev->msi_vectors = NULL; + vdev->nr_vectors = 0; + vdev->interrupt = VFIO_INT_NONE; + + vfio_enable_intx(vdev); +} + +static void vfio_disable_msix(VFIODevice *vdev) +{ + msix_unset_vector_notifiers(&vdev->pdev); + + if (vdev->nr_vectors) { + vfio_disable_irqindex(vdev, VFIO_PCI_MSIX_IRQ_INDEX); + } + + vfio_disable_msi_common(vdev); + + DPRINTF("%s(%04x:%02x:%02x.%x, msi%s)\n", __func__, + vdev->host.domain, vdev->host.bus, vdev->host.slot, + vdev->host.function, msix ? "x" : ""); +} + +static void vfio_disable_msi(VFIODevice *vdev) { int i; - vfio_disable_irqindex(vdev, msix ? VFIO_PCI_MSIX_IRQ_INDEX : - VFIO_PCI_MSI_IRQ_INDEX); + vfio_disable_irqindex(vdev, VFIO_PCI_MSI_IRQ_INDEX); for (i = 0; i < vdev->nr_vectors; i++) { VFIOMSIVector *vector = &vdev->msi_vectors[i]; @@ -558,26 +586,15 @@ static void vfio_disable_msi_x(VFIODevice *vdev, bool msix) NULL, NULL, NULL); } - if (msix) { - msix_vector_unuse(&vdev->pdev, i); - } - event_notifier_cleanup(&vector->interrupt); } - g_free(vdev->msi_vectors); - vdev->msi_vectors = NULL; - vdev->nr_vectors = 0; - - if (!msix) { - msi_set_qsize(&vdev->pdev, 0); /* Actually still means 1 vector */ - } + vfio_disable_msi_common(vdev); - DPRINTF("%s(%04x:%02x:%02x.%x, msi%s)\n", __func__, - vdev->host.domain, vdev->host.bus, vdev->host.slot, - vdev->host.function, msix ? "x" : ""); + msi_set_qsize(&vdev->pdev, 0); /* Actually still means 1 vector */ - vfio_enable_intx(vdev); + DPRINTF("%s(%04x:%02x:%02x.%x)\n", __func__, vdev->host.domain, + vdev->host.bus, vdev->host.slot, vdev->host.function); } /* @@ -763,7 +780,7 @@ static void vfio_pci_write_config(PCIDevice *pdev, uint32_t addr, if (!was_enabled && is_enabled) { vfio_enable_msi(vdev); } else if (was_enabled && !is_enabled) { - vfio_disable_msi_x(vdev, false); + vfio_disable_msi(vdev); } } @@ -776,9 +793,9 @@ static void vfio_pci_write_config(PCIDevice *pdev, uint32_t addr, is_enabled = msix_enabled(pdev); if (!was_enabled && is_enabled) { - /* vfio_msix_vector_use handles this automatically */ + vfio_enable_msix(vdev); } else if (was_enabled && !is_enabled) { - vfio_disable_msi_x(vdev, true); + vfio_disable_msix(vdev); } } } @@ -973,10 +990,10 @@ static void vfio_disable_interrupts(VFIODevice *vdev) vfio_disable_intx(vdev); break; case VFIO_INT_MSI: - vfio_disable_msi_x(vdev, false); + vfio_disable_msi(vdev); break; case VFIO_INT_MSIX: - vfio_disable_msi_x(vdev, true); + vfio_disable_msix(vdev); break; } } @@ -1094,15 +1111,6 @@ static int vfio_setup_msix(VFIODevice *vdev, int pos) return ret; } - ret = msix_set_vector_notifiers(&vdev->pdev, vfio_msix_vector_use, - vfio_msix_vector_release); - if (ret) { - error_report("vfio: msix_set_vector_notifiers failed %d\n", ret); - msix_uninit(&vdev->pdev, &vdev->bars[vdev->msix->table_bar].mem, - &vdev->bars[vdev->msix->pba_bar].mem); - return ret; - } - return 0; } @@ -1111,12 +1119,6 @@ static void vfio_teardown_msi(VFIODevice *vdev) msi_uninit(&vdev->pdev); if (vdev->msix) { - /* FIXME: Why can't unset just silently do nothing?? */ - if (vdev->pdev.msix_vector_use_notifier && - vdev->pdev.msix_vector_release_notifier) { - msix_unset_vector_notifiers(&vdev->pdev); - } - msix_uninit(&vdev->pdev, &vdev->bars[vdev->msix->table_bar].mem, &vdev->bars[vdev->msix->pba_bar].mem); }