thr3ads.net - Nouveau - [Nouveau] [PATCH v3 07/12] PCI: Set ports for discrete USB4 controllers appropriately [Feb 2022]

If this information is useful, please help other people find it:
Share via:

Mario Limonciello

2022-Feb-11 19:32 UTC

[Nouveau] [PATCH v3 00/12] Overhaul `is_thunderbolt`

Various drivers in the kernel use `is_thunderbolt` or
`pci_is_thunderbolt_attached` to designate behaving differently
from a device that is internally in the machine. This relies upon checks
for a specific capability only set on Intel controllers.

Non-Intel USB4 designs should also match this designation so that they
can be treated the same regardless of the host they're connected to.

As part of adding the generic USB4 controller code, it was realized that
`is_thunderbolt` and `pcie_is_thunderbolt_attached` have been overloaded.

Instead migrate to using removable attribute from device core.

Changes from v2->v3:
- Add various tags for patches that haven't changed from v2->v3
- Add new patches for Mika's suggestions:
  * Moving Apple Thunderbolt D3 declaration into quirks
  * Detect PCIe root port used for PCIe tunneling on integrated controllers
    using `usb4-host-interface`
  * Detect PCIe root port used for PCIe tunneling on discrete controllers
    using the USB4 DVSEC specification

Changes from v1->v2:
 - Add Alex's tag to first patch
 - Move lack of command completion into a quirk (Lukas)
 - Drop `is_thunderbolt` attribute and `pci_is_thunderbolt_attached` and
   use device core removable attribute instead
 - Adjust all consumers of old attribute to use removable

Note: this spans USB/DRM/platform-x86/PCI trees.
As a majority of the changes are in PCI, it should probably come through
that tree if possible.

Mario Limonciello (12):
  thunderbolt: move definition of PCI_CLASS_SERIAL_USB_USB4
  PCI: Move `is_thunderbolt` check for lack of command completed to a
    quirk
  PCI: Move check for old Apple Thunderbolt controllers into a quirk
  PCI: Drop the `is_thunderbolt` attribute from PCI core
  PCI: Detect root port of internal USB4 devices by
    `usb4-host-interface`
  PCI: Explicitly mark USB4 NHI devices as removable
  PCI: Set ports for discrete USB4 controllers appropriately
  drm/amd: drop the use of `pci_is_thunderbolt_attached`
  drm/nouveau: drop the use of `pci_is_thunderbolt_attached`
  drm/radeon: drop the use of `pci_is_thunderbolt_attached`
  platform/x86: amd-gmux: drop the use of `pci_is_thunderbolt_attached`
  PCI: drop `pci_is_thunderbolt_attached`

 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_vga.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_device.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_kms.c     |  2 +-
 drivers/pci/hotplug/pciehp_hpc.c        |  6 +--
 drivers/pci/pci-acpi.c                  | 10 ++++
 drivers/pci/pci.c                       | 12 +++--
 drivers/pci/pci.h                       |  5 ++
 drivers/pci/probe.c                     | 55 ++++++++++++++-----
 drivers/pci/quirks.c                    | 70 +++++++++++++++++++++++++
 drivers/platform/x86/apple-gmux.c       |  2 +-
 drivers/thunderbolt/nhi.h               |  2 -
 include/linux/pci.h                     | 25 +--------
 include/linux/pci_ids.h                 |  3 ++
 15 files changed, 148 insertions(+), 56 deletions(-)

-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 01/12] thunderbolt: move definition of PCI_CLASS_SERIAL_USB_USB4

This PCI class definition of the USB4 device is currently located only in
the thunderbolt driver.

It will be needed by a few other drivers for upcoming changes. Move it into
the common include file.

Acked-by: Alex Deucher <alexander.deucher at amd.com>
Acked-by: Mika Westerberg <mika.westerberg at linux.intel.com>
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/thunderbolt/nhi.h | 2 --
 include/linux/pci_ids.h   | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index 69083aab2736..79e980b51f94 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -81,6 +81,4 @@ extern const struct tb_nhi_ops icl_nhi_ops;
 #define PCI_DEVICE_ID_INTEL_TGL_H_NHI0			0x9a1f
 #define PCI_DEVICE_ID_INTEL_TGL_H_NHI1			0x9a21
 
-#define PCI_CLASS_SERIAL_USB_USB4			0x0c0340
-
 #endif
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index aad54c666407..61b161d914f0 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -116,6 +116,7 @@
 #define PCI_CLASS_SERIAL_USB_OHCI	0x0c0310
 #define PCI_CLASS_SERIAL_USB_EHCI	0x0c0320
 #define PCI_CLASS_SERIAL_USB_XHCI	0x0c0330
+#define PCI_CLASS_SERIAL_USB_USB4	0x0c0340
 #define PCI_CLASS_SERIAL_USB_DEVICE	0x0c03fe
 #define PCI_CLASS_SERIAL_FIBER		0x0c04
 #define PCI_CLASS_SERIAL_SMBUS		0x0c05
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 02/12] PCI: Move `is_thunderbolt` check for lack of command completed to a quirk

The `is_thunderbolt` check is currently used to indicate the lack of
command completed support for a number of older Thunderbolt devices.

This however is heavy handed and should have been done via a quirk.  Move
the affected devices outlined in commit 493fb50e958c ("PCI: pciehp: Assume
NoCompl+ for Thunderbolt ports") into pci quirks.

Suggested-by: Lukas Wunner <lukas at wunner.de>
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/pci/hotplug/pciehp_hpc.c |  6 +-----
 drivers/pci/quirks.c             | 17 +++++++++++++++++
 include/linux/pci.h              |  2 ++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 1c1ebf3dad43..e4c42b24aba8 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -996,11 +996,7 @@ struct controller *pcie_init(struct pcie_device *dev)
 	if (pdev->hotplug_user_indicators)
 		slot_cap &= ~(PCI_EXP_SLTCAP_AIP | PCI_EXP_SLTCAP_PIP);
 
-	/*
-	 * We assume no Thunderbolt controllers support Command Complete events,
-	 * but some controllers falsely claim they do.
-	 */
-	if (pdev->is_thunderbolt)
+	if (pdev->no_cmd_complete)
 		slot_cap |= PCI_EXP_SLTCAP_NCCS;
 
 	ctrl->slot_cap = slot_cap;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index d2dd6a6cda60..6d3c88edde00 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3675,6 +3675,23 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PORT_RIDGE,
 			quirk_thunderbolt_hotplug_msi);
 
+static void quirk_thunderbolt_command_completed(struct pci_dev *pdev)
+{
+	pdev->no_cmd_complete = 1;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LIGHT_RIDGE,
+			quirk_thunderbolt_command_completed);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EAGLE_RIDGE,
+			quirk_thunderbolt_command_completed);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LIGHT_PEAK,
+			quirk_thunderbolt_command_completed);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
+			quirk_thunderbolt_command_completed);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_2C,
+			quirk_thunderbolt_command_completed);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PORT_RIDGE,
+			quirk_thunderbolt_command_completed);
+
 #ifdef CONFIG_ACPI
 /*
  * Apple: Shutdown Cactus Ridge Thunderbolt controller.
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8253a5413d7c..1e5b769e42fc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -443,6 +443,8 @@ struct pci_dev {
 	unsigned int	is_hotplug_bridge:1;
 	unsigned int	shpc_managed:1;		/* SHPC owned by shpchp */
 	unsigned int	is_thunderbolt:1;	/* Thunderbolt controller */
+	unsigned int	no_cmd_complete:1;	/* Lies about command completed events */
+
 	/*
 	 * Devices marked being untrusted are the ones that can potentially
 	 * execute DMA attacks and similar. They are typically connected
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 03/12] PCI: Move check for old Apple Thunderbolt controllers into a quirk

`pci_bridge_d3_possible` currently checks explicitly for a Thunderbolt
controller to indicate that D3 is possible.  As this is used solely
for older Apple systems, move it into a quirk that enumerates across
all Intel TBT controllers.

Suggested-by: Mika Westerberg <mika.westerberg at linux.intel.com>
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/pci/pci.c    | 12 +++++-----
 drivers/pci/quirks.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9ecce435fb3f..5002e214c9a6 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1064,7 +1064,13 @@ static inline bool platform_pci_bridge_d3(struct pci_dev
*dev)
 	if (pci_use_mid_pm())
 		return false;
 
-	return acpi_pci_bridge_d3(dev);
+	if (acpi_pci_bridge_d3(dev))
+		return true;
+
+	if (device_property_read_bool(&dev->dev,
"HotPlugSupportInD3"))
+		return true;
+
+	return false;
 }
 
 /**
@@ -2954,10 +2960,6 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
 		if (pci_bridge_d3_force)
 			return true;
 
-		/* Even the oldest 2010 Thunderbolt controller supports D3. */
-		if (bridge->is_thunderbolt)
-			return true;
-
 		/* Platform might know better if the bridge supports D3 */
 		if (platform_pci_bridge_d3(bridge))
 			return true;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6d3c88edde00..aaf098ca7d54 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3756,6 +3756,59 @@ DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
 			       quirk_apple_poweroff_thunderbolt);
 #endif
 
+/* Apple machines as old as 2010 can do D3 with Thunderbolt controllers, but
don't specify
+ * it in the ACPI tables
+ */
+static void quirk_apple_d3_thunderbolt(struct pci_dev *dev)
+{
+	struct property_entry properties[] = {
+		PROPERTY_ENTRY_BOOL("HotPlugSupportInD3"),
+		{},
+	};
+
+	if (!x86_apple_machine)
+		return;
+
+	if (device_create_managed_software_node(&dev->dev, properties, NULL))
+		pci_warn(dev, "could not add HotPlugSupportInD3 property");
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LIGHT_RIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EAGLE_RIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LIGHT_PEAK,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_2C,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PORT_RIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_REDWOOD_RIDGE_2C_NHI,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_REDWOOD_RIDGE_2C_BRIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_REDWOOD_RIDGE_4C_NHI,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_REDWOOD_RIDGE_4C_BRIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_NHI,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_NHI,
+			quirk_apple_d3_thunderbolt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE,
+			quirk_apple_d3_thunderbolt);
+
 /*
  * 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
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 04/12] PCI: Drop the `is_thunderbolt` attribute from PCI core

The `is_thunderbolt` attribute is currently a dumping ground for a
variety of things.

Instead use the driver core removable attribute to indicate the
detail a device is attached to a thunderbolt or USB4 chain.

Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/pci/probe.c               | 20 +++++++-------------
 drivers/platform/x86/apple-gmux.c |  2 +-
 include/linux/pci.h               |  5 ++---
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 17a969942d37..e41656cdd8f0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1577,16 +1577,6 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev)
 		pdev->is_hotplug_bridge = 1;
 }
 
-static void set_pcie_thunderbolt(struct pci_dev *dev)
-{
-	u16 vsec;
-
-	/* Is the device part of a Thunderbolt controller? */
-	vsec = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL,
PCI_VSEC_ID_INTEL_TBT);
-	if (vsec)
-		dev->is_thunderbolt = 1;
-}
-
 static void set_pcie_untrusted(struct pci_dev *dev)
 {
 	struct pci_dev *parent;
@@ -1603,6 +1593,10 @@ static void set_pcie_untrusted(struct pci_dev *dev)
 static void pci_set_removable(struct pci_dev *dev)
 {
 	struct pci_dev *parent = pci_upstream_bridge(dev);
+	u16 vsec;
+
+	/* Is the device a Thunderbolt controller? */
+	vsec = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL,
PCI_VSEC_ID_INTEL_TBT);
 
 	/*
 	 * We (only) consider everything downstream from an external_facing
@@ -1615,8 +1609,9 @@ static void pci_set_removable(struct pci_dev *dev)
 	 * accessible to user / may not be removed by end user, and thus not
 	 * exposed as "removable" to userspace.
 	 */
-	if (parent &&
-	    (parent->external_facing || dev_is_removable(&parent->dev)))
+	if (vsec ||
+	    (parent &&
+	    (parent->external_facing || dev_is_removable(&parent->dev))))
 		dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
 }
 
@@ -1860,7 +1855,6 @@ int pci_setup_device(struct pci_dev *dev)
 	dev->cfg_size = pci_cfg_space_size(dev);
 
 	/* Need to have dev->cfg_size ready */
-	set_pcie_thunderbolt(dev);
 
 	set_pcie_untrusted(dev);
 
diff --git a/drivers/platform/x86/apple-gmux.c
b/drivers/platform/x86/apple-gmux.c
index 57553f9b4d1d..04232fbc7d56 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -596,7 +596,7 @@ static int gmux_resume(struct device *dev)
 
 static int is_thunderbolt(struct device *dev, void *data)
 {
-	return to_pci_dev(dev)->is_thunderbolt;
+	return pci_is_thunderbolt_attached(to_pci_dev(dev));
 }
 
 static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1e5b769e42fc..d9719eb14654 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -442,7 +442,6 @@ struct pci_dev {
 	unsigned int	is_virtfn:1;
 	unsigned int	is_hotplug_bridge:1;
 	unsigned int	shpc_managed:1;		/* SHPC owned by shpchp */
-	unsigned int	is_thunderbolt:1;	/* Thunderbolt controller */
 	unsigned int	no_cmd_complete:1;	/* Lies about command completed events */
 
 	/*
@@ -2447,11 +2446,11 @@ static inline bool pci_is_thunderbolt_attached(struct
pci_dev *pdev)
 {
 	struct pci_dev *parent = pdev;
 
-	if (pdev->is_thunderbolt)
+	if (dev_is_removable(&pdev->dev))
 		return true;
 
 	while ((parent = pci_upstream_bridge(parent)))
-		if (parent->is_thunderbolt)
+		if (dev_is_removable(&parent->dev))
 			return true;
 
 	return false;
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`

The root port used for PCIe tunneling should be marked as removable to
ensure that the entire chain is marked removable.

This can be done by looking for the device property specified in
the ACPI tables `usb4-host-interface`.

Suggested-by: Mika Westerberg <mika.westerberg at linux.intel.com>
Link:
https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#mapping-native-protocols-pcie-displayport-tunneled-through-usb4-to-usb4-host-routers
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/pci/pci-acpi.c | 10 ++++++++++
 drivers/pci/pci.h      |  5 +++++
 drivers/pci/probe.c    |  1 +
 3 files changed, 16 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index a42dbf448860..6368e5633b1b 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1391,6 +1391,16 @@ void pci_acpi_cleanup(struct device *dev, struct
acpi_device *adev)
 	}
 }
 
+bool pci_acpi_is_usb4(struct pci_dev *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
+
+	if (!adev)
+		return false;
+	return fwnode_property_present(acpi_fwnode_handle(adev),
+				       "usb4-host-interface");
+}
+
 static struct fwnode_handle *(*pci_msi_get_fwnode_cb)(struct device *dev);
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 3d60cabde1a1..359607c0542d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -695,6 +695,7 @@ void acpi_pci_refresh_power_state(struct pci_dev *dev);
 int acpi_pci_wakeup(struct pci_dev *dev, bool enable);
 bool acpi_pci_need_resume(struct pci_dev *dev);
 pci_power_t acpi_pci_choose_state(struct pci_dev *pdev);
+bool pci_acpi_is_usb4(struct pci_dev *dev);
 #else
 static inline int pci_dev_acpi_reset(struct pci_dev *dev, bool probe)
 {
@@ -734,6 +735,10 @@ static inline pci_power_t acpi_pci_choose_state(struct
pci_dev *pdev)
 {
 	return PCI_POWER_ERROR;
 }
+static inline bool pci_acpi_is_usb4(struct pci_dev *dev)
+{
+	return false;
+}
 #endif
 
 #ifdef CONFIG_PCIEASPM
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e41656cdd8f0..2693211d31cf 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1610,6 +1610,7 @@ static void pci_set_removable(struct pci_dev *dev)
 	 * exposed as "removable" to userspace.
 	 */
 	if (vsec ||
+	    pci_acpi_is_usb4(dev) ||
 	    (parent &&
 	    (parent->external_facing || dev_is_removable(&parent->dev))))
 		dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 06/12] PCI: Explicitly mark USB4 NHI devices as removable

USB4 class devices are also removable like Intel Thunderbolt devices.

Drivers of downstream devices use this information to declare functional
differences in how the drivers perform by knowing that they are connected
to an upstream TBT/USB4 port.

Reviewed-by: Macpaul Lin <macpaul.lin at mediatek.com>
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/pci/probe.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2693211d31cf..67ca33188cba 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1610,6 +1610,7 @@ static void pci_set_removable(struct pci_dev *dev)
 	 * exposed as "removable" to userspace.
 	 */
 	if (vsec ||
+	    dev->class == PCI_CLASS_SERIAL_USB_USB4 ||
 	    pci_acpi_is_usb4(dev) ||
 	    (parent &&
 	    (parent->external_facing || dev_is_removable(&parent->dev))))
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 07/12] PCI: Set ports for discrete USB4 controllers appropriately

Discrete USB4 controllers won't have ACPI nodes specifying which
PCIe or XHCI port they are linked with.

In order to set the removable attribute appropriately, use the
USB4 DVSEC extended capabability set on these root ports to determine
if they are located on a discrete USB4 controller.

Suggested-by: Mika Westerberg <mika.westerberg at linux.intel.com>
Link: https://usb.org/sites/default/files/USB4%20Specification%2020211116.zip
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/pci/probe.c     | 33 +++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 67ca33188cba..1ed3e24db11e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -25,6 +25,8 @@
 #define CARDBUS_LATENCY_TIMER	176	/* secondary latency timer */
 #define CARDBUS_RESERVE_BUSNR	3
 
+#define PCI_DVSEC_ID_USB4	0x23
+
 static struct resource busn_resource = {
 	.name	= "PCI busn",
 	.start	= 0,
@@ -1590,6 +1592,36 @@ static void set_pcie_untrusted(struct pci_dev *dev)
 		dev->untrusted = true;
 }
 
+static bool pci_is_discrete_usb4(struct pci_dev *dev)
+{
+	int dvsec_val = 0, pos;
+	u32 hdr;
+
+	/* USB4 spec says vendors can use either */
+	pos = pci_find_dvsec_capability(dev,
+					PCI_VENDOR_ID_INTEL,
+					PCI_DVSEC_ID_USB4);
+	if (pos) {
+		dvsec_val = 0x06;
+	} else {
+		pos = pci_find_dvsec_capability(dev,
+						PCI_VENDOR_ID_USB_IF,
+						PCI_DVSEC_ID_USB4);
+		if (pos)
+			dvsec_val = 0x01;
+	}
+	if (!dvsec_val)
+		return false;
+
+	pci_read_config_dword(dev, pos + PCI_DVSEC_HEADER2, &hdr);
+	if ((hdr & GENMASK(15, 0)) != dvsec_val)
+		return false;
+	/* this port is used for either NHI/PCIe tunnel/USB tunnel */
+	if (hdr & GENMASK(18, 16))
+		return true;
+	return false;
+}
+
 static void pci_set_removable(struct pci_dev *dev)
 {
 	struct pci_dev *parent = pci_upstream_bridge(dev);
@@ -1612,6 +1644,7 @@ static void pci_set_removable(struct pci_dev *dev)
 	if (vsec ||
 	    dev->class == PCI_CLASS_SERIAL_USB_USB4 ||
 	    pci_acpi_is_usb4(dev) ||
+	    pci_is_discrete_usb4(dev) ||
 	    (parent &&
 	    (parent->external_facing || dev_is_removable(&parent->dev))))
 		dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 61b161d914f0..271326e058b9 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3097,4 +3097,6 @@
 
 #define PCI_VENDOR_ID_NCUBE		0x10ff
 
+#define PCI_VENDOR_ID_USB_IF		0x1EC0
+
 #endif /* _LINUX_PCI_IDS_H */
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 08/12] drm/amd: drop the use of `pci_is_thunderbolt_attached`

Currently `pci_is_thunderbolt_attached` is used to indicate a device
is connected externally.

The PCI core now marks such devices as removable and downstream drivers
can use this instead.

Reviewed-by: Macpaul Lin <macpaul.lin at mediatek.com>
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1ebb91db2274..6dbf5753b5be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -161,7 +161,7 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev,
unsigned long flags)
 	    (amdgpu_is_atpx_hybrid() ||
 	     amdgpu_has_atpx_dgpu_power_cntl()) &&
 	    ((flags & AMD_IS_APU) == 0) &&
-	    !pci_is_thunderbolt_attached(to_pci_dev(dev->dev)))
+	    !dev_is_removable(&adev->pdev->dev))
 		flags |= AMD_IS_PX;
 
 	parent = pci_upstream_bridge(adev->pdev);
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
index ee7cab37dfd5..2c5d74d836f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
@@ -382,7 +382,7 @@ static void nbio_v2_3_enable_aspm(struct amdgpu_device
*adev,
 
 		data |= NAVI10_PCIE__LC_L0S_INACTIVITY_DEFAULT <<
PCIE_LC_CNTL__LC_L0S_INACTIVITY__SHIFT;
 
-		if (pci_is_thunderbolt_attached(adev->pdev))
+		if (dev_is_removable(&adev->pdev->dev))
 			data |= NAVI10_PCIE__LC_L1_INACTIVITY_TBT_DEFAULT  <<
PCIE_LC_CNTL__LC_L1_INACTIVITY__SHIFT;
 		else
 			data |= NAVI10_PCIE__LC_L1_INACTIVITY_DEFAULT <<
PCIE_LC_CNTL__LC_L1_INACTIVITY__SHIFT;
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 09/12] drm/nouveau: drop the use of `pci_is_thunderbolt_attached`

Currently `pci_is_thunderbolt_attached` is used to indicate a device
is connected externally.

The PCI core now marks such devices as removable and downstream drivers
can use this instead.

Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_vga.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c
b/drivers/gpu/drm/nouveau/nouveau_vga.c
index 60cd8c0463df..2c8008cb38e0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -97,7 +97,7 @@ nouveau_vga_init(struct nouveau_drm *drm)
 	vga_client_register(pdev, nouveau_vga_set_decode);
 
 	/* don't register Thunderbolt eGPU with vga_switcheroo */
-	if (pci_is_thunderbolt_attached(pdev))
+	if (dev_is_removable(&pdev->dev))
 		return;
 
 	vga_switcheroo_register_client(pdev, &nouveau_switcheroo_ops, runtime);
@@ -120,7 +120,7 @@ nouveau_vga_fini(struct nouveau_drm *drm)
 
 	vga_client_unregister(pdev);
 
-	if (pci_is_thunderbolt_attached(pdev))
+	if (dev_is_removable(&pdev->dev))
 		return;
 
 	vga_switcheroo_unregister_client(pdev);
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 10/12] drm/radeon: drop the use of `pci_is_thunderbolt_attached`

Currently `pci_is_thunderbolt_attached` is used to indicate a device
is connected externally.

The PCI core now marks such devices as removable and downstream drivers
can use this instead.

Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/gpu/drm/radeon/radeon_device.c | 4 ++--
 drivers/gpu/drm/radeon/radeon_kms.c    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 4f0fbf667431..5117fce23b3f 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1439,7 +1439,7 @@ int radeon_device_init(struct radeon_device *rdev,
 
 	if (rdev->flags & RADEON_IS_PX)
 		runtime = true;
-	if (!pci_is_thunderbolt_attached(rdev->pdev))
+	if (!dev_is_removable(&rdev->pdev->dev))
 		vga_switcheroo_register_client(rdev->pdev,
 					       &radeon_switcheroo_ops, runtime);
 	if (runtime)
@@ -1527,7 +1527,7 @@ void radeon_device_fini(struct radeon_device *rdev)
 	/* evict vram memory */
 	radeon_bo_evict_vram(rdev);
 	radeon_fini(rdev);
-	if (!pci_is_thunderbolt_attached(rdev->pdev))
+	if (!dev_is_removable(&rdev->pdev->dev))
 		vga_switcheroo_unregister_client(rdev->pdev);
 	if (rdev->flags & RADEON_IS_PX)
 		vga_switcheroo_fini_domain_pm_ops(rdev->dev);
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
b/drivers/gpu/drm/radeon/radeon_kms.c
index 11ad210919c8..e01ee7a5cf5d 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -139,7 +139,7 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned
long flags)
 	if ((radeon_runtime_pm != 0) &&
 	    radeon_has_atpx() &&
 	    ((flags & RADEON_IS_IGP) == 0) &&
-	    !pci_is_thunderbolt_attached(pdev))
+	    !dev_is_removable(&pdev->dev))
 		flags |= RADEON_IS_PX;
 
 	/* radeon_device_init should report only fatal error
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 11/12] platform/x86: amd-gmux: drop the use of `pci_is_thunderbolt_attached`

Currently `pci_is_thunderbolt_attached` is used to indicate a device
is connected externally.

The PCI core now marks such devices as removable and downstream drivers
can use this instead.

Acked-by: Hans de Goede <hdegoede at redhat.com>
Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 drivers/platform/x86/apple-gmux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/apple-gmux.c
b/drivers/platform/x86/apple-gmux.c
index 04232fbc7d56..ffac15b9befd 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -596,7 +596,7 @@ static int gmux_resume(struct device *dev)
 
 static int is_thunderbolt(struct device *dev, void *data)
 {
-	return pci_is_thunderbolt_attached(to_pci_dev(dev));
+	return dev_is_removable(dev);
 }
 
 static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
-- 
2.34.1

Mario Limonciello

2022-Feb-11 19:32 UTC

head link

[Nouveau] [PATCH v3 12/12] PCI: drop `pci_is_thunderbolt_attached`

Currently `pci_is_thunderbolt_attached` is used to indicate a device
is connected externally.

As all drivers now look at the removable attribute, drop this function.

Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
---
 include/linux/pci.h | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index d9719eb14654..089e7e36a0d9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2434,28 +2434,6 @@ static inline bool pci_ari_enabled(struct pci_bus *bus)
 	return bus->self && bus->self->ari_enabled;
 }
 
-/**
- * pci_is_thunderbolt_attached - whether device is on a Thunderbolt daisy chain
- * @pdev: PCI device to check
- *
- * Walk upwards from @pdev and check for each encountered bridge if it's
part
- * of a Thunderbolt controller.  Reaching the host bridge means @pdev is not
- * Thunderbolt-attached.  (But rather soldered to the mainboard usually.)
- */
-static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
-{
-	struct pci_dev *parent = pdev;
-
-	if (dev_is_removable(&pdev->dev))
-		return true;
-
-	while ((parent = pci_upstream_bridge(parent)))
-		if (dev_is_removable(&parent->dev))
-			return true;
-
-	return false;
-}
-
 #if defined(CONFIG_PCIEPORTBUS) || defined(CONFIG_EEH)
 void pci_uevent_ers(struct pci_dev *pdev, enum  pci_ers_result err_type);
 #endif
-- 
2.34.1

Bjorn Helgaas

2022-Feb-11 21:02 UTC

head link

[Nouveau] [PATCH v3 01/12] thunderbolt: move definition of PCI_CLASS_SERIAL_USB_USB4

On Fri, Feb 11, 2022 at 01:32:39PM -0600, Mario Limonciello
wrote:> This PCI class definition of the USB4 device is currently located only in
> the thunderbolt driver.
> 
> It will be needed by a few other drivers for upcoming changes. Move it into
> the common include file.
> 
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
> Acked-by: Mika Westerberg <mika.westerberg at linux.intel.com>
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
I would change the subject to:

  PCI: Add USB4 class definition

because this seems like more of a PCI thing than a Thunderbolt thing,
but either way:

Acked-by: Bjorn Helgaas <bhelgaas at google.com>
> ---
>  drivers/thunderbolt/nhi.h | 2 --
>  include/linux/pci_ids.h   | 1 +
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
> index 69083aab2736..79e980b51f94 100644
> --- a/drivers/thunderbolt/nhi.h
> +++ b/drivers/thunderbolt/nhi.h
> @@ -81,6 +81,4 @@ extern const struct tb_nhi_ops icl_nhi_ops;
>  #define PCI_DEVICE_ID_INTEL_TGL_H_NHI0			0x9a1f
>  #define PCI_DEVICE_ID_INTEL_TGL_H_NHI1			0x9a21
>  
> -#define PCI_CLASS_SERIAL_USB_USB4			0x0c0340
> -
>  #endif
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index aad54c666407..61b161d914f0 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -116,6 +116,7 @@
>  #define PCI_CLASS_SERIAL_USB_OHCI	0x0c0310
>  #define PCI_CLASS_SERIAL_USB_EHCI	0x0c0320
>  #define PCI_CLASS_SERIAL_USB_XHCI	0x0c0330
> +#define PCI_CLASS_SERIAL_USB_USB4	0x0c0340
>  #define PCI_CLASS_SERIAL_USB_DEVICE	0x0c03fe
>  #define PCI_CLASS_SERIAL_FIBER		0x0c04
>  #define PCI_CLASS_SERIAL_SMBUS		0x0c05
> -- 
> 2.34.1
>

Bjorn Helgaas

2022-Feb-11 21:20 UTC

head link

[Nouveau] [PATCH v3 02/12] PCI: Move `is_thunderbolt` check for lack of command completed to a quirk

Update subject to something like:

  PCI: pciehp: Quirk broken Command Completed support on Intel Thunderbolt
controllers

On Fri, Feb 11, 2022 at 01:32:40PM -0600, Mario Limonciello
wrote:> The `is_thunderbolt` check is currently used to indicate the lack of
> command completed support for a number of older Thunderbolt devices.
> 
> This however is heavy handed and should have been done via a quirk.  Move
> the affected devices outlined in commit 493fb50e958c ("PCI: pciehp:
Assume
> NoCompl+ for Thunderbolt ports") into pci quirks.
> 
> Suggested-by: Lukas Wunner <lukas at wunner.de>
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>  drivers/pci/hotplug/pciehp_hpc.c |  6 +-----
>  drivers/pci/quirks.c             | 17 +++++++++++++++++
>  include/linux/pci.h              |  2 ++
>  3 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c
b/drivers/pci/hotplug/pciehp_hpc.c
> index 1c1ebf3dad43..e4c42b24aba8 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -996,11 +996,7 @@ struct controller *pcie_init(struct pcie_device *dev)
>  	if (pdev->hotplug_user_indicators)
>  		slot_cap &= ~(PCI_EXP_SLTCAP_AIP | PCI_EXP_SLTCAP_PIP);
>  
> -	/*
> -	 * We assume no Thunderbolt controllers support Command Complete events,
> -	 * but some controllers falsely claim they do.
> -	 */
> -	if (pdev->is_thunderbolt)
> +	if (pdev->no_cmd_complete)
>  		slot_cap |= PCI_EXP_SLTCAP_NCCS;
>  
>  	ctrl->slot_cap = slot_cap;
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index d2dd6a6cda60..6d3c88edde00 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3675,6 +3675,23 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_PORT_RIDGE,
>  			quirk_thunderbolt_hotplug_msi);
Please add a comment above to the effect that PCI_EXP_SLTCAP_NCCS
being clear means the controller generates a Command Completed software
notification when it completes a command, and these controllers don't
generate those notifications even though PCI_EXP_SLTCAP_NCCS is clear
(PCIe r6.0, sec 7.5.3.9).
> +static void quirk_thunderbolt_command_completed(struct pci_dev *pdev)
Rename to quirk_no_command_completed().  This doesn't have anything to
do with Thunderbolt; it's just that the affected devices happen to be
Thunderbolt controllers.
> +{
> +	pdev->no_cmd_complete = 1;
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_LIGHT_RIDGE,
> +			quirk_thunderbolt_command_completed);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_EAGLE_RIDGE,
> +			quirk_thunderbolt_command_completed);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_LIGHT_PEAK,
> +			quirk_thunderbolt_command_completed);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
> +			quirk_thunderbolt_command_completed);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_2C,
> +			quirk_thunderbolt_command_completed);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_PORT_RIDGE,
> +			quirk_thunderbolt_command_completed);
Can we put these in drivers/pci/hotplug/pciehp_hpc.c?  We already
have a few similar quirks there.
>  #ifdef CONFIG_ACPI
>  /*
>   * Apple: Shutdown Cactus Ridge Thunderbolt controller.
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8253a5413d7c..1e5b769e42fc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -443,6 +443,8 @@ struct pci_dev {
>  	unsigned int	is_hotplug_bridge:1;
>  	unsigned int	shpc_managed:1;		/* SHPC owned by shpchp */
>  	unsigned int	is_thunderbolt:1;	/* Thunderbolt controller */
> +	unsigned int	no_cmd_complete:1;	/* Lies about command completed events */
> +
>  	/*
>  	 * Devices marked being untrusted are the ones that can potentially
>  	 * execute DMA attacks and similar. They are typically connected
> -- 
> 2.34.1
>

Bjorn Helgaas

2022-Feb-11 21:45 UTC

head link

[Nouveau] [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`

On Fri, Feb 11, 2022 at 01:32:43PM -0600, Mario Limonciello
wrote:> The root port used for PCIe tunneling should be marked as removable to
> ensure that the entire chain is marked removable.
> 
> This can be done by looking for the device property specified in
> the ACPI tables `usb4-host-interface`.
> 
> Suggested-by: Mika Westerberg <mika.westerberg at linux.intel.com>
> Link:
https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#mapping-native-protocols-pcie-displayport-tunneled-through-usb4-to-usb4-host-routers
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>  drivers/pci/pci-acpi.c | 10 ++++++++++
>  drivers/pci/pci.h      |  5 +++++
>  drivers/pci/probe.c    |  1 +
>  3 files changed, 16 insertions(+)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a42dbf448860..6368e5633b1b 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1391,6 +1391,16 @@ void pci_acpi_cleanup(struct device *dev, struct
acpi_device *adev)
>  	}
>  }
>  
> +bool pci_acpi_is_usb4(struct pci_dev *dev)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
> +
> +	if (!adev)
> +		return false;
> +	return fwnode_property_present(acpi_fwnode_handle(adev),
> +				       "usb4-host-interface");
Maybe it's obvious to everybody but me that "USB4" means this
device
is removable.  The Microsoft reference above doesn't say anything
about removability.

My expectation is that "USB" (like "PCI" and
"PCIe") tells me
something about how a device is electrically connected and how
software can operate it.  It doesn't really tell me anything about
whether those electrical connections are permanent, made through an
internal slot, or made through an external connector and cable.
> +}
> +
>  static struct fwnode_handle *(*pci_msi_get_fwnode_cb)(struct device *dev);
>  
>  /**
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 3d60cabde1a1..359607c0542d 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -695,6 +695,7 @@ void acpi_pci_refresh_power_state(struct pci_dev *dev);
>  int acpi_pci_wakeup(struct pci_dev *dev, bool enable);
>  bool acpi_pci_need_resume(struct pci_dev *dev);
>  pci_power_t acpi_pci_choose_state(struct pci_dev *pdev);
> +bool pci_acpi_is_usb4(struct pci_dev *dev);
>  #else
>  static inline int pci_dev_acpi_reset(struct pci_dev *dev, bool probe)
>  {
> @@ -734,6 +735,10 @@ static inline pci_power_t acpi_pci_choose_state(struct
pci_dev *pdev)
>  {
>  	return PCI_POWER_ERROR;
>  }
> +static inline bool pci_acpi_is_usb4(struct pci_dev *dev)
> +{
> +	return false;
> +}
>  #endif
>  
>  #ifdef CONFIG_PCIEASPM
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index e41656cdd8f0..2693211d31cf 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1610,6 +1610,7 @@ static void pci_set_removable(struct pci_dev *dev)
>  	 * exposed as "removable" to userspace.
>  	 */
>  	if (vsec ||
> +	    pci_acpi_is_usb4(dev) ||
>  	    (parent &&
>  	    (parent->external_facing ||
dev_is_removable(&parent->dev))))
>  		dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
> -- 
> 2.34.1
>

Bjorn Helgaas

2022-Feb-11 21:58 UTC

head link

[Nouveau] [PATCH v3 06/12] PCI: Explicitly mark USB4 NHI devices as removable

On Fri, Feb 11, 2022 at 01:32:44PM -0600, Mario Limonciello
wrote:> USB4 class devices are also removable like Intel Thunderbolt devices.
Spec reference, please.
> Drivers of downstream devices use this information to declare functional
> differences in how the drivers perform by knowing that they are connected
> to an upstream TBT/USB4 port.
> 
> Reviewed-by: Macpaul Lin <macpaul.lin at mediatek.com>
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>  drivers/pci/probe.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 2693211d31cf..67ca33188cba 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1610,6 +1610,7 @@ static void pci_set_removable(struct pci_dev *dev)
>  	 * exposed as "removable" to userspace.
>  	 */
>  	if (vsec ||
> +	    dev->class == PCI_CLASS_SERIAL_USB_USB4 ||
>  	    pci_acpi_is_usb4(dev) ||
>  	    (parent &&
>  	    (parent->external_facing ||
dev_is_removable(&parent->dev))))
> -- 
> 2.34.1
>

Bjorn Helgaas

2022-Feb-11 22:10 UTC

head link

[Nouveau] [PATCH v3 07/12] PCI: Set ports for discrete USB4 controllers appropriately

Make the subject specific, not just "appropriately."  I think
you're
marking something *removable*, so include that.

On Fri, Feb 11, 2022 at 01:32:45PM -0600, Mario Limonciello
wrote:> Discrete USB4 controllers won't have ACPI nodes specifying which
> PCIe or XHCI port they are linked with.
> 
> In order to set the removable attribute appropriately, use the
> USB4 DVSEC extended capabability set on these root ports to determine
> if they are located on a discrete USB4 controller.
s/capabability/capability/
> Suggested-by: Mika Westerberg <mika.westerberg at linux.intel.com>
> Link:
https://usb.org/sites/default/files/USB4%20Specification%2020211116.zip
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
>  drivers/pci/probe.c     | 33 +++++++++++++++++++++++++++++++++
>  include/linux/pci_ids.h |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 67ca33188cba..1ed3e24db11e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -25,6 +25,8 @@
>  #define CARDBUS_LATENCY_TIMER	176	/* secondary latency timer */
>  #define CARDBUS_RESERVE_BUSNR	3
>  
> +#define PCI_DVSEC_ID_USB4	0x23
> +
>  static struct resource busn_resource = {
>  	.name	= "PCI busn",
>  	.start	= 0,
> @@ -1590,6 +1592,36 @@ static void set_pcie_untrusted(struct pci_dev *dev)
>  		dev->untrusted = true;
>  }
>  
> +static bool pci_is_discrete_usb4(struct pci_dev *dev)
> +{
> +	int dvsec_val = 0, pos;
> +	u32 hdr;
> +
> +	/* USB4 spec says vendors can use either */
> +	pos = pci_find_dvsec_capability(dev,
> +					PCI_VENDOR_ID_INTEL,
> +					PCI_DVSEC_ID_USB4);
> +	if (pos) {
> +		dvsec_val = 0x06;
> +	} else {
> +		pos = pci_find_dvsec_capability(dev,
> +						PCI_VENDOR_ID_USB_IF,
> +						PCI_DVSEC_ID_USB4);
> +		if (pos)
> +			dvsec_val = 0x01;
> +	}
> +	if (!dvsec_val)
> +		return false;
> +
> +	pci_read_config_dword(dev, pos + PCI_DVSEC_HEADER2, &hdr);
> +	if ((hdr & GENMASK(15, 0)) != dvsec_val)
> +		return false;
> +	/* this port is used for either NHI/PCIe tunnel/USB tunnel */
Capitalize comment like others in this file.

Spec reference would be helpful, too.  I don't know how to verify
any of these values.  The Link: above is great, but name, revision,
section number would be even better.
> +	if (hdr & GENMASK(18, 16))
> +		return true;
> +	return false;
> +}
> +
>  static void pci_set_removable(struct pci_dev *dev)
>  {
>  	struct pci_dev *parent = pci_upstream_bridge(dev);
> @@ -1612,6 +1644,7 @@ static void pci_set_removable(struct pci_dev *dev)
>  	if (vsec ||
>  	    dev->class == PCI_CLASS_SERIAL_USB_USB4 ||
>  	    pci_acpi_is_usb4(dev) ||
> +	    pci_is_discrete_usb4(dev) ||
>  	    (parent &&
>  	    (parent->external_facing ||
dev_is_removable(&parent->dev))))
>  		dev_set_removable(&dev->dev, DEVICE_REMOVABLE);
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 61b161d914f0..271326e058b9 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -3097,4 +3097,6 @@
>  
>  #define PCI_VENDOR_ID_NCUBE		0x10ff
>  
> +#define PCI_VENDOR_ID_USB_IF		0x1EC0
This file is supposed to be sorted by Vendor ID.  PCI_VENDOR_ID_XEN,
PCI_VENDOR_ID_OCZ, and PCI_VENDOR_ID_NCUBE screwed up, but you can put
USB_IF in the correct spot.

It's not 100%, but most entries use lower-case hex.

Nouveau - Feb 2022 - [PATCH v3 07/12] PCI: Set ports for discrete USB4 controllers appropriately

[Nouveau] [PATCH v3 00/12] Overhaul `is_thunderbolt`

[Nouveau] [PATCH v3 01/12] thunderbolt: move definition of PCI_CLASS_SERIAL_USB_USB4

[Nouveau] [PATCH v3 02/12] PCI: Move `is_thunderbolt` check for lack of command completed to a quirk

[Nouveau] [PATCH v3 03/12] PCI: Move check for old Apple Thunderbolt controllers into a quirk

[Nouveau] [PATCH v3 04/12] PCI: Drop the `is_thunderbolt` attribute from PCI core

[Nouveau] [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`

[Nouveau] [PATCH v3 06/12] PCI: Explicitly mark USB4 NHI devices as removable

[Nouveau] [PATCH v3 07/12] PCI: Set ports for discrete USB4 controllers appropriately

[Nouveau] [PATCH v3 08/12] drm/amd: drop the use of `pci_is_thunderbolt_attached`

[Nouveau] [PATCH v3 09/12] drm/nouveau: drop the use of `pci_is_thunderbolt_attached`

[Nouveau] [PATCH v3 10/12] drm/radeon: drop the use of `pci_is_thunderbolt_attached`

[Nouveau] [PATCH v3 11/12] platform/x86: amd-gmux: drop the use of `pci_is_thunderbolt_attached`

[Nouveau] [PATCH v3 12/12] PCI: drop `pci_is_thunderbolt_attached`

[Nouveau] [PATCH v3 01/12] thunderbolt: move definition of PCI_CLASS_SERIAL_USB_USB4

[Nouveau] [PATCH v3 02/12] PCI: Move `is_thunderbolt` check for lack of command completed to a quirk

[Nouveau] [PATCH v3 05/12] PCI: Detect root port of internal USB4 devices by `usb4-host-interface`

[Nouveau] [PATCH v3 06/12] PCI: Explicitly mark USB4 NHI devices as removable

[Nouveau] [PATCH v3 07/12] PCI: Set ports for discrete USB4 controllers appropriately