linux-kernel - Re: [PATCH v5 2/2] PCI: amd-mdb: Add support for PCIe RP PERST# signal handling

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: <20250718163150.GA2700763@bhelgaas>
Date: Fri, 18 Jul 2025 11:31:50 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: "Musham, Sai Krishna" <sai.krishna.musham@....com>
Cc: "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	"kw@...ux.com" <kw@...ux.com>, "mani@...nel.org" <mani@...nel.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: [PATCH v5 2/2] PCI: amd-mdb: Add support for PCIe RP PERST#
 signal handling

On Fri, Jul 18, 2025 at 04:30:32AM +0000, Musham, Sai Krishna wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Bjorn,
> 
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@...nel.org>
> > Sent: Saturday, July 12, 2025 4:49 AM
> > To: Musham, Sai Krishna <sai.krishna.musham@....com>
> > Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com; mani@...nel.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: [PATCH v5 2/2] PCI: amd-mdb: Add support for PCIe RP PERST#
> > signal handling
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On Fri, Jul 11, 2025 at 10:53:57AM +0530, Sai Krishna Musham wrote:
> > > Add support for handling the AMD Versal Gen 2 MDB PCIe Root Port PERST#
> > > signal via a GPIO by parsing the new PCIe bridge node to acquire the
> > > reset GPIO. If the bridge node is not found, fall back to acquiring it
> > > from the PCIe node.
> > >
> > > As part of this, update the interrupt controller node parsing to use
> > > of_get_child_by_name() instead of of_get_next_child(), since the PCIe
> > > node now has multiple children. This ensures the correct node is
> > > selected during initialization.

> > > +      * If amd_mdb_parse_pcie_port returns -ENODEV, it indicates that the
> > > +      * PCIe Bridge node was not found in the device tree. This is not
> > > +      * considered a fatal error and will trigger a fallback where the
> > > +      * reset GPIO is acquired directly from the PCIe node.
> > > +      */
> > > +     if (ret && ret != -ENODEV) {
> > > +             return ret;
> > > +     } else if (ret == -ENODEV) {
> >
> > The "ret" checking seems unnecessarily complicated.
> >
> > > +             dev_info(dev, "Falling back to acquire reset GPIO from PCIe node\n");
> >
> > I don't think this is worthy of a message.  If there are DTs in the
> > field that were valid once, they continue to be valid forever, and
> > there's no point in complaining about them.
> >
> > https://lore.kernel.org/all/20250702-perst-v5-2-920b3d1f6ee1@qti.qualcomm.com/
> > has a good example of how to this fallback nicely.
> >
> > Otherwise looks good to me.
> 
> Thanks for the feedback. I've removed the fallback message and simplified the "ret"
> checking. Could you please confirm if this looks good for v6?
> 
>         if (ret == -ENODEV) {
> 
>                 /* Request the GPIO for PCIe reset signal and assert */
>                 pcie->perst_gpio = devm_gpiod_get_optional(dev, "reset",
>                                                            GPIOD_OUT_HIGH);
>                 if (IS_ERR(pcie->perst_gpio))
>                         return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
>                                              "Failed to request reset GPIO\n");
>         } else if (ret) {
>                 return ret;
>         }

Looks good to me.  It's important to note that this -ENODEV fallback
uses the PERST# GPIO described in the host bridge, not in a Root Port,
but I think your comment above includes this.

Bjorn