All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Esther Shimanovich <eshimanovich@chromium.org>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Rajat Jain <rajatja@google.com>,
	 Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	 Esther Shimanovich <eshimanovich@chromium.org>
Subject: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8
Date: Thu, 21 Dec 2023 15:53:42 -0500	[thread overview]
Message-ID: <20231221-thunderbolt-pci-patch-4-v4-1-2e136e57c9bc@chromium.org> (raw)

On Lenovo X1 Carbon Gen 7/8 devices, when a platform enables a policy to
distrust removable PCI devices, the build-in USB-C ports stop working at
all.
This happens because these X1 Carbon models have a unique feature; a
Thunderbolt controller that is discrete from the SoC. The software sees
this controller, and incorrectly assumes it is a removable PCI device,
even though it is fixed to the computer and is wired to the computer's
own USB-C ports.

Relabel all the components of the JHL6540 controller as DEVICE_FIXED,
and where applicable, external_facing.

Ensure that the security policy to distrust external PCI devices works
as intended, and that the device's USB-C ports are able to enumerate
even when the policy is enabled.

Signed-off-by: Esther Shimanovich <eshimanovich@chromium.org>
---
Changes in v4:
- replaced a dmi check in the rootport quirk with a subsystem vendor and
  device check.
- Link to v3: https://lore.kernel.org/r/20231220-thunderbolt-pci-patch-4-v3-1-056fd1717d06@chromium.org

Changes in v3:
- removed redundant dmi check, as the subsystem vendor check is
  sufficient
- switched to PCI_VENDOR_ID_LENOVO instead of hex code
- Link to v2: https://lore.kernel.org/r/20231219-thunderbolt-pci-patch-4-v2-1-ec2d7af45a9b@chromium.org

Changes in v2:
- nothing new, v1 was just a test run to see if the ASCII diagram would
  be rendered properly in mutt and k-9
- for folks using gmail, make sure to select "show original" on the top
  right, as otherwise the diagram will be garbled by the standard
  non-monospace font
- Link to v1: https://lore.kernel.org/r/20231219-thunderbolt-pci-patch-4-v1-1-4e8e3773f0a9@chromium.org
---
 drivers/pci/quirks.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index ea476252280a..34e43323ff14 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3873,6 +3873,118 @@ DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
 			       quirk_apple_poweroff_thunderbolt);
 #endif
 
+/*
+ * On most ThinkPad Carbon 7/8s, JHL6540 Thunderbolt 3 bridges are set
+ * incorrectly as DEVICE_REMOVABLE despite being built into the device.
+ * This is the side effect of a unique hardware configuration.
+ *
+ * Normally, Thunderbolt functionality is integrated to the SoC and
+ * its root ports.
+ *
+ *                          Most devices:
+ *                    root port --> USB-C port
+ *
+ * But X1 Carbon Gen 7/8 uses Whiskey Lake and Comet Lake SoC, which
+ * don't come with Thunderbolt functionality. Therefore, a discrete
+ * Thunderbolt Host PCI controller was added between the root port and
+ * the USB-C port.
+ *
+ *                        Thinkpad Carbon 7/8s
+ *                 (w/ Whiskey Lake and Comet Lake SoC):
+ *                root port -->  JHL6540   --> USB-C port
+ *
+ * Because the root port is labeled by FW as "ExternalFacingPort", as
+ * required by the DMAR ACPI spec, the JHL6540 chip is inaccurately
+ * labeled as DEVICE_REMOVABLE by the kernel pci driver.
+ * Therefore, the built-in USB-C ports do not enumerate when policies
+ * forbidding external pci devices are enforced.
+ *
+ * This fix relabels the pci components in the built-in JHL6540 chip as
+ * DEVICE_FIXED, ensuring that the built-in USB-C ports always enumerate
+ * properly as intended.
+ *
+ * This fix also labels the external facing components of the JHL6540 as
+ * external_facing, so that the pci attach policy works as intended.
+ *
+ * The ASCII diagram below describes the pci layout of the JHL6540 chip.
+ *
+ *                         Root Port
+ *                 [8086:02b4] or [8086:9db4]
+ *                             |
+ *                        JHL6540 Chip
+ *     __________________________________________________
+ *    |                      Bridge                      |
+ *    |        PCI ID ->  [8086:15d3]                    |
+ *    |         DEVFN ->      (00)                       |
+ *    |       _________________|__________________       |
+ *    |      |           |            |           |      |
+ *    |    Bridge     Bridge        Bridge      Bridge   |
+ *    | [8086:15d3] [8086:15d3]  [8086:15d3] [8086:15d3] |
+ *    |    (00)        (08)         (10)        (20)     |
+ *    |      |           |            |           |      |
+ *    |     NHI          |     USB Controller     |      |
+ *    | [8086:15d2]      |       [8086:15d4]      |      |
+ *    |    (00)          |          (00)          |      |
+ *    |      |___________|            |___________|      |
+ *    |____________|________________________|____________|
+ *                 |                        |
+ *             USB-C Port               USB-C Port
+ *
+ *
+ * Based on what a JHL6549 pci component's pci id, subsystem device id
+ * and devfn are, we can infer if it is fixed and if it faces a usb port;
+ * which would mean it is external facing.
+ * This quirk uses these values to identify the pci components and set the
+ * properties accordingly.
+ */
+static void carbon_X1_fixup_relabel_alpine_ridge(struct pci_dev *dev)
+{
+	/* Is this JHL6540 PCI component embedded in a Lenovo device? */
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_LENOVO)
+		return;
+
+	/* Is this JHL6540 PCI component embedded in an X1 Carbon Gen 7/8? */
+	if (dev->subsystem_device != 0x22be && // Gen 8
+	    dev->subsystem_device != 0x2292) { // Gen 7
+		return;
+	}
+
+	dev_set_removable(&dev->dev, DEVICE_FIXED);
+
+	/* Not all 0x15d3 components are external facing */
+	if (dev->device == 0x15d3 &&
+	    dev->devfn != 0x08 &&
+	    dev->devfn != 0x20) {
+		return;
+	}
+
+	dev->external_facing = true;
+}
+
+/*
+ * We also need to relabel the root port as a consequence of changing
+ * the JHL6540's PCIE hierarchy.
+ */
+static void carbon_X1_fixup_rootport_not_removable(struct pci_dev *dev)
+{
+	/* Is this JHL6540 PCI component embedded in a Lenovo device? */
+	if (dev->subsystem_vendor != PCI_VENDOR_ID_LENOVO)
+		return;
+
+	/* Is this JHL6540 PCI component embedded in an X1 Carbon Gen 7/8? */
+	if (dev->subsystem_device != 0x22be && // Gen 8
+	    dev->subsystem_device != 0x2292) { // Gen 7
+		return;
+	}
+
+	dev->external_facing = false;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x15d3, carbon_X1_fixup_relabel_alpine_ridge);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x15d2, carbon_X1_fixup_relabel_alpine_ridge);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x15d4, carbon_X1_fixup_relabel_alpine_ridge);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x02b4, carbon_X1_fixup_rootport_not_removable);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x9db4, carbon_X1_fixup_rootport_not_removable);
+
 /*
  * Following are device-specific reset methods which can be used to
  * reset a single function if other methods (e.g. FLR, PM D0->D3) are

---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20231219-thunderbolt-pci-patch-4-ede71cb833c4

Best regards,
-- 
Esther Shimanovich <eshimanovich@chromium.org>


             reply	other threads:[~2023-12-21 20:53 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21 20:53 Esther Shimanovich [this message]
2023-12-21 23:15 ` [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8 Dmitry Torokhov
2023-12-27  0:15 ` Bjorn Helgaas
2023-12-28 13:25 ` Lukas Wunner
2023-12-28 13:39   ` Mika Westerberg
2024-01-17 21:21     ` Esther Shimanovich
2024-01-18  6:00       ` Mika Westerberg
2024-01-18 15:47         ` Mario Limonciello
2024-01-18 16:12           ` Dmitry Torokhov
2024-01-18 16:21             ` Dmitry Torokhov
2024-01-19  5:37             ` Mika Westerberg
2024-01-19  7:48               ` Mika Westerberg
2024-01-19 10:22                 ` Mika Westerberg
2024-01-19 16:03                   ` Esther Shimanovich
2024-01-22  6:10                     ` Mika Westerberg
2024-01-22 23:50                   ` Mario Limonciello
2024-01-23  6:18                     ` Mika Westerberg
2024-01-25 23:45                       ` Esther Shimanovich
2024-04-15 22:34                         ` Esther Shimanovich
2024-04-16  5:03                           ` Mika Westerberg
2024-04-18 19:43                             ` Esther Shimanovich
2024-04-19  4:49                               ` Mika Westerberg
2024-04-22 19:17                                 ` Esther Shimanovich
2024-04-22 19:21                                   ` Mario Limonciello
2024-04-23  5:33                                     ` Mika Westerberg
2024-04-23  8:31                                       ` Lukas Wunner
2024-04-23  8:40                                         ` Mika Westerberg
2024-04-23 16:59                                       ` Mario Limonciello
2024-04-24  8:56                                         ` Mika Westerberg
2024-04-25 21:16                                           ` Esther Shimanovich
2024-04-26  4:52                                             ` Mika Westerberg
2024-04-26 15:58                                               ` Esther Shimanovich
2024-04-27  5:35                                               ` Lukas Wunner
2024-04-27  7:41                                                 ` Mika Westerberg
2024-04-27  7:08                                             ` Lukas Wunner
2024-04-27 15:09                                             ` Lukas Wunner
2024-05-01 22:23                                               ` Esther Shimanovich
2024-05-02  4:38                                                 ` Mika Westerberg
2024-05-02  9:54                                                   ` Mario Limonciello
2024-05-02 10:07                                                     ` Mika Westerberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231221-thunderbolt-pci-patch-4-v4-1-2e136e57c9bc@chromium.org \
    --to=eshimanovich@chromium.org \
    --cc=bhelgaas@google.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rajatja@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.