Message-ID: <e256f7c5-d096-de28-3148-22b44d45f9fa@linux.intel.com>
Date: Fri, 22 Aug 2025 18:04:54 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: linux-pci@...r.kernel.org
cc: Andreas Larsson <andreas@...sler.com>, Bjorn Helgaas <bhelgaas@...gle.com>, 
    "David S. Miller" <davem@...emloft.net>, 
    Geert Uytterhoeven <geert@...ux-m68k.org>, linux-m68k@...ts.linux-m68k.org, 
    linux-mips@...r.kernel.org, sparclinux@...r.kernel.org, 
    Thomas Bogendoerfer <tsbogend@...ha.franken.de>, 
    Christian König <christian.koenig@....com>, 
    Yinghai Lu <yinghai@...nel.org>, Igor Mammedov <imammedo@...hat.com>, 
    "Rafael J . Wysocki" <rafael@...nel.org>, 
    Jonathan Cameron <Jonathan.Cameron@...wei.com>, 
    Lorenzo Pieralisi <lorenzo.pieralisi@....com>, 
    Krzysztof Wilczyński <kw@...ux.com>, 
    LKML <linux-kernel@...r.kernel.org>, 
    Michał Winiarski <michal.winiarski@...el.com>, 
    linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 00/24] PCI: Bridge window selection improvements

On Fri, 22 Aug 2025, Ilpo Järvinen wrote:

> This series is based on top of the three resource fitting and
> assignment algorithm fixes (v3).

I realized I didn't include a link to those patches. It's this series: 

https://lore.kernel.org/linux-pci/20250822123359.16305-1-ilpo.jarvinen@linux.intel.com/

I'm sorry about the extra hassle.

-- 
 i.

> PCI resource fitting and assignment code needs to find the bridge
> window a resource belongs to in multiple places, yet, no common
> function for that exists. Thus, each site has its own version of
> the decision, each with their own corner cases, misbehaviors, and
> some resulting in complex interfaces between internal functions.
> 
> This series tries to rectify the situation by adding two new functions
> to select the bridge window. To support these functions, bridge windows
> must always contain their type information in flags which requires
> modifying the flags behavior for bridge window resources.
> 
> I've hit problems related to zeroed resource flags so many times by now
> that I've already lost count which has highlighted over and over again
> that clearing type information is not a good idea. As also proven by
> some changes of this series, retaining the flags for bridge windows
> ended up fixing existing issues (although kernel ended up recovering
> from the worst problem graciously and the other just results in dormant
> code).
> 
> This series only changes resource flags behavior for bridge windows.
> The sensible direction is to make a similar change for the other
> resources as well eventually but making that change involves more
> uncertainty and is not strictly necessary yet. Driver code outside of
> PCI core could have assumptions about the flags, whereas bridge windows
> are mostly internal to PCI core code (or should be, sane endpoint
> drivers shouldn't be messing with the bridge windows). Thus, limiting
> the flags behavior changes to bridge windows for now is safer than
> attempting full behavioral change in a single step.
> 
> 
> I've tried to look out for any trouble that code under arch/ could
> cause after the flags start to behave differently and therefore ended
> up consolidating arch/ code to use pci_enable_resources(). My
> impression is that strictly speaking only the MIPS code would break
> similar to PCI core's copy of pci_enable_resources(), the others were
> much more lax in checking so they'd likely keep working but
> consolidation seemed still the best approach there as the enable checks
> seemed diverging for no apparent reason.
> 
> Most sites are converted by this change. There are three known places
> that are not yet converted:
> 
>   - fail_type based logic in __assign_resources_sorted():
>     I'm expecting to cover this along with the resizable BAR
>     changes as I need to change the fallback logic anyway (one
>     of the motivators what got me started with this series,
>     I need an easy way to acquire the bridge window during
>     retries/fallbacks if maximum sized BARs do not fit, which
>     is what this series provides).
> 
>   - Failure detection after BAR resize: Keeps using the type
>     based heuristic for failure detection. It isn't very clear how
>     to decide which assignment failures should be counted and which
>     not. There could be pre-existing failures that keep happening
>     that end up blocking BAR resize but that's no worse than behavior
>     before this series. How to identify the relevant failures does
>     not look straightforward given the current structures. This
>     clearly needs more thought before coding any solution.
> 
>   - resource assignment itself: This is a very complex change
>     due to bus and kernel resources abstractions and might not be
>     realistic any time soon.
> 
> I'd have wanted to also get rid of pci_bridge_check_ranges() that
> (re)adds type information which seemed now unnecessary. It turns out,
> however, that root windows still require so it will have to wait for
> now.
> 
> This change has been tested on a large number of machine I've access to
> which come with heterogeneous PCI configurations. Some resources
> retained their original addresses now also with pci=realloc because
> this series fixed the unnecessary release(+assign) of those resources.
> Other than that, nothing worth of note from that testing.
> 
> 
> My test coverage is x86 centric unfortunately so I'd appreciate if
> somebody with access to non-x86 archs takes the effort to test this
> series.
> 
> Info for potential testers:
> 
> Usually, it's enough to gather lspci -vvv pre and post the series, and
> use diff to see whether the resources remained the same and also check
> that the same drivers are still bound to the devices to confirm that
> devices got properly enabled (also shown by lspci -vvv). I normally
> test both with and without pci=realloc. In case of a trouble, besides
> lspci -vvv output, providing pre and post dmesg and /proc/iomem
> contents would be helpful, please take the dmesg with dyndbg="file
> drivers/pci/*.c +p" on the kernel cmdline.
> 
> Ilpo Järvinen (24):
>   m68k/PCI: Use pci_enable_resources() in pcibios_enable_device()
>   sparc/PCI: Remove pcibios_enable_device() as they do nothing extra
>   MIPS: PCI: Use pci_enable_resources()
>   PCI: Move find_bus_resource_of_type() earlier
>   PCI: Refactor find_bus_resource_of_type() logic checks
>   PCI: Always claim bridge window before its setup
>   PCI: Disable non-claimed bridge window
>   PCI: Use pci_release_resource() instead of release_resource()
>   PCI: Enable bridge even if bridge window fails to assign
>   PCI: Preserve bridge window resource type flags
>   PCI: Add defines for bridge window indexing
>   PCI: Add bridge window selection functions
>   PCI: Fix finding bridge window in pci_reassign_bridge_resources()
>   PCI: Warn if bridge window cannot be released when resizing BAR
>   PCI: Use pbus_select_window() during BAR resize
>   PCI: Use pbus_select_window_for_type() during IO window sizing
>   PCI: Rename resource variable from r to res
>   PCI: Use pbus_select_window() in space available checker
>   PCI: Use pbus_select_window_for_type() during mem window sizing
>   PCI: Refactor distributing available memory to use loops
>   PCI: Refactor remove_dev_resources() to use pbus_select_window()
>   PCI: Add pci_setup_one_bridge_window()
>   PCI: Pass bridge window to pci_bus_release_bridge_resources()
>   PCI: Alter misleading recursion to pci_bus_release_bridge_resources()
> 
>  arch/m68k/kernel/pcibios.c   |  39 +-
>  arch/mips/pci/pci-legacy.c   |  38 +-
>  arch/sparc/kernel/leon_pci.c |  27 --
>  arch/sparc/kernel/pci.c      |  27 --
>  arch/sparc/kernel/pcic.c     |  27 --
>  drivers/pci/bus.c            |   3 +
>  drivers/pci/pci-sysfs.c      |  27 +-
>  drivers/pci/pci.h            |   8 +-
>  drivers/pci/probe.c          |  35 +-
>  drivers/pci/setup-bus.c      | 798 ++++++++++++++++++-----------------
>  drivers/pci/setup-res.c      |  46 +-
>  include/linux/pci.h          |   5 +-
>  12 files changed, 504 insertions(+), 576 deletions(-)
> 
> 
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> prerequisite-patch-id: 801e8dd3aa9847d4945cb7d8958574a6006004ab
> prerequisite-patch-id: 0233311f04e3ea013676b6cc00626410bbe11e41
> prerequisite-patch-id: 9841faf37d56c1acf1167559613e862ef62e509d
> 

Powered by blists - more mailing lists