From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39626) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZuZ1-0006ua-60 for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:06:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YZuYx-0008Ed-Lu for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:06:10 -0400 Date: Mon, 23 Mar 2015 16:06:56 +1100 From: David Gibson Message-ID: <20150323050656.GR25043@voom.fritz.box> References: <1426523486-9794-1-git-send-email-gwshan@linux.vnet.ibm.com> <20150320060401.GY5741@voom.redhat.com> <20150320062729.GA13024@shangw> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="19HmC3QOnaNVzKTI" Content-Disposition: inline In-Reply-To: <20150320062729.GA13024@shangw> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 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: alex.williamson@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --19HmC3QOnaNVzKTI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 20, 2015 at 05:27:29PM +1100, Gavin Shan wrote: > On Fri, Mar 20, 2015 at 05:04:01PM +1100, David Gibson wrote: > >On Tue, Mar 17, 2015 at 03:31:24AM +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 > >> --- > >> v2: vfio_container_eeh_event() stub for !CONFIG_PCI and separate > >> error message for this function. Dropped vfio_put_group() > >> on NULL group > >> --- > >> hw/vfio/Makefile.objs | 6 +++++- > >> hw/vfio/common.c | 7 +++++++ > >> hw/vfio/pci-stub.c | 17 +++++++++++++++++ > >> hw/vfio/pci.c | 38 ++++++++++++++++++++++++++++++++++++++ > >> include/hw/vfio/vfio.h | 2 ++ > >> 5 files changed, 69 insertions(+), 1 deletion(-) > >> create mode 100644 hw/vfio/pci-stub.c > >>=20 > >> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs > >> index e31f30e..1b8a065 100644 > >> --- a/hw/vfio/Makefile.objs > >> +++ b/hw/vfio/Makefile.objs > >> @@ -1,4 +1,8 @@ > >> ifeq ($(CONFIG_LINUX), y) > >> obj-$(CONFIG_SOFTMMU) +=3D common.o > >> -obj-$(CONFIG_PCI) +=3D pci.o > >> +ifeq ($(CONFIG_PCI), y) > >> +obj-y +=3D pci.o > >> +else > >> +obj-y +=3D pci-stub.o > >> +endif > >> endif > >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c > >> index 148eb53..ed07814 100644 > >> --- a/hw/vfio/common.c > >> +++ b/hw/vfio/common.c > >> @@ -949,7 +949,14 @@ int vfio_container_ioctl(AddressSpace *as, int32_= t groupid, > >> switch (req) { > >> case VFIO_CHECK_EXTENSION: > >> case VFIO_IOMMU_SPAPR_TCE_GET_INFO: > >> + break; > >> case VFIO_EEH_PE_OP: > >> + if (vfio_container_eeh_event(as, groupid, param) !=3D 0) { > > > >I really dislike the idea of having an arbitrarily complex side effect > >from a function whose name suggest's it's just a trivial wrapper > >around the ioctl(). > > >=20 > Ok. I guess you would like putting the complex in the callers of > vfio_container_ioctl(). Well.. maybe. I'd also be happy if helper functions were implemeneted which both called the ioctl() and did the other necessary pieces. They should just be called something that indicates their full function, not a name which suggests they're just an ioctl wrapper. > All those callers are implemtend in sPAPR > platform (spapr_pci_vfio.c). I didn't put the logic there because: >=20 > - VFIOPCIDevice is invisible to sPAPR platform. It seems kind of silly that VFIOPCIDevice isn't visible to spapr_pci_vfio.c. > - We can't tell one particular PCI device attached to sPAPRPHBState > is the basement of VFIOPCIDevice, or emulated PCI device. I'm not sure what you mean by that. --=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 --19HmC3QOnaNVzKTI Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVD59wAAoJEGw4ysog2bOSVK0QAL5Z6Cq9ovK3D4xKv2OF1iO5 27jG/WdYKRmpfkZ+6jKqG9zlKJ6hi+ebbM2tIvpgzWYap2lddwBlAthjpOK1y+Z7 wUYVuZtQTXq1AGEhXefB7JEX+Pc15sMhR7Gzm5WkFvk5TZwKtrze+IfKYxy/16xX +S5FliNgzAFY6Hg44xJ4fLI29HCG7YdI/a2ibWVHc7pVjJgcZ3PJPDU3iS+h4iTk 925cNepgNWXtGXeWfOmnVSs0EUSnj8BDZrKAIup9x/pvEAltQ8iFBonu5nj4b+MP L2wwyB89mVfY5+aimew13sRkqFKC70+fSBVF8eATUzSuS3+uSBAtpgIRzEG8l06h VbHJLmtaVqSxxLiagorA/abVPMdA9TgrFXqQu1jWzJqgN8d4uKYwkll/ooQYGfyK XLhJ7aUpuiIiUiyBNqJScRjXo0vFb/HWBResFSmVjXfPOHELp7vyWVQ4L1evMwpx LWlpFhl6EzGY+7xiOtPbg0ng25bT5f7c0CilHSHhMvB5e+cLMt7lWib+SfEyKZnV bG0lzCjzDDwuxQGwjllibTUWKJpE/KBDae+r4xJjld0lCF2fq48I3S/xxfoMGIrm dxzCdvyWxuEjHmbUSO4oZni92DEhxlYIrnnUeqiZ0aI/MEX5lbWEqTiee+6psysh SWBAy05RyFaBko2HZhvT =2Shh -----END PGP SIGNATURE----- --19HmC3QOnaNVzKTI--