From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36683) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMNJX-0006Ho-CJ for qemu-devel@nongnu.org; Tue, 26 Aug 2014 16:26:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XMNJR-0001KD-1h for qemu-devel@nongnu.org; Tue, 26 Aug 2014 16:25:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49952) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMNJQ-0001K4-PK for qemu-devel@nongnu.org; Tue, 26 Aug 2014 16:25:52 -0400 Message-ID: <1409084747.2906.147.camel@ul30vt.home> From: Alex Williamson Date: Tue, 26 Aug 2014 14:25:47 -0600 In-Reply-To: <1408528328-11560-3-git-send-email-gwshan@linux.vnet.ibm.com> References: <1408528328-11560-1-git-send-email-gwshan@linux.vnet.ibm.com> <1408528328-11560-3-git-send-email-gwshan@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 2/2] 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, qemu-devel@nongnu.org On Wed, 2014-08-20 at 19:52 +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 | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > index 1a3e7eb..3cf7f02 100644 > --- a/hw/misc/vfio.c > +++ b/hw/misc/vfio.c > @@ -4424,6 +4424,8 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid, > { > VFIOGroup *group; > VFIOContainer *container; > + VFIODevice *vdev; > + struct vfio_eeh_pe_op *arg; Define these within the scope of the case since they're not used outside of it. > int ret = -1; > > group = vfio_get_group(groupid, as); > @@ -4442,7 +4444,29 @@ 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: > + 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) { > + if (msix_enabled(&vdev->pdev)) { > + vfio_disable_msix(vdev); > + } > + > + msix_reset(&vdev->pdev); > + } We already have vfio_disable_interrupts(), can't we use that (blindly)? Do we really need to call msix_reset()? If so, should vfio_disable_msix() call it? > + > + break; Extraneous break > + } > + > break; > default: > /* Return an error on unknown requests */ Thanks, Alex