linux-kernel - Re: [PATCH v2] PCI: dwc: Set PORT_LOGIC_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: <854259d9-c9e6-4b25-8787-0999099bbf4d@quicinc.com>
Date: Thu, 24 Apr 2025 13:09:59 +0800
From: Qiang Yu <quic_qianyu@...cinc.com>
To: "Wenbin Yao (Consultant)" <quic_wenbyao@...cinc.com>,
        Niklas Cassel
	<cassel@...nel.org>
CC: <jingoohan1@...il.com>, <manivannan.sadhasivam@...aro.org>,
        <lpieralisi@...nel.org>, <kw@...ux.com>, <robh@...nel.org>,
        <bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <krishna.chundru@....qualcomm.com>,
        <quic_vbadigan@...cinc.com>, <quic_mrana@...cinc.com>,
        <quic_cang@...cinc.com>
Subject: Re: [PATCH v2] PCI: dwc: Set PORT_LOGIC_LINK_WIDTH to one lane


On 4/24/2025 10:49 AM, Wenbin Yao (Consultant) wrote:
> On 4/23/2025 9:24 PM, Niklas Cassel wrote:
>> On Tue, Apr 22, 2025 at 06:36:23PM +0800, Wenbin Yao wrote:
>>> As per DWC PCIe registers description 4.30a, section 1.13.43, 
>>> NUM_OF_LANES
>>> named as PORT_LOGIC_LINK_WIDTH in PCIe DWC driver, is referred to as 
>>> the
>>> "Predetermined Number of Lanes" in section 4.2.6.2.1 of the PCI Express
>>> Base 3.0 Specification, revision 1.0. This section explains the 
>>> conditions
>>> need be satisfied for entering Polling.Configuration:
>>>
>>> "Next state is Polling.Configuration after at least 1024 TS1 Ordered 
>>> Sets
>>> were transmitted, and all Lanes that detected a Receiver during Detect
>>> receive eight consecutive training sequences.
>>>
>>> Otherwise, after a 24 ms timeout the next state is:
>>> Polling.Configuration if
>>> (i) Any Lane, which detected a Receiver during Detect, received eight
>>> consecutive training sequences and a minimum of 1024 TS1 Ordered 
>>> Sets are
>>> transmitted after receiving one TS1 or TS2 Ordered Set.
>>> And
>>> (ii) At least a predetermined set of Lanes that detected a Receiver 
>>> during
>>> Detect have detected an exit from Electrical Idle at least once since
>>> entering Polling.Active.
>>>
>>> Note: This may prevent one or more bad Receivers or Transmitters from
>>> holding up a valid Link from being configured, and allow for additional
>>> training in Polling.Configuration. The exact set of predetermined 
>>> Lanes is
>>> implementation specific.
>>>
>>> Note: Any Lane that receives eight consecutive TS1 or TS2 Ordered Sets
>>> should have detected an exit from Electrical Idle at least once since
>>> entering Polling.Active."
>>>
>>> In a PCIe link that supports multiple lanes, if 
>>> PORT_LOGIC_LINK_WIDTH is
>>> set to lane width hardware supports, all lanes that detect a receiver
>>> during the Detect phase must receive eight consecutive training 
>>> sequences.
>>> Otherwise, the LTSSM cannot enter Polling.Configuration and link 
>>> training
>>> will fail.
>>>
>>> Therefore, always set PORT_LOGIC_LINK_WIDTH to 1, regardless of the 
>>> number
>>> of lanes the port actually supports, to make linking up more robust. 
>>> This
>>> setting will not affect the intended link width if all lanes are
>>> functional. Additionally, the link can still be established with at 
>>> least
>>> one lane if other lanes are faulty.
>>>
>>> Co-developed-by: Qiang Yu <quic_qianyu@...cinc.com>
>>> Signed-off-by: Qiang Yu <quic_qianyu@...cinc.com>
>>> Signed-off-by: Wenbin Yao <quic_wenbyao@...cinc.com>
>>> ---
>>> Changes in v2:
>>> - Reword commit message.
>>> - Link to v1: 
>>> https://lore.kernel.org/all/1524e971-8433-1e2d-b39e-65bad0d6c6ce@quicinc.com/
>>>
>>>   drivers/pci/controller/dwc/pcie-designware.c | 5 +----
>>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c 
>>> b/drivers/pci/controller/dwc/pcie-designware.c
>>> index 97d76d3dc..be348b341 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>> @@ -797,22 +797,19 @@ static void 
>>> dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
>>>       /* Set link width speed control register */
>>>       lwsc = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>>>       lwsc &= ~PORT_LOGIC_LINK_WIDTH_MASK;
>>> +    lwsc |= PORT_LOGIC_LINK_WIDTH_1_LANES;
>>>       switch (num_lanes) {
>>>       case 1:
>>>           plc |= PORT_LINK_MODE_1_LANES;
>>> -        lwsc |= PORT_LOGIC_LINK_WIDTH_1_LANES;
>>>           break;
>>>       case 2:
>>>           plc |= PORT_LINK_MODE_2_LANES;
>>> -        lwsc |= PORT_LOGIC_LINK_WIDTH_2_LANES;
>>>           break;
>>>       case 4:
>>>           plc |= PORT_LINK_MODE_4_LANES;
>>> -        lwsc |= PORT_LOGIC_LINK_WIDTH_4_LANES;
>>>           break;
>>>       case 8:
>>>           plc |= PORT_LINK_MODE_8_LANES;
>>> -        lwsc |= PORT_LOGIC_LINK_WIDTH_8_LANES;
>>>           break;
>>>       default:
>>>           dev_err(pci->dev, "num-lanes %u: invalid value\n", 
>>> num_lanes);
>>> -- 
>>> 2.34.1
>>>
>> I still see the link to my EP (which also have this patch) using all
>> four lanes according to lspci, so:
>>
>> Tested-by: Niklas Cassel <cassel@...nel.org>
Thank you, Niklas, for kindly testing this patch and providing feedback.
Wenbin seems to have misunderstood you; sorry for this.
>
> This setting will not affect the intended link width if all lanes are
> functional. Additionally, the link can still be established with at least
> one lane if other lanes are faulty.
>
-- 
With best wishes
Qiang Yu