On Thu, Sep 04, 2025 at 01:11:22AM +0800, Hans Zhang wrote:

On 2025/9/3 01:48, Bjorn Helgaas wrote:

quoted

On Fri, Jun 20, 2025 at 11:55:06PM +0800, Hans Zhang wrote:

quoted

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

Apparently Root Port MPS configuration is different from that for
downstream devices?

Yes, at the very beginning, the situation I tested was like the previous
reply:
https://lore.kernel.org/linux-pci/bb40385c-6839-484c-90b2-d6c7ecb95ba9@163.com/ (local)

I meant that this commit log suggests the *code path* is different for
Root Ports than other devices.

quoted

quoted

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. Specifically, when configuring the MPS
for a PCIe device, if the device is a root port and the bus tuning is not
disabled (PCIE_BUS_TUNE_OFF), the MPS is set to 128 << dev->pcie_mpss to
match the Root Port's maximum supported payload size. The Max Read Request
Size (MRRS) is subsequently adjusted through existing companion logic to
maintain compatibility with PCIe specifications.

Note that this initial setting of the root port MPS to the maximum might
be reduced later during the enumeration of downstream devices if any of
those devices do not support the maximum MPS of the root port.

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.

quoted

Update the MRRS "to maintain compatibility" part.  I'm dubious about
there being a spec compatibility issue with respect to MRRS.  Cite the
relevant section if there is an issue.

The description is inaccurate. I will correct it.

I plan to modify the commit message as follows:
If there are any incorrect descriptions, please correct them. Thank you very
much.

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.

With this patch, during the host controller probing phase and when PCIe
bus tuning is enabled, the implementation configures Root Port MPS
settings to their hardware-supported maximum values. Specifically, when
configuring the MPS for a PCIe device, if the device is a Root Port and
the bus tuning is not disabled (PCIE_BUS_TUNE_OFF), the MPS is set to
128 << dev->pcie_mpss to match the Root Port's maximum supported payload
size. The Max Read Request Size (MRRS) is subsequently adjusted by
existing logic in pci_configure_mps() to ensure it is not less than the
MPS, maintaining compliance with PCIe specifications (see PCIe r7.0,
sec 7.5.3.4).

AFAICS, sec 7.5.3.4 doesn't say anything about a relationship between
MRRS and MPS.  MPS is a constraint on the payload size of TLPs with
data (Memory Writes, Completions with Data, etc).  MRRS is a
constraint on the Length field in a Memory Read Request.  A single
Memory Read can be satisified with several Completions.  MPS is one of
the things that determines how many Completions are required.

This has more details and a lot of good discussion:
https://www.xilinx.com/support/documentation/white_papers/wp350.pdf