Re: [PATCH] PCI: dwc: ep: Fix regression in dw_pcie_ep_raise_msi_irq()

[Niklas Cassel <cassel@kernel.org>]

On Tue, Feb 10, 2026 at 01:32:05PM -0600, Bjorn Helgaas wrote:
> IIUC, the sequence is like this:
> 
>   - nvmet-pci-epf calls pci_epc_raise_irq() leading to
>     dw_pcie_ep_raise_msi_irq()
> 
>   - dw_pcie_ep_raise_msi_irq() reads PCI_MSI_ADDRESS_*, maps msg_addr,
>     and saves it in ep->msi_msg_addr
> 
>   - host updates PCI_MSI_ADDRESS_*
> 
>   - nvmet-pci-epf calls pci_epc_raise_irq() again
> 
>   - dw_pcie_ep_raise_msi_irq() reads PCI_MSI_ADDRESS_*, notices that
>     msg_addr has changed, and WARNs and returns -EINVAL
> 
> and this patch makes it so the second time through
> dw_pcie_ep_raise_msi_irq(), we notice the msg_addr change, remove the
> old mapping, and map it again with the new address.

Correct.


> 
> Isn't there still a race between host updates of PCI_MSI_ADDRESS_* and
> endpoint reads of those registers?  We can't prevent the host from
> updating PCI_MSI_ADDRESS_* between dw_pcie_ep_map_addr() and the
> writel(), so maybe it's impossible to prevent the theoretical race
> there, and all we can really do is mitigate what we expect to be a
> single change at boot time of the host?

Normally, the MSI target address is not changed during runtime.

The spec allows changing the MSI-X address/data pair when the corresponding
vector is *masked*, and classifies the behavior as undefined if address/data
pair gets changed while the vector is *unmasked*.

AFAICT, it does not mention anything for MSI, so I do not think it is allowed
to be changed during runtime.

The only reason why it is changed here is because UEFI/BIOS will have one
MSI target address, and then once Linux boots, it will use another MSI target
address. (So it only changes once.)


> 
> Even for that single change, it looks like the host could update
> PCI_MSI_ADDRESS_* simultaneously with dw_pcie_ep_raise_msi_irq(), 
> leading to mapping a half-updated msg_addr.  This part we *could*
> prevent by re-reading PCI_MSI_ADDRESS_* to detect a partial update.

The problem that commit 8719c64e76bf ("PCI: dwc: ep: Cache MSI outbound iATU
mapping") fixes is that the DWC controller does not handle when the outbound
iATU is re-programmed when there are ongoing outbound transactions.

What commit 8719c64e76bf ("PCI: dwc: ep: Cache MSI outbound iATU mapping")
did was to map it once on startup, that way we don't re-program the outbound
iATU on every pci_epc_raise_irq() call. Before this commit, every
pci_epc_raise_irq() call could potentially cause ongoing outbound transactions
to be sent untranslated. Often the transactions that were sent untranslated
did not appear to be the MSI writel() itself, but other transactions performed
by the eDMA.

So even with this patch, since we are still not mapping + unmapping the MSI
target address on every pci_epc_raise_irq(), it should just be a single time
where we might trigger this problematic behavior (when MSI target address
changes, UEFI -> Linux). (Which is still much better than possibly triggering
this problematic behavior on every pci_epc_raise_irq().)


> 
> Probably unrelated question: the pci_epc_raise_irq() path doesn't seem
> to check PCI_MSI_FLAGS_ENABLE or PCI_MSIX_FLAGS_ENABLE.  Is that
> intended?

I guess we could improve pci_epc_raise_irq() to check those bits, in a
separate patch.

Mani, thoughts?


Kind regards,
Niklas