Re: [PATCH 3/5] PCI: dwc: Remove MSI/MSIX capability if iMSI-RX is used as MSI controller

[Qiang Yu <qiang.yu@oss.qualcomm.com>]

On Sat, Dec 27, 2025 at 10:51:26AM +0530, Manivannan Sadhasivam wrote:
> On Fri, Dec 26, 2025 at 03:31:23PM -0600, Bjorn Helgaas wrote:
> > In subject, s/MSIX/MSI-X/ to match spec and other usage.
> > 
> > On Sun, Nov 09, 2025 at 10:59:42PM -0800, Qiang Yu wrote:
> > > Some platforms may not support ITS (Interrupt Translation Service) and
> > > MBI (Message Based Interrupt), or there are not enough available empty SPI
> > > lines for MBI, in which case the msi-map and msi-parent property will not
> > > be provided in device tree node. For those cases, the DWC PCIe driver
> > > defaults to using the iMSI-RX module as MSI controller. However, due to
> > > DWC IP design, iMSI-RX cannot generate MSI interrupts for Root Ports even
> > > when MSI is properly configured and supported as iMSI-RX will only monitor
> > > and intercept incoming MSI TLPs from PCIe link, but the memory write
> > > generated by Root Port are internal system bus transactions instead of
> > > PCIe TLPs, so they are ignored.
> > > 
> > > This leads to interrupts such as PME, AER from the Root Port not received
> > > on the host and the users have to resort to workarounds such as passing
> > > "pcie_pme=nomsi" cmdline parameter.
> > 
> > This will be great, thanks a lot for working on this.  This has been a
> > long-standing irritation with this DWC IP.
> > 
> > > To ensure reliable interrupt handling, remove MSI and MSI-X capabilities
> > > from Root Ports when using iMSI-RX as MSI controller, which is indicated
> > > by has_msi_ctrl == true. This forces a fallback to INTx interrupts,
> > > eliminating the need for manual kernel command line workarounds.
> > > 
> > > With this behavior:
> > > - Platforms with ITS/MBI support use ITS/MBI MSI for interrupts from all
> > >   components.
> > > - Platforms without ITS/MBI support fall back to INTx for Root Ports and
> > >   use iMSI-RX for other PCI devices.
> > > 
> > > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware-host.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 20c9333bcb1c4812e2fd96047a49944574df1e6f..3724aa7f9b356bfba33a6515e2c62a3170aef1e9 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -1083,6 +1083,16 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> > >  
> > >  	dw_pcie_dbi_ro_wr_dis(pci);
> > >  
> > > +	/*
> > > +	 * If iMSI-RX module is used as the MSI controller, remove MSI and
> > > +	 * MSI-X capabilities from PCIe Root Ports to ensure fallback to INTx
> > > +	 * interrupt handling.
> > > +	 */
> > > +	if (pp->has_msi_ctrl) {
> > > +		dw_pcie_remove_capability(pci, PCI_CAP_ID_MSI);
> > > +		dw_pcie_remove_capability(pci, PCI_CAP_ID_MSIX);
> > > +	}
> > 
> > "has_msi_ctrl" doesn't seem like a good name here because there's no
> > documentation about what it means, and "has_msi_ctrl" is completely
> > generic while "iMSI-RX" is very specific.
> > 
> 
> This predates my involvement with DWC drivers, but I guess it expands to 'has
> internal MSI controller' and 'internal' probably means iMSI-RX. But I agree that
> the naming could be improved to something like 'imsi_rx_available' or
> 'has_imsi_rx'. I'll take a stab at it in a separate patch.
> 
> > And apparently platforms with ITS/MBI *can* generate MSIs from Root
> > Ports, but "has_msi_ctrl" would be false for them?  This is really
> > hard to read.
> > 
> 
> Yes.
> 
> > pp->has_msi_ctrl is set by qcom_pcie_ecam_host_init() and IIUC, for
> > any platform that lacks .msi_init() and the "msi-parent" and "msi-map"
> > properties.
> > 
> > The qcom_pcie_ecam_host_init() case is weird because it looks like it
> > abuses the pci_ecam_ops.init() callback to initialize MSI stuff, not
> > ECAM stuff.  Maybe that MSI init could be done in qcom_pcie_probe()
> > right after it calls pci_host_common_ecam_create()?
> > 
> 
> I think it should be possible to initialize MSI after
> pci_host_common_ecam_create(). Let me fix *this* and above in a separate series.

The qcom_pcie_ecam_host_init() is used by firmware-managed targets and the 
function unconditionally sets has_msi_ctrl = true without checking for 
"msi-parent" or "msi-map" properties in the device tree. So I think
firmware should take care of removing MSI/MSIX cap.

- Qiang Yu
> 
> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்