From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulanit Subject: Re: [PATCH 1/2 v5] iommu/amd: Fix logic for clearing the IOMMU interrupt bits Date: Mon, 17 Jun 2013 13:59:31 -0500 Message-ID: <51BF5C93.8000408@amd.com> References: <1370840751-11277-1-git-send-email-suravee.suthikulpanit@amd.com> <51B5CCF002000078000DC9A5@nat28.tlf.novell.com> <20130610121855.GE8802@ocelot.phlegethon.org> <51B5E58802000078000DCA7E@nat28.tlf.novell.com> <51B6E40A02000078000DCF6C@nat28.tlf<51B6E40A02000078000DCF6C@nat28.tlf.novell.com> <51B7ACB5.7070805@amd.com> <51B8303302000078000DD6B6@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51B8303302000078000DD6B6@nat28.tlf.novell.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: Jan Beulich Cc: Tim Deegan , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 6/12/2013 1:24 AM, Jan Beulich wrote: >>>> On 12.06.13 at 01:03, Suravee Suthikulanit > wrote: >> On 6/11/2013 1:47 AM, Jan Beulich wrote: >>> @@ -611,22 +608,33 @@ static void iommu_check_event_log(struct >>> u32 entry; >>> unsigned long flags; >>> >>> + /* 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 */ >>> + /* 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); >>> - >>> - /* reset interrupt status bit */ >>> - entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); >>> - iommu_set_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT); >>> - >>> - writel(entry, iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET); >>> + 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); >>> + /* >>> + * Re-schedule the tasklet to handle eventual log entries added >>> + * between reading the log above and re-enabling the interrupt. >>> + */ >>> + tasklet_schedule(&amd_iommu_irq_tasklet); >>> + } >>> + } >> If more entries are added to the event log during the time that event >> log interrupt is disabled (in the control register), >> the IOMMU hardware will generate interrupt once the the interrupt enable >> bit in the control register changes from 0 to 1 and set the status >> register. Since the "iommu_interrupt_handler" code is already calling >> "schedule_tasklet", we should not need to "re-schedule" tasklet here. >> I have confirmed the hardware behavior described with the hardware >> designer. This is also the same on the PPR log. > And also the same between v1 and v2 hardware? Again, I'd like to > be on the safe side, and rather do a reschedule too much than one > too little. And in any case, having your documentation made more > precise in these respects would be much appreciated. > > Jan > > Acked: Suravee Suthikulpanit