[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aTfavdjoc2ob2j2g@ryzen>
Date: Tue, 9 Dec 2025 09:15:57 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Koichiro Den <den@...inux.co.jp>
Cc: ntb@...ts.linux.dev, linux-pci@...r.kernel.org,
dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
Frank.Li@....com, mani@...nel.org, kwilczynski@...nel.org,
kishon@...nel.org, bhelgaas@...gle.com, corbet@....net,
vkoul@...nel.org, jdmason@...zu.us, dave.jiang@...el.com,
allenbh@...il.com, Basavaraj.Natikar@....com,
Shyam-sundar.S-k@....com, kurt.schwemmer@...rosemi.com,
logang@...tatee.com, jingoohan1@...il.com, lpieralisi@...nel.org,
robh@...nel.org, jbrunet@...libre.com, fancer.lancer@...il.com,
arnd@...db.de, pstanner@...hat.com, elfring@...rs.sourceforge.net
Subject: Re: [RFC PATCH v2 19/27] PCI: dwc: ep: Cache MSI outbound iATU
mapping
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
Powered by blists - more mailing lists