linux-kernel - Re: [PATCH] PCI: stm32: Fix LTSSM EP race with start link.

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] [day] [month] [year] [list]

Message-ID: <20251114185928.GA2335574@bhelgaas>
Date: Fri, 14 Nov 2025 12:59:28 -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 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?

> 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>
>