| 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:
<AS8PR04MB8833BDE50B260CE235DF51258CA8A@AS8PR04MB8833.eurprd04.prod.outlook.com>
Date: Thu, 18 Dec 2025 08:48:53 +0000
From: Hongxing Zhu <hongxing.zhu@....com>
To: Manivannan Sadhasivam <mani@...nel.org>
CC: Shawn Lin <shawn.lin@...k-chips.com>, Frank Li <frank.li@....com>,
Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>, Jingoo Han
<jingoohan1@...il.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kwilczynski@...nel.org>, Rob
Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
"zhangsenchuan@...incomputing.com" <zhangsenchuan@...incomputing.com>
Subject: RE: [PATCH 2/2] PCI: dwc: Do not return failure from
dw_pcie_wait_for_link() if link is in Detect/Poll state
> -----Original Message-----
> From: Manivannan Sadhasivam <mani@...nel.org>
> Sent: 2025年12月18日 16:15
> To: Hongxing Zhu <hongxing.zhu@....com>
> Cc: Shawn Lin <shawn.lin@...k-chips.com>; Frank Li <frank.li@....com>;
> Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>;
> Jingoo Han <jingoohan1@...il.com>; Lorenzo Pieralisi <lpieralisi@...nel.org>;
> Krzysztof Wilczyński <kwilczynski@...nel.org>; Rob Herring
> <robh@...nel.org>; Bjorn Helgaas <bhelgaas@...gle.com>;
> linux-pci@...r.kernel.org; linux-kernel@...r.kernel.org;
> vincent.guittot@...aro.org; zhangsenchuan@...incomputing.com
> Subject: Re: [PATCH 2/2] PCI: dwc: Do not return failure from
> dw_pcie_wait_for_link() if link is in Detect/Poll state
>
> On Fri, Nov 21, 2025 at 09:56:05PM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Nov 21, 2025 at 05:21:58AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Manivannan Sadhasivam <mani@...nel.org>
> > > > Sent: 2025年11月20日 13:37
> > > > To: Shawn Lin <shawn.lin@...k-chips.com>
> > > > Cc: Hongxing Zhu <hongxing.zhu@....com>; Frank Li
> > > > <frank.li@....com>; Manivannan Sadhasivam
> > > > <manivannan.sadhasivam@....qualcomm.com>;
> > > > Jingoo Han <jingoohan1@...il.com>; Lorenzo Pieralisi
> > > > <lpieralisi@...nel.org>; Krzysztof Wilczyński
> > > > <kwilczynski@...nel.org>; Rob Herring <robh@...nel.org>; Bjorn
> > > > Helgaas <bhelgaas@...gle.com>; linux-pci@...r.kernel.org;
> > > > linux-kernel@...r.kernel.org; vincent.guittot@...aro.org;
> > > > zhangsenchuan@...incomputing.com
> > > > Subject: Re: [PATCH 2/2] PCI: dwc: Do not return failure from
> > > > dw_pcie_wait_for_link() if link is in Detect/Poll state
> > > >
> > > > + Richard, Frank
> > > >
> > > > On Thu, Nov 20, 2025 at 09:13:24AM +0800, Shawn Lin wrote:
> > > > > 在 2025/11/20 星期四 2:10, Manivannan Sadhasivam 写道:
> > > > > > dw_pcie_wait_for_link() API waits for the link to be up and
> > > > > > returns failure if the link is not up within the 1 second
> > > > > > interval. But if there was no device connected to the bus,
> > > > > > then the link up failure would
> > > > be expected.
> > > > > > In that case, the callers might want to skip the failure in a
> > > > > > hope that the link will be up later when a device gets connected.
> > > > > >
> > > > > > One of the callers, dw_pcie_host_init() is currently skipping
> > > > > > the failure irrespective of the link state, in an assumption
> > > > > > that the link may come up later. But this assumption is wrong,
> > > > > > since LTSSM states other than Detect and Poll during link
> > > > > > training phase are considered to be fatal and the link needs to be
> retrained.
> > > > > >
> > > > > > So to avoid callers making wrong assumptions, skip returning
> > > > > > failure from
> > > > > > dw_pcie_wait_for_link() if the link is in Detect or Poll state
> > > > > > after timeout and also check the return value of the API in
> > > > dw_pcie_host_init().
> > > > > >
> > > > > > Signed-off-by: Manivannan Sadhasivam
> > > > > > <manivannan.sadhasivam@....qualcomm.com>
> > > > > > ---
> > > > > > drivers/pci/controller/dwc/pcie-designware-host.c | 8 +++++---
> > > > > > drivers/pci/controller/dwc/pcie-designware.c | 8 ++++++++
> > > > > > 2 files changed, 13 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > > > index 8fe3454f3b13..8c4845fd24aa 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > > > @@ -671,9 +671,11 @@ int dw_pcie_host_init(struct dw_pcie_rp
> *pp)
> > > > > > * If there is no Link Up IRQ, we should not bypass the delay
> > > > > > * because that would require users to manually rescan for
> devices.
> > > > > > */
> > > > > > - if (!pp->use_linkup_irq)
> > > > > > - /* Ignore errors, the link may come up later */
> > > > > > - dw_pcie_wait_for_link(pci);
> > > > > > + if (!pp->use_linkup_irq) {
> > > > > > + ret = dw_pcie_wait_for_link(pci);
> > > > > > + if (ret)
> > > > > > + goto err_stop_link;
> > > > > > + }
> > > > > > ret = pci_host_probe(bridge);
> > > > > > if (ret)
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > index c644216995f6..fe13c6b10ccb 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > @@ -651,6 +651,14 @@ int dw_pcie_wait_for_link(struct dw_pcie
> *pci)
> > > > > > }
> > > > > > if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
> > > > > > + /*
> > > > > > + * If the link is in Detect or Poll state, it indicates that no
> > > > > > + * device is connected. So return success to allow the device
> to
> > > > > > + * show up later.
> > > > > > + */
> > > > > > + if (dw_pcie_get_ltssm(pci) <=
> DW_PCIE_LTSSM_DETECT_WAIT)
> > > > > > + return 0;
> > > > >
> > > > > I'm afraid this might not be true. If there is no devices
> > > > > connected or the device connected without power supplied, it
> > > > > means there is no far-end pull-up termination resistor from TX
> > > > > view of RC. TX pulse detection signal from the RC side will not
> > > > > undergo voltage division, and its LTSSM state machine will only
> > > > > toggle between DW_PCIE_LTSSM_DETECT_QUIET and
> DW_PCIE_LTSSM_DETECT_ACT.
> > > > >
> > > >
> > > > I must admit that I just inherited this check from
> dw_pcie_suspend_noirq().
> > > > But I cross checked the PCIe base spec and it mentions clearly
> > > > that the LTSSM will be in Detect.Quiet/Active states if no
> > > > endpoint is detected i.e., within the 1s timeout, the LTSSM
> > > > should've transitioned back to these Detect states.
> > > >
> > > > I'm wondering why we are checking for Poll and other states in
> > > > dw_pcie_suspend_noirq(). I believe the intention was to check for
> > > > the presence of an endpoint or not.
> > > >
> > > > Richard, Frank, thoughts?
> > > >
> > > Hi Mani:
> > > Yes, it is.
> > > In my initial upstreaming patches, the intention to check this state
> > > is to figure out that there is an endpoint device or not.
> > >
> >
> > If so, why do we need to check for LTSSM states other than
> > DW_PCIE_LTSSM_DETECT_QUIET and DW_PCIE_LTSSM_DETECT_ACT?
> >
> > Did spec mandate it or you did it for some specific reason?
> >
>
> Hongxing, ping!
Hi Mani:
Sorry to reply late.
Refer to "Advanced Information: Lane Reversal and Broken Lanes" of PCIe DM
Controller data book released by Synopsys.
"
Note: Flip: When the LTSSM transitions to DETECT_WAIT, meaning that some lanes
are detected in DETECT_ACT, but not all; and lane0 is not detected, the LTSSM
autonomously activates a lane flip operation.
"
It seems that LTSSM might transition to DETECT_WAIT in the FLIP case.
So, to be save, just use the max condition here.
Best Regards
Richard Zhu
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists