From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54719) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOxGH-0004ux-A9 for qemu-devel@nongnu.org; Tue, 02 Sep 2014 19:13:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XOxG7-0000oh-VR for qemu-devel@nongnu.org; Tue, 02 Sep 2014 19:13:17 -0400 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:34852) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOxG7-0000oG-BD for qemu-devel@nongnu.org; Tue, 02 Sep 2014 19:13:07 -0400 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 3 Sep 2014 09:13:02 +1000 Received: from d23relay07.au.ibm.com (d23relay07.au.ibm.com [9.190.26.37]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id AC67A2CE8047 for ; Wed, 3 Sep 2014 09:12:57 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay07.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id s82NE1Gh13500452 for ; Wed, 3 Sep 2014 09:14:01 +1000 Received: from d23av01.au.ibm.com (localhost [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s82NCumM011879 for ; Wed, 3 Sep 2014 09:12:56 +1000 Date: Wed, 3 Sep 2014 09:12:57 +1000 From: Gavin Shan Message-ID: <20140902231256.GA4795@shangw> References: <1409532812-8644-1-git-send-email-gwshan@linux.vnet.ibm.com> <1409532812-8644-3-git-send-email-gwshan@linux.vnet.ibm.com> <1409688642.3804.114.camel@ul30vt.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1409688642.3804.114.camel@ul30vt.home> Subject: Re: [Qemu-devel] [RFC PATCH v2 2/2] 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: Alex Williamson Cc: aik@ozlabs.ru, Gavin Shan , qemu-devel@nongnu.org On Tue, Sep 02, 2014 at 02:10:42PM -0600, Alex Williamson wrote: >On Mon, 2014-09-01 at 10:53 +1000, 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 >> --- >> hw/misc/vfio.c | 32 +++++++++++++++++++++++++++++++- >> 1 file changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c >> index 1a3e7eb..1f55051 100644 >> --- a/hw/misc/vfio.c >> +++ b/hw/misc/vfio.c >> @@ -2724,6 +2724,17 @@ static void vfio_disable_interrupts(VFIODevice *vdev) >> } >> } >> >> +static void vfio_disable_and_reset_interrupts(VFIODevice *vdev) >> +{ >> + vfio_disable_interrupts(vdev); >> + >> + switch (vdev->interrupt) { >> + case VFIO_INT_MSIX: >> + msix_reset(&vdev->pdev); >> + break; >> + } > >This is apparently untested because vdev->interrupt should never be set >to VFIO_INT_MSIX after vfio_disable_interrupts(). Also, you need to >update the normal reset path to call msix_reset() unless it's already >happening via another reset handler. Thanks, > Yes, I didn't test this revision. I'll change the code according to your comments and retest, then send v3 for your comments. Thanks, Gavin >Alex > >> +} >> + >> static int vfio_setup_msi(VFIODevice *vdev, int pos) >> { >> uint16_t ctrl; >> @@ -4442,8 +4453,27 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid, >> switch (req) { >> case VFIO_CHECK_EXTENSION: >> case VFIO_IOMMU_SPAPR_TCE_GET_INFO: >> - case VFIO_EEH_PE_OP: >> break; >> + case VFIO_EEH_PE_OP: { >> + VFIODevice *vdev; >> + struct vfio_eeh_pe_op *arg = (struct vfio_eeh_pe_op *)param; >> + >> + switch (arg->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(vdev, &group->device_list, next) { >> + vfio_disable_and_reset_interrupts(vdev); >> + } >> + } >> + >> + break; >> + } >> default: >> /* Return an error on unknown requests */ >> error_report("vfio: unsupported ioctl %X", req); > > >