Re: [PATCH v3] PCI: Skip Target Speed quirk on clamped ports with no link

On 8/1/26 22:11, Andreas Wild wrote:
> From: "Maciej W. Rozycki" <[email protected]>
> 
> Since commit 72780f796468 ("PCI: Always lift 2.5GT/s restriction in PCIe
> failed link retraining") the Target Speed quirk lifts a firmware-imposed
> 2.5GT/s restriction on any downstream port, without checking whether the
> link is up.  Where nothing is plugged in, the retraining that follows can
> never complete, so each attempt costs PCIE_LINK_RETRAIN_TIMEOUT_MS.  The
> quirk makes two of them -- the initial one and the restore on the error
> path -- adding a fixed 2 s to every boot.
> 
> On an MSI PRO Z690-A WIFI DDR4 (Intel 600 Series PCH) with one empty x1
> slot, running v7.2-rc5:
> 
>  0.541 pci 0000:00:1c.0: removing 2.5GT/s downstream link speed restriction
>  1.541 pci 0000:00:1c.0: retraining failed
>  2.541 pci 0000:00:1c.2: [8086:7aba] type 01 class 0x060400

TWIMC, this patch fixes a regression Edward reported here:
https://bugzilla.kernel.org/show_bug.cgi?id=221801

[side note: the report could be more specific, yes -- I wanted to ask
Edward to clarify a few things (like the actual slowdown) before
forwarding it, but it didn't came to that when I noticed this patch and
asked Edward to just check if it helped -- which is did.]

Ciao, Thorsten
> Where the Link Speed has already been clamped at 2.5GT/s and no link has
> been established there is nothing worth doing, which is what the kerneldoc
> for the quirk already describes: the restriction is to be lifted where
> firmware arranged it "and the port reports its link already being up".
> Bail out early in that case, before either the ASM2824 workaround or the
> removal of the restriction is considered.
> 
> Ports whose link is up are unaffected, and so is the ASM2824 workaround,
> which is reached with the Target Link Speed not clamped.
> 
> With this applied the quirk returns without touching the port: both
> messages are gone, enumeration proceeds from 0000:00:1c.0 to 0000:00:1c.2
> in 1 ms rather than 2 s, and the systemd "kernel" boot phase goes from
> 3.011 s to 1.036 s.
> 
> Fixes: 72780f796468 ("PCI: Always lift 2.5GT/s restriction in PCIe failed link retraining")
> Cc: [email protected]
> Reported-by: Andreas Wild <[email protected]>
> Closes: https://lore.kernel.org/lkml/[email protected]/
> Tested-by: Andreas Wild <[email protected]>
> Signed-off-by: Maciej W. Rozycki <[email protected]>
> Signed-off-by: Andreas Wild <[email protected]>
> ---
> v3:
>  - Replaced with Maciej's much simpler approach: bail out of the quirk
>    entirely when the Target Link Speed is already clamped at 2.5GT/s and no
>    link has been established, rather than programming the speed and skipping
>    only the retraining.  One function, no new API, no bwctrl changes.
>  - Note this leaves the Target Link Speed clamped on such a port, where v2
>    left it at the Port's maximum.  A device hot-plugged there later trains at
>    2.5GT/s: pcie_wait_for_link_delay() only calls the quirk when
>    pcie_wait_for_link_status() fails, so a link that comes up cleanly at
>    2.5GT/s never re-runs it.  Flagging in case that matters; the clamp is
>    firmware's, so honouring it on an unoccupied Port seems defensible.
>  - v2: https://lore.kernel.org/lkml/[email protected]/
>  - v1: https://lore.kernel.org/lkml/[email protected]/
>  drivers/pci/quirks.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b09f27f..9e407c4 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -108,7 +108,11 @@ int pcie_failed_link_retrain(struct pci_dev *dev)
>  
>  	pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
>  	pcie_capability_read_word(dev, PCI_EXP_LNKCTL2, &oldlnkctl2);
> -	if (!(lnksta & PCI_EXP_LNKSTA_DLLLA) && pcie_lbms_seen(dev, lnksta)) {
> +	if (lnksta & PCI_EXP_LNKSTA_DLLLA) {
> +		;
> +	} else if (PCIE_LNKCTL2_TLS2SPEED(oldlnkctl2) == PCIE_SPEED_2_5GT) {
> +		return ret;
> +	} else if (pcie_lbms_seen(dev, lnksta)) {
>  		pci_info(dev, "broken device, retraining non-functional downstream link at 2.5GT/s\n");
>  		ret = pcie_set_target_speed(dev, PCIE_SPEED_2_5GT, false);
>  		if (ret)