From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Lukas Wunner <lukas@wunner.de>
Cc: Esther Shimanovich <eshimanovich@chromium.org>,
Mario Limonciello <mario.limonciello@amd.com>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Rajat Jain <rajatja@google.com>
Subject: Re: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8
Date: Fri, 10 May 2024 08:26:16 +0300 [thread overview]
Message-ID: <20240510052616.GC4162345@black.fi.intel.com> (raw)
In-Reply-To: <ZjsKPSgV39SF0gdX@wunner.de>
Hi,
On Wed, May 08, 2024 at 07:14:37AM +0200, Lukas Wunner wrote:
> On Wed, May 01, 2024 at 06:23:28PM -0400, Esther Shimanovich wrote:
> > On Sat, Apr 27, 2024 at 3:17AM Lukas Wunner <lukas@wunner.de> wrote:
> > That is correct, when the user-visible issue occurs, no driver is
> > bound to the NHI and XHCI. The discrete JHL chip is not permitted to
> > attach to the external-facing root port because of the security
> > policy, so the NHI and XHCI are not seen by the computer.
>
> Could you rework your patch to only rectify the NHI's and XHCI's
> device properties and leave the bridges untouched?
As an alternative, I also spent some time to figure out whether there is
a way to detect the integrated Thunderbolt PCIe Root Ports and turns out
it is not "impossible" at least :) Basically it is a list of Ice Lake
and Tiger Lake Thunderbolt PCIe Root Ports. Everything after this is
using the "usb4-host-interface" device property.
I went a head and did a patch that, instead of relabeling, sets the
"untrusted" and "removable" flags based on this and some heuristics (to
figure out the discrete controller) directly on the source. I did some
testing over the hardware I have here and it sets the flags like this:
- Everything below integrated Thunderbolt/USB4 PCIe root ports are
marked as "untrusted" and "removable", this includes the Ice Lake
and Tiger Lake ones. Whereas the NHI and xHCI here are untouched.
- Everything below discrete Thunderbolt/USB4 host controller PCIe
downstream ports that are behind a PCIe Root Port with
"external_facing" set are marked as "untrusted" and "removable"
whereas endpoints are still "trusted" and "fixed".
I'm sharing the code below. @Esther, you may use it as you like, parts
of it or just ignore the whole thing completely.
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 1325fbae2f28..38bc80c931d6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1612,6 +1612,127 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
dev->is_thunderbolt = 1;
}
+static bool pcie_switch_directly_under(struct pci_dev *bridge,
+ struct pci_dev *parent,
+ struct pci_dev *pdev)
+{
+ u64 serial, upstream_serial;
+
+ /*
+ * Check the type of the device to make sure it is part of a PCIe
+ * switch and if it is try to match the serial numbers too with
+ * the assumption that they all share the same serial number.
+ */
+ switch (pci_pcie_type(pdev)) {
+ case PCI_EXP_TYPE_UPSTREAM:
+ if (parent == bridge)
+ return pci_get_dsn(pdev) != 0;
+ break;
+
+ case PCI_EXP_TYPE_DOWNSTREAM:
+ if (pci_pcie_type(parent) == PCI_EXP_TYPE_UPSTREAM) {
+ upstream_serial = pci_get_dsn(parent);
+ if (!upstream_serial)
+ return false;
+ serial = pci_get_dsn(pdev);
+ if (!serial)
+ return false;
+ if (serial != upstream_serial)
+ return false;
+ parent = pci_upstream_bridge(parent);
+ if (parent == bridge)
+ return true;
+ }
+ break;
+
+ case PCI_EXP_TYPE_ENDPOINT:
+ if (pci_pcie_type(parent) == PCI_EXP_TYPE_DOWNSTREAM) {
+ serial = pci_get_dsn(parent);
+ if (!serial)
+ return false;
+ parent = pci_upstream_bridge(parent);
+ if (parent && pci_pcie_type(parent) == PCI_EXP_TYPE_UPSTREAM) {
+ upstream_serial = pci_get_dsn(parent);
+ if (!upstream_serial)
+ return false;
+ if (serial != upstream_serial)
+ return false;
+ parent = pci_upstream_bridge(parent);
+ if (parent == bridge)
+ return true;
+ }
+ }
+ break;
+ }
+
+ return false;
+}
+
+static bool pcie_has_usb4_host_interface(struct pci_dev *pdev)
+{
+ struct fwnode_handle *fwnode;
+
+ /*
+ * For USB4 the tunneled PCIe root or downstream ports are marked with
+ * the "usb4-host-interface" property so we look for that first. This
+ * should cover the most cases.
+ */
+ fwnode = fwnode_find_reference(dev_fwnode(&pdev->dev),
+ "usb4-host-interface", 0);
+ if (!IS_ERR(fwnode)) {
+ fwnode_handle_put(fwnode);
+ return true;
+ }
+
+ /*
+ * Any integrated Thunderbolt 3/4 PCIe root ports from Intel
+ * before Alder Lake do not have the above device property so we
+ * use their PCI IDs instead. All these are tunneled. This list
+ * is not expected to grow.
+ */
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
+ switch (pdev->device) {
+ /* Ice Lake Thunderbolt 3 PCIe Root Ports */
+ case 0x8a1d:
+ case 0x8a1f:
+ case 0x8a21:
+ case 0x8a23:
+ /* Tiger Lake-LP Thunderbolt 4 PCIe Root Ports */
+ case 0x9a23:
+ case 0x9a25:
+ case 0x9a27:
+ case 0x9a29:
+ /* Tiger Lake-H Thunderbolt 4 PCIe Root Ports */
+ case 0x9a2b:
+ case 0x9a2d:
+ case 0x9a2f:
+ case 0x9a31:
+ return true;
+ }
+ }
+
+ return false;
+}
+
+/* root->external_facing is true, parent != NULL */
+static bool pcie_is_tunneled(struct pci_dev *root, struct pci_dev *parent,
+ struct pci_dev *pdev)
+{
+ /* Anything directly behind a "usb4-host-interface" is tunneled */
+ if (pcie_has_usb4_host_interface(parent))
+ return true;
+
+ /*
+ * Check if this is a discrete Thunderbolt/USB4 controller that is
+ * directly behind a PCIe Root Port marked as "ExternalFacingPort".
+ * These are not behind a PCIe tunnel.
+ */
+ if (pcie_switch_directly_under(root, parent, pdev))
+ return false;
+
+ return true;
+}
+
static void set_pcie_untrusted(struct pci_dev *dev)
{
struct pci_dev *parent;
@@ -1621,8 +1742,32 @@ static void set_pcie_untrusted(struct pci_dev *dev)
* untrusted as well.
*/
parent = pci_upstream_bridge(dev);
- if (parent && (parent->untrusted || parent->external_facing))
- dev->untrusted = true;
+ if (parent) {
+ struct pci_dev *root;
+
+ /* If parent is untrusted so are we */
+ if (parent->untrusted) {
+ pci_dbg(dev, "marking as untrusted\n");
+ dev->untrusted = true;
+ return;
+ }
+
+ root = pcie_find_root_port(dev);
+ if (root && root->external_facing) {
+ /*
+ * Only PCIe root ports can be marked as
+ * "ExternalFacingPort", However, in case of a
+ * discrete Thunderbolt/USB4 controller only its
+ * downstream facing ports are actually
+ * something that are exposed to the wild so we
+ * only mark devices behind those as untrusted.
+ */
+ if (pcie_is_tunneled(root, parent, dev)) {
+ pci_dbg(dev, "marking as untrusted\n");
+ dev->untrusted = true;
+ }
+ }
+ }
}
static void pci_set_removable(struct pci_dev *dev)
@@ -1639,10 +1784,15 @@ static void pci_set_removable(struct pci_dev *dev)
* the port is marked with external_facing, such devices are less
* accessible to user / may not be removed by end user, and thus not
* exposed as "removable" to userspace.
+ *
+ * These are the same devices marked as untrusted by the above
+ * function. The ports and endpoints part of the discrete
+ * Thunderbolt/USB4 controller are not marked as removable.
*/
- if (parent &&
- (parent->external_facing || dev_is_removable(&parent->dev)))
+ if (dev->untrusted || (parent && dev_is_removable(&parent->dev))) {
+ pci_dbg(dev, "marking as removable\n");
dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
+ }
}
/**
next prev parent reply other threads:[~2024-05-10 5:26 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-21 20:53 [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8 Esther Shimanovich
2023-12-21 23:15 ` 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
2024-05-08 5:14 ` Lukas Wunner
2024-05-10 5:26 ` Mika Westerberg [this message]
2024-05-10 15:44 ` Esther Shimanovich
2024-05-11 4:38 ` Mika Westerberg
2024-05-11 5:43 ` Mika Westerberg
2024-05-15 18:53 ` Esther Shimanovich
2024-05-15 20:35 ` Lukas Wunner
2024-05-15 20:51 ` Lukas Wunner
2024-05-15 21:44 ` Esther Shimanovich
2024-05-16 8:30 ` Mika Westerberg
2024-05-16 10:03 ` Mika Westerberg
2024-05-16 9:16 ` 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=20240510052616.GC4162345@black.fi.intel.com \
--to=mika.westerberg@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=dmitry.torokhov@gmail.com \
--cc=eshimanovich@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=mario.limonciello@amd.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).