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@xxxxxxxxxxxxxxx> 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