linux-kernel - [PATCH v2] PCI/portdrv: Disable bwctrl service if port is fixed at 2.5 GT/s

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20241213-fix_bwctrl_thunderbolt-v2-1-b52fef641dfc@kernel.org>
Date: Fri, 13 Dec 2024 21:56:14 +0100
From: Niklas Schnelle <niks@...nel.org>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
 Bjorn Helgaas <bhelgaas@...gle.com>, Lukas Wunner <lukas@...ner.de>
Cc: linux-pci@...r.kernel.org, Rob Herring <robh@...nel.org>, 
 Krzysztof Wilczy??ski <kw@...ux.com>, 
 "Maciej W . Rozycki" <macro@...am.me.uk>, 
 Jonathan Cameron <Jonathan.Cameron@...wei.com>, 
 Alexandru Gagniuc <mr.nuke.me@...il.com>, 
 Krishna chaitanya chundru <quic_krichai@...cinc.com>, 
 Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, 
 "Rafael J . Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org, 
 Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>, 
 linux-kernel@...r.kernel.org, Daniel Lezcano <daniel.lezcano@...aro.org>, 
 Amit Kucheria <amitk@...nel.org>, Zhang Rui <rui.zhang@...el.com>, 
 Christophe JAILLET <christophe.jaillet@...adoo.fr>, 
 Mika Westerberg <mika.westerberg@...ux.intel.com>, 
 Lorenzo Pieralisi <lpieralisi@...nel.org>, 
 Niklas Schnelle <niks@...nel.org>
Subject: [PATCH v2] PCI/portdrv: Disable bwctrl service if port is fixed at
 2.5 GT/s

Trying to enable bwctrl on an Intel JHL7540 (Titan Ridge) based
Thunderbolt port causes a boot hang on at least some systems though the
exact reason is not yet understood. As per the spec Thunderbolt PCIe
Downstream Ports have a fake Max Link Speed of 2.5 GT/s (USB4 v2 sec
11.2.1):

   "Max Link Speed field in the Link Capabilities Register set to 0001b
    (data rate of 2.5 GT/s only).
    Note: These settings do not represent actual throughput.
    Throughput is implementation specific and based on the USB4 Fabric
    performance."

More generally if 2.5 GT/s is the only supported link speed there is no
point in throtteling as this is already the lowest possible PCIe speed
so don't advertise the capability stopping bwctrl from being probed on
these ports.

The PCIe r6.2 specification section 7.5.3.18 recommends to primarily
utilize the Supported Link Speeds Vector instead of the Max Link Speeds
field to prevent confusion if future specifications allow devices not
to support lower speeds. This concern does not apply however when
specifically targeting devices claiming support only for 2.5 GT/s.

Link: https://lore.kernel.org/linux-pci/Z1R4VNwCOlh9Sg9n@wunner.de/
Fixes: 665745f27487 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller")
Tested-by: Niklas Schnelle <niks@...nel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
Signed-off-by: Niklas Schnelle <niks@...nel.org>
---
Note: This issue causes a boot hang on my personal workstation.

While there is an ongoing discussion about generalizing this to all
devices with a single supported speed. It turns out however that in my
case dev->supported_speeds incorrectly claims 2.5-8 GT/s requiring
a seperate second fix. So in the interest of simplicity and because I'll
be out from the 19th until January, I'd like to propose to do this simple
fix to the boot hang now and take the time to figure out a more general
approach afterwards.
---
Changes in v2:
- Improve commit message to mention the specific controller and
  why using the Max Link Speeds field should be fine here.
- Add a comment (Lukas)
- Add R-b's (no change to logic).
- Link to v1: https://lore.kernel.org/r/20241207-fix_bwctrl_thunderbolt-v1-1-b711f572a705@kernel.org
---
 drivers/pci/pcie/portdrv.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 5e10306b63081b1ddd13e0a545418e2a8610c14c..9edd10f4f516c1fb5416a810d884739d82d08587 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -270,7 +270,12 @@ static int get_port_device_capability(struct pci_dev *dev)
 		u32 linkcap;

 		pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &linkcap);
-		if (linkcap & PCI_EXP_LNKCAP_LBNC)
+		/*
+		 * Enable bandwidth control only if more than just the base
+		 * speed of 2.5 GT/s is supported.
+		 */
+		if (linkcap & PCI_EXP_LNKCAP_LBNC &&
+		    (linkcap & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB)
 			services |= PCIE_PORT_SERVICE_BWCTRL;
 	}

---
base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
change-id: 20241207-fix_bwctrl_thunderbolt-bd1f96b3d98f

Best regards,
-- 
Niklas Schnelle <niks@...nel.org>