| 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
| ||
|
Message-ID: <20251117200020.GA2518034@bhelgaas>
Date: Mon, 17 Nov 2025 14:00:20 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Christian Bruel <christian.bruel@...s.st.com>
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kwilczynski@...nel.org>,
Manivannan Sadhasivam <mani@...nel.org>,
Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
linux-pci@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: stm32: Fix LTSSM EP race with start link.
On Mon, Nov 17, 2025 at 01:04:47PM +0100, Christian Bruel wrote:
> On 11/14/25 19:59, Bjorn Helgaas wrote:
> > On Fri, Nov 14, 2025 at 08:45:52AM +0100, Christian Bruel wrote:
> > > If the host has deasserted PERST# and started link training before the link
> > > is started on EP side, enabling LTSSM before the endpoint registers are
> > > initialized in the perst_irq handler results in probing incorrect values.
> > >
> > > Thus, wait for the PERST# level-triggered interrupt to start link training
> > > at the end of initialization and cleanup the stm32_pcie_[start stop]_link
> > > functions.
> >
> > I've seen this kind of thing in other drivers, and I wondered whether
> > it was safe because the host asserts and deasserts PERST#
> > asynchronously, independent of anything the endpoint is doing.
> >
> > I assume it's possible that the host deasserts PERST# before this
> > driver has the stm32_pcie_ep_perst_irq_thread() thread set up. If
> > that happens and the driver doesn't see the PERST# interrupt, does
> > everything still work correctly?
>
> yes it does. the PERST# interrupt is level-triggered and, if already
> de-asserted, fires only when enabled (it is NOAUTOEN) with start_link.
>
> At that point, the host can enumerate by performing a manual rescan or
> rebind the PCIe driver, restarting the entire probe sequence.
>
> Tested the pcie_epf_test driver with various power-up sequences: full
> power-up the host or device first, and stop or standby PM suspend/resume.
Help me think through this. I guess the interesting case is when the
host boots first and enumerates devices before the stm32 endpoint is
ready, right?
I suppose the endpoint LTSSM is initially disabled, so the link is
down, and the host enumeration didn't find anything?
Where does the link come up? I see the pci_epc_start_store() that
eventually leads to stm32_pcie_start_link(), which enables perst_irq.
Since you requested perst_irq with IRQF_TRIGGER_HIGH, and PERST# is
deasserted, does that trigger stm32_pcie_ep_perst_irq_thread() and
call stm32_pcie_perst_deassert() to enable the LTSSM?
When the link comes up, if the Downstream Port supports hotplug and
pciehp is enabled, it might notice the link-up event and treat this as
a hot-add?
Otherwise the user would have to manually rescan to notice the
endpoint?
> > > Signed-off-by: Christian Bruel <christian.bruel@...s.st.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-stm32-ep.c | 38 ++++++------------------------
> > > 1 file changed, 7 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-stm32-ep.c b/drivers/pci/controller/dwc/pcie-stm32-ep.c
> > > index 3400c7cd2d88a279c49ef36a99fc7537c381c384..d0654bb43759bb8d0f0d7badbf7bdae839241fcf 100644
> > > --- a/drivers/pci/controller/dwc/pcie-stm32-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-stm32-ep.c
> > > @@ -37,36 +37,9 @@ static void stm32_pcie_ep_init(struct dw_pcie_ep *ep)
> > > dw_pcie_ep_reset_bar(pci, bar);
> > > }
> > > -static int stm32_pcie_enable_link(struct dw_pcie *pci)
> > > -{
> > > - struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> > > -
> > > - regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> > > - STM32MP25_PCIECR_LTSSM_EN,
> > > - STM32MP25_PCIECR_LTSSM_EN);
> > > -
> > > - return dw_pcie_wait_for_link(pci);
> > > -}
> > > -
> > > -static void stm32_pcie_disable_link(struct dw_pcie *pci)
> > > -{
> > > - struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> > > -
> > > - regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR, STM32MP25_PCIECR_LTSSM_EN, 0);
> > > -}
> > > -
> > > static int stm32_pcie_start_link(struct dw_pcie *pci)
> > > {
> > > struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> > > - int ret;
> > > -
> > > - dev_dbg(pci->dev, "Enable link\n");
> > > -
> > > - ret = stm32_pcie_enable_link(pci);
> > > - if (ret) {
> > > - dev_err(pci->dev, "PCIe cannot establish link: %d\n", ret);
> > > - return ret;
> > > - }
> > > enable_irq(stm32_pcie->perst_irq);
> > > @@ -77,11 +50,7 @@ static void stm32_pcie_stop_link(struct dw_pcie *pci)
> > > {
> > > struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
> > > - dev_dbg(pci->dev, "Disable link\n");
> > > -
> > > disable_irq(stm32_pcie->perst_irq);
> > > -
> > > - stm32_pcie_disable_link(pci);
> > > }
> > > static int stm32_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > > @@ -152,6 +121,8 @@ static void stm32_pcie_perst_assert(struct dw_pcie *pci)
> > > dev_dbg(dev, "PERST asserted by host\n");
> > > + regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR, STM32MP25_PCIECR_LTSSM_EN, 0);
> > > +
> > > pci_epc_deinit_notify(ep->epc);
> > > stm32_pcie_disable_resources(stm32_pcie);
> > > @@ -192,6 +163,11 @@ static void stm32_pcie_perst_deassert(struct dw_pcie *pci)
> > > pci_epc_init_notify(ep->epc);
> > > + /* Enable link training */
> > > + regmap_update_bits(stm32_pcie->regmap, SYSCFG_PCIECR,
> > > + STM32MP25_PCIECR_LTSSM_EN,
> > > + STM32MP25_PCIECR_LTSSM_EN);
> > > +
> > > return;
> > > err_disable_resources:
> > >
> > > ---
> > > base-commit: 31115ecec74fe5c679a149d7037009f26b3aa8a9
> > > change-id: 20251113-perst_ep-0b57b9679cf9
> > >
> > > Best regards,
> > > --
> > > Christian Bruel <christian.bruel@...s.st.com>
> > >
>
Powered by blists - more mailing lists