Re: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8

From: Mario Limonciello
Date: Tue Apr 23 2024 - 12:59:53 EST


On 4/23/2024 00:33, Mika Westerberg wrote:
On Mon, Apr 22, 2024 at 02:21:18PM -0500, Mario Limonciello wrote:
On 4/22/2024 14:17, Esther Shimanovich wrote:
Thanks for the explanation! I still don't fully understand how that
would work for my use case.

Perhaps it would be better for me to describe the case I am trying to
protect against.

To rehash, this quirk was written for devices with discrete
Thunderbolt controllers.

For example,
CometLake_CPU -> AlpineRidge_Chip -> USB-C Port
This device has the ExternalFacingPort property in ACPI.
My quirk relabels the Alpine Ridge chip as "fixed" and
external-facing, so that devices attached to the USB-C port could be
labeled as "removable"

Let's say we have a TigerLake CPU, which has integrated
Thunderbolt/USB4 capabilities:

TigerLake_ThunderboltCPU -> USB-C Port
This device also has the ExternalFacingPort property in ACPI and lacks
the usb4-host-interface property in the ACPI.

My worry is that someone could take an Alpine Ridge Chip Thunderbolt
Dock and attach it to the TigerLake CPU

TigerLake_ThunderboltCPU -> USB-C Port -> AlpineRidge_Dock

If that were to happen, this quirk would incorrectly label the Alpine
Ridge Dock as "fixed" instead of "removable".

My thinking was that we could prevent this scenario from occurring if
we filtered this quirk not to apply on CPU's like Tiger Lake, with
integrated Thunderbolt/USB4 capabilities.

ExternalFacingPort is found both on the Comet Lake ACPI and also on
the Tiger Lake ACPI. So I can't use that to distinguish between CPUs
which don't have integrated Thunderbolt, like Comet Lake, and CPUs
with integrated Thunderbolt, like Tiger Lake.

I am looking for something that can tell me if the device's Root Port
has the Thunderbolt controller upstream to it or not.
Is there anything like that?
Or perhaps should I add a check which compares the name of the
device's CPU with a list of CPUs that this quirk can be applied to?
Or is there some way I can identify the Thunderbolt controller, then
determine if it's upstream or downstream from the root port?
Or are Alpine Ridge docks not something to worry about at all?

My thought was once you have a device as untrusted, everything else
connected to it should "also" be untrusted.

I think what you are looking for is that anything behind a PCIe tunnel
should not have this applied. IIRC the AMD GPU or some code there were
going to add identification of "virtual" links to the bandwidth
calculation functionality.

@Mario, do you remember if this was done already and if that could maybe
be re-used here?

Yeah there was a series that I worked on a few spins a while back specifically in the context of eGPUs to identify virtual links and take them out of bandwidth calculations.

It didn't get merged, I recall it got stalled on various feedback and I didn't dust it off because the series also did prompt discussions about the reasoning that amdgpu was doing this in the first place. It turned out to be a bad assumption in the code and I instead made a change to amdgpu to not look at the whole topology but just the link partner (466a7d115326ece682c2b60d1c77d1d0b9010b4f if anyone is curious).


The other way I think is something like this:

- If it does not have "usb4-host-interface" property (or behind a port
that has that). These are all tunneled (e.g virtual).

- It is directly connected to a PCIe root port with
"ExternalFacingPort" and it has sibling device that is "Thunderbolt
NHI". This is because you can only have "NHI" on a host router
according to the USB4 spec.

I may be forgetting something though.