Re: [PATCH 1/1] PCI: Add CONFIG_PCI_NOSPEED_QUIRK to remove pcie_failed_link_retrain

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jul 16, 2025 at 01:02:06PM -0600, Matthew W Carlis wrote:
> It is desirable to be able to remove pcie_failed_link_retrain for some
> systems which are known to have PCIe devices with good LTSSM behavior
> or a high degree of compatibility and which may be required to endure
> large numbers of hot-plug events or DPC triggers & always arrive at the
> maximum link speed. It appears that there is a degree of variability
> in DSP/RP behavior in terms of setting the LBMS bit & therefore
> difficult to tune pcie_failed_link_retrain with a very high degree
> of accuracy in terms of never forcing a device to Gen1 that would
> be able to arrive at its maximum speed on its own.
> 
> Signed-off-by: Matthew W Carlis <mattc@purestorage.com>
> ---
>  drivers/pci/Kconfig  | 9 +++++++++
>  drivers/pci/pci.h    | 8 +++++++-
>  drivers/pci/quirks.c | 3 +++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 9c0e4aaf4e8c..8f01808231f7 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -68,6 +68,15 @@ config PCI_QUIRKS
>  	  Disable this only if your target machine is unaffected by PCI
>  	  quirks.
>  
> +config PCI_NOSPEED_QUIRK
> +	default n
> +	bool "Remove forced Gen1 link speed Gen1 quirk" if EXPERT
> +	help
> +	  This disables a workaround that will guide the PCIe link to
> +	  2.5GT/s speed if it thinks the link has failed to train. Enable
> +	  this if you think this workaround is forcing the link to 2.5GT/s
> +	  when it should not.

This seems awfully specific to me, really too specific to carry in the
upstream tree.  pcie_failed_link_retrain() is itself ridiculously
specific, and I'm not sure we should even keep carrying that.

Maybe we should just accept that broken hardware exists and add quirks
to limit link speed or tell the user to buy a working device.

>  config PCI_DEBUG
>  	bool "PCI Debugging"
>  	depends on DEBUG_KERNEL
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 12215ee72afb..51fddc6419f3 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -785,7 +785,6 @@ void pci_acs_init(struct pci_dev *dev);
>  int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
>  int pci_dev_specific_enable_acs(struct pci_dev *dev);
>  int pci_dev_specific_disable_acs_redir(struct pci_dev *dev);
> -int pcie_failed_link_retrain(struct pci_dev *dev);
>  #else
>  static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev,
>  					       u16 acs_flags)
> @@ -800,11 +799,18 @@ static inline int pci_dev_specific_disable_acs_redir(struct pci_dev *dev)
>  {
>  	return -ENOTTY;
>  }
> +#endif
> +
> +#ifdef CONFIG_PCI_QUIRKS
> +#ifndef CONFIG_PCI_NOSPEED_QUIRK
> +int pcie_failed_link_retrain(struct pci_dev *dev);
> +#else
>  static inline int pcie_failed_link_retrain(struct pci_dev *dev)
>  {
>  	return -ENOTTY;
>  }
>  #endif
> +#endif
>  
>  /* PCI error reporting and recovery */
>  pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 39bb0c025119..d2d06f9ec983 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -36,6 +36,8 @@
>  #include <linux/switchtec.h>
>  #include "pci.h"
>  
> +#ifndef CONFIG_PCI_NOSPEED_QUIRK
> +
>  static bool pcie_lbms_seen(struct pci_dev *dev, u16 lnksta)
>  {
>  	if (test_bit(PCI_LINK_LBMS_SEEN, &dev->priv_flags))
> @@ -140,6 +142,7 @@ int pcie_failed_link_retrain(struct pci_dev *dev)
>  
>  	return ret;
>  }
> +#endif
>  
>  static ktime_t fixup_debug_start(struct pci_dev *dev,
>  				 void (*fn)(struct pci_dev *dev))
> -- 
> 2.46.0
>