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

[Tim Deegan <tim@xxxxxxx>]

At 07:40 +0100 on 11 Jun (1370936413), Jan Beulich wrote:
> >>> 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.

I think it's pretty clear from the text that the bit gets set when an
entry is written, even if an interrupt doesn't get raised.  To me, that
means we can replace the mandatory re-run on interrupt enable with a
check of the status register (i.e. the check we'll do for erratum 787
anyway).

But if you're not comfortable with that, the v5 you just posted looks
correct to me, and (at least until we have restartable IOMMU faults)
that path won't be hot enough to worry about the efficiency.  So both v5
patches are Reviewed-by: Tim Deegan <tim@xxxxxxx>.

Cheers,

Tim.

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