<suravee.suthikulpanit@amd.com>
2013-Apr-18 18:41 UTC
[PATCH 2/2] iommu/amd: Workaround for ERBT1312
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
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.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
xen/drivers/passthrough/amd/iommu_init.c | 41 ++++++++++++++++++++----------
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_init.c
b/xen/drivers/passthrough/amd/iommu_init.c
index f1af9de..75cf104 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -615,16 +615,23 @@ static void iommu_check_event_log(struct amd_iommu *iommu)
sizeof(event_entry_t), parse_event_log_entry);
spin_lock_irqsave(&iommu->lock, flags);
-
- /*check event overflow */
+
entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
- if ( iommu_get_bit(entry, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT) )
- iommu_reset_log(iommu, &iommu->event_log,
set_iommu_event_log_control);
+ while (entry & IOMMU_STATUS_EVENT_LOG_INT_MASK) {
+
+ /*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);
- /* reset interrupt status bit */
- writel(IOMMU_STATUS_EVENT_LOG_INT_MASK,
- iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+ /* reset interrupt status bit */
+ writel(IOMMU_STATUS_EVENT_LOG_INT_MASK,
+ iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+
+ /* This is a hardware bug workaround (ERBT1312) */
+ entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+ }
spin_unlock_irqrestore(&iommu->lock, flags);
}
@@ -681,18 +688,24 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu)
iommu_read_log(iommu, &iommu->ppr_log,
sizeof(ppr_entry_t), parse_ppr_log_entry);
-
+
spin_lock_irqsave(&iommu->lock, flags);
- /*check event overflow */
entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
- if ( iommu_get_bit(entry, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT) )
- iommu_reset_log(iommu, &iommu->ppr_log,
set_iommu_ppr_log_control);
+ while (entry & IOMMU_STATUS_PPR_LOG_INT_MASK) {
+
+ /*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);
- /* reset interrupt status bit */
- writel(IOMMU_STATUS_PPR_LOG_INT_MASK,
- iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+ /* reset interrupt status bit */
+ writel(IOMMU_STATUS_PPR_LOG_INT_MASK,
+ iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+
+ /* This is a hardware bug workaround (ERBT1312) */
+ entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+ }
spin_unlock_irqrestore(&iommu->lock, flags);
}
--
1.7.10.4
>>> On 18.04.13 at 20:41, <suravee.suthikulpanit@amd.com> wrote:What is ERBT1312? Please reference at least the title of the document containing this in the description, so that in few months/years time one can still reconstruct what this refers to.> --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -615,16 +615,23 @@ static void iommu_check_event_log(struct amd_iommu *iommu) > sizeof(event_entry_t), parse_event_log_entry); > > spin_lock_irqsave(&iommu->lock, flags); > - > - /*check event overflow */ > + > entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > > - if ( iommu_get_bit(entry, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT) ) > - iommu_reset_log(iommu, &iommu->event_log, set_iommu_event_log_control); > + while (entry & IOMMU_STATUS_EVENT_LOG_INT_MASK) { > + > + /*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); > > - /* reset interrupt status bit */ > - writel(IOMMU_STATUS_EVENT_LOG_INT_MASK, > - iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + /* reset interrupt status bit */ > + writel(IOMMU_STATUS_EVENT_LOG_INT_MASK, > + iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + > + /* This is a hardware bug workaround (ERBT1312) */ > + entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + } > > spin_unlock_irqrestore(&iommu->lock, flags); > } > @@ -681,18 +688,24 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu) > > iommu_read_log(iommu, &iommu->ppr_log, > sizeof(ppr_entry_t), parse_ppr_log_entry); > - > + > spin_lock_irqsave(&iommu->lock, flags); > > - /*check event overflow */ > entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > > - if ( iommu_get_bit(entry, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT) ) > - iommu_reset_log(iommu, &iommu->ppr_log, set_iommu_ppr_log_control); > + while (entry & IOMMU_STATUS_PPR_LOG_INT_MASK) { > + > + /*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); > > - /* reset interrupt status bit */ > - writel(IOMMU_STATUS_PPR_LOG_INT_MASK, > - iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + /* reset interrupt status bit */ > + writel(IOMMU_STATUS_PPR_LOG_INT_MASK, > + iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + > + /* This is a hardware bug workaround (ERBT1312) */ > + entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); > + } > > spin_unlock_irqrestore(&iommu->lock, flags); > }This needs cleaning up from a coding style perspective: No hard tabs, braces on separate lines, spaces inside the outermost parentheses following keywords, a space following the opening /* of comments, proper indentation of multi-line function arguments. Jan
>>> On 18.04.13 at 20:41, <suravee.suthikulpanit@amd.com> wrote: > 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.The Completion Wait Interrupt bit is unaffected by this erratum? Or else, is it guaranteed that flush_command_buffer() wouldn''t need a similar fix? Jan