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

[Koichiro Den]

On Tue, Dec 09, 2025 at 09:15:57AM +0100, Niklas Cassel wrote:
> 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.

Sorry about my late response, and thank you for handling this:
https://lore.kernel.org/all/20251210071358.2267494-2-cassel@xxxxxxxxxx/

Koichiro

> 
> 
> 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