Hi Trent, On 10/12/2018 20:34, Trent Piepho wrote: > On Mon, 2018-12-10 at 18:31 +0000, Marc Zyngier wrote: >> >>> I had concerns about what appears to be an unnecessary extra lock taken >>> before handling an interrupt and enabling all MSIs even if nothing has >>> tried to enable them. >> >> Regarding the lock: I'm quite puzzled that you consider it >> "unnecessary", given that all the DWC callbacks expect such a locking. I >> suspect you are considering from a pure performance angle, and I'd >> suggest that you post numbers showing the unacceptable overhead of an >> otherwise uncontended lock. > > The difference is that the other callbacks can be called outside the > path of handling an interrupt. Any thread could possibly make a call > to mask the interrupt at any time, so the lock is needed. > > But the ack is only called in the path of handling an irq. That's why > its different. There's already a system insuring that callback is not > called reentrantly. If the lock was ever taken when it was attempted > to be locked, then we'd already be broken. > >> As for enabling all MSIs upfront, same thing. Please demonstrate how >> harmful it is, given that they are all masked by default, consistently >> with what other interrupt controllers are doing. > > Without any information on the controller, who knows what the > differences between a masked and enabled vs a disabled msi are? I > don't know there is a difference, but on the other hand, you don't know > there isn't. It's like a dozen lines of code to not change the > behavior. That is not true, I've replied on 13/18/2018 with that information. I leave here again the information provided. *PCIE_MSI_INTRx_MASK* When an MSI is received for a masked interrupt, the corresponding status bit gets set in the interrupt status register but the controller will not signal it. As soon as the masked interrupt is unmasked and assuming the status bit is still set the controller will signal it. *PCIE_MSI_INTRx_ENABLE* Enables/disables every interrupt. When an MSI is received from a disabled interrupt, no status bit gets set in MSI controller interrupt status register, in another word, the interruption will be lost. Regards, Gustavo >
