Re: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts

[Stefan Roese]

On 10/22/25 11:55, mani@xxxxxxxxxx wrote:
> On Wed, Oct 22, 2025 at 08:59:19AM +0200, Stefan Roese wrote:
> > Hi Bjorn,
> > Hi Ravi,
> >
> > On 10/21/25 23:28, Bjorn Helgaas wrote:
> > > On Tue, Oct 21, 2025 at 08:55:41PM +0000, Bandi, Ravi Kumar wrote:
> > > > > On Tue, Oct 21, 2025 at 05:46:17PM +0000, Bandi, Ravi Kumar wrote:
> > > > > > > On Oct 21, 2025, at 10:23 AM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> > > > > > > On Sat, Sep 20, 2025 at 10:52:32PM +0000, Ravi Kumar Bandi wrote:
> > > > > > > > The pcie-xilinx-dma-pl driver does not enable INTx interrupts
> > > > > > > > after initializing the port, preventing INTx interrupts from
> > > > > > > > PCIe endpoints from flowing through the Xilinx XDMA root port
> > > > > > > > bridge. This issue affects kernel 6.6.0 and later versions.
> > > > > > > >
> > > > > > > > This patch allows INTx interrupts generated by PCIe endpoints
> > > > > > > > to flow through the root port. Tested the fix on a board with
> > > > > > > > two endpoints generating INTx interrupts. Interrupts are
> > > > > > > > properly detected and serviced. The /proc/interrupts output
> > > > > > > > shows:
> > > > > > > >
> > > > > > > > [...]
> > > > > > > > 32:        320          0  pl_dma:RC-Event  16 Level     400000000.axi-pcie, azdrv
> > > > > > > > 52:        470          0  pl_dma:RC-Event  16 Level     500000000.axi-pcie, azdrv
> > > > > > > > [...]
> >
> > First a comment on this IRQ logging:
> >
> > These lines do NOT refer to the INTx IRQ(s) but the controller internal
> > "events" (errors etc). Please see this log for INTx on my Versal
> > platform with pci_irqd_intx_xlate added:
> >
> >   24:          0          0  pl_dma:RC-Event   0 Level     LINK_DOWN
> >   25:          0          0  pl_dma:RC-Event   3 Level     HOT_RESET
> >   26:          0          0  pl_dma:RC-Event   8 Level     CFG_TIMEOUT
> >   27:          0          0  pl_dma:RC-Event   9 Level     CORRECTABLE
> >   28:          0          0  pl_dma:RC-Event  10 Level     NONFATAL
> >   29:          0          0  pl_dma:RC-Event  11 Level     FATAL
> >   30:          0          0  pl_dma:RC-Event  20 Level     SLV_UNSUPP
> >   31:          0          0  pl_dma:RC-Event  21 Level     SLV_UNEXP
> >   32:          0          0  pl_dma:RC-Event  22 Level     SLV_COMPL
> >   33:          0          0  pl_dma:RC-Event  23 Level     SLV_ERRP
> >   34:          0          0  pl_dma:RC-Event  24 Level     SLV_CMPABT
> >   35:          0          0  pl_dma:RC-Event  25 Level     SLV_ILLBUR
> >   36:          0          0  pl_dma:RC-Event  26 Level     MST_DECERR
> >   37:          0          0  pl_dma:RC-Event  27 Level     MST_SLVERR
> >   38:         94          0  pl_dma:RC-Event  16 Level     84000000.axi-pcie
> >   39:         94          0  pl_dma:INTx   0 Level     nvme0q0, nvme0q1
> >
> > The last line shows the INTx IRQs here ('pl_dma:INTx' vs 'pl_dma:RC-
> > Event').
> >
> > More below...
> >
> > > > > > > > Changes since v1::
> > > > > > > > - Fixed commit message per reviewer's comments
> > > > > > > >
> > > > > > > > Fixes: 8d786149d78c ("PCI: xilinx-xdma: Add Xilinx XDMA Root Port driver")
> > > > > > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > > > > > Signed-off-by: Ravi Kumar Bandi <ravib@xxxxxxxxxx>
> > > > > > >
> > > > > > > Hi Ravi, obviously you tested this, but I don't know how to reconcile
> > > > > > > this with Stefan's INTx fix at
> > > > > > > https://lore.kernel.org/r/20251021154322.973640-1-stefan.roese@xxxxxxxxxxx
> > > > > > >
> > > > > > > Does Stefan's fix need to be squashed into this patch?
> > > > > >
> > > > > > Sure, we can squash Stefan’s fix into this.
> > > > >
> > > > > I know we *can* squash them.
> > > > >
> > > > > I want to know why things worked for you and Stefan when they
> > > > > *weren't* squashed:
> > > > >
> > > > >    - Why did INTx work for you even without Stefan's patch.  Did you
> > > > >      get INTx interrupts but not the right ones, e.g., did the device
> > > > >      signal INTA but it was received as INTB?
> > > >
> > > > I saw that interrupts were being generated by the endpoint device,
> > > > but I didn’t specifically check if they were correctly translated in
> > > > the controller. I noticed that the new driver wasn't explicitly
> > > > enabling the interrupts, so my first approach was to enable them,
> > > > which helped the interrupts flow through.
> > >
> > > OK, I'll assume the interrupts happened but the driver might not have
> > > been able to handle them correctly, e.g., it was prepared for INTA but
> > > got INTB or similar.
> > >
> > > > >    - Why did Stefan's patch work for him even without your patch.  How
> > > > >      could Stefan's INTx work without the CSR writes to enable
> > > > >      interrupts?
> > > >
> > > > I'm not entirely sure if there are any other dependencies in the
> > > > FPGA bitstream. I'll investigate further and get back to you.
> > >
> > > Stefan clarified in a private message that he had applied your patch
> > > first, so this mystery is solved.
> >
> > Yes. I applied Ravi's patch first and still got no INTx delivered
> > to the nvme driver. That's what me triggered to dig deeper here and
> > resulted in this v2 patch with pci_irqd_intx_xlate added.
> >
> > BTW:
> > I re-tested just now w/o Ravi's patch and the INTx worked. Still I think
> > Ravi's patch is valid and should be applied...
>
> How come INTx is working without the patch from Ravi which enabled INTx routing
> in the controller? Was it enabled by default in the hardware?

Yes, this is my best guess right now. I could double-check here, but
IMHO it makes sense to enable it "manually" as done with Ravi's patch
to not rely on this default setup at all.

Thanks,
Stefan