[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2 V2] iommu/amd: Workaround for erratum 787
>>> On 10.06.13 at 15:55, "Jan Beulich" <JBeulich@xxxxxxxx> wrote: >>>> On 10.06.13 at 14:43, Tim Deegan <tim@xxxxxxx> wrote: >> How about: >> write-to-clear status.pending >> process the log >> if (status.pending) >> reschedule the tasklet >> else >> unmask the interrupt. > > Actually, I don't think that's right: Clearing the pending bit with > the respective interrupt source disabled doesn't allow the > pending bit to become set again upon arrival of a new log entry, > and hence we might still be leaving entries unhandled for an > indefinite period of time. So I now think we need to do > > write-to-clear status.pending > process the log > unmask the interrupt > process the log again > if (status.pending) > reschedule the tasklet > > Of course we could skip the unmask and parse-again if the > interrupt wasn't masked in the first place, which is possible since > it gets masked only for any IOMMU that had its interrupt handler > executed before the tasklet gets busy on it. So with this done I now realized that all of these transformations don't really belong in this erratum workaround patch. They either should be a prereq patch (or folded into patch 1), or a follow-up one, with the one here then nevertheless going back to the original simple loop approach. Do you view this any different? Here is what one of the two functions now looks like, just for reference: static void iommu_check_event_log(struct amd_iommu *iommu) { u32 entry; unsigned long flags; for ( ; ; ) { /* RW1C interrupt status bit */ writel(IOMMU_STATUS_EVENT_LOG_INT_MASK, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); iommu_read_log(iommu, &iommu->event_log, 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 ( entry & IOMMU_STATUS_EVENT_OVERFLOW_MASK ) iommu_reset_log(iommu, &iommu->event_log, set_iommu_event_log_control); else { entry = readl(iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET); if ( !(entry & IOMMU_CONTROL_EVENT_LOG_INT_MASK) ) { entry |= IOMMU_CONTROL_EVENT_LOG_INT_MASK; writel(entry, iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET); spin_unlock_irqrestore(&iommu->lock, flags); continue; } } break; } /* * Workaround for erratum787: * Re-check to make sure the bit has been cleared. */ entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); if ( entry & IOMMU_STATUS_EVENT_LOG_INT_MASK ) tasklet_schedule(&amd_iommu_irq_tasklet); spin_unlock_irqrestore(&iommu->lock, flags); } You'll note that even here we're having a loop again, which you presumably won't like much. The only alternative I see is to run iommu_read_log() with iommu->lock held, which has the caveat of the function itself taking a lock (albeit - without having done any proving yet - I think that lock is taken completely pointlessly). In any case - the erratum workaround is really just the comment and three following lines. Everything else belongs elsewhere imo. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |