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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20231103054807.GN17433@black.fi.intel.com>
Date:   Fri, 3 Nov 2023 07:48:07 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Mario Limonciello <mario.limonciello@....com>
Cc:     Lukas Wunner <lukas@...ner.de>, Bjorn Helgaas <helgaas@...nel.org>,
        bhelgaas@...gle.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 Thu, Nov 02, 2023 at 11:22:05AM -0500, Mario Limonciello wrote:
> 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.

Just to add here, for discrete (eg. add in USB4 host controllers) the
USB4 spec defines DVSEC capability that can be used to figure out the
same information as the ACPI above so at least we should make the code
work so that adding the DVSEC support later on is still possible with
minimal effort :)

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

I tend to agree with this. There can be other "mediums" than
USB4/Thunderbolt that can carry PCIe packets.