public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Shawn Jin <shawn.jin@asteralabs.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	 "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [BUG] PCIe bridge resource allocation creates invalid limit addresses after Secondary Bus Reset recovery
Date: Mon, 16 Mar 2026 12:28:46 +0200 (EET)	[thread overview]
Message-ID: <2a53776e-1e5e-74b0-1079-1ac8163efccd@linux.intel.com> (raw)
In-Reply-To: <LV8P221MB147282D317C952564A739B369945A@LV8P221MB1472.NAMP221.PROD.OUTLOOK.COM>

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

On Fri, 13 Mar 2026, Shawn Jin wrote:

> Hi Ilpo and Bjorn,
> 
> A quick update on the test result with the kernel 6.19.6-1-default in openSUSE Tumbleweed.
> 
> Summary:
> 1. With PCI realloc off, the bridge windows were reassigned as before, the ending addresses look normal, all ending with 0xffff.
> 2. With PCI realloc on, the non-pref window ending address is still incorrect: 0xe1a55554. But the pref window looks right.
> [  482.943430] [   T1435] pci 0000:99:01.0: bridge window [mem 0x134000000000-0x1347ffffffff 64bit pref]: assigned
> [  482.943431] [   T1435] pci 0000:99:01.0: bridge window [mem 0xe1a00000-0xe1a55554]: assigned

Was this window initially zero sized? That would explain why align becomes 
zero.

> Details:
> Focused topology:
> -[0000:94]---01.0-[95-9e]----00.0-[96-9e]----01.0-[98-9c]----00.0-[99-9c]--+-00.0-[9a]--
>                                                                            +-01.0-[9b]----00.0  Device 1e3e:0002
>                                                                            \-02.0-[9c]--
> Test 1: realloc OFF
> // Cold boot
> [    2.132565] [      T1] pci 0000:99:00.0: PCI bridge to [bus 9a]
> [    2.133202] [      T1] pci 0000:99:01.0: PCI bridge to [bus 9b]
> [    2.133413] [      T1] pci 0000:99:01.0:   bridge window [mem 0xe9600000-0xe96fffff]
> [    2.133555] [      T1] pci 0000:99:01.0:   bridge window [mem 0x13b000000000-0x13b7ffffffff 64bit pref]
> [    2.133827] [      T1] pci 0000:99:02.0: PCI bridge to [bus 9c]
> [    2.134453] [      T1] pci 0000:98:00.0: PCI bridge to [bus 99-9c]
> [    2.134565] [      T1] pci 0000:98:00.0:   bridge window [mem 0xe9600000-0xe96fffff]
> [    2.134647] [      T1] pci 0000:98:00.0:   bridge window [mem 0x13b000000000-0x13b7ffffffff 64bit pref]
> [    2.134809] [      T1] pci 0000:96:01.0: PCI bridge to [bus 98-9c]
> [    2.134830] [      T1] pci 0000:96:01.0:   bridge window [mem 0xe9600000-0xe96fffff]
> [    2.134845] [      T1] pci 0000:96:01.0:   bridge window [mem 0x13b000000000-0x13b7ffffffff 64bit pref]
> // after SBR
> [  202.341320] [   T1430] pci 0000:99:01.0: PCI bridge to [bus 9b]
> [  202.341539] [   T1430] pci 0000:99:01.0:   bridge window [mem 0xe9600000-0xe96fffff]
> [  202.341680] [   T1430] pci 0000:99:01.0:   bridge window [mem 0x13b000000000-0x13b7ffffffff 64bit pref]
> [  202.341950] [   T1430] pci 0000:99:02.0: PCI bridge to [bus 9c]
> [  202.342573] [   T1430] pci 0000:98:00.0: PCI bridge to [bus 99-9c]
> [  202.342685] [   T1430] pci 0000:98:00.0:   bridge window [mem 0xe9600000-0xe96fffff]
> [  202.342766] [   T1430] pci 0000:98:00.0:   bridge window [mem 0x13b000000000-0x13b7ffffffff 64bit pref]
> [  202.342929] [   T1430] pcieport 0000:96:01.0: PCI bridge to [bus 98-9c]
> [  202.342951] [   T1430] pcieport 0000:96:01.0:   bridge window [mem 0xe9600000-0xe96fffff]
> [  202.342964] [   T1430] pcieport 0000:96:01.0:   bridge window [mem 0x13b000000000-0x13b7ffffffff 64bit pref]
> 
> // although the bridges don't have IO windows, the dbg message showing the window extension may be a bit worrisome.
> [  202.340536] [   T1430] pci 0000:99:00.0: bridge window [io  size 0x0000 disabled] extended by 0x0000000000000555
> [  202.340538] [   T1430] pci 0000:99:01.0: bridge window [io  size 0x0000 disabled] extended by 0x0000000000000555
> [  202.340538] [   T1430] pci 0000:99:02.0: bridge window [io  size 0x0000 disabled] extended by 0x0000000000000555

This again is thanks to align being 0 because resource_size() == 0 so 
ALIGN_DOWN_IF_NONZERO() doesn't do anything useful.

I wonder if it would make sense to handle the minimum alignment of bridge 
windows properly in pci_resource_alignment(). It currently can return 0 
for bridge windows but we've well-defined alignment lower bounds for those 
which I think could be applied right there at source instead of having 
the callers have to worry about getting the alignment lower bounding 
right.

-- 
 i.

> Test 2: realloc ON
> // code boot
> [    2.146003] [      T1] pci_bus 0000:96: resource 1 [mem 0xe1800000-0xe1dfffff]
> [    2.146004] [      T1] pci_bus 0000:96: resource 2 [mem 0x134000000000-0x1348004fffff 64bit pref]
> [    2.146007] [      T1] pci_bus 0000:98: resource 0 [io  size 0x1000]
> [    2.146008] [      T1] pci_bus 0000:98: resource 1 [mem 0xe1a00000-0xe1bfffff]
> [    2.146009] [      T1] pci_bus 0000:98: resource 2 [mem 0x134000000000-0x1347ffffffff 64bit pref]
> [    2.146010] [      T1] pci_bus 0000:99: resource 1 [mem 0xe1a00000-0xe1afffff]
> [    2.146011] [      T1] pci_bus 0000:99: resource 2 [mem 0x134000000000-0x1347ffffffff 64bit pref]
> [    2.146012] [      T1] pci_bus 0000:9b: resource 1 [mem 0xe1a00000-0xe1afffff]
> [    2.146013] [      T1] pci_bus 0000:9b: resource 2 [mem 0x134000000000-0x1347ffffffff 64bit pref]
> 
> // after SBR
> // USP 98:00.0's windows still look correct
> [  482.943427] [   T1435] pci 0000:98:00.0: bridge window [mem 0x134000000000-0x1347ffffffff 64bit pref]: assigned
> [  482.943429] [   T1435] pci 0000:98:00.0: bridge window [mem 0xe1a00000-0xe1bfffff]: assigned
> // DSP 99:01.0 pref window is good
> [  482.943430] [   T1435] pci 0000:99:01.0: bridge window [mem 0x134000000000-0x1347ffffffff 64bit pref]: assigned
> // DSP 99:01.0 non-pref window ending addr is wrong: 0xe1a55554
> [  482.943431] [   T1435] pci 0000:99:01.0: bridge window [mem 0xe1a00000-0xe1a55554]: assigned
> [  482.943432] [   T1435] pci 0000:99:00.0: PCI bridge to [bus 9a]
> [  482.944063] [   T1435] pci 0000:9b:00.0: BAR 0 [mem 0x134000000000-0x1347ffffffff 64bit pref]: assigned
> [  482.944164] [   T1435] pci 0000:9b:00.0: BAR 2 [mem 0xe1a00000-0xe1a3ffff]: assigned
> [  482.944202] [   T1435] pci 0000:99:01.0: PCI bridge to [bus 9b]
> [  482.944424] [   T1435] pci 0000:99:01.0:   bridge window [mem 0xe1a00000-0xe1a55554]
> [  482.944565] [   T1435] pci 0000:99:01.0:   bridge window [mem 0x134000000000-0x1347ffffffff 64bit pref]
> [  482.944837] [   T1435] pci 0000:99:02.0: PCI bridge to [bus 9c]
> [  482.945463] [   T1435] pci 0000:98:00.0: PCI bridge to [bus 99-9c]
> [  482.945575] [   T1435] pci 0000:98:00.0:   bridge window [mem 0xe1a00000-0xe1bfffff]
> [  482.945657] [   T1435] pci 0000:98:00.0:   bridge window [mem 0x134000000000-0x1347ffffffff 64bit pref]
> [  482.945820] [   T1435] pcieport 0000:96:01.0: PCI bridge to [bus 98-9c]
> [  482.945842] [   T1435] pcieport 0000:96:01.0:   bridge window [mem 0xe1a00000-0xe1bfffff]
> [  482.945856] [   T1435] pcieport 0000:96:01.0:   bridge window [mem 0x134000000000-0x1347ffffffff 64bit pref]
> [  482.945883] [   T1435] PCI: No. 2 try to assign unassigned res
> [  482.945885] [   T1435] pci 0000:99:00.0: disabling bridge window [mem size 0x00000000 64bit pref disabled] to [bus 9a] (unused)
> // DSP 99:00.0 doesn't have any EP attached. But the mem size 0x55555 looks very suspicious.
> [  482.945886] [   T1435] pci 0000:99:00.0: disabling bridge window [mem size 0x00055555 disabled] to [bus 9a] (unused)
> [  482.945887] [   T1435] pci 0000:99:02.0: disabling bridge window [mem size 0x00000000 64bit pref disabled] to [bus 9c] (unused)
> // Same for DSP 99:02.0
> [  482.945888] [   T1435] pci 0000:99:02.0: disabling bridge window [mem size 0x00055555 disabled] to [bus 9c] (unused)
> 
> Hope the above info gives you some hints.
>
> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Thursday, March 12, 2026 10:48 AM
> To: Shawn Jin <shawn.jin@asteralabs.com>
> Cc: Bjorn Helgaas <helgaas@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; linux-pci@vger.kernel.org <linux-pci@vger.kernel.org>
> Subject: Re: [BUG] PCIe bridge resource allocation creates invalid limit addresses after Secondary Bus Reset recovery
> 
> [EXTERNAL EMAIL]
> 
> On Thu, 12 Mar 2026, Shawn Jin wrote:
> 
> > Hi Ilpo,
> >
> > Thanks for looking into the issue. Just a quick question. So the patch
> > you linked and the patch you attached is for the latest kernel, not 6.8.
> > Right? I'll try to install the latest kernel first and rerun my test and
> > keep you updated.
> 
> Yes, they are for the latest trees but I can look into backporting them if
> needed.
> 
> --
>  i.
> 
> > ________________________________________
> > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Sent: Thursday, March 12, 2026 6:24 AM
> > To: Shawn Jin <shawn.jin@asteralabs.com>
> > Cc: Bjorn Helgaas <helgaas@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; linux-pci@vger.kernel.org <linux-pci@vger.kernel.org>
> > Subject: Re: [BUG] PCIe bridge resource allocation creates invalid limit addresses after Secondary Bus Reset recovery
> >
> > [EXTERNAL EMAIL]
> >
> > Hi,
> > First of all, many of the changes related to this additional resource
> > distribution do not really make sense to me.
> >
> > There's one fix that tries to rectify to most gross wrongness in the
> > adjustment itself because it caused a regression (not yet applied):
> >
> > https://lore.kernel.org/linux-pci/20260219153951.68869-1-ilpo.jarvinen@linux.intel.com/
> >
> > It probably doesn't address the bug you see here though it may hide it.
> >
> > > [  796.604869] pcieport 0000:96:01.0: distributing available resources
> > > [  796.604872] pci 0000:98:00.0: bridge window [mem 0x00100000-0x001fffff] extended by 0x0000000000100000
> > > [  796.604876] pci 0000:99:00.0: bridge window [??? 0x00000000 flags 0x0] extended by 0x0000000000055555
> > > [  796.604880] pci 0000:99:01.0: bridge window [mem 0x00100000-0x001fffff] shrunken by 0x00000000000aaaac
> > > [  796.604883] pci 0000:99:01.0: bridge window [mem 0x800000000-0xfffffffff 64bit pref] shrunken by 0x0000000000000001
> > > [  796.604886] pci 0000:99:02.0: bridge window [??? 0x00000000 flags 0x0] extended by 0x0000000000055555
> >
> > Note how it's not just -1 that seems wrong, the other numbers too seem
> > non-round (to 1M) as well.
> >
> > There seems to be a number of problems with this entire algorithm.
> > I think the -1 comes from remove_dev_resource() that is called for a
> > resource which does have !res->flags (+ res->start=res->end=0 ->
> > resource_size(res) == 1).
> >
> > So could you please try the patch below.
> >
> > Not that the fix makes everything okay IMO but perhaps it addresses this
> > particular case. It's not even clear to me how some of the cases with not
> > valid bridge windows should be dealt with while distributing the
> > additional memory between the bridges.
> >
> > The 55555 cases probably come from align == 0 which makes
> > ALIGN_DOWN_IF_NONZERO() to produce non-aligning sizes. The fix patch below
> > might address it.
> >
> > I don't (yet) understand how that aaaac came to be, I suppose it somehow
> > relates to this doing something unexpected so if you want to check that
> > out, it would be helpful:
> >                 align = pci_resource_alignment(bridge, res);
> >                 if (!resource_assigned(res) && align)
> >                         available[i].start = min(ALIGN(available[i].start, align),
> >                                                  available[i].end + 1);
> >
> > Another option is remove_dev_resource() doing something that leads to it.
> >
> > It might be worth adding debug traps for available[i] getting non-1M
> > aligned but it's not very clear to me how and where.
> >
> > From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
> > Date: Thu, 12 Mar 2026 14:59:43 +0200
> > Subject: [PATCH 1/1] PCI: Skip not valid bridge windows while distributing
> >  resources
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > pci_bus_distribute_available_resources() distributes available
> > resources between downstream bridges, however, it does not take
> > into account cases where a bridge window is not valid.
> >
> > Skip touching bridge windows that are not valid.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  drivers/pci/setup-bus.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index 61f769aaa2f6..4fcb5e0c44b4 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -1868,6 +1868,9 @@ static void remove_dev_resource(struct resource *avail, struct pci_dev *dev,
> >  {
> >         resource_size_t size, align, tmp;
> >
> > +       if (!res->flags)
> > +               return;
> > +
> >         size = resource_size(res);
> >         if (!size)
> >                 return;
> > @@ -1922,6 +1925,8 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
> >                 available[i] = available_in[i];
> > +               if (!res->flags)
> > +                       continue;
> >                 /*
> >                  * The alignment of this bridge is yet to be considered,
> >                  * hence it must be done now before extending its bridge
> > @@ -1993,6 +1998,8 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
> >                 for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
> >                         res = pci_resource_n(dev, PCI_BRIDGE_RESOURCES + i);
> >
> > +                       if (!res->flags)
> > +                               continue;
> >                         /*
> >                          * Make sure the split resource space is properly
> >                          * aligned for bridge windows (align it down to
> > base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
> > --
> > 2.39.5
> >
> 

  reply	other threads:[~2026-03-16 10:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 22:00 [BUG] PCIe bridge resource allocation creates invalid limit addresses after Secondary Bus Reset recovery Shawn Jin
2026-03-11 23:19 ` Bjorn Helgaas
2026-03-12  0:02   ` Shawn Jin
2026-03-12  1:03     ` Shawn Jin
2026-03-12 13:24       ` Ilpo Järvinen
2026-03-12 17:14         ` Shawn Jin
2026-03-12 17:48           ` Ilpo Järvinen
2026-03-13 16:48             ` Shawn Jin
2026-03-16 10:28               ` Ilpo Järvinen [this message]
2026-03-16 17:26                 ` Shawn Jin
2026-03-12 17:34     ` Bjorn Helgaas
2026-03-12 17:40       ` Shawn Jin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2a53776e-1e5e-74b0-1079-1ac8163efccd@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=shawn.jin@asteralabs.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox