From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59273) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVsED-0001vo-5D for qemu-devel@nongnu.org; Wed, 11 Mar 2015 21:48:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVsE6-0006ID-7z for qemu-devel@nongnu.org; Wed, 11 Mar 2015 21:48:01 -0400 Date: Thu, 12 Mar 2015 12:04:59 +1100 From: David Gibson Message-ID: <20150312010459.GQ11973@voom.redhat.com> References: <1426054314-19564-1-git-send-email-gwshan@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="e9fMjeYs+GPci+mg" Content-Disposition: inline In-Reply-To: <1426054314-19564-1-git-send-email-gwshan@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH 1/3] VFIO: Clear stale MSIx table during 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 --e9fMjeYs+GPci+mg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 11, 2015 at 05:11:52PM +1100, Gavin Shan wrote: > The PCI device MSIx table is cleaned out in hardware after EEH PE > reset. However, we still hold the stale MSIx entries in QEMU, which > should be cleared accordingly. Otherwise, we will run into another > (recursive) EEH error and the PCI devices contained in the PE have > to be offlined exceptionally. >=20 > The patch clears stale MSIx table before EEH PE reset so that MSIx > table could be restored properly after EEH PE reset. >=20 > Signed-off-by: Gavin Shan > --- > hw/vfio/common.c | 6 +++++- > hw/vfio/pci.c | 39 +++++++++++++++++++++++++++++++++++++++ > include/hw/vfio/vfio.h | 3 ++- > 3 files changed, 46 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 148eb53..e3833f4 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -949,8 +949,12 @@ int vfio_container_ioctl(AddressSpace *as, int32_t g= roupid, > switch (req) { > case VFIO_CHECK_EXTENSION: > case VFIO_IOMMU_SPAPR_TCE_GET_INFO: > - case VFIO_EEH_PE_OP: > break; > + case VFIO_EEH_PE_OP: > + if (!vfio_container_eeh_event(as, groupid, param)) { Please use =3D=3D 0 not !, remembering that !some_function() is the success case hurts my brain. > + break; > + } > + /* fallthru */ It doesn't look like the fallthrough will generate the correct error message: it will say "unsupported ioctl" but vfio_container_eeh_event() could fail for some other reason. > default: > /* Return an error on unknown requests */ > error_report("vfio: unsupported ioctl %X", req); > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 6b80539..8c4a8cb 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -3319,6 +3319,45 @@ static void vfio_unregister_req_notifier(VFIOPCIDe= vice *vdev) > vdev->req_enabled =3D false; > } > =20 > +int vfio_container_eeh_event(AddressSpace *as, int32_t groupid, > + struct vfio_eeh_pe_op *op) > +{ > + VFIOGroup *group; > + VFIODevice *vbasedev; > + VFIOPCIDevice *vdev; > + > + group =3D vfio_get_group(groupid, as); > + if (!group) { > + vfio_put_group(group); Is vfio_put_group(NULL) really what you want? > + error_report("vfio: group %d not found\n", groupid); > + return -1; > + } > + > + switch (op->op) { > + case VFIO_EEH_PE_RESET_HOT: > + case VFIO_EEH_PE_RESET_FUNDAMENTAL: > + /* > + * The MSIx table will be cleaned out by reset. We need > + * 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. > + */ > + QLIST_FOREACH(vbasedev, &group->device_list, next) { > + vdev =3D container_of(vbasedev, VFIOPCIDevice, vbasedev); > + if (msix_enabled(&vdev->pdev)) { > + vfio_disable_msix(vdev); > + } > + > + msix_reset(&vdev->pdev); > + } > + > + break; > + } > + > + vfio_put_group(group); > + return 0; > +} > + > static int vfio_initfn(PCIDevice *pdev) > { > VFIOPCIDevice *vdev =3D DO_UPCAST(VFIOPCIDevice, pdev, pdev); > diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h > index 0b26cd8..99528a3 100644 > --- a/include/hw/vfio/vfio.h > +++ b/include/hw/vfio/vfio.h > @@ -5,5 +5,6 @@ > =20 > extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid, > int req, void *param); > - > +extern int vfio_container_eeh_event(AddressSpace *as, int32_t groupid, > + struct vfio_eeh_pe_op *op); > #endif --=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 --e9fMjeYs+GPci+mg Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVAOY7AAoJEGw4ysog2bOS2yoP/3ce92+xuIRz1HoCjV7IBJQd 30bfurcLW0HUMuEGxS1sEBpi65KjY5ZlxKZzwolq9BVv2+g8dKFUEz83deBU+zoV URqoZQlV9gSvlKgYUA99BHO/qE4pzvxfHGVE3bRe0daw6H9MweqUEAerOdyFF7PH SIHDoU1a6NJKKssL7L3CZnjfEfP3f+Unc9ZB1sH7Km7tZ3knkqtYh5HuwkGW+MUU gOpesPuHpljjQpD9ocqiejhBhXw0IAE+5TRSoECFRy9ZIH8qjxBz7kkJX6QEgI29 bHyv8zoDvKB5gnGbrEK2V4aIJ/9/joLZR5zdqCOOIXEkcPNTP/0pFPvrEfLnFHHo UKY9CPopImPq1JepYheaeA5FIpC9XEDkNFuwxOen7QnwZ6jqKihJesZAWPwoZO/D tH2O5nswsKPuI0xfDKunDwJgm3Lw8hQzem6qv1dkVv3fhKTw6v8uk0P+xD7C3hHJ WGxv5a8JEOK/NlOrRwHShMOfCRf7yyUYux9894u00Q+ac3Xwo2ZjUnKBxVlvIwkX ZeiJkLlXcr0ZuVZpzaA4PaQ24oq1eN2dOXyy3oNGC2bKOQ49HI2MBof2c+vczYeW YrzqexOamFKUhTsgFSBdsiQmS5j4FUD52nCV5gGre2g28u5JOo37HtNDnreLEiXy eLd7b0+zFgLjroE6f5cz =bZ3H -----END PGP SIGNATURE----- --e9fMjeYs+GPci+mg--