From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVsEA-0001v1-0C for qemu-devel@nongnu.org; Wed, 11 Mar 2015 21:47:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVsE5-0006I7-Vf for qemu-devel@nongnu.org; Wed, 11 Mar 2015 21:47:57 -0400 Date: Thu, 12 Mar 2015 12:48:16 +1100 From: David Gibson Message-ID: <20150312014816.GR11973@voom.redhat.com> References: <1426054314-19564-1-git-send-email-gwshan@linux.vnet.ibm.com> <1426054314-19564-2-git-send-email-gwshan@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="3dBJfKlFjfsS/piO" Content-Disposition: inline In-Reply-To: <1426054314-19564-2-git-send-email-gwshan@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH 2/3] VFIO: Clear INTx pending state on EEH reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gavin Shan Cc: aik@ozlabs.ru, alex.williamson@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de --3dBJfKlFjfsS/piO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 11, 2015 at 05:11:53PM +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. >=20 > # 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) >=20 > The patch clears "VFIOPCIDevice->intx.pending" after EEH reset > is completed on the PE, which contains the adapter. In turn, the > mmap'ed BAR regions can be reenabled to avoid EEH recovery failure. >=20 > Signed-off-by: Gavin Shan > --- > hw/vfio/pci.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) >=20 > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 8c4a8cb..55e0904 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -3352,6 +3352,20 @@ int vfio_container_eeh_event(AddressSpace *as, int= 32_t groupid, > } > =20 > break; > + case VFIO_EEH_PE_RESET_DEACTIVATE: > + /* > + * We might have INTx interrupt whose handler disabled the > + * memory mapped BARs. Without clearing the INTx pending > + * state, the timer kicked by the INTx interrupt handler > + * won't enable those disabled memory mapped BARs, which > + * leads EEH recovery failure. > + */ > + QLIST_FOREACH(vbasedev, &group->device_list, next) { > + vdev =3D container_of(vbasedev, VFIOPCIDevice, vbasedev); > + vdev->intx.pending =3D false; > + } > + > + break; > } > =20 > vfio_put_group(group); I'm not sure that invoking these side effects from the low-level ioctl() wrapper makes a lot of sense. Wouldn't it be clearer to explicitly do the necessary state cleanup in the reset callers. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --3dBJfKlFjfsS/piO Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVAPBgAAoJEGw4ysog2bOS1CsQANv7/Yh5NcZmnfBwN94h7hqZ MoWAIFSe1tFFoCmfpdMwSkeqLXmwdjZkT2m1SsCVfgI0rkDf+u9lrSx2wAuCYcTC GrT9sCmCdAjPECd64NaTiXOANbMPWbPSyMUtY6eFEwwNpnGdNSaK/a7QMkfLG0vv FjwD7NcWKVGaMjTRG1a1PrX3ngfW1mxDi3pW2lpN9f82UD9QvOhcAYMsrx1w/00p 2kRKU472bnCpFFJD0H2XUhtoYpzDdc5KOFfNVFOGDqCWPdaVcYrwKAXd2jWR2Y/i 7qKRD4fgGR+XsVU7o8jEsKIIlIToAbVSc01bYd7tO5lO31G/jFqX615RCNOfIGQW iIGy+ujenYTrrXdVMWXRarRgy62xVXLRJlxZSiyzExFyE9XBK6kLAISrhkwkXgHz T8wunpWjbovaLQKG+9WmAwgZzXaPlrJuLWsS3KYJfPaGZC0szVDBH0lY6BZiikRp 77Yx4aN6nIqEn4ScVPnFoxB89tTd05dNKjwI4r6WFgXskUU71dUZL5ykcmkw/Q+j SLVmhziu9KzZQt0RFrlPUllGIhefuQobvbRgOkCuFpXlPx3SKsz5Lgd99dX39O3t PAyJbjQ8p0tNA0GGerAeqFsDrf/gQ4ae0bw7ykce1utffjDoPGd++Was0780ma8s jOvjMX8gc1QggMJFV8by =W7yS -----END PGP SIGNATURE----- --3dBJfKlFjfsS/piO--