Re: [Xen-devel] [PATCH 1/2 v5] iommu/amd: Fix logic for clearing the IOMMU interrupt bits

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.06.13 at 01:03, Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>
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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel