Re: [PATCH] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unmapping the cached MSI iATU window without flushing posted writes can cause AXI bridge lockups.

Pre-existing issues:
- [High] The cached outbound iATU mapping for MSI interrupts does not track the endpoint function number, causing misrouted interrupts.
--

--- Patch [1]: [PATCH] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU ---
commit f97e3684cb2dd4f7108472fedeb7f80a99737691
Author: Niklas Cassel <cassel@kernel.org>

    PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU
    
    The persistent MSI iATU mapping can conflict with the dynamic MSI-X iATU
    mapping, since they are both using ep->msi_mem_phys.
    
    If dw_pcie_ep_raise_msi_irq() caches the iATU mapping, and then
    dw_pcie_ep_raise_msix_irq() is called, it maps the same address to a new
    window/iATU.
    
    Link: https://lore.kernel.org/linux-pci/20260729051542.DC2741F000E9@smtp.kernel.org/

> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 7d2794945704e..051b234260352 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -1124,6 +1124,15 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  		return -EPERM;
>  	}
>  
> +	/*
> +	 * ep->msi_iatu_mapped means that an MSI target address is cached,
> +	 * unmap it first so that we can reuse ep->msi_mem_phys for MSI-X.
> +	 */
> +	if (ep->msi_iatu_mapped) {
> +		dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->msi_mem_phys);
> +		ep->msi_iatu_mapped = false;
> +	}

[Severity: High]
Does unmapping the cached MSI iATU window here without flushing posted writes
risk an AXI bridge lockup?

In a multi-function endpoint, if Function 0 raises an MSI, the write is posted
to the AXI bridge and the iATU mapping is intentionally kept active to avoid
unmapping while in-flight.

If Function 1 immediately raises an MSI-X, this unconditionally unmaps the
cached MSI mapping. Since there is no dummy read or flush before
dw_pcie_ep_unmap_addr(), the previous MSI write might still be in the AXI
bridge's posted write buffer when its iATU translation window is disabled,
which could violate hardware constraints and lead to a lockup or dropped MSI
writes.

[Severity: High]
This is a pre-existing issue, but does the pre-existing MSI caching logic
cause misrouted interrupts for multi-function endpoints?

Looking at dw_pcie_ep_raise_msi_irq(), the caching logic erroneously skips
iATU reconfiguration solely based on the target address and size, failing
to check for a mismatch in the requested func_no:

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_ep_raise_msi_irq() {
    ...
	if (ep->msi_iatu_mapped && (ep->msi_msg_addr != msg_addr ||
				    ep->msi_map_size != map_size)) {
		dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->msi_mem_phys);
		ep->msi_iatu_mapped = false;
	}
    ...
}

If multiple functions share the same host MSI target address and size, the
second function to raise an MSI will reuse the iATU mapping cached by the
first function.

Since the DWC iATU mapping is programmed with the func_no of the original
function, subsequent MSIs from other functions will be sent with the wrong
Requester ID. This could lead to IOMMU faults or interrupt routing
failures on the host.

The struct dw_pcie_ep appears to be missing a field to store the cached
func_no to properly track this:

drivers/pci/controller/dwc/pcie-designware.h:struct dw_pcie_ep {
    ...
	/* MSI outbound iATU state */
	bool			msi_iatu_mapped;
	u64			msi_msg_addr;
	size_t			msi_map_size;
};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729213520.1187704-2-cassel@kernel.org?part=1