[PATCH v2 0/4] PCI: Patch series to support Thunderbolt without any BIOS support

From: Nicholas Johnson
Date: Mon Mar 11 2019 - 12:22:59 EST


_________________________________________________________________

If possible, please try to include this in the upcoming release. I have
been slow in getting PATCH v2 out but overall, it should not be too much
to do.

You have seen the first patch before, which is the most complicated. It
is based entirely on Mika Westerberg's code and he was happy that the
changes are fine. So this should help a little in getting it through.

The second is merely comments and no code - easy to inspect.

The third has very few lines and should be easy to test and revert in
the worst case.

The fourth brings the most changes functionally, but has nothing in it
algorithmically. It is just taking command line parameters and setting
the variables - hence it should be easy to visually inspect for
correctness.

I have worked very hard on this and would be very appreciative if it
could make the merge window so that the next eight weeks can be spent by
me responding to any potential concerns about it.

Thank you!
_________________________________________________________________


This new revision of my patches solves some small problems and tidies up
the commit notes, migrating background information into the cover
letter.

Together, these patches allow for a Thunderbolt 3 Titan Ridge add-in
card to be supported on any system with a PCI bus, without an inkling of
firmware support. I have been using the following (somewhat overkill)
kernel arguments for many weeks now with two dual-port add-in cards:

pci=assign-busses,hpbussize=0x33,realloc,hp_io_size=0,hp_mmio_size=256M,hp_mmio_pref_size=64G,nocrs
[required on some other platforms] pcie_ports=native

This exceeds the official Intel Thunderbolt implementation in that
arbitrary memory may be easily assigned, allowing for cards like Xeon
Phi coprocessor with massive BARs (the size of their onboard RAM) to be
placed on Thunderbolt.

PATCH 0001 solves the resource alignment bug here, which also applies to
external graphics: https://bugzilla.kernel.org/show_bug.cgi?id=199581
The resource alignment bug is not Thunderbolt-specific, and applies to
nested PCI hotplug bridges. Hence, it needed to be fixed regardless of
Thunderbolt. But Thunderbolt is the most likely use case affected by the
bug.

PATCH 0002 is just comment clean-up and no code is changed. I noticed
that a lot of the block comments in drivers/pci/setup-bus.c did not meet
the kernel coding style guidelines so I fixed them all when I needed to
change a comment. This has been separated into its own patch upon
request of the maintainers.

PATCH 0003 fixes a bug that is nasty, but rarely shows. This bugfix is
critical for that next Thunderbolt patch. I will provide the previously
requested dmesg output showing the bug in another email. For this, I
will be using an unmodified kernel and show how requesting
hpmemsize=256M on my machine results in 512M being allocated in MMIO
window, and hence the assignment failing because there is not enough
space. I can show it assigning 256M in MMIO successfully after my patch
is applied.

Symptoms: When the kernel makes multiple passes at assigning resources,
it can sometimes assign double the hpmemsize specified in kernel
parameters into the MMIO bridge window.

The cause: pbus_size_io() and pbus_size_mem() both call
find_free_bus_resource() to identify which bridge window of the bus
matches the required resource type. Because this function explicitly
skips resources with a parent, it will return NULL if the desired
resource has been successfully assigned in a previous pass. The
functions pbus_size_io() and pbus_size_mem() return -ENOSPC if
find_free_bus_resource() returns NULL, meaning an assigned resource is
interpreted as "out of space", which can result in
__pci_bus_size_bridges allocating the required size again in another
window.

The solution: my proposed patch renames find_free_bus_resource() to
find_bus_resource_of_type() and modifies it to not skip resources with
r->parent. The name change is because returning an assigned resource
makes the resource potentially not "free". The calling functions,
pbus_size_io() and pbus_size_mem() have checks added for b_res->parent
and they return 0 (success) in this case. This is because a resource
with a parent is already assigned and should be interpreted as a
success, not a failure.

Testing: I have been using this proposed patch for many weeks now and it
appears to work flawlessly. It has the added benefit of slashing the
number of attempts taken to assign a given BAR, meaning a much cleaner
dmesg. I am so confident in it that if it were not to be accepted, I
would continue to compile my own kernel each release with it included,
for my own use.

PATCH 0004 allows MMIO and MMIO_PREF sizes to be requested independently
in kernel parameters. This is the user-facing patch that delivers the
functional requirement of being able to specify the resource sizes
independently. The other patches are prep-work for this patch, allowing
it to work flawlessly. This will also make at least one other person
very happy, providing a solution where none previously existed (if this
is accepted, I will be answering their question with this patch):
https://superuser.com/questions/1054657/how-can-i-reserve-hotplug-bridges-memory-only-for-prefetchable-memory-using-the

Nicholas Johnson (4):
PCI: Consider alignment of hot-added bridges when distributing
available resources
PCI: Cleanup comments in setup-bus.c to meet kernel coding style
guidelines
PCI: Fix serious bug when sizing bridges with additional size
PCI: modify kernel parameters to differentiate between MMIO and
MMIO_PREF sizes

.../admin-guide/kernel-parameters.txt | 21 +-
drivers/pci/pci.c | 39 +-
drivers/pci/setup-bus.c | 513 +++++++++---------
include/linux/pci.h | 3 +-
4 files changed, 323 insertions(+), 253 deletions(-)

--
2.20.1