Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU
@ 2026-07-30 13:31 Niklas Cassel
  2026-07-30 13:31 ` [PATCH v2 1/2] PCI: dwc: ep: Flush the cached MSI address before unmap Niklas Cassel
  2026-07-30 13:31 ` [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU Niklas Cassel
  0 siblings, 2 replies; 6+ messages in thread
From: Niklas Cassel @ 2026-07-30 13:31 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Niklas Cassel, Koichiro Den
  Cc: linux-pci

Hello all,

Here comes some fixes for MSI/MSI-X unmap problems in the DWC driver
reported by Sashiko.

Changes since v1:
-Added patch 1/2.
-Added a flush in patch 2/2.

Kind regards,
Niklas

Niklas Cassel (2):
  PCI: dwc: ep: Flush the cached MSI address before unmap
  PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU

 drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h    |  1 +
 2 files changed, 15 insertions(+)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] PCI: dwc: ep: Flush the cached MSI address before unmap
  2026-07-30 13:31 [PATCH v2 0/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU Niklas Cassel
@ 2026-07-30 13:31 ` Niklas Cassel
  2026-07-30 13:48   ` sashiko-bot
  2026-07-30 13:31 ` [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU Niklas Cassel
  1 sibling, 1 reply; 6+ messages in thread
From: Niklas Cassel @ 2026-07-30 13:31 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Niklas Cassel
  Cc: Sashiko, linux-pci

While we do a flush before unmap in dw_pcie_ep_raise_msix_irq(),
see commit c22533c66cca ("PCI: dwc: ep: Flush MSI-X write before unmapping
its ATU entry"), we do not do a flush before unmap in
dw_pcie_ep_raise_msi_irq(), if the cached MSI target address changed.

Fix this by doing a flush before unmap.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-pci/20260729214859.B9E2B1F00A3A@smtp.kernel.org/
Fixes: 468711a40d5d ("PCI: dwc: ep: Refresh MSI Message Address cache on change")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 3 +++
 drivers/pci/controller/dwc/pcie-designware.h    | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 7d2794945704..3128e7ae8c5f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -1032,6 +1032,8 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 		 * there is no unified way to check if we have operations in
 		 * flight, thus we don't know if we should WARN() or not.
 		 */
+		/* flush posted write before unmap */
+		readl(ep->msi_mem + ep->msi_iatu_mapped_offset);
 		dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->msi_mem_phys);
 		ep->msi_iatu_mapped = false;
 	}
@@ -1044,6 +1046,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 			return ret;
 
 		ep->msi_iatu_mapped = true;
+		ep->msi_iatu_mapped_offset = offset;
 		ep->msi_msg_addr = msg_addr;
 		ep->msi_map_size = map_size;
 	}
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index de4b245b1758..1d2139474157 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -518,6 +518,7 @@ struct dw_pcie_ep {
 
 	/* MSI outbound iATU state */
 	bool			msi_iatu_mapped;
+	size_t			msi_iatu_mapped_offset;
 	u64			msi_msg_addr;
 	size_t			msi_map_size;
 };
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU
  2026-07-30 13:31 [PATCH v2 0/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU Niklas Cassel
  2026-07-30 13:31 ` [PATCH v2 1/2] PCI: dwc: ep: Flush the cached MSI address before unmap Niklas Cassel
@ 2026-07-30 13:31 ` Niklas Cassel
  2026-07-30 13:51   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Niklas Cassel @ 2026-07-30 13:31 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Koichiro Den, Niklas Cassel
  Cc: Sashiko, linux-pci

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.

When dw_pcie_ep_raise_msix_irq() later calls dw_pcie_ep_unmap_addr(), the
lookup function, dw_pcie_find_index(), returns the first iATU index which
has the address mapped.

This means that dw_pcie_ep_raise_msix_irq() can unmap the address mapped
by dw_pcie_ep_raise_msi_irq(), without clearing ep->msi_iatu_mapped.

If there is a cached MSI iATU mapping, let dw_pcie_ep_raise_msix_irq()
unmap it first, so that we won't have two different iATUs mapping the
same address.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-pci/20260729051542.DC2741F000E9@smtp.kernel.org/
Fixes: 8719c64e76bf ("PCI: dwc: ep: Cache MSI outbound iATU mapping")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 3128e7ae8c5f..7cd169d45890 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);
+		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);
 	ret = dw_pcie_ep_map_addr(epc, func_no, 0, ep->msi_mem_phys, msg_addr,
 				  map_size);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] PCI: dwc: ep: Flush the cached MSI address before unmap
  2026-07-30 13:31 ` [PATCH v2 1/2] PCI: dwc: ep: Flush the cached MSI address before unmap Niklas Cassel
@ 2026-07-30 13:48   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-30 13:48 UTC (permalink / raw)
  To: Niklas Cassel; +Cc: linux-pci

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

Pre-existing issues:
- [High] The cached MSI iATU mapping logic in dw_pcie_ep_raise_msi_irq() omits checking func_no, causing MSIs from subsequent functions to be sent with the wrong Requester ID.
- [High] Missing flush of posted MSI writes before unmapping the ATU window in dw_pcie_ep_stop().
- [High] Hardware address conflict and ATU window leak due to shared ep->msi_mem_phys between MSI and MSI-X.
--

--- Patch [1]: [PATCH v2 1/2] PCI: dwc: ep: Flush the cached MSI address before unmap ---
commit 61e06ab5c5d18de814a8a431d661886fcd30c6d7
Author: Niklas Cassel <cassel@kernel.org>

PCI: dwc: ep: Flush the cached MSI address before unmap

While we do a flush before unmap in dw_pcie_ep_raise_msix_irq(), we do
not do a flush before unmap in dw_pcie_ep_raise_msi_irq(), if the cached
MSI target address changed. Fix this by doing a flush before unmap.

Link: https://lore.kernel.org/linux-pci/20260729214859.B9E2B1F00A3A@smtp.kernel.org/

> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -1032,6 +1032,8 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  		 * there is no unified way to check if we have operations in
>  		 * flight, thus we don't know if we should WARN() or not.
>  		 */
> +		/* flush posted write before unmap */
> +		readl(ep->msi_mem + ep->msi_iatu_mapped_offset);

[Severity: High]
This is a pre-existing issue, but since this patch adds a flush here to
drain posted writes, is a similar flush needed in dw_pcie_ep_stop()?

During device stop, dw_pcie_ep_stop() unmaps the MSI ATU window:

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;
	}
    ...
}

Would stopping the controller and unmapping the window without a dummy
read to drain the AXI bridge cause in-flight MSI writes to be dropped?

>  		dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->msi_mem_phys);

[Severity: High]
This isn't a bug introduced by this patch, but does the MSI iATU mapping
logic need to track the function number?

Currently, struct dw_pcie_ep lacks a field to track which function number
is mapped. The caching logic in dw_pcie_ep_raise_msi_irq() checks the
target address and size, but omits the function number.

If Function 0 raises an MSI, the ATU is programmed with Function 0's
Requester ID. If Function 1 then raises an MSI to the identical target
address, the cache check succeeds and bypasses ATU reprogramming.

Might this cause Function 1's MSIs to be transmitted via the ATU window
still configured with Function 0's Requester ID, leading to dropped
interrupts by the host IOMMU?


[Severity: High]
This is a pre-existing issue, but does sharing ep->msi_mem_phys between
MSI and MSI-X cause hardware address conflicts?

If dw_pcie_ep_raise_msi_irq() maps an ATU window for ep->msi_mem_phys,
and subsequently dw_pcie_ep_raise_msix_irq() unconditionally allocates
and maps a new ATU window for the exact same physical address, overlapping
translation windows are created.

When dw_pcie_ep_unmap_addr() is called, it searches for the physical address
and unmaps the first matching window:

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_find_index() {
    ...
	for_each_set_bit(index, ep->ob_window_map, pci->num_ob_windows) {
		if (ep->outbound_addr[index] != addr)
			continue;
		*atu_index = index;
		return 0;
	}
    ...
}

Would this incorrectly return the MSI window's index, unmapping the MSI
window instead of the MSI-X window, and permanently leaking the newly
allocated MSI-X ATU window?

>  		ep->msi_iatu_mapped = false;
>  	}

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU
  2026-07-30 13:31 ` [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU Niklas Cassel
@ 2026-07-30 13:51   ` sashiko-bot
  2026-07-30 14:16     ` Niklas Cassel
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-07-30 13:51 UTC (permalink / raw)
  To: Niklas Cassel; +Cc: linux-pci

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU
  2026-07-30 13:51   ` sashiko-bot
@ 2026-07-30 14:16     ` Niklas Cassel
  0 siblings, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2026-07-30 14:16 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-pci

On Thu, Jul 30, 2026 at 01:51:16PM +0000, sashiko-bot@kernel.org wrote:
> 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.

(snip)

> [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);
> 

I don't see how this comment is valid.

I tested this patch with two Rock 5Bs, one in EP mode and one in RC mode.

The PCIe controller on the host:
https://github.com/torvalds/linux/blob/v7.2-rc5/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi#L370
is indeed using GIC ITS, and I saw no issues at all.

Additionally, we have already been doing a readl() on the MSI-X target
address, for every single MSI-X, since commit c22533c66cca ("PCI: dwc: ep:
Flush MSI-X write before unmapping its ATU entry"), first included in
v7.0-rc2, and not a single soul has complained since then.

So personally I am not too concerned.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-30 14:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 13:31 [PATCH v2 0/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU Niklas Cassel
2026-07-30 13:31 ` [PATCH v2 1/2] PCI: dwc: ep: Flush the cached MSI address before unmap Niklas Cassel
2026-07-30 13:48   ` sashiko-bot
2026-07-30 13:31 ` [PATCH v2 2/2] PCI: dwc: ep: Fix unmap potentially unmapping the wrong iATU Niklas Cassel
2026-07-30 13:51   ` sashiko-bot
2026-07-30 14:16     ` Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox