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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 9796 bytes --]

On Mon, 9 Nov 2026, Bjorn Helgaas wrote:

> 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")?

Those should as safe as it gets.

The honor user alignment request change would be safe as well when it 
comes to not yet hitting the regression territory but since I've not 
tested intermediate configurations that extensively I'd keep it either-or 
for the rest.

> 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.

The regression is the same genoa iomem blackhole we discussed in the v3 
thread:

https://lore.kernel.org/linux-pci/20261013130948.6231-9-ilpo.jarvinen@linux.intel.com
/

There were multiple possible possible ways to quirk this and it comes down 
to which one you (Bjorn) prefer. I just left my quirk patch out of these 
v4 and v5 series because it seemed the least likely to be the one taken.

If you want me to test a specific quirk approach on the affected system, 
just let me know. I already did test reverting your change that freed the 
e820 reserved regions for iomem use and that did prevent the regression.


(I now seem to realize I forgot to include Mario while sending v5. I've 
added him to this now.)

Mario was willing to pursue contacting the reporters of the earlier 
problems with a pre-prepared branch to test to see if my placement series 
could resolve the issues reported (earlier). Depending the results of that 
test, we might have yet another way to quirk it.

Mario,

There were quite many changes in v5 so I'd prefer the test branch using 
v5.

-- 
 i.