Re: [RFC PATCH v2 19/27] PCI: dwc: ep: Cache MSI outbound iATU mapping

[Niklas Cassel <cassel@kernel.org>]

On Mon, Dec 08, 2025 at 08:57:19AM +0100, Niklas Cassel wrote:
> On Sun, Nov 30, 2025 at 01:03:57AM +0900, Koichiro Den wrote:
>
> I don't like that this patch modifies dw_pcie_ep_raise_msi_irq() but does
> not modify dw_pcie_ep_raise_msix_irq()
>
> both functions call dw_pcie_ep_map_addr() before doing the writel(),
> so I think they should be treated the same.

Btw, when using nvmet-pci-epf:
drivers/nvme/target/pci-epf.c

With queue depth == 32, I get:
[  106.585811] arm-smmu-v3 fc900000.iommu: event 0x10 received:
[  106.586349] arm-smmu-v3 fc900000.iommu:      0x0000010000000010
[  106.586846] arm-smmu-v3 fc900000.iommu:      0x0000020000000000
[  106.587341] arm-smmu-v3 fc900000.iommu:      0x000000090000f040
[  106.587841] arm-smmu-v3 fc900000.iommu:      0x0000000000000000
[  106.588335] arm-smmu-v3 fc900000.iommu: event: F_TRANSLATION client: 0000:01:00.0 sid: 0x100 ssid: 0x0 iova: 0x90000f040 ipa: 0x0
[  106.589383] arm-smmu-v3 fc900000.iommu: unpriv data write s1 "Input address caused fault" stag: 0x0

(If I only run with queue depth == 1, I cannot trigger this IOMMU error.)


So, I really think that this patch is important, as it solves a real
problem also for the nvmet-pci-epf driver.


With this patch applied, I cannot trigger the IOMMU error,
so I really think that you should send this a a stand alone patch.


I still think that we need to modify dw_pcie_ep_raise_msix_irq() in a similar
way.


Perhaps instead of:


	if (!ep->msi_iatu_mapped) {
		ret = dw_pcie_ep_map_addr(epc, func_no, 0,
					  ep->msi_mem_phys, msg_addr,
					  map_size);
		if (ret)
			return ret;

		ep->msi_iatu_mapped = true;
		ep->msi_msg_addr = msg_addr;
		ep->msi_map_size = map_size;
	} else if (WARN_ON_ONCE(ep->msi_msg_addr != msg_addr ||
				ep->msi_map_size != map_size)) {
		/*
		 * The host changed the MSI target address or the required
		 * mapping size. Reprogramming the iATU at runtime is unsafe
		 * on this controller, so bail out instead of trying to update
		 * the existing region.
		 */
		return -EINVAL;
	}

	writel(msg_data | (interrupt_num - 1), ep->msi_mem + offset);



We could modify both dw_pcie_ep_raise_msix_irq and dw_pcie_ep_raise_msi_irq
to do something like:



	if (!ep->msi_iatu_mapped) {
		ret = dw_pcie_ep_map_addr(epc, func_no, 0,
					  ep->msi_mem_phys, msg_addr,
					  map_size);
		if (ret)
			return ret;

		ep->msi_iatu_mapped = true;
		ep->msi_msg_addr = msg_addr;
		ep->msi_map_size = map_size;
	} else if (WARN_ON_ONCE(ep->msi_msg_addr != msg_addr ||
				ep->msi_map_size != map_size)) {
		/*
		 * Host driver might have changed from MSI to MSI-X
		 * or the other way around.
		 */
		 dw_pcie_ep_unmap_addr(epc, 0, 0, ep->msi_mem_phys);
		 ret = dw_pcie_ep_map_addr(epc, func_no, 0,
		 			   ep->msi_mem_phys, msg_addr,
					   map_size);
		if (ret)
			return ret;
	}

	writel(msg_data | (interrupt_num - 1), ep->msi_mem + offset);



I think that should work without needing to introuce any
.start_engine() / .stop_engine() APIs.



Kind regards,
Niklas