linux-kernel - Re: [PATCH 2/2] PCI: Ignore PCIe ports used for tunneling in pcie_bandwidth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Nov 2023 11:22:05 -0500
From:   Mario Limonciello <mario.limonciello@....com>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Bjorn Helgaas <helgaas@...nel.org>, bhelgaas@...gle.com,
        mika.westerberg@...ux.intel.com, andreas.noever@...il.com,
        michael.jamet@...el.com, YehezkelShB@...il.com,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, Alexander.Deucher@....com
Subject: Re: [PATCH 2/2] PCI: Ignore PCIe ports used for tunneling in
 pcie_bandwidth_available()

On 11/2/2023 10:33, Lukas Wunner wrote:
> On Thu, Nov 02, 2023 at 10:26:31AM -0500, Mario Limonciello wrote:
>> On 11/2/2023 10:21, Lukas Wunner wrote:
>>> On Wed, Nov 01, 2023 at 08:14:31PM -0500, Mario Limonciello wrote:
>>>> Considering this I think it's a good idea to move that creation of the
>>>> device link into drivers/pci/pci-acpi.c and store a bit in struct
>>>> pci_device to indicate it's a tunneled port.
>>>>
>>>> Then 'thunderbolt' can look for this directly instead of walking all
>>>> the FW nodes.
>>>>
>>>> pcie_bandwidth_available() can just look at the tunneled port bit
>>>> instead of the existence of the device link.
>>>
>>> pci_is_thunderbolt_attached() should already be doing exactly what
>>> you want to achieve with the new bit.  It tells you whether a PCI
>>> device is behind a Thunderbolt tunnel.  So I don't think a new bit
>>> is actually needed.
>>
>> It's only for a device connected to an Intel TBT3 controller though; it
>> won't apply to USB4.
> 
> Time to resurrect this patch here...? :)
> 
> https://lore.kernel.org/all/20220204182820.130339-3-mario.limonciello@amd.com/

That thought crossed my mind, but I don't think it's actually correct.
That's the major reason I didn't resurrect that series.

The PCIe topology looks like this:

├─PCIe tunneled root port
|  └─PCIe bridge/switch (TBT3 or USB4 hub)
|    └─PCIe device
└─PCIe root port
   └─USB 4 Router

In this topology the USB4 PCIe class device is going to be the USB4 
router.  This *isn't* a tunneled device.

The two problematic devices are going to be that PCIe bridge (TBT or 
USB4 hub) and PCIe tunneled root port.
Looking for the class is going to mark the wrong device for the "USB 4 
Router".

I looked through the USB4 spec again and I don't see any way that such a 
port can be distinguished.

I feel the correct way to identify it is via the relationship specified 
in ACPI.

FWIW I also think that that all the kernel users of 
pci_is_thunderbolt_attached() *should* be using dev_is_removable().

amdgpu is going to be switching over to this as one of the fixes I 
mentioned for that bug:
https://patchwork.freedesktop.org/patch/564738/

If nouveau and radeon also switch over we can probably should axe the 
function pci_is_thunderbolt_attached() all together.

If you guys agree I can send out a separate series for this to go after 
the amdgpu patch merges.