Re: [PATCH] PCI: dwc: Fix interrupt race in when handling MSI

[Trent Piepho <tpiepho@impinj.com>]

On Mon, 2018-11-12 at 16:01 +0000, Lorenzo Pieralisi wrote:
> On Thu, Nov 08, 2018 at 08:51:38PM +0000, Trent Piepho wrote:
> > 
> > The reason is that the GIC interrupt above the dwc is non-reentrant. 
> > It remains masked (aka active[1]) during this entire process (1 to 10).
> >  This means every MSI is effectively already masked.  So masking the
> > active MSI(s) a 2nd time gains nothing besides preventing some extra
> > edges for a masked interrupt going to the ARM GIC.
> > 
> > In theory, if the GIC interrupt handler was reentrant, then on receipt
> > of a new MSI we could re-enter the dwc handler on a different CPU and
> > run the new MSI (a different MSI!) at the same time as the original MSI
> > handler is still running.
> > 
> > There difference here is that by unmasking in the interrupt in the GIC
> > before the dwc handler is finished, masking an individual MSI in the
> > dwc is no longer a 2nd redundant masking.
> 
> There is not (and there must not be) anything in DWC HW that allows
> us assume its "interrupt controller" is cascaded to a GIC. It may
> correspond to what we are debugging but it is an assumption we must not
> make and its driver has to work regardless of what its interrupt
> controller is connected to, that's the basis of hierarchical IRQ chips
> management, at least from my narrow and (most likely) incomplete
> knowledge of the IRQ subsystem.

If you mean we can't assume the dwc is cascaded from specifically a
GIC, totally agree.

If you mean we can't assume the dwc is cascaded from "something", then
I disagree.  The driver can't possibly be at the top level.  How would
the cpu ever enter it?

What I think really is what matters here is if the the chain the dwc is
cascaded from allows the dwc to be called reentrantly or not.

On my system, the code base I inspected does not allow that.  I don't
know of any written Linux interrupt policy that says this must be the
case.  If it is not the case, then I see other points in the dwc driver
that are potentially racy.