Re: [RFC] PCI: Fix the issue of link speed downgrade after link retraining

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Lukas, -cc stable]

On Thu, Nov 07, 2024 at 10:37:58PM +0800, Jinhui Guo wrote:
> The link speed is downgraded to 2.5 GT/s when a Samsung NVMe device
> is hotplugged into a Intel PCIe root port [8086:0db0].
> 
> ```
> +-[0000:3c]-+-00.0  Intel Corporation Ice Lake Memory Map/VT-d
> |           ...
> |           +02.0-[3d]----00.0  Samsung Electronics Co Ltd Device a80e
> ```
> 
> Some printing information can be obtained when the issue emerges.
> "Card present" is reported twice via external interrupts due to
> a slight tremor when the Samsung NVMe device is plugged in.
> The failure of the link activation for the first time leads to
> the link speed of the root port being mistakenly downgraded to 2.5G/s.
> 
> ```
> [ 8223.419682] pcieport 0000:3d:02.0: pciehp: Slot(1): Card present
> [ 8224.449714] pcieport 0000:3d:02.0: broken device, retraining non-functional downstream link at 2.5GT/s
> [ 8225.518723] pcieport 0000:3d:02.0: pciehp: Slot(1): Card present
> [ 8225.518726] pcieport 0000:3d:02.0: pciehp: Slot(1): Link up
> ```

No need for markdown in commit logs.  Drop timestamps (I don't think
they are telling us anything useful here).  Indent quoted material a
couple spaces.

Nothing here looks specific to the Intel Root Port or the NVMe device;
I assume any hot-added device could see the same problem, at least
with pciehp.

> To avoid wrongly setting the link speed to 2.5GT/s, only allow
> specific pcie devices to perform link retrain.

s/pcie/PCIe/

> Fixes: a89c82249c37 ("PCI: Work around PCIe link training failures")
> Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>
> ---
>  drivers/pci/quirks.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index dccb60c1d9cc..59858156003b 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -91,7 +91,8 @@ int pcie_failed_link_retrain(struct pci_dev *dev)
>  	int ret = -ENOTTY;
>  
>  	if (!pci_is_pcie(dev) || !pcie_downstream_port(dev) ||
> -	    !pcie_cap_has_lnkctl2(dev) || !dev->link_active_reporting)
> +	    !pcie_cap_has_lnkctl2(dev) || !dev->link_active_reporting ||
> +		!pci_match_id(ids, dev))
>  		return ret;
>  
>  	pcie_capability_read_word(dev, PCI_EXP_LNKCTL2, &lnkctl2);
> @@ -119,8 +120,7 @@ int pcie_failed_link_retrain(struct pci_dev *dev)
>  	}
>  
>  	if ((lnksta & PCI_EXP_LNKSTA_DLLLA) &&
> -	    (lnkctl2 & PCI_EXP_LNKCTL2_TLS) == PCI_EXP_LNKCTL2_TLS_2_5GT &&
> -	    pci_match_id(ids, dev)) {
> +	    (lnkctl2 & PCI_EXP_LNKCTL2_TLS) == PCI_EXP_LNKCTL2_TLS_2_5GT) {
>  		u32 lnkcap;
>  
>  		pci_info(dev, "removing 2.5GT/s downstream link speed restriction\n");
> -- 
> 2.20.1
>