| 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
| ||
|
Message-ID: <PSXP216MB043840E2DE9B81AC8797F63A80530@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
Date: Wed, 18 Dec 2019 00:54:25 +0000
From: Nicholas Johnson <nicholas.johnson-opensource@...look.com.au>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Logan Gunthorpe <logang@...tatee.com>
Subject: Re: [PATCH v12 0/4] PCI: Patch series to improve Thunderbolt
enumeration
On Tue, Dec 17, 2019 at 09:12:48AM -0600, Bjorn Helgaas wrote:
> On Mon, Dec 09, 2019 at 12:59:29PM +0000, Nicholas Johnson wrote:
> > Hi all,
> >
> > Since last time:
> > Reverse Christmas tree for a couple of variables
> >
> > Changed while to whilst (sounds more formal, and so that
> > grepping for "while" only brings up code)
> >
> > Made sure they still apply to latest Linux v5.5-rc1
> >
> > Kind regards,
> > Nicholas
> >
> > Nicholas Johnson (4):
> > PCI: Consider alignment of hot-added bridges when distributing
> > available resources
> > PCI: In extend_bridge_window() change available to new_size
> > PCI: Change extend_bridge_window() to set resource size directly
> > PCI: Allow extend_bridge_window() to shrink resource if necessary
>
> I have tentatively applied these to pci/resource for v5.6, but I need
> some kind of high-level description for why we want these changes.
I could not find these in linux-next (whereas it was almost immediate
last time), so this must be why.
>
> The commit logs describe what the code does, and that's good, but we
> really need a little more of the *why* and what the user-visible
> benefit is. I know some of this was in earlier postings, but it seems
> to have gotten lost along the way.
Is this explanation going into the commit notes, or is this just to get
it past reviewers, Greg K-H and Linus Torvalds?
Here is an attempt:
Although Thunderbolt 3 does not need special treatment like Cardbus, it
does require the kernel to be up to date with PCI hotplug and capable of
assigning resources when native enumeration mode is used (as opposed to
BIOS-assisted mode). Operating systems have neglected this over the
years, as PCI hotplug is not widely used.
Thunderbolt has a structure where each downstream facing port is a PCI
hotplug bridge. Peripheral devices may have a second Thunderbolt port
for daisy chaining more devices, much like Firewire did.
In this example, the controller is at bus 03, and this controller has
two ports. Bus 05 is the NHI (Native Host Interface) and Bus 39 is the
USB controller. Bus 07 is a Thunderbolt dock, as is Bus 0d, daisy
chained after the first dock. The hotplug bridges are 04.01, 04.04,
07.04, 0d.04, and depending on the implementation, 1c.4 under the host
controller.
+-1c.4-[03-6c]----00.0-[04-6c]--+-00.0-[05]----00.0
| +-01.0-[06-38]----00.0-[07-38]--+-00.0-[08]----00.0
| | +-01.0-[09]----00.0
| | +-02.0-[0a]--
| | +-03.0-[0b]--
| | \-04.0-[0c-38]----00.0-[0d-38]--+-00.0-[0e]----00.0
| | +-01.0-[0f]----00.0
| | +-02.0-[10]--
| | +-03.0-[11]--
| | \-04.0-[12-38]--
| +-02.0-[39]----00.0
| \-04.0-[3a-6c]--
The host controller implementation appears differently in new Intel Ice
Lake systems, with the Thunderbolt controllers integrated into the CPU,
with the Thunderbolt ports appearing as chipset root ports.
Ice Lake with 2/4 Thunderbolt 3 ports implemented (0d.0 is the USB
controller and 0d.2 is NHI #0):
-[0000:00]-+-00.0
+-02.0
+-04.0
+-07.0-[01-2b]--
+-07.1-[2c-56]--
+-0d.0
+-0d.2
Thunderbolt is unusual in that we have nested hotplug bridges. Mika
Westerberg <mika.westerberg@...ux.intel.com> has done most of the work
required to assign all unused resources (busn and mem) from the parent
bridge window to the downstream hotplug bridge, allowing for more
Thunderbolt devices to be daisy-chained. However, in
drivers/pci/setup-bus.c in pci_bus_distribute_available_resources(), I
noticed problems with hot-adding Thunderbolt devices containing PCI
devices with resource alignment above the minimum 1M. Furthermore, I
found it more reliable to cease the use of additional size resource
lists, which are considered "optional" when allocating. The operation of
pci_bus_distribute_available_resources() is only as guaranteed as the
resource assignment. None of this would work with resources assigned by
the kernel parameters
pci=hpmmiosize=nn[KMG],hpmmiosize=nn[KMG],hpmmioprefsize=nn[KMG] - it
would only work if the resources under the root port were assigned by
firmware.
I have solved the alignment problem with patch 1/4 in
pci_bus_distribute_available_resources() and the remaining issues with
patches 2-4 by changing extend_bridge_window() which is now named
adjust_bridge_window(). You can find the link to the bug report filed by
Mika Westerberg in patch 1/4 commit log.
To sum up, although this could be applicable to any hypothetical
application with nested PCI hotplug bridges, the intended end user
facing case is Thunderbolt 3 with native enumeration mode. My changes:
- Allow for hot-adding PCI devices with >1M alignment of BARs
- Offer improved resilience and reliability due to removal of add_size
- Allow this to work with resources assigned with the kernel parameters
pci=hpmmiosize=nn[KMG],hpmmiosize=nn[KMG],hpmmioprefsize=nn[KMG] as
opposed to only firmware-allocated resources.
Please let me know if this is not adequate for the required purposes.
Thanks!
Kind regards,
Nicholas Johnson
>
> Bjorn
Powered by blists - more mailing lists