* [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
* 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
* [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 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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.