On Tue, 2018-11-13 at 00:41 +0000, Gustavo Pimentel wrote:
On 09/11/2018 11:34, Marc Zyngier wrote:
Gustavo, here's one simple ask. Please reply to this email with a step by step, register by register description of how an MSI must be handled on this HW. We do need it, and we need it now.
Hi Marc, I thought that I did respond to your question on Nov 8. However I will repeat it and add some extra information to it now.
About the registers:
PCIE_MSI_INTR0_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_INTR0_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.
If the MSI is unmasked and the status bit is still set, *not* from a new MSI but rather from one that arrived before the MSI was masked, does the controller still signal it?
I would suspect the answer is no: only a new MSI will cause the interrupt be signaled on unmask. And then only if the status bit has not been cleared before the unmask (as you stated already).
But for that to be the case, there must be another bit of interrupt status (i.e., "pending") that we don't know about yet. The bit in the status register alone is not enough.
This pending bit would need to be set on a 0->1 transition of the status bit. An MSI arriving while the status bit is already 1 does not trigger a new interrupt.
About this new callbacks:
The dw_pci_bottom_mask() and dw_pci_bottom_unmask() callbacks were added on Linux kernel v4.17 on PCI: dwc: Move MSI IRQs allocation to IRQ domains hierarchical API patch [1] and based on what I've seen so far, before this patch there were no interrupt masking been performed.
Yes. Or rather, the status bit being set effectively masks the interrupt.
Based on the information provided, its very likely that I should use the PCIE_MSI_INTR0_MASK register instead of PCIE_MSI_INTR0_ENABLE on the dw_pci_bottom_mask() and dw_pci_bottom_unmask() callbacks. As soon as I return from Linux plumbers conference, I will test the system using the PCIE_MSI_INTR0_MASK register.
Using enable to mask the interrupt is broken. It will allow an MSI to be lost if it arrives while the MSI in not enabled. It is impossible to prevent that from racing against the pci device driver's final check that no MSI-signaled condition in the hardware is present.