Message-ID: <54feb448-b8b9-49ca-bf29-02b9884c048c@amd.com>
Date: Thu, 22 Aug 2024 10:56:44 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Esther Shimanovich <eshimanovich@...omium.org>,
 Bjorn Helgaas <bhelgaas@...gle.com>, Rajat Jain <rajatja@...gle.com>,
 "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>, iommu@...ts.linux.dev,
 Lukas Wunner <lukas@...ner.de>, linux-pci@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] PCI: Detect and trust built-in Thunderbolt chips

On 8/22/2024 10:53, Mika Westerberg wrote:
> Hi,
> 
> On Thu, Aug 22, 2024 at 10:29:55AM -0500, Mario Limonciello wrote:
>> On 8/15/2024 14:07, 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.
>>>
>>> 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.
>>>
>>>      This host controller can be identified by virtue of being located
>>>      directly below an external-facing root port that lacks integrated
>>>      Thunderbolt. Label it as trusted and fixed.
>>>
>>>      Everything downstream from it is untrusted and removable.
>>>
>>> The ExternalFacingPort ACPI property is described here:
>>> Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports
>>>
>>> Suggested-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
>>> Signed-off-by: Esther Shimanovich <eshimanovich@...omium.org>
>>> Tested-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
>>> Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
>>> ---
>>> While working with devices that have discrete Thunderbolt chips, I
>>> noticed that their internal TBT chips are inaccurately labeled as
>>> untrusted and removable.
>>>
>>> I've observed that this issue impacts all computers with internal,
>>> discrete Intel JHL Thunderbolt chips, such as JHL6240, JHL6340, JHL6540,
>>> and JHL7540, across multiple device manufacturers such as Lenovo, Dell,
>>> and HP.
>>>
>>> This affects the execution of any downstream security policy that
>>> relies on the "untrusted" or "removable" flags.
>>>
>>> I initially submitted a quirk to resolve this, which was too small in
>>> scope, and after some discussion, Mika proposed a more thorough fix:
>>> https://lore.kernel.org/lkml/20240510052616.GC4162345@black.fi.intel.com
>>> I refactored it and am submitting as a new patch.
>>
>> My apologies on my delayed response, I've been OOO a while.
>>
>> I had a test with this patch on an AMD Phoenix system on top of 6.11-rc4.  I
>> am noticing that with it, it's now flagging an internal PCI host bridge as
>> untrusted.  I added some extra debugging and it falls through to the last
>> case of pcie_is_tunneled() where it returns true.
>>
>> Here is the topology of the system:
>>
>> -[0000:00]-+-00.0
>>             +-00.2
>>             +-01.0
>>             +-01.3-[01]----00.0
>>             +-02.0
>>             +-02.1-[02]----00.0
>>             +-02.4-[03]----00.0
>>             +-03.0
>>             +-03.1-[04-62]--
>>             +-04.0
>>             +-04.1-[63-c1]--
>>             +-08.0
>>             +-08.1-[c2]--+-00.0
>>             |            +-00.1
>>             |            +-00.2
>>             |            +-00.3
>>             |            +-00.4
>>             |            +-00.5
>>             |            +-00.6
>>             |            \-00.7
>>             +-08.2-[c3]--+-00.0
>>             |            \-00.1
>>             +-08.3-[c4]--+-00.0
>>             |            +-00.3
>>             |            +-00.4
>>             |            +-00.5
>>             |            \-00.6
>>             +-14.0
>>             +-14.3
>>             +-18.0
>>             +-18.1
>>             +-18.2
>>             +-18.3
>>             +-18.4
>>             +-18.5
>>             +-18.6
>>             \-18.7
>>
>> Here are the details of all devices on the system:
>>
>> 00:00.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14e8]
>> 00:00.2 IOMMU [0806]: Advanced Micro Devices, Inc. [AMD] Device [1022:14e9]
>> 00:01.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ea]
>> 00:01.3 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ee]
>> 00:02.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ea]
>> 00:02.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ee]
>> 00:02.4 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ee]
>> 00:03.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ea]
>> 00:03.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h
>> USB4/Thunderbolt PCIe tunnel [1022:14ef]
>> 00:04.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ea]
>> 00:04.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h
>> USB4/Thunderbolt PCIe tunnel [1022:14ef]
>> 00:08.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14ea]
>> 00:08.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14eb]
>> 00:08.2 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14eb]
>> 00:08.3 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14eb]
>> 00:14.0 SMBus [0c05]: Advanced Micro Devices, Inc. [AMD] FCH SMBus
>> Controller [1022:790b] (rev 71)
>> 00:14.3 ISA bridge [0601]: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge
>> [1022:790e] (rev 51)
>> 00:18.0 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f0]
>> 00:18.1 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f1]
>> 00:18.2 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f2]
>> 00:18.3 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f3]
>> 00:18.4 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f4]
>> 00:18.5 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f5]
>> 00:18.6 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f6]
>> 00:18.7 Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:14f7]
>> 01:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
>> RTL8111/8168/8211/8411 PCI Express Gigabit Ethernet Controller [10ec:8168]
>> (rev 1c)
>> 02:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. RTS5261 PCI
>> Express Card Reader [10ec:5261] (rev 01)
>> 03:00.0 Non-Volatile memory controller [0108]: Samsung Electronics Co Ltd
>> NVMe SSD Controller PM9A1/PM9A3/980PRO [144d:a80a]
>> c2:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc.
>> [AMD/ATI] Phoenix1 [1002:15bf] (rev 03)
>> c2:00.1 Audio device [0403]: Advanced Micro Devices, Inc. [AMD/ATI]
>> Rembrandt Radeon High Definition Audio Controller [1002:1640]
>> c2:00.2 Encryption controller [1080]: Advanced Micro Devices, Inc. [AMD]
>> Family 19h (Model 74h) CCP/PSP 3.0 Device [1022:15c7]
>> c2:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:15b9]
>> c2:00.4 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:15ba]
>> c2:00.5 Multimedia controller [0480]: Advanced Micro Devices, Inc. [AMD]
>> ACP/ACP3X/ACP6x Audio Coprocessor [1022:15e2] (rev 63)
>> c2:00.6 Audio device [0403]: Advanced Micro Devices, Inc. [AMD] Family
>> 17h/19h HD Audio Controller [1022:15e3]
>> c2:00.7 Signal processing controller [1180]: Advanced Micro Devices, Inc.
>> [AMD] Device [1022:164a]
>> c3:00.0 Non-Essential Instrumentation [1300]: Advanced Micro Devices, Inc.
>> [AMD] Device [1022:14ec]
>> c3:00.1 Signal processing controller [1180]: Advanced Micro Devices, Inc.
>> [AMD] AMD IPU Device [1022:1502]
>> c4:00.0 Non-Essential Instrumentation [1300]: Advanced Micro Devices, Inc.
>> [AMD] Device [1022:14ec]
>> c4:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:15c0]
>> c4:00.4 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device
>> [1022:15c1]
>> c4:00.5 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink
>> Sardine USB4/Thunderbolt NHI controller #1 [1022:1668]
>> c4:00.6 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink
>> Sardine USB4/Thunderbolt NHI controller #2 [1022:1669]
>>
>> Here's the snippet from the kernel log with the patch in place.  You can see
>> it flagged 00:02.0 as untrusted and removable, but it definitely isn't.
> 
> Is it marked as ExternalFacingPort in the ACPI tables?

No; it doesn't have an ACPI companion device.

Powered by blists - more mailing lists