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@xxxxxxxxx> 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);
+ }
}
/**
