* [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling
@ 2025-10-21 15:43 Stefan Roese
2025-10-21 15:53 ` Bjorn Helgaas
2025-10-21 17:30 ` Sean Anderson
0 siblings, 2 replies; 6+ messages in thread
From: Stefan Roese @ 2025-10-21 15:43 UTC (permalink / raw)
To: linux-pci
Cc: Sean Anderson, Manivannan Sadhasivam, Ravi Kumar Bandi,
Thippeswamy Havalige, Michal Simek, Bjorn Helgaas
While testing with NVMe drives connected to the Versal QDMA PL PCIe RP
on our platform I noticed that with MSI disabled (e.g. via pci=nomsi)
the NVMe interrupts are not delivered to the host CPU resulting in
timeouts while probing.
Debugging has shown, that the hwirq numbers passed to this device driver
(1...4, 1=INTA etc) need to get adjusted to match the numbers in the
controller registers bits (0...3).
This patch now adds pci_irqd_intx_xlate to the INTx IRQ domain ops,
handling this IRQ number translation correctly.
Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
Cc: Sean Anderson <sean.anderson@linux.dev>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Ravi Kumar Bandi <ravib@amazon.com>
Cc: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Cc: Michal Simek <michal.simek@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
v2:
- Use pci_irqd_intx_xlate to handle this IRQ number translation as suggested
by Sean (thanks again)
drivers/pci/controller/pcie-xilinx-dma-pl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/pcie-xilinx-dma-pl.c b/drivers/pci/controller/pcie-xilinx-dma-pl.c
index 84888eda990b2..80095457ec531 100644
--- a/drivers/pci/controller/pcie-xilinx-dma-pl.c
+++ b/drivers/pci/controller/pcie-xilinx-dma-pl.c
@@ -370,6 +370,7 @@ static int xilinx_pl_dma_pcie_intx_map(struct irq_domain *domain,
/* INTx IRQ Domain operations */
static const struct irq_domain_ops intx_domain_ops = {
.map = xilinx_pl_dma_pcie_intx_map,
+ .xlate = pci_irqd_intx_xlate,
};
static irqreturn_t xilinx_pl_dma_pcie_msi_handler_high(int irq, void *args)
--
2.51.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling
2025-10-21 15:43 [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling Stefan Roese
@ 2025-10-21 15:53 ` Bjorn Helgaas
2025-10-21 16:02 ` Stefan Roese
2025-10-21 16:26 ` Sean Anderson
2025-10-21 17:30 ` Sean Anderson
1 sibling, 2 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2025-10-21 15:53 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-pci, Sean Anderson, Manivannan Sadhasivam, Ravi Kumar Bandi,
Thippeswamy Havalige, Michal Simek, Bjorn Helgaas
On Tue, Oct 21, 2025 at 05:43:22PM +0200, Stefan Roese wrote:
> While testing with NVMe drives connected to the Versal QDMA PL PCIe RP
> on our platform I noticed that with MSI disabled (e.g. via pci=nomsi)
> the NVMe interrupts are not delivered to the host CPU resulting in
> timeouts while probing.
>
> Debugging has shown, that the hwirq numbers passed to this device driver
> (1...4, 1=INTA etc) need to get adjusted to match the numbers in the
> controller registers bits (0...3).
>
> This patch now adds pci_irqd_intx_xlate to the INTx IRQ domain ops,
> handling this IRQ number translation correctly.
s/pcie-xilinx-dma-pl:/xilinx-xdma:/ # in subject
s/has shown, that/has shown that/
s/This patch now adds/Add/
s/pci_irqd_intx_xlate/pci_irqd_intx_xlate()/
We'll do this when applying, no need to repost for this.
I wonder how many other drivers have this issue.
pci_irqd_intx_xlate() is used only by:
dwc/pci-dra7xx.c
pcie-altera.c
pcie-xilinx-nwl.c
pcie-xilinx.c
pcie-xilinx-dma-pl.c # this patch
Is there something different about these drivers that means they need
it when all the others don't?
> Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
> Cc: Sean Anderson <sean.anderson@linux.dev>
> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Ravi Kumar Bandi <ravib@amazon.com>
> Cc: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Cc: Michal Simek <michal.simek@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
> v2:
> - Use pci_irqd_intx_xlate to handle this IRQ number translation as suggested
> by Sean (thanks again)
>
> drivers/pci/controller/pcie-xilinx-dma-pl.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-dma-pl.c b/drivers/pci/controller/pcie-xilinx-dma-pl.c
> index 84888eda990b2..80095457ec531 100644
> --- a/drivers/pci/controller/pcie-xilinx-dma-pl.c
> +++ b/drivers/pci/controller/pcie-xilinx-dma-pl.c
> @@ -370,6 +370,7 @@ static int xilinx_pl_dma_pcie_intx_map(struct irq_domain *domain,
> /* INTx IRQ Domain operations */
> static const struct irq_domain_ops intx_domain_ops = {
> .map = xilinx_pl_dma_pcie_intx_map,
> + .xlate = pci_irqd_intx_xlate,
> };
>
> static irqreturn_t xilinx_pl_dma_pcie_msi_handler_high(int irq, void *args)
> --
> 2.51.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling
2025-10-21 15:53 ` Bjorn Helgaas
@ 2025-10-21 16:02 ` Stefan Roese
2025-10-21 16:15 ` Bjorn Helgaas
2025-10-21 16:26 ` Sean Anderson
1 sibling, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2025-10-21 16:02 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Sean Anderson, Manivannan Sadhasivam, Ravi Kumar Bandi,
Thippeswamy Havalige, Michal Simek, Bjorn Helgaas
Hi Bjorn,
On 10/21/25 17:53, Bjorn Helgaas wrote:
> On Tue, Oct 21, 2025 at 05:43:22PM +0200, Stefan Roese wrote:
>> While testing with NVMe drives connected to the Versal QDMA PL PCIe RP
>> on our platform I noticed that with MSI disabled (e.g. via pci=nomsi)
>> the NVMe interrupts are not delivered to the host CPU resulting in
>> timeouts while probing.
>>
>> Debugging has shown, that the hwirq numbers passed to this device driver
>> (1...4, 1=INTA etc) need to get adjusted to match the numbers in the
>> controller registers bits (0...3).
>>
>> This patch now adds pci_irqd_intx_xlate to the INTx IRQ domain ops,
>> handling this IRQ number translation correctly.
>
> s/pcie-xilinx-dma-pl:/xilinx-xdma:/ # in subject
> s/has shown, that/has shown that/
> s/This patch now adds/Add/
> s/pci_irqd_intx_xlate/pci_irqd_intx_xlate()/
>
> We'll do this when applying, no need to repost for this.
Okay, thanks.
> I wonder how many other drivers have this issue.
> pci_irqd_intx_xlate() is used only by:
>
> dwc/pci-dra7xx.c
> pcie-altera.c
> pcie-xilinx-nwl.c
> pcie-xilinx.c
> pcie-xilinx-dma-pl.c # this patch
>
> Is there something different about these drivers that means they need
> it when all the others don't?
I can't really tell. I'm pretty sure that this driver also needs
pci_irqd_intx_xlate():
pcie-xilinx-cpm.c
I can't test it right now though. Perhaps in a few weeks though.
My best guess is, that legacy PCI IRQs are very rarely (not at all?)
used and therefor tested these days. On our ZynqMP / Versal platforms
this is currently sometimes used, as the RP does not support MSI-X
(only MSI which is very unfortunate - and legacy of course). So it
might be that some other drivers are missing this intx_xlate as well.
Should be easy to test by booting with pci=nomsi.
Thanks,
Stefan
>> Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
>> Cc: Sean Anderson <sean.anderson@linux.dev>
>> Cc: Manivannan Sadhasivam <mani@kernel.org>
>> Cc: Ravi Kumar Bandi <ravib@amazon.com>
>> Cc: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
>> Cc: Michal Simek <michal.simek@amd.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>> v2:
>> - Use pci_irqd_intx_xlate to handle this IRQ number translation as suggested
>> by Sean (thanks again)
>>
>> drivers/pci/controller/pcie-xilinx-dma-pl.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/controller/pcie-xilinx-dma-pl.c b/drivers/pci/controller/pcie-xilinx-dma-pl.c
>> index 84888eda990b2..80095457ec531 100644
>> --- a/drivers/pci/controller/pcie-xilinx-dma-pl.c
>> +++ b/drivers/pci/controller/pcie-xilinx-dma-pl.c
>> @@ -370,6 +370,7 @@ static int xilinx_pl_dma_pcie_intx_map(struct irq_domain *domain,
>> /* INTx IRQ Domain operations */
>> static const struct irq_domain_ops intx_domain_ops = {
>> .map = xilinx_pl_dma_pcie_intx_map,
>> + .xlate = pci_irqd_intx_xlate,
>> };
>>
>> static irqreturn_t xilinx_pl_dma_pcie_msi_handler_high(int irq, void *args)
>> --
>> 2.51.1
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling
2025-10-21 16:02 ` Stefan Roese
@ 2025-10-21 16:15 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2025-10-21 16:15 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-pci, Sean Anderson, Manivannan Sadhasivam, Ravi Kumar Bandi,
Thippeswamy Havalige, Michal Simek, Bjorn Helgaas
On Tue, Oct 21, 2025 at 06:02:40PM +0200, Stefan Roese wrote:
> On 10/21/25 17:53, Bjorn Helgaas wrote:
> > On Tue, Oct 21, 2025 at 05:43:22PM +0200, Stefan Roese wrote:
> > > While testing with NVMe drives connected to the Versal QDMA PL PCIe RP
> > > on our platform I noticed that with MSI disabled (e.g. via pci=nomsi)
> > > the NVMe interrupts are not delivered to the host CPU resulting in
> > > timeouts while probing.
> > >
> > > Debugging has shown, that the hwirq numbers passed to this device driver
> > > (1...4, 1=INTA etc) need to get adjusted to match the numbers in the
> > > controller registers bits (0...3).
> > >
> > > This patch now adds pci_irqd_intx_xlate to the INTx IRQ domain ops,
> > > handling this IRQ number translation correctly.
> ...
> > I wonder how many other drivers have this issue.
> > pci_irqd_intx_xlate() is used only by:
> >
> > dwc/pci-dra7xx.c
> > pcie-altera.c
> > pcie-xilinx-nwl.c
> > pcie-xilinx.c
> > pcie-xilinx-dma-pl.c # this patch
> >
> > Is there something different about these drivers that means they need
> > it when all the others don't?
>
> I can't really tell. I'm pretty sure that this driver also needs
> pci_irqd_intx_xlate():
>
> pcie-xilinx-cpm.c
>
> I can't test it right now though. Perhaps in a few weeks though.
>
> My best guess is, that legacy PCI IRQs are very rarely (not at all?)
> used and therefor tested these days. On our ZynqMP / Versal platforms this
> is currently sometimes used, as the RP does not support MSI-X
> (only MSI which is very unfortunate - and legacy of course). So it
> might be that some other drivers are missing this intx_xlate as well.
> Should be easy to test by booting with pci=nomsi.
I suspect many other drivers are also broken but not tested with INTx.
I guess we should ask about testing when merging new drivers that
claim to support INTx.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling
2025-10-21 15:53 ` Bjorn Helgaas
2025-10-21 16:02 ` Stefan Roese
@ 2025-10-21 16:26 ` Sean Anderson
1 sibling, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2025-10-21 16:26 UTC (permalink / raw)
To: Bjorn Helgaas, Stefan Roese
Cc: linux-pci, Manivannan Sadhasivam, Ravi Kumar Bandi,
Thippeswamy Havalige, Michal Simek, Bjorn Helgaas
On 10/21/25 11:53, Bjorn Helgaas wrote:
> On Tue, Oct 21, 2025 at 05:43:22PM +0200, Stefan Roese wrote:
>> While testing with NVMe drives connected to the Versal QDMA PL PCIe RP
>> on our platform I noticed that with MSI disabled (e.g. via pci=nomsi)
>> the NVMe interrupts are not delivered to the host CPU resulting in
>> timeouts while probing.
>>
>> Debugging has shown, that the hwirq numbers passed to this device driver
>> (1...4, 1=INTA etc) need to get adjusted to match the numbers in the
>> controller registers bits (0...3).
>>
>> This patch now adds pci_irqd_intx_xlate to the INTx IRQ domain ops,
>> handling this IRQ number translation correctly.
>
> s/pcie-xilinx-dma-pl:/xilinx-xdma:/ # in subject
> s/has shown, that/has shown that/
> s/This patch now adds/Add/
> s/pci_irqd_intx_xlate/pci_irqd_intx_xlate()/
>
> We'll do this when applying, no need to repost for this.
>
> I wonder how many other drivers have this issue.
> pci_irqd_intx_xlate() is used only by:
>
> dwc/pci-dra7xx.c
> pcie-altera.c
> pcie-xilinx-nwl.c
> pcie-xilinx.c
> pcie-xilinx-dma-pl.c # this patch
>
> Is there something different about these drivers that means they need
> it when all the others don't?
I think many PCI controllers just don't differentiate between legacy
interrupts. e.g. I looked at NXP's LS1046 which is based on DWC and it
does
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0000 0 0 1 &gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 2 &gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
which I believe maps all root port legacy interrupts to a single shared
GIC interrupt. Which sort of defeats the purpose of swizzling, but most
stuff uses MSI(-X) now so I guess they figured a status register wasn't
worth the verification.
And yes, I don't think legacy interrupts are tested very much these
days.
--Sean
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling
2025-10-21 15:43 [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling Stefan Roese
2025-10-21 15:53 ` Bjorn Helgaas
@ 2025-10-21 17:30 ` Sean Anderson
1 sibling, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2025-10-21 17:30 UTC (permalink / raw)
To: Stefan Roese, linux-pci
Cc: Manivannan Sadhasivam, Ravi Kumar Bandi, Thippeswamy Havalige,
Michal Simek, Bjorn Helgaas
On 10/21/25 11:43, Stefan Roese wrote:
> While testing with NVMe drives connected to the Versal QDMA PL PCIe RP
> on our platform I noticed that with MSI disabled (e.g. via pci=nomsi)
> the NVMe interrupts are not delivered to the host CPU resulting in
> timeouts while probing.
>
> Debugging has shown, that the hwirq numbers passed to this device driver
> (1...4, 1=INTA etc) need to get adjusted to match the numbers in the
> controller registers bits (0...3).
>
> This patch now adds pci_irqd_intx_xlate to the INTx IRQ domain ops,
> handling this IRQ number translation correctly.
>
> Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
> Cc: Sean Anderson <sean.anderson@linux.dev>
> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Ravi Kumar Bandi <ravib@amazon.com>
> Cc: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Cc: Michal Simek <michal.simek@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
> v2:
> - Use pci_irqd_intx_xlate to handle this IRQ number translation as suggested
> by Sean (thanks again)
>
> drivers/pci/controller/pcie-xilinx-dma-pl.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-dma-pl.c b/drivers/pci/controller/pcie-xilinx-dma-pl.c
> index 84888eda990b2..80095457ec531 100644
> --- a/drivers/pci/controller/pcie-xilinx-dma-pl.c
> +++ b/drivers/pci/controller/pcie-xilinx-dma-pl.c
> @@ -370,6 +370,7 @@ static int xilinx_pl_dma_pcie_intx_map(struct irq_domain *domain,
> /* INTx IRQ Domain operations */
> static const struct irq_domain_ops intx_domain_ops = {
> .map = xilinx_pl_dma_pcie_intx_map,
> + .xlate = pci_irqd_intx_xlate,
> };
>
> static irqreturn_t xilinx_pl_dma_pcie_msi_handler_high(int irq, void *args)
Reviewed-by: Sean Anderson <sean.anderson@linux.dev>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-21 17:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-21 15:43 [PATCH v2] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling Stefan Roese
2025-10-21 15:53 ` Bjorn Helgaas
2025-10-21 16:02 ` Stefan Roese
2025-10-21 16:15 ` Bjorn Helgaas
2025-10-21 16:26 ` Sean Anderson
2025-10-21 17:30 ` Sean Anderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox