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:
 <DM4PR12MB61581827F4D90B4A23EA7401CDA42@DM4PR12MB6158.namprd12.prod.outlook.com>
Date: Mon, 24 Mar 2025 09:29:39 +0000
From: "Musham, Sai Krishna" <sai.krishna.musham@....com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
CC: "bhelgaas@...gle.com" <bhelgaas@...gle.com>, "lpieralisi@...nel.org"
	<lpieralisi@...nel.org>, "kw@...ux.com" <kw@...ux.com>, "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 v4 2/2] PCI: xilinx-cpm: Add support for PCIe RP PERST#
 signal

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Mani,

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> Sent: Friday, March 21, 2025 6:22 PM
> To: Musham, Sai Krishna <sai.krishna.musham@....com>
> Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com; 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 v4 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 Tue, Mar 18, 2025 at 02:56:48PM +0530, Sai Krishna Musham wrote:
> > Add PCIe IP reset along with GPIO-based control for the PCIe Root Port
> > PERST# signal. Synchronizing the PCIe IP reset with the PERST#
> > signal's assertion and deassertion avoids Link Training failures.
> >
> > Add clear firewall after Link reset for CPM5NC.
> >
> > Adapt to use GPIO framework and make reset optional to maintain
> > backward compatibility with existing DTBs.
> >
> > Signed-off-by: Sai Krishna Musham <sai.krishna.musham@....com>
> > ---
> > Changes for v4:
> > - Add PCIe PERST# support for CPM5NC.
> > - Add PCIe IP reset along with PERST# to avoid Link Training Errors.
> > - Remove PCIE_T_PVPERL_MS define and PCIE_T_RRS_READY_MS after
> >   PERST# deassert.
> > - Move PCIe PERST# assert and deassert logic to
> >   xilinx_cpm_pcie_init_port() before cpm_pcie_link_up(), since
> >   Interrupts enable and PCIe RP bridge enable should be done after
> >   Link up.
> > - Update commit message.
> >
> > Changes for v3:
> > - Use PCIE_T_PVPERL_MS define.
> >
> > Changes for v2:
> > - Make the request GPIO optional.
> > - Correct the reset sequence as per PERST#
> > - Update commit message
> > ---
> >  drivers/pci/controller/pcie-xilinx-cpm.c | 66
> > +++++++++++++++++++++++-
> >  1 file changed, 65 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c
> > b/drivers/pci/controller/pcie-xilinx-cpm.c
> > index d0ab187d917f..fd1fee2f614b 100644
> > --- a/drivers/pci/controller/pcie-xilinx-cpm.c
> > +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> > @@ -6,6 +6,8 @@
> >   */
> >
> >  #include <linux/bitfield.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/irq.h>
> >  #include <linux/irqchip.h>
> > @@ -21,6 +23,13 @@
> >  #include "pcie-xilinx-common.h"
> >
> >  /* Register definitions */
> > +#define XILINX_CPM_PCIE0_RST         0x00000308
> > +#define XILINX_CPM5_PCIE0_RST                0x00000318
> > +#define XILINX_CPM5_PCIE1_RST                0x0000031C
> > +#define XILINX_CPM5NC_PCIE0_RST              0x00000324
> > +
> > +#define XILINX_CPM5NC_PCIE0_FW               0x00001140
> > +
> >  #define XILINX_CPM_PCIE_REG_IDR              0x00000E10
> >  #define XILINX_CPM_PCIE_REG_IMR              0x00000E14
> >  #define XILINX_CPM_PCIE_REG_PSCR     0x00000E1C
> > @@ -99,6 +108,7 @@ struct xilinx_cpm_variant {
> >       u32 ir_status;
> >       u32 ir_enable;
> >       u32 ir_misc_value;
> > +     u32 cpm_pcie_rst;
> >  };
> >
> >  /**
> > @@ -106,6 +116,8 @@ struct xilinx_cpm_variant {
> >   * @dev: Device pointer
> >   * @reg_base: Bridge Register Base
> >   * @cpm_base: CPM System Level Control and Status Register(SLCR) Base
> > + * @crx_base: CPM Clock and Reset Control Registers Base
> > + * @cpm5nc_base: CPM5NC Control and Status Registers Base
> >   * @intx_domain: Legacy IRQ domain pointer
> >   * @cpm_domain: CPM IRQ domain pointer
> >   * @cfg: Holds mappings of config space window @@ -118,6 +130,8 @@
> > struct xilinx_cpm_pcie {
> >       struct device                   *dev;
> >       void __iomem                    *reg_base;
> >       void __iomem                    *cpm_base;
> > +     void __iomem                    *crx_base;
> > +     void __iomem                    *cpm5nc_base;
> >       struct irq_domain               *intx_domain;
> >       struct irq_domain               *cpm_domain;
> >       struct pci_config_window        *cfg;
> > @@ -478,9 +492,42 @@ static int xilinx_cpm_setup_irq(struct
> > xilinx_cpm_pcie *port)  static void xilinx_cpm_pcie_init_port(struct
> > xilinx_cpm_pcie *port)  {
> >       const struct xilinx_cpm_variant *variant = port->variant;
> > +     struct device *dev = port->dev;
> > +     struct gpio_desc *reset_gpio;
> > +
> > +     /* Request the GPIO for PCIe reset signal */
> > +     reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > +     if (IS_ERR(reset_gpio)) {
> > +             dev_err(dev, "Failed to request reset GPIO\n");
> > +             return;
> > +     }
> > +
> > +     /* Assert the reset signal */
> > +     gpiod_set_value(reset_gpio, 1);
> >
> > -     if (variant->version == CPM5NC_HOST)
> > +     /* Assert the PCIe IP reset */
> > +     writel_relaxed(0x1, port->crx_base + variant->cpm_pcie_rst);
> > +
> > +     /* Controller specific delay */
> > +     udelay(50);
> > +
> > +     /* Deassert the PCIe IP reset */
> > +     writel_relaxed(0x0, port->crx_base + variant->cpm_pcie_rst);
> > +
> > +     /* Deassert the reset signal */
> > +     gpiod_set_value(reset_gpio, 0);
> > +     mdelay(PCIE_T_RRS_READY_MS);
> > +
> > +     if (variant->version == CPM5NC_HOST) {
> > +             /* Clear Firewall */
>
> On top of Krzk's review:
>
> What does this 'firewall' mean? Clearly, not something defined in the PCIe spec.
> Also, you made it independent of PERST# line. So is it really needed for platforms
> not supporting PERST#?
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்

Firewall is internal to CPM5NC IP, it is asserted when the PCIe Link goes DOWN and then will not allow further PCIe transactions. So, firewall mode should be cleared after the PERST# sequence.
- Sai Krishna

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ