From patchwork Sun Dec 15 10:20:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 13908714 X-Patchwork-Delegate: kw@linux.com Received: from bmailout2.hostsharing.net (bmailout2.hostsharing.net [83.223.78.240]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E5848320E for ; Sun, 15 Dec 2024 10:25:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=83.223.78.240 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734258349; cv=none; b=g9/TSaaceJTprNouksFd7iqNWknWzxbycTyOHOoDbTtkgqQAyATc7itqIGzUjh9OGOC2AG5nxg30RoRoz1vINUici2M8ekvQdL24JH+YXxlFwaSCDsDZ16RIpSb7PMppEJrAXU56lfJci1zCGTz6vex3sfqucJk17k3nrWaIMP8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734258349; c=relaxed/simple; bh=oWpU5prI21m4cP+NKLzxEDgdeezpsvRNbUyd64mu8QQ=; h=Message-ID:In-Reply-To:References:From:Date:Subject:To:Cc; b=coJQC/2Kn1TMIVw99pbD1QiZitVx/NopPZjcgqtGAwyW+x35mfUPd9QB1g7vGzSLE6p2gAzPF2DDUSY77nt5yXJiaPJOL8ybsBQUI+qbPvuQaAJPbPpc9OFfRDf8QB36/WmHH4LCu6Z1Df3Jcp5vs5RYni1waWNjJDMuDHeRH+s= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de; spf=pass smtp.mailfrom=wunner.de; arc=none smtp.client-ip=83.223.78.240 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=wunner.de Received: from h08.hostsharing.net (h08.hostsharing.net [IPv6:2a01:37:1000::53df:5f1c:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS RSA CA G1" (verified OK)) by bmailout2.hostsharing.net (Postfix) with ESMTPS id 3140E2800B485; Sun, 15 Dec 2024 11:25:37 +0100 (CET) Received: by h08.hostsharing.net (Postfix, from userid 100393) id 1EAAA440FDE; Sun, 15 Dec 2024 11:25:37 +0100 (CET) Message-ID: <1a07f35cdfda64ca1d5154cc85ca1dd5f01137d3.1734257330.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sun, 15 Dec 2024 11:20:51 +0100 Subject: [PATCH for-linus v2 1/3] PCI: Assume 2.5 GT/s if Max Link Speed is undefined To: Bjorn Helgaas Cc: linux-pci@vger.kernel.org, Niklas Schnelle , Ilpo Jarvinen , Jonathan Cameron , Mika Westerberg , "Maciej W. Rozycki" , Mario Limonciello Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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 Reviewed-by: Jonathan Cameron --- 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"); + 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; From patchwork Sun Dec 15 10:20:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 13908715 X-Patchwork-Delegate: kw@linux.com Received: from bmailout1.hostsharing.net (bmailout1.hostsharing.net [83.223.95.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8CDAF320E for ; Sun, 15 Dec 2024 10:28:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=83.223.95.100 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734258527; cv=none; b=ME6CN0hibBj106bG0wQLENTUVYzLgAlsZrI6tnWWSbabe61bESGsbOJ7ZA4MfXLdh+jMjKjHDS1ElL1h1tSjfbF487eQh+ir8CDbWBOBstMt5lqkJ/9Jn6at3YqsIWHcDAWBce8oMPjqv9osAT7it2aKs9ypMKp6M/aAkUlQiUw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734258527; c=relaxed/simple; bh=9ehiABYxiI4eRXAS6TyK6068SM12XaHvZ1mG3J+trpg=; h=Message-ID:In-Reply-To:References:From:Date:Subject:MIME-Version: Content-Type:To:Cc; b=IfPQkeu3z2HlQH7N3YlAzEqru34vpP3uHYLwBb4Nv8EGryu3X9+UqrAyE9py5otNJatlj5HRkzu8Bx+GqDXX8pI7uCjN3mT71xg6gpRW7gk789oJZ6TfrXcisdZGKbHAvlIxxPYy4ndp6RIdMiODFvq0qSuZWI89c1i3tvp3tZA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de; spf=pass smtp.mailfrom=wunner.de; arc=none smtp.client-ip=83.223.95.100 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=wunner.de Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS RSA CA G1" (verified OK)) by bmailout1.hostsharing.net (Postfix) with ESMTPS id 7F8383000D7A4; Sun, 15 Dec 2024 11:28:42 +0100 (CET) Received: by h08.hostsharing.net (Postfix, from userid 100393) id 665F7356243; Sun, 15 Dec 2024 11:28:42 +0100 (CET) Message-ID: <0044d6cd05ad20ec3a6ec5a8a22b6ab652e251fe.1734257330.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sun, 15 Dec 2024 11:20:52 +0100 Subject: [PATCH for-linus v2 2/3] PCI: Honor Max Link Speed when determining supported speeds Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: Bjorn Helgaas Cc: linux-pci@vger.kernel.org, Niklas Schnelle , Ilpo Jarvinen , Jonathan Cameron , Mika Westerberg , "Maciej W. Rozycki" , Mario Limonciello The Supported Link Speeds Vector in the Link Capabilities 2 Register indicates the *supported* link speeds. The Max Link Speed field in the Link Capabilities Register indicates the *maximum* of those speeds. pcie_get_supported_speeds() neglects to honor the Max Link Speed field and will thus incorrectly deem higher speeds as supported. Fix it. One user-visible issue addressed here is an incorrect value in the sysfs attribute "max_link_speed". But the main motivation is a boot hang reported by Niklas: Intel JHL7540 "Titan Ridge 2018" Thunderbolt controllers supports 2.5-8 GT/s speeds, but indicate 2.5 GT/s as maximum. Ilpo recalls seeing this on more devices. It can be explained by the controller's Downstream Ports supporting 8 GT/s if an Endpoint is attached, but limiting to 2.5 GT/s if the port interfaces to a PCIe Adapter, in accordance with USB4 v2 sec 11.2.1: "This section defines the functionality of an Internal PCIe Port that interfaces to a PCIe Adapter. [...] The Logical sub-block shall update the PCIe configuration registers with the following characteristics: [...] 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." The present commit is not sufficient on its own to fix Niklas' boot hang, but it is a prerequisite. Fixes: d2bd39c0456b ("PCI: Store all PCIe Supported Link Speeds") Reported-by: Niklas Schnelle Closes: https://lore.kernel.org/r/70829798889c6d779ca0f6cd3260a765780d1369.camel@kernel.org/ Signed-off-by: Lukas Wunner Cc: Ilpo Järvinen Tested-by: Niklas Schnelle Reviewed-by: Jonathan Cameron --- drivers/pci/pci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index ab0ef7b6c798..9f672399e688 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -6247,6 +6247,10 @@ u8 pcie_get_supported_speeds(struct pci_dev *dev) pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2); speeds = lnkcap2 & PCI_EXP_LNKCAP2_SLS; + /* Ignore speeds higher than Max Link Speed */ + speeds &= GENMASK(lnkcap & PCI_EXP_LNKCAP_SLS, + PCI_EXP_LNKCAP2_SLS_2_5GB); + /* PCIe r3.0-compliant */ if (speeds) return speeds; From patchwork Sun Dec 15 10:20:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 13908716 X-Patchwork-Delegate: kw@linux.com Received: from bmailout3.hostsharing.net (bmailout3.hostsharing.net [176.9.242.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E090814263 for ; Sun, 15 Dec 2024 10:33:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=176.9.242.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734258809; cv=none; b=NqvN5JLw3q1jrWapRJoJxwp2ho+jiXVq5FOm5yu0nSLA5G3S8f78CxDJPgtZPCZ5371DZhjc40XZwgDf/a/I3EuaWSO5Nvt9I89mYbQ99s6Z7RfVauGdqlekZLjqRCHQScIAxvYCRlKndAuCb20ZiAj80aVPDbjgVmZ8QFqXKE0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734258809; c=relaxed/simple; bh=S++7ZrTlPYNdgzCgM9hV3GVnQFP/LB6nPqC//qTjKh8=; h=Message-ID:In-Reply-To:References:From:Date:Subject:MIME-Version: Content-Type:To:Cc; b=CrUe2DQSGRdoewcLlsXWi3X0WlFdzG5v5XvdF/r5X4avFDTqzkJ7eRCBL20m8biwKCfOQp3IEX4X/anL/nf42NISwfn4EHZ9Bg8Zooi0I7mqBEsmSvlNepiTYCXSWiKPr5FJdaiaL+VJOKeDG4XKop3qefkO/b7b64pp4IKo5TE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de; spf=pass smtp.mailfrom=wunner.de; arc=none smtp.client-ip=176.9.242.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wunner.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=wunner.de Received: from h08.hostsharing.net (h08.hostsharing.net [IPv6:2a01:37:1000::53df:5f1c:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS RSA CA G1" (verified OK)) by bmailout3.hostsharing.net (Postfix) with ESMTPS id BCB0B100E2020; Sun, 15 Dec 2024 11:33:19 +0100 (CET) Received: by h08.hostsharing.net (Postfix, from userid 100393) id 872053D6A2C; Sun, 15 Dec 2024 11:33:19 +0100 (CET) Message-ID: <2292e75dcf26f1c6d7c1f715edfe0e49f079149d.1734257330.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sun, 15 Dec 2024 11:20:53 +0100 Subject: [PATCH for-linus v2 3/3] PCI/bwctrl: Enable only if more than one speed is supported Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: Bjorn Helgaas Cc: linux-pci@vger.kernel.org, Niklas Schnelle , Ilpo Jarvinen , Jonathan Cameron , Mika Westerberg , "Maciej W. Rozycki" , Mario Limonciello If a PCIe port only supports a single speed, enabling bandwidth control is pointless: There's no need to monitor autonomous speed changes, nor can the speed be changed. Not enabling it saves a small amount of memory and compute resources, but also fixes a boot hang reported by Niklas: It occurs when enabling bandwidth control on Downstream Ports of Intel JHL7540 "Titan Ridge 2018" Thunderbolt controllers. The ports only support 2.5 GT/s in accordance with USB4 v2 sec 11.2.1, so the present commit works around the issue. PCIe r6.2 sec 8.2.1 prescribes that: "A device must support 2.5 GT/s and is not permitted to skip support for any data rates between 2.5 GT/s and the highest supported rate." Consequently, bandwidth control is currently only disabled if a port doesn't support higher speeds than 2.5 GT/s. However the Implementation Note in PCIe r6.2 sec 7.5.3.18 cautions: "It is strongly encouraged that software primarily utilize the Supported Link Speeds Vector instead of the Max Link Speed field, so that software can determine the exact set of supported speeds on current and future hardware. This can avoid software being confused if a future specification defines Links that do not require support for all slower speeds." In other words, future revisions of the PCIe Base Spec may allow gaps in the Supported Link Speeds Vector. To be future-proof, don't just check whether speeds above 2.5 GT/s are supported, but rather check whether *more than one* speed is supported. Fixes: 665745f27487 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller") Reported-by: Niklas Schnelle Closes: https://lore.kernel.org/r/db8e457fcd155436449b035e8791a8241b0df400.camel@kernel.org/ Signed-off-by: Lukas Wunner Cc: Ilpo Järvinen Tested-by: Niklas Schnelle Reviewed-by: Jonathan Cameron Reviewed-by: Mario Limonciello --- drivers/pci/pcie/portdrv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c index 5e10306b6308..02e73099bad0 100644 --- a/drivers/pci/pcie/portdrv.c +++ b/drivers/pci/pcie/portdrv.c @@ -265,12 +265,14 @@ static int get_port_device_capability(struct pci_dev *dev) (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER))) services |= PCIE_PORT_SERVICE_DPC; + /* Enable bandwidth control if more than one speed is supported. */ if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) { u32 linkcap; pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &linkcap); - if (linkcap & PCI_EXP_LNKCAP_LBNC) + if (linkcap & PCI_EXP_LNKCAP_LBNC && + hweight8(dev->supported_speeds) > 1) services |= PCIE_PORT_SERVICE_BWCTRL; }