[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240826043118.GN1532424@black.fi.intel.com>
Date: Mon, 26 Aug 2024 07:31:18 +0300
From: Mika Westerberg <mika.westerberg@...ux.intel.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Esther Shimanovich <eshimanovich@...omium.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Rajat Jain <rajatja@...gle.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Mario Limonciello <mario.limonciello@....com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
iommu@...ts.linux.dev, Lukas Wunner <lukas@...ner.de>,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] PCI: Detect and trust built-in Thunderbolt chips
On Sat, Aug 24, 2024 at 11:20:42AM -0500, Bjorn Helgaas wrote:
> On Sat, Aug 24, 2024 at 07:26:35AM +0300, Mika Westerberg wrote:
> > On Fri, Aug 23, 2024 at 04:12:54PM -0500, Bjorn Helgaas wrote:
> > > On Fri, Aug 23, 2024 at 04:53:16PM +0000, Esther Shimanovich wrote:
> > > > Some computers with CPUs that lack Thunderbolt features use discrete
> > > > Thunderbolt chips to add Thunderbolt functionality. These Thunderbolt
> > > > chips are located within the chassis; between the root port labeled
> > > > ExternalFacingPort and the USB-C port.
> > >
> > > Is this a firmware defect? I asked this before, and I interpret your
> > > answer of "ExternalFacingPort is not 100% accurate all of the time" as
> > > "yes, this is a firmware defect." That should be part of the commit
> > > log and code comments.
> > >
> > > We (of course) have to work around firmware defects, but workarounds
> > > need to be labeled as such instead of masquerading as generic code.
> > >
> > > > These Thunderbolt PCIe devices should be labeled as fixed and trusted,
> > > > as they are built into the computer. Otherwise, security policies that
> > > > rely on those flags may have unintended results, such as preventing
> > > > USB-C ports from enumerating.
> > > >
> > > > Detect the above scenario through the process of elimination.
> > > >
> > > > 1) Integrated Thunderbolt host controllers already have Thunderbolt
> > > > implemented, so anything outside their external facing root port is
> > > > removable and untrusted.
> > > >
> > > > Detect them using the following properties:
> > > >
> > > > - Most integrated host controllers have the usb4-host-interface
> > > > ACPI property, as described here:
> > > > Link: https://learn.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
> > > >
> > > > - Integrated Thunderbolt PCIe root ports before Alder Lake do not
> > > > have the usb4-host-interface ACPI property. Identify those with
> > > > their PCI IDs instead.
> > > >
> > > > 2) If a root port does not have integrated Thunderbolt capabilities, but
> > > > has the ExternalFacingPort ACPI property, that means the manufacturer
> > > > has opted to use a discrete Thunderbolt host controller that is
> > > > built into the computer.
> > >
> > > Unconvincing. If a Root Port has an external connector, is it
> > > impossible to plug in a Thunderbolt device to that connector? I
> > > assume the wires from a Root Port could be traces on a PCB to a
> > > soldered-down Thunderbolt controller, OR could be wires to a connector
> > > where a Thunderbolt controller could be plugged in. How could we tell
> > > the difference?
> >
> > You are talking about soldered down controller vs. add-in card (e.g PCIe
> > slot)? We don't really distinguish those.
>
> That's kind of my point. We're depending on the platform using
> ExternalFacingPort to tell us whether there's an external connector,
> and in this case it sounds like the platform is lying to us.
It is defined only for PCIe Root Ports (for reasons unknown to me) so
there is no way to put it under the PCIe Downstream Ports itself that
are tunneled. The platform does the best it can here.
> What about PCI_EXP_FLAGS_SLOT? If a discrete Thunderbolt controller
> is built into the platform, maybe there would be no reason for the
> Root Port to set Slot Implemented and provide the Slot Capabilities/
> Control/Status registers.
It is a regular PCIe device with regular PCIe link upstream and it will
even be hot-removable during the firmware upgrade.
Powered by blists - more mailing lists