From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH 2/2 V2] iommu/amd: Workaround for erratum 787 Date: Mon, 10 Jun 2013 10:35:32 +0100 Message-ID: <20130610093532.GA8802@ocelot.phlegethon.org> References: <1370840751-11277-1-git-send-email-suravee.suthikulpanit@amd.com> <1370840751-11277-2-git-send-email-suravee.suthikulpanit@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1370840751-11277-2-git-send-email-suravee.suthikulpanit@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: suravee.suthikulpanit@amd.com Cc: JBeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org At 00:05 -0500 on 10 Jun (1370822751), suravee.suthikulpanit@amd.com wrote: > From: Suravee Suthikulpanit > > The IOMMU interrupt handling in bottom half must clear the PPR log interrupt > and event log interrupt bits to re-enable the interrupt. This is done by > writing 1 to the memory mapped register to clear the bit. Due to hardware bug, > if the driver tries to clear this bit while the IOMMU hardware also setting > this bit, the conflict will result with the bit being set. If the interrupt > handling code does not make sure to clear this bit, subsequent changes in the > event/PPR logs will no longer generating interrupts, and would result if > buffer overflow. After clearing the bits, the driver must read back > the register to verify. Is there a risk of livelock here? That is, if some device is causing a lot of IOMMU faults, a CPU could get stuck in this loop re-enabling interrupts as fast as they are raised. The solution suggested in the erratum seems better: if the bit is set after clearing, process the interrupts again (i.e. run/schedule the top-half handler). That way the bottom-half handler will definitely terminate and the system can make some progress. Cheers, Tim. > Signed-off-by: Suravee Suthikulpanit > --- > V2 changes: > - Coding style fixes > > xen/drivers/passthrough/amd/iommu_init.c | 36 ++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 12 deletions(-) > > diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c > index a85c63f..048a2e6 100644 > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -616,15 +616,21 @@ static void iommu_check_event_log(struct amd_iommu *iommu) > > spin_lock_irqsave(&iommu->lock, flags); > > - /*check event overflow */ > entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + while (entry & (1 << IOMMU_STATUS_EVENT_LOG_INT_SHIFT)) > + { > + /* Check event overflow */ > + if ( iommu_get_bit(entry, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT) ) > + iommu_reset_log(iommu, &iommu->event_log, set_iommu_event_log_control); > > - if ( iommu_get_bit(entry, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT) ) > - iommu_reset_log(iommu, &iommu->event_log, set_iommu_event_log_control); > + /* RW1C interrupt status bit */ > + writel((1 << IOMMU_STATUS_EVENT_LOG_INT_SHIFT), > + iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > > - /* RW1C interrupt status bit */ > - writel((1 << IOMMU_STATUS_EVENT_LOG_INT_SHIFT), > - iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + /* Workaround for erratum787: > + * Re-check to make sure the bit has been cleared */ > + entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + } > > spin_unlock_irqrestore(&iommu->lock, flags); > } > @@ -684,15 +690,21 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu) > > spin_lock_irqsave(&iommu->lock, flags); > > - /*check event overflow */ > entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + while ( entry & (1 << IOMMU_STATUS_PPR_LOG_INT_SHIFT ) ) > + { > + /* Check event overflow */ > + if ( iommu_get_bit(entry, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT) ) > + iommu_reset_log(iommu, &iommu->ppr_log, set_iommu_ppr_log_control); > > - if ( iommu_get_bit(entry, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT) ) > - iommu_reset_log(iommu, &iommu->ppr_log, set_iommu_ppr_log_control); > + /* RW1C interrupt status bit */ > + writel((1 << IOMMU_STATUS_PPR_LOG_INT_SHIFT), > + iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > > - /* RW1C interrupt status bit */ > - writel((1 << IOMMU_STATUS_PPR_LOG_INT_SHIFT), > - iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + /* Workaround for erratum787: > + * Re-check to make sure the bit has been cleared */ > + entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + } > > spin_unlock_irqrestore(&iommu->lock, flags); > } > -- > 1.7.10.4 > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel