Re: [PATCH v2 1/2] PCI: Configure root port MPS to hardware maximum during host probing

[Hans Zhang <18255117159@163.com>]

On 2025/4/25 18:23, Niklas Cassel wrote:
> On Fri, Apr 25, 2025 at 05:57:07PM +0800, Hans Zhang wrote:
>> Current PCIe initialization logic may leave root ports operating with
>> non-optimal Maximum Payload Size (MPS) settings. While downstream device
>> configuration is handled during bus enumeration, root port MPS values
>> inherited from firmware or hardware defaults might not utilize the full
>> capabilities supported by the controller hardware. This can result in
>> suboptimal data transfer efficiency across the PCIe hierarchy.
>>
>> During host controller probing phase, when PCIe bus tuning is enabled,
>> the implementation now configures root port MPS settings to their
>> hardware-supported maximum values. By iterating through bridge devices
>> under the root bus and identifying PCIe root ports, each port's MPS is set
>> to 128 << pcie_mpss to match the device's maximum supported payload size.
>> The Max Read Request Size (MRRS) is subsequently adjusted through existing
>> companion logic to maintain compatibility with PCIe specifications.
>>
>> Explicit initialization at host probing stage ensures consistent PCIe
>> topology configuration before downstream devices perform their own MPS
>> negotiations. This proactive approach addresses platform-specific
>> requirements where controller drivers depend on properly initialized root
>> port settings, while maintaining backward compatibility through
>> PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
>> utilized without altering existing device negotiation behaviors.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
> 
> Perhaps Mani deserves a Suggested-by tag?
> 

Dear Niklas,

Thank you very much for your reply. Ok. Sorry, I missed it. I 'm going 
to add Suggested-by tag.

> 
>> ---
>>   drivers/pci/probe.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 364fa2a514f8..3973c593fdcf 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -3206,6 +3206,7 @@ EXPORT_SYMBOL_GPL(pci_create_root_bus);
>>   int pci_host_probe(struct pci_host_bridge *bridge)
>>   {
>>   	struct pci_bus *bus, *child;
>> +	struct pci_dev *dev;
>>   	int ret;
>>   
>>   	pci_lock_rescan_remove();
>> @@ -3228,6 +3229,17 @@ int pci_host_probe(struct pci_host_bridge *bridge)
>>   	 */
>>   	pci_assign_unassigned_root_bus_resources(bus);
>>   
>> +	if (pcie_bus_config != PCIE_BUS_TUNE_OFF) {
>> +		/* Configure root ports MPS to be MPSS by default */
>> +		for_each_pci_bridge(dev, bus) {
>> +			if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
>> +				continue;
>> +
>> +			pcie_write_mps(dev, 128 << dev->pcie_mpss);
>> +			pcie_write_mrrs(dev);
> 
> The comment says configure MPS, but the code also calls pcie_write_mrrs().
> 
> Should we update the comment or should we remove the call to pcie_write_mrrs()?
> 

I have tested and found that the result is the same whether 
pcie_write_mrrs() is called or not.

> Note that even when calling pcie_write_mrrs(), it will not update MRRS for the
> common case (pcie_bus_config == PCIE_BUS_DEFAULT).

But I discovered a problem:

0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag- RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes



			Should the DevCtl MaxPayload be 256B?

But I tested that the file reading and writing were normal. Is the 
display of 512B here what we expected?

Root Port 0003:30:00.0 has the same problem. May I ask what your opinion is?


		......
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 00
                  DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
                          ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
                          MaxPayload 256 bytes, MaxReadReq 512 bytes
		......





Several PCIe ports that I enabled.

root@cix-localhost:~# cat /proc/version
Linux version 6.15.0-rc2-00015-gced1536aaf04-dirty (hans@hans) ......
root@cix-localhost:~# lspci
0000:c0:00.0 PCI bridge: Device 1f6c:0001
0000:c1:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller S4LV008[Pascal]
0001:90:00.0 PCI bridge: Device 1f6c:0001
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO
0003:30:00.0 PCI bridge: Device 1f6c:0001
0003:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller (rev 05)root@cix-localhost:~# lspci -vvv
0000:c0:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag+ RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes
		......
0000:c1:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller S4LV008[Pascal] (prog-if 02 [NVM Express])
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
                          ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
                          MaxPayload 512 bytes, MaxReadReq 512 bytes
		......
0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag- RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes
		......
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 00
                  DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
                          ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
                          MaxPayload 256 bytes, MaxReadReq 512 bytes
		......
0003:30:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag- RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes
		......
0003:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller (rev 05)
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 01
                  DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
<512ns, L1 <64us
                          ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
                          MaxPayload 256 bytes, MaxReadReq 4096 bytes
          ......

Best regards,
Hans


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic