Re: [PATCH v5 00/14] PCI: Resource placement algorithm fixes

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Nov 06, 2026 at 03:43:21PM +0200, Ilpo Järvinen wrote:
> V4 got quite many comments from sashiko, some of them were a bit
> sketchy (bogus scenarios such as IOV BARs on a bridge) but there was
> still good feedback hidden under the clearly flawed facade.
> 
> There have been major changes so any retesting is appreciated (the
> 4 candidates patch is largely the same so I kept the Tested-by tags but
> everything around has been changing so even that is on the borderline
> when it comes to the validity of the existing tags).
> 
> I ended up dropping the nested window fix which I had anyway realized
> would not be necessary with the current resource code. I had earlier
> planned to change the resource side code to only try start addresses
> that qualify to size+align requirements and had thus kept the nested
> win fix. But it just makes PCI side more complicated requiring extra
> tweaks and checks. With that patch dropped, pci_align_resource() can
> alone decide the placement making the code much simpler.
> 
> Second major change is rewriting the gap calculation, which required
> including pbus_size_mem() refactoring that I was planning on doing
> anyway.
> 
> As with v4, the quirk is not included and one approach should still
> be selected to solve the Genoa/Zen4 iomem blackhole but that's not
> likely the one I proposed within my series so I left it out.
> 
> This series also does not attempt to address cases where user alignment
> override does not propagate to window size calculations and thus not
> into the align given to resource placement code (reported by sashiko).
> These are pre-existing issues and while they should be fixed
> eventually, I've no plans on increasing the scope and complexity of
> this series that is focused on solving real regressions caused by the
> commit 9036bd0efcb6 ("PCI: Align head space better").
> 
> For those who haven't read the previous version intros (no changes
> from v4 in this):
> 
> This series generalizes the resource placement algorithm. The commit
> 9036bd0efcb6 ("PCI: Align head space better") special cased one
> composite resource remainder case to fix a regression (in a long chain
> of regression fixes). Unfortunately, it introduced another regression
> (or a few, to be more accurate).
> 
> Instead of building more special tricks, generalize the PCI resource
> placement algorithm so it returns placement that gives better chances
> for the greedy assignment algorithm to find suitable space for
> subsequent assignments.
> 
> In short words, the new approach tries to find a placement for the
> current resource that blocks as little of the free space range as
> possible, considering both alignment and continuous free span of the
> remaining free space.
> 
> In addition, the series corrects composite resource sizing to account
> for gaps that have to be added due to alignment constraints and
> remainder space not fully connecting (filling space all the way to
> the bridge window align).
> 
> More detailed explanations in the patches themselves.
> 
> Unfortunately, this also causes yet another regression due to shuffling
> resources around. I've left my quirk out of this version as it seems
> the least likely approach to be used to resolve the issue. Some approach
> should still be applied but that decision seems largely independent of
> this series. The main part of the quirk discussion is here:
> 
>   https://lore.kernel.org/linux-pci/20261013130948.6231-9-ilpo.jarvinen@linux.intel.com/
> 
> I find it likely that after this series the resources are packed tight
> enough to solve also
>   https://bugzilla.kernel.org/show_bug.cgi?id=220016
> from more than a year back but that remains to be seen (if we can still
> get the reporter to test it).
> 
> 
> I planning on making another series to properly honor
> pci=resource_alignment and improve locking as I realized the current
> approach allows parameter change to race with the fitting and
> assignment algorithm.
> 
> v5:
> - Reordered series, "simple" patches first
> - Added patch to skip resource with IORESOURCE_DISABLED
> - Added patches to fix __ffs*() (__ffs64 & zero handling)
> - Fix 2 more 1UL -> 1ULL, renamed roundup/down funcs to _u64
> - Rework considering user/arch min align, always take it into
>   account during placement if present (sashiko)
> - Rename pci_init_pci_resreq_data() -> pci_init_resreq_data() and make
>   it non-inline func
> - Drop nested window fix (unnecessary and w/o complexity is less)
> - Rename .user_align -> .min_align
> - Use struct to store candidate selection parameters
> - Rework candidate selection precedence
> - Calculate odd-multiple starting "2 blocks resource" using both blocks
> - Add a tie breaker to candidate selection using alignment of the left
>   edge
> - Do not use candidate logic if there's no full block or
>   rr->min_align > align (the latter is due to pre-existing problems
>   elsewhere).
> - If no candidate is possible, return right edge of the empty res
>   (not strictly necessary because one of the candidates is at res->start
>    and the caller should use large enough align if user align is present
>    but it doesn't hurt either)
> - Refactor pbus_size_mem() to support the new gap calculation
> - Reworked gap calculation entirely
> - Added debug print to sizing function
> - Fix many grammar errors / typos / consistency improvements
> 
> v4:
> - Drop my own quirk patch (one approach should still be picked!)
> - Use 1ULL to not lose bits in 32-bit case (sashiko)
> - Copy avail flags from full_avail.flags (sashiko)
> - Always return "resize" from pci_specified_resource_alignment() (sashiko)
> 
> v3:
> - Add roundup/down_pow_of_two64() (new patch) to avoid problems if
>   resource_size_t is 64-bit and unsigned long is 32-bit + use __ffs64()
>   (multiple potential issues mentioned by sashiko)
> - Do not print res->parent in parisc debug code as the resource is never
>   in the resource tree (found by sashiko).
> - Do not override user-alignment with the remainder moves (sashiko)
> - Consider optional sizes properly when sizing bridge window gaps (sashiko)
> - Move genoa quirk under arch/x86/
> - Rename quirk's static resource variable
> 
> v2:
> - Add patch to cleanup parisc debug print (do dyndbg conversion)
> - Add patch to prevent remainder tricks when alignment override is used
>   (found by sashiko)
> - Do not use PCIBIOS_MIN_IO/MEM but 1U as min align. The former seem
>   address space lower bounds even if parisc arch code misleadingly
>   uses them for ALIGN() call.
> - Fix res->end + 1 overflow case (found by sashiko)
> - Tweak debug print formatting
> - Reworked the iomem black hole quirk (now based on the bridge)
> 
> 
> Ilpo Järvinen (14):
>   resource: Mark free space assigned
>   parisc/PCI: Clean up resource debug print & use dynamic debug
>   PCI: Don't size or assign disabled BARs
>   PCI: Use __ffs64() for resource_size_t typed align
>   PCI: Prevent zero align from reaching __ffs64()
>   linux/log2.h: Add 64-bit roundup/down_pow_of_two_u64() variants
>   PCI: Derive aligns[] array size in a define
>   PCI: Honor alignment overrides
>   PCI: Place resources to either edge of the window
>   PCI: Separate align/raw size calculation from pbus_size_mem()
>   PCI: Calculate bridge window with optional sizes from scratch
>   PCI: Move size/align calculation fully into __pbus_size_mem()
>   PCI: Fix composite resource sizing
>   PCI: Add debug printing to window sizing

It looks like the early ones are straightforward and good-to-go.  How
many could we just apply in order to get them out of the way?
I'm guessing at least the first six (up to and including "PCI: Derive
aligns[] array size in a define")?

You mentioned "this also causes yet another regression" so I assume
there's something we're not quite ready to merge yet?  I'd like to get
the rest in linux-next as soon as we can.

>  arch/alpha/kernel/pci.c          |   3 +-
>  arch/arm/kernel/bios32.c         |   5 +-
>  arch/m68k/kernel/pcibios.c       |   4 +-
>  arch/mips/pci/pci-generic.c      |   5 +-
>  arch/mips/pci/pci-legacy.c       |   5 +-
>  arch/parisc/kernel/pci.c         |  20 +--
>  arch/powerpc/kernel/pci-common.c |   5 +-
>  arch/sh/drivers/pci/pci.c        |   5 +-
>  arch/x86/pci/i386.c              |   5 +-
>  arch/xtensa/kernel/pci.c         |   5 +-
>  drivers/char/agp/intel-gtt.c     |   5 +-
>  drivers/gpu/drm/i915/i915_gmch.c |   5 +-
>  drivers/pci/pci.c                |   4 +-
>  drivers/pci/pci.h                |   6 +
>  drivers/pci/setup-bus.c          | 263 ++++++++++++++++++++-----------
>  drivers/pci/setup-res.c          | 246 ++++++++++++++++++++++++++---
>  include/linux/log2.h             |  52 ++++++
>  include/linux/pci.h              |  15 +-
>  kernel/resource.c                |   3 +-
>  19 files changed, 510 insertions(+), 151 deletions(-)
> 
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> -- 
> 2.47.3
>