linux-kernel - Re: [RESEND PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP PERST# signal

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250617144654.GA1135267@bhelgaas>
Date: Tue, 17 Jun 2025 09:46:54 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: "Musham, Sai Krishna" <sai.krishna.musham@....com>
Cc: Manivannan Sadhasivam <mani@...nel.org>,
	"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	"kw@...ux.com" <kw@...ux.com>,
	"manivannan.sadhasivam@...aro.org" <manivannan.sadhasivam@...aro.org>,
	"robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>,
	"cassel@...nel.org" <cassel@...nel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Simek, Michal" <michal.simek@....com>,
	"Gogada, Bharat Kumar" <bharat.kumar.gogada@....com>,
	"Havalige, Thippeswamy" <thippeswamy.havalige@....com>
Subject: Re: [RESEND PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP
 PERST# signal

On Tue, Jun 17, 2025 at 04:14:37AM +0000, Musham, Sai Krishna wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Manivannan,
> 
> > -----Original Message-----
> > From: Manivannan Sadhasivam <mani@...nel.org>
> > Sent: Thursday, June 12, 2025 10:49 PM
> > To: Musham, Sai Krishna <sai.krishna.musham@....com>
> > Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com;
> > manivannan.sadhasivam@...aro.org; robh@...nel.org; krzk+dt@...nel.org;
> > conor+dt@...nel.org; cassel@...nel.org; linux-pci@...r.kernel.org;
> > devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Simek, Michal
> > <michal.simek@....com>; Gogada, Bharat Kumar
> > <bharat.kumar.gogada@....com>; Havalige, Thippeswamy
> > <thippeswamy.havalige@....com>
> > Subject: Re: [RESEND PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP
> > PERST# signal
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On Mon, Apr 14, 2025 at 08:53:04AM +0530, Sai Krishna Musham wrote:
> > > Add support for handling the PCIe Root Port (RP) PERST# signal using
> > > the GPIO framework, along with the PCIe IP reset. This reset is
> > > managed by the driver and occurs after the Initial Power Up sequence
> > > (PCIe CEM r6.0, 2.2.1) is handled in hardware before the driver's probe
> > > function is called.

> > > +     if (do_reset) {
> > > +             /* Assert the PCIe IP reset */
> > > +             writel_relaxed(0x1, port->crx_base + variant->cpm_pcie_rst);
> > > +
> > > +             /*
> > > +              * "PERST# active time", as per Table 2-10: Power Sequencing
> > > +              * and Reset Signal Timings of the PCIe Electromechanical
> > > +              * Specification, Revision 6.0, symbol "T_PERST".
> > > +              */
> > > +             udelay(100);
> >
> > Are you sure that you need T_PERST here and not T_PVPERL? T_PERST
> > is only valid while resuming from D3Cold i.e., after power up,
> > while T_PVPERL is valid during the power up, which is usually the
> > case when a controller driver probes. Is your driver relying on
> > power being enabled by the bootloader and the driver just toggling
> > PERST# to perform conventional reset of the endpoint?
> 
> Thanks for pointing that out. Yes, the power-up sequence is handled
> by the hardware, and the driver relies on power being enabled by it.
> We're only toggling the PERST# signal in the driver to perform a
> conventional reset of the endpoint. So, I'm confident that T_PERST
> is the appropriate timing reference here, not T_PVPERL.
> 
> Additionally, this delay was recommended by our hardware team, who
> confirmed that the power-up sequence is managed in hardware logic,
> and that T_PERST is the appropriate timing to apply in this context.
> 
> I also checked pci.h but couldn't find a predefined macro for
> T_PERST, so I used 100.  Please let me know if there's a preferred
> macro I should be using instead.

If we need a new macro, please add it.  Include a citation to the
relevant section of the spec ("PCIe CEM r6.0, sec 2.11.2"; table
numbers don't appear in the table of contents so they're hard to
find), and include the units ("_US", I guess) in the macro name.

Given a comment at the macro definition, you don't need to repeat it
at all the uses.

Bjorn