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

[Suravee Suthikulanit <suravee.suthikulpanit@amd.com>]

On 6/12/2013 1:24 AM, Jan Beulich wrote:
>>>> On 12.06.13 at 01:03, Suravee Suthikulanit <suravee.suthikulpanit@amd.com>
> 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 <suravee.suthikulpanit@amd.com>