From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58594) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXyrE-0004Nu-8U for qemu-devel@nongnu.org; Tue, 17 Mar 2015 17:17:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YXyrA-0001FE-Om for qemu-devel@nongnu.org; Tue, 17 Mar 2015 17:17:00 -0400 Message-ID: <1426627006.3643.342.camel@redhat.com> From: Alex Williamson Date: Tue, 17 Mar 2015 15:16:46 -0600 In-Reply-To: <1426523486-9794-2-git-send-email-gwshan@linux.vnet.ibm.com> References: <1426523486-9794-1-git-send-email-gwshan@linux.vnet.ibm.com> <1426523486-9794-2-git-send-email-gwshan@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/3] VFIO: Disable INTx interrupt on EEH reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gavin Shan Cc: aik@ozlabs.ru, qemu-devel@nongnu.org, agraf@suse.de, qemu-ppc@nongnu.org, david@gibson.dropbear.id.au On Tue, 2015-03-17 at 03:31 +1100, Gavin Shan wrote: > When Linux guest recovers from EEH error on the following Emulex > adapter, the MSIx interrupts are disabled and the INTx emulation > is enabled. One INTx interrupt is injected to the guest by host > because of detected pending INTx interrupts on the adapter. QEMU > disables mmap'ed BAR regions and starts a timer to enable those > regions at later point the INTx interrupt handler. Unfortunately, > "VFIOPCIDevice->intx.pending" isn't cleared, meaning those disabled > mapp'ed BAR regions won't be reenabled properly. It leads to EEH > recovery failure at guest side because of hanged MMIO access. > > # lspci | grep Emulex > 0000:01:00.0 Ethernet controller: Emulex Corporation \ > OneConnect 10Gb NIC (be3) (rev 02) > 0000:01:00.1 Ethernet controller: Emulex Corporation \ > OneConnect 10Gb NIC (be3) (rev 02) > > The patch disables INTx interrupt before doing EEH reset to avoid > the issue. > > Signed-off-by: Gavin Shan > --- > hw/vfio/pci.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index fca1edc..bfa3d0c 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -3340,6 +3340,14 @@ int vfio_container_eeh_event(AddressSpace *as, int32_t groupid, > * disable it so that it can be reenabled properly. Also, > * the cached MSIx table should be cleared as it's not > * reflecting the contents in hardware. > + * > + * We might have INTx interrupt whose handler disables the > + * memory mapped BARs. The INTx pending state can't be > + * cleared with memory BAR access in slow path. The timer > + * kicked by the INTx interrupt handler won't enable those > + * disabled memory mapped BARs, which leads to hanged MMIO > + * register access and EEH recovery failure. We simply disable > + * INTx if it has been enabled. > */ This feels like a quick hack for a problem we don't really understand. Why is INTx being fired through QEMU rather than KVM? Why isn't the INTx re-enabling happening since this is exactly the scenario where it's supposed to work (ie. INTx occurs, BAR mmap disabled, guest accesses BAR, mmap re-enabled, INTx unmasked)? > QLIST_FOREACH(vbasedev, &group->device_list, next) { > vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev); > @@ -3348,6 +3356,11 @@ int vfio_container_eeh_event(AddressSpace *as, int32_t groupid, > } > > msix_reset(&vdev->pdev); > + > + /* Disable INTx */ > + if (vdev->interrupt == VFIO_INT_INTx) { > + vfio_disable_intx(vdev); > + } > } > > break;