lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <brekq5jmgnotwpshcksxefpg2adm4vlsbuncazdg32sdpxqjwj@annnvyzshrys>
Date: Wed, 22 Oct 2025 16:02:24 +0530
From: "mani@...nel.org" <mani@...nel.org>
To: "Havalige, Thippeswamy" <thippeswamy.havalige@....com>
Cc: Stefan Roese <stefan.roese@...lbox.org>, 
	Bjorn Helgaas <helgaas@...nel.org>, "Bandi, Ravi Kumar" <ravib@...zon.com>, 
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>, "bhelgaas@...gle.com" <bhelgaas@...gle.com>, 
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>, "kwilczynski@...nel.org" <kwilczynski@...nel.org>, 
	"robh@...nel.org" <robh@...nel.org>, "Simek, Michal" <michal.simek@....com>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"stable@...r.kernel.org" <stable@...r.kernel.org>, Sean Anderson <sean.anderson@...ux.dev>
Subject: Re: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts

On Wed, Oct 22, 2025 at 10:08:44AM +0000, Havalige, Thippeswamy wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Stefan,
> 
> > -----Original Message-----
> > From: Stefan Roese <stefan.roese@...lbox.org>
> > Sent: Wednesday, October 22, 2025 3:29 PM
> > To: mani@...nel.org
> > Cc: Bjorn Helgaas <helgaas@...nel.org>; Bandi, Ravi Kumar
> > <ravib@...zon.com>; Havalige, Thippeswamy
> > <thippeswamy.havalige@....com>; lpieralisi@...nel.org;
> > bhelgaas@...gle.com; linux-pci@...r.kernel.org; kwilczynski@...nel.org;
> > robh@...nel.org; Simek, Michal <michal.simek@....com>; linux-arm-
> > kernel@...ts.infradead.org; linux-kernel@...r.kernel.org;
> > stable@...r.kernel.org; Sean Anderson <sean.anderson@...ux.dev>
> > Subject: Re: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts
> >
> > On 10/22/25 11:55, mani@...nel.org 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@...nel.org>
> > 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@...r.kernel.org
> > >>>>>>>> Signed-off-by: Ravi Kumar Bandi <ravib@...zon.com>
> > >>>>>>>
> > >>>>>>> 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@m
> > >>>>>>> ailbox.org
> > >>>>>>>
> > >>>>>>> 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.
> Hardware doesn't enable this bits by default, INTx didn't work since there is a miss match in the DT property which doesn't require pci_irqd_intx_xlate.
> 
> interrupt-map = <0 0 0 1 &pcie_intc_0 0>,
> <0 0 0 2 &pcie_intc_0 1>,
> <0 0 0 3 &pcie_intc_0 2>,
> <0 0 0 4 &pcie_intc_0 3>;
> 

Ok. This makes me believe that we do not need Stefan's patch [1] and need just
this patch from Ravi.

- Mani

[1] https://lore.kernel.org/linux-pci/20251021154322.973640-1-stefan.roese@mailbox.org/

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ