From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43043) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZusg-0004xM-89 for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:26:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YZusR-00056X-3k for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:26:30 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:42825) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZusQ-00056F-GV for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:26:15 -0400 Received: from /spool/local by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 23 Mar 2015 15:26:11 +1000 Date: Mon, 23 Mar 2015 16:25:10 +1100 From: Gavin Shan Message-ID: <20150323052510.GA9147@shangw> References: <1426523486-9794-1-git-send-email-gwshan@linux.vnet.ibm.com> <20150320060401.GY5741@voom.redhat.com> <20150320062729.GA13024@shangw> <20150323050656.GR25043@voom.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150323050656.GR25043@voom.fritz.box> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/3] VFIO: Clear stale MSIx table during EEH reset Reply-To: Gavin Shan List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: alex.williamson@redhat.com, qemu-ppc@nongnu.org, Gavin Shan , qemu-devel@nongnu.org On Mon, Mar 23, 2015 at 04:06:56PM +1100, David Gibson wrote: >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. >> >> >> >> The patch clears stale MSIx table before EEH PE reset so that MSIx >> >> table could be restored properly after EEH PE reset. >> >> >> >> 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 >> >> >> >> 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) += common.o >> >> -obj-$(CONFIG_PCI) += pci.o >> >> +ifeq ($(CONFIG_PCI), y) >> >> +obj-y += pci.o >> >> +else >> >> +obj-y += 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) != 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(). >> > >> >> 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. > Indeed, vfio_container_ioctl() isn't indicating what the function is doing. How about renaming it to vfio_container_event_and_ioctl()? I'm always bad at giving a good function name :) Thanks, Gavin >> All those callers are implemtend in sPAPR >> platform (spapr_pci_vfio.c). I didn't put the logic there because: >> >> - 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. > > >-- >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