Re: [Xen-devel] [PATCH 2/2 V2] iommu/amd: Workaround for erratum 787

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.06.13 at 18:31, Tim Deegan <tim@xxxxxxx> wrote:
> At 16:03 +0100 on 10 Jun (1370880211), Jan Beulich wrote:
> From my reading of the IOMMU spec the pending bit gets set whether the
> enable bit is set or not:
> 
>   PPRInt: peripheral page service request interrupt. Revision 1:
>   RO. Reset 0b. Reserved.  Revision 2: RW1C. Reset 0b. 1=PPR request
>   entry written to the PPR log by the IOMMU. 0=No PPR entry written to
>   the PPR log by the IOMMU. An interrupt is generated when PPRInt=1b and
>   MMIO Offset 0018h[PPRIntEn]=1b.
> 
> and
> 
>   EventLogInt: event log interrupt. RW1C. Reset 0b. 1=Event entry
>   written to the event log by the IOMMU. 0=No event entry written to the
>   event log by the IOMMU. An interrupt is generated when EventLogInt=1b
>   and MMIO Offset 0018h[EventIntEn]=1b.
> 
> so we should be OK without a second pass if the pending bit is still
> clear after unmasking the interrupt.

Do we want to rely on this? I don't think so, as the names of the
bits suggest otherwise. I'll send v5 in a minute, and I think this will
work for both possible cases.

>> So with this done I now realized that all of these transformations
>> don't really belong in this erratum workaround patch.
> 
> Agreed.  I think this reshuffle to avoid lost entries should be its own
> patch, and the erratum 787 one should follow it -- unless the logic we
> end up with handles erratum 787 as a side-effect. :)

Actually merging it into patch 1 seemed the more natural thing in
the end, as some of the effects of the patch before this re-shuffle
really require these further adjustments to be done at once.

>> You'll note that even here we're having a loop again, which you
>> presumably won't like much.
> 
> Well, this time the event handling is inside the loop, so it ought to
> catch and disable bad passthrough devices.  I'd still prefer having the
> tasklet schedule itself and terminate, but I'm happy to defer to your
> taste.

I came to the same conclusion - it'll re-schedule the tasklet now
both for the interrupt re-enabling case as well as (in patch 2) for
the interrupt-bit-still-or-again-set one.

Jan


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