linux-kernel - Re: Re: [PATCH v2 3/3] PCI: dwc: Check for the device presence during suspend and resume

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: <62a83468.243.19a801a6d02.Coremail.zhangsenchuan@eswincomputing.com>
Date: Fri, 14 Nov 2025 10:03:26 +0800 (GMT+08:00)
From: zhangsenchuan <zhangsenchuan@...incomputing.com>
To: "Manivannan Sadhasivam" <mani@...nel.org>,
	"Bjorn Helgaas" <helgaas@...nel.org>
Cc: "Manivannan Sadhasivam" <manivannan.sadhasivam@....qualcomm.com>,
	lpieralisi@...nel.org, kwilczynski@...nel.org, bhelgaas@...gle.com,
	will@...nel.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, robh@...nel.org,
	linux-arm-msm@...r.kernel.org, vincent.guittot@...aro.org,
	"Frank Li" <Frank.li@....com>
Subject: Re: Re: [PATCH v2 3/3] PCI: dwc: Check for the device presence
 during suspend and resume




> -----Original Messages-----
> From: "Manivannan Sadhasivam" <mani@...nel.org>
> Send time:Friday, 14/11/2025 01:01:27
> To: "Bjorn Helgaas" <helgaas@...nel.org>
> Cc: "Manivannan Sadhasivam" <manivannan.sadhasivam@....qualcomm.com>, lpieralisi@...nel.org, kwilczynski@...nel.org, bhelgaas@...gle.com, will@...nel.org, linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, robh@...nel.org, linux-arm-msm@...r.kernel.org, zhangsenchuan@...incomputing.com, vincent.guittot@...aro.org, "Frank Li" <Frank.li@....com>
> Subject: Re: [PATCH v2 3/3] PCI: dwc: Check for the device presence during suspend and resume
> 
> On Thu, Nov 13, 2025 at 10:40:13AM -0600, Bjorn Helgaas wrote:
> > [+cc Frank]
> > 
> > On Fri, Nov 07, 2025 at 10:13:19AM +0530, Manivannan Sadhasivam wrote:
> > > If there is no device available under the Root Ports, there is no point in
> > > sending PME_Turn_Off and waiting for L2/L3 transition during suspend, it
> > > will result in a timeout. Hence, skip those steps if no device is available
> > > during suspend.
> > > 
> > > During resume, do not wait for the link up if there was no device connected
> > > before suspend. It is very unlikely that a device will get connected while
> > > the host system was suspended.
> > > 
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware-host.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 20c9333bcb1c..5a39e7139ec9 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -20,6 +20,7 @@
> > >  #include <linux/platform_device.h>
> > >  
> > >  #include "../../pci.h"
> > > +#include "../pci-host-common.h"
> > >  #include "pcie-designware.h"
> > >  
> > >  static struct pci_ops dw_pcie_ops;
> > > @@ -1129,6 +1130,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> > >  	u32 val;
> > >  	int ret;
> > >  
> > > +	if (!pci_root_ports_have_device(pci->pp.bridge->bus))
> > > +		goto stop_link;
> > 
> > This looks racy.  Maybe it's still OK, but I think it would be good to
> > include a comment to acknowledge that and explain why either outcome
> > is acceptable, e.g., if a user removes a device during suspend, it
> > results in a timeout but nothing more terrible.
> > 
> 
> Ok.
> 
> > >  	/*
> > >  	 * If L1SS is supported, then do not put the link into L2 as some
> > >  	 * devices such as NVMe expect low resume latency.
> > > @@ -1162,6 +1166,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> > >  	 */
> > >  	udelay(1);
> > >  
> > > +stop_link:
> > >  	dw_pcie_stop_link(pci);
> > >  	if (pci->pp.ops->deinit)
> > >  		pci->pp.ops->deinit(&pci->pp);
> > > @@ -1195,6 +1200,14 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > +	/*
> > > +	 * If there was no device before suspend, skip waiting for link up as
> > > +	 * it is bound to fail. It is very unlikely that a device will get
> > > +	 * connected *during* suspend.
> > 
> > I'm not convinced.  Unlike the suspend side, where the race window is
> > tiny, here the window is the entire time the system is suspended, and
> > at least in laptop usage, there's no reason I would hesitate to plug
> > something in while suspended.
> > 
> 
> In that case, we just need to do:
> 
> 	/* Ignore errors as there could be no devices connected */
> 	dw_pcie_wait_for_link()
> 
> I wanted to avoid the timeout if we knew that there was no device connected
> during suspend.
> 
Hi, Mani

Although it will ignore the judgment during normal device connection.
Fortunately, this function will print prompt information. Perhaps, this is a
good choice. There is also such a practice in the dw_pcie_host_init function.
for example in the dw_pcie_host_init:

    /* Ignore errors. the link may come up later */
     dw_pcie_wait_for_link(pci);

Perhaps there are other better methods. For now, this is what I have seen.

Kind regards,
Senchuan Zhang