lkml.org 
[lkml]   [2024]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8
    On Tue, Apr 23, 2024 at 11:59:36AM -0500, Mario Limonciello wrote:
    > 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).

    Okay that makes sense. Thanks!

    \
     
     \ /
      Last update: 2024-05-27 18:00    [W:4.303 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site