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

[sashiko-bot@kernel.org]

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

New issues:
- [High] Using `readl()` on the host's MSI target address to flush posted writes generates a dangerous PCIe Memory Read Request, causing kernel panics.

Pre-existing issues:
- [High] The cached MSI iATU mapping incorrectly reuses a mapping for different functions, leading to wrong PCIe Requester IDs.
- [Medium] Missing synchronization/flush of posted writes before unmapping MSI iATU window in `dw_pcie_ep_stop()`.
- [Medium] The MSI-X table entries are read using native CPU endianness, causing incorrect values on Big Endian endpoints.
--

--- Patch [2]: [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU ---
commit 43c93e2ba0a475079cedb73c82f8ed9973d5ab90
Author: Niklas Cassel <cassel@kernel.org>
PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU

This patch fixes a conflict between persistent MSI iATU mappings and dynamic
MSI-X iATU mappings by ensuring the cached MSI mapping is unmapped before
raising an MSI-X interrupt.

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 3128e7ae8c5fd..7cd169d458909 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -1127,6 +1127,17 @@ 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) {
> +		/* flush posted write before unmap */
> +		readl(ep->msi_mem + ep->msi_iatu_mapped_offset);

[Severity: High]
Does using readl() on the host's MSI target address generate a dangerous
non-posted PCIe Memory Read Request?

Most host interrupt controllers (like GIC ITS or APIC) do not support reads
to their doorbell addresses and will reply with an Unsupported Request (UR).

The DWC controller translates the UR completion into an AXI Slave Error,
which can trigger a synchronous external abort and kernel panic on the
endpoint CPU.

This concern applies to flushing posted writes in both
dw_pcie_ep_raise_msix_irq() and dw_pcie_ep_raise_msi_irq().

> +		dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->msi_mem_phys);
> +		ep->msi_iatu_mapped = false;
> +	}
> +
>  	msg_addr = dw_pcie_ep_align_addr(epc, msg_addr, &map_size, &offset);

[Severity: High]
This is a pre-existing issue, but I noticed the cached MSI iATU mapping might
incorrectly reuse a mapping for different functions.

Looking at dw_pcie_ep_raise_msi_irq():

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)) {
        ...
}

Since dw_pcie_ep_map_addr() programs the outbound ATU with a specific func_no,
if this reuses the existing mapping without checking func_no, will it send
the PCIe TLP with the first function's Requester ID?

This could cause MSIs from multi-function endpoints to be dropped or misrouted
by the host's interrupt controller or IOMMU.

[Severity: Medium]
This isn't a bug introduced by this patch, but unmapping an iATU window while
posted writes are in flight on the AXI bridge could cause undefined behavior
and interconnect faults.

In dw_pcie_ep_stop():

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_ep_stop() {
        ...
        if (ep->msi_iatu_mapped) {
                dw_pcie_ep_unmap_addr(epc, 0, 0, ep->msi_mem_phys);
                ep->msi_iatu_mapped = false;
        }
        ...
}

Is it safe to tear down the iATU here without any synchronization or delay
to ensure in-flight AXI operations have completed?

[Severity: Medium]
This is a pre-existing issue, but reading MSI-X table entries natively can
yield byte-swapped values on Big Endian endpoint CPUs.

Looking at dw_pcie_ep_raise_msix_irq():

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_ep_raise_msix_irq() {
        ...
        msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr;
        msg_data = msix_tbl[(interrupt_num - 1)].msg_data;
        vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl;
        ...
}

Since this memory is a PCIe BAR programmed by the host over the PCIe link,
the data should be in little-endian format. Should these accesses use
endianness conversions like le64_to_cpu() and le32_to_cpu() to ensure
correct values?

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