Linux PCI subsystem development
 help / color / mirror / Atom feed
* [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