Re: [PATCH for-linus v2 1/3] PCI: Assume 2.5 GT/s if Max Link Speed is undefined

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/15/2024 04:20, Lukas Wunner wrote:
Broken PCIe devices may not set any of the bits in the Link Capabilities
Register's "Max Link Speed" field.  Assume 2.5 GT/s in such a case,
which is the lowest possible PCIe speed.  It must be supported by every
device per PCIe r6.2 sec 8.2.1.

Emit a message informing about the malformed field.  Use KERN_INFO
severity to minimize annoyance.  This will help silicon validation
engineers take note of the issue so that regular users hopefully never
see it.

There is currently no known affected product, but a subsequent commit
will honor the Max Link Speed field when determining supported speeds
and depends on the field being well-formed.  (It uses the Max Link Speed
as highest bit in a GENMASK(highest, lowest) macro and if the field is
zero, that would result in GENMASK(0, lowest).)

Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
---
  drivers/pci/pci.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 35dc9f249b86..ab0ef7b6c798 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6233,6 +6233,13 @@ u8 pcie_get_supported_speeds(struct pci_dev *dev)
  	u32 lnkcap2, lnkcap;
  	u8 speeds;
+ /* A device must support 2.5 GT/s (PCIe r6.2 sec 8.2.1) */
+	pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
+	if (!(lnkcap & PCI_EXP_LNKCAP_SLS)) {
+		pci_info(dev, "Undefined Max Link Speed; assume 2.5 GT/s\n");

As it's just theoretical, shouldn't it be noisier? I'm thinking at least pci_warn(). Otherwise if this goes in and stays at pci_info() it's going to be a lot easier to miss.

Whereas at least messages that are warn or err get a more thorough look at during hardware bring up.

+		return PCI_EXP_LNKCAP2_SLS_2_5GB;
+	}
+
  	/*
  	 * Speeds retain the reserved 0 at LSB before PCIe Supported Link
  	 * Speeds Vector to allow using SLS Vector bit defines directly.
@@ -6244,8 +6251,6 @@ u8 pcie_get_supported_speeds(struct pci_dev *dev)
  	if (speeds)
  		return speeds;
- pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
-
  	/* Synthesize from the Max Link Speed field */
  	if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB)
  		speeds = PCI_EXP_LNKCAP2_SLS_5_0GB | PCI_EXP_LNKCAP2_SLS_2_5GB;





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux