Received-SPF: pass (google.com: domain of linux-kernel+bounces-32374-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 22 Jan 2024 08:10:05 +0200
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Esther Shimanovich <eshimanovich@chromium.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Lukas Wunner <lukas@wunner.de>, 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
Message-ID: <20240122061005.GF2543524@black.fi.intel.com>
References: <20231228132517.GA12586@wunner.de>
 <20231228133949.GG2543524@black.fi.intel.com>
 <CA+Y6NJFQq39WSSwHwm37ZQV8_rwX+6k5r+0uUs_d1+UyGGLqUw@mail.gmail.com>
 <20240118060002.GV2543524@black.fi.intel.com>
 <23ee70d5-d6c0-4dff-aeac-08cc48b11c54@amd.com>
 <ZalOCPrVA52wyFfv@google.com>
 <20240119053756.GC2543524@black.fi.intel.com>
 <20240119074829.GD2543524@black.fi.intel.com>
 <20240119102258.GE2543524@black.fi.intel.com>
 <CA+Y6NJHhTaroqJKEvOebRvbTdgkxW8tqFvq5MrOVE9swmwmtOw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CA+Y6NJHhTaroqJKEvOebRvbTdgkxW8tqFvq5MrOVE9swmwmtOw@mail.gmail.com>

On Fri, Jan 19, 2024 at 11:03:18AM -0500, Esther Shimanovich wrote:
> On Thu, Jan 18, 2024 at 1:01 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > Well that's pretty much all Intel Titan Ridge and Maple Ridge based
> > systems. Some early ones did not use IOMMU but all the rest do.
> ...
> > Before Intel Ice Lake it was all discrete and it is still discrete with
> > the Barlow Ridge controller who will have exact same ExternalFacing port
> > as the previous models.
> 
> Next week I'll try those devices in our inventory to see if I can find
> another one with this bug. I'll get back to you on that!
> 
> On Fri, Jan 19, 2024 at 2:58 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > Now if I understand the reason behind this patch is actually not about
> > "removability" that much than about identifying a trusted vs. untrusted
> > device and attaching a driver to those. I was under impression that
> > there is already a solution to this in ChromeOS kernel. It has an
> > allowlist of drivers that are allowed to attach these devices and that
> > includes the PCIe port drivers, xhci_hcd and the thunderbolt driver,
> > possibly something else too. Is this not working for your case?
> 
> This device shouldn’t be treated as a removable thunderbolt device
> that is enabled by policy because it is an internal device that should
> be trusted in the first place.
> Even so, while learning about this problem I tried modifying the
> ChromeOS policy but it ended up not fixing the issue because it seems
> like there is an expectation for it to see an existing “fixed”
> thunderbolt port before it loads anything else. But the fixed
> thunderbolt port is prevented from enumerating during bootup, before
> the policy has a chance to work.

Have you contacted the Chrome kernel folks about this? Perhaps they
have thought about this already?

> On Fri, Jan 19, 2024 at 5:23 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > The below "might" work:
> >
> > 1. A device that is directly behind a PCIe root or downstream port that
> >    has ->external_facing == 1.
> >
> > 2. It is a PCIe endpoint.
> >
> > 3. It is a sibling to or has any of the below PCI IDs (see
> >    drivers/thunderbolt/nhi.h for the definitions):
> 
> External pci devices seem to have the same kinds of chips in them. So
> this wouldn’t distinguish the “fixed but discrete” embedded pci
> devices from the “removable” pcie through usb devices. My monitor with
> thunderbolt capabilities has the JHL7540 chip in it. From the kernel's
> perspective, I have only found that the subsystem id is what
> distinguishes these devices.
> 
> That is, unless I am missing something in your proposal that would
> distinguish a fixed JHL6540 chip from an external JHL6540 chip. Please
> correct me on any assumptions I get wrong!

Yes, you are missing the 1. that it needs to be directly behind the PCIe
root or downstream port that is marked as ->external_facing, and the
fact that there can't be NHI's (that's the host controller with the IDs
I listed in 3.) anywhere else except starting the topology according the
USB4 spec (and the same applies to Thunderbolt 1-3).