drivers/pci/bus.c | 10 ++---- drivers/pci/setup-bus.c | 80 +++++++++++++++++++++++++++++++++++++---- include/linux/ioport.h | 44 ++++++++++++++++++++--- include/linux/pci.h | 5 +-- kernel/resource.c | 68 ++++++++++++++++------------------- 5 files changed, 148 insertions(+), 59 deletions(-)
Hi all,
Here's a series that contains two fixes to PCI bridge window sizing
algorithm. Together, they should enable remove & rescan cycle to work
for a PCI bus that has PCI devices with optional resources and/or
disparity in BAR sizes.
For the second fix, I chose to expose find_empty_resource_slot() from
kernel/resource.c because it should increase accuracy of the cannot-fit
decision (currently that function is called find_resource()). In order
to do that sensibly, a few improvements seemed in order to make its
interface and name of the function sane before exposing it. Thus, the
few extra patches on resource side.
Unfortunately I don't have a reason to suspect these would help with
the issues related to the currently ongoing resource regression
thread [1].
[1] https://lore.kernel.org/linux-pci/ZXpaNCLiDM+Kv38H@marvin.atrad.com.au/
v2:
- Add "typedef" to kerneldoc to get correct formatting
- Use RESOURCE_SIZE_MAX instead of literal
- Remove unnecessary checks for io{port/mem}_resource
- Apply a few style tweaks from Andy
Ilpo Järvinen (7):
PCI: Fix resource double counting on remove & rescan
resource: Rename find_resource() to find_empty_resource_slot()
resource: Document find_empty_resource_slot() and resource_constraint
resource: Use typedef for alignf callback
resource: Handle simple alignment inside __find_empty_resource_slot()
resource: Export find_empty_resource_slot()
PCI: Relax bridge window tail sizing rules
drivers/pci/bus.c | 10 ++----
drivers/pci/setup-bus.c | 80 +++++++++++++++++++++++++++++++++++++----
include/linux/ioport.h | 44 ++++++++++++++++++++---
include/linux/pci.h | 5 +--
kernel/resource.c | 68 ++++++++++++++++-------------------
5 files changed, 148 insertions(+), 59 deletions(-)
--
2.30.2
On Thu, 28 Dec 2023 18:57:00 +0200
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> Hi all,
>
> Here's a series that contains two fixes to PCI bridge window sizing
> algorithm. Together, they should enable remove & rescan cycle to work
> for a PCI bus that has PCI devices with optional resources and/or
> disparity in BAR sizes.
>
> For the second fix, I chose to expose find_empty_resource_slot() from
> kernel/resource.c because it should increase accuracy of the cannot-fit
> decision (currently that function is called find_resource()). In order
> to do that sensibly, a few improvements seemed in order to make its
> interface and name of the function sane before exposing it. Thus, the
> few extra patches on resource side.
>
> Unfortunately I don't have a reason to suspect these would help with
> the issues related to the currently ongoing resource regression
> thread [1].
>
> [1] https://lore.kernel.org/linux-pci/ZXpaNCLiDM+Kv38H@marvin.atrad.com.au/
>
Hi Ilpo
I've hit what looks to be a similar issue to this (not fixed by this series :()
QEMU setup with a CXL PCI Expander bridge a single RP and a 4 port
port switch with CXL Type 3 devices below ports 0 and 1. (2 and 3 are empty but
each has a single BAR). RP and DSP on the switch all hotplug capable bridges.
Note that if I touch almost anything about the configuration it all works, but
this particular combination doesn't. I can add a 3rd empty port or remove 1
or add or remove an EP below the switch and it all succeeds.
arm64 setup and I'd rather not set the DSM to not reenumerate (because
there should be no problem doing so.
Note that arm64 support in general isn't upstream in qemu yet (at least partly
because we haven't figure out how to do PXB bus enumeration on DT) but can
be found at gitlab.com/jic23/qemu (not including the vsec additions to expand
the available CRS entries for the root bridge)
I've added EDK2 support and the vsec structures to expand the windows massively
so there 'should' be no issue with tight fits. However, the large CRS
entries for the root bridge don't seem to get picked up.
I did some digging and the 0c bus has those windows, but not the 0c:00.0
(the root port). I can't work out how extra space is supposed to get distributed
to root ports.
Problem chunk with the kernel enumeration is that the first CXL type3 device
has 3 bars, but the range has been shrunken to the point where only one of them
fits.
pci 0000:0f:00.0: BAR 2 [mem 0x20000000-0x2003ffff 64bit]: assigned
pci 0000:0f:00.0: BAR 0 [mem size 0x00010000 64bit]: can't assign; no space
pci 0000:0f:00.0: BAR 0 [mem size 0x00010000 64bit]: failed to assign
pci 0000:0f:00.0: BAR 4 [mem size 0x00001000]: can't assign; no space
pci 0000:0f:00.0: BAR 4 [mem size 0x00001000]: failed to assign
pci 0000:0e:00.0: PCI bridge to [bus 0f]
pci 0000:0e:00.0: bridge window [mem 0x20000000-0x2003ffff]
pci 0000:0e:00.0: bridge window [mem 0x20c00000-0x20cfffff 64bit pref]
pci 0000:10:00.0: BAR 2 [mem 0x20100000-0x2013ffff 64bit]: assigned
pci 0000:10:00.0: BAR 0 [mem 0x20140000-0x2014ffff 64bit]: assigned
pci 0000:10:00.0: BAR 4 [mem 0x20150000-0x20150fff]: assigned
pci 0000:0e:01.0: PCI bridge to [bus 10]
pci 0000:0e:01.0: bridge window [mem 0x20100000-0x2017ffff]
pci 0000:0e:01.0: bridge window [mem 0x20d00000-0x20dfffff 64bit pref]
AS you can see the window for that first device is simply too small.
EDK2 ends up with a /proc/iomap of
(kernel hacked as if the DSM was there to prevent reenumeration.
0b000080-0b0000ff : PRP0001:00
10000000-1fffffff : PCI Bus 0000:00
10000000-101fffff : PCI Bus 0000:0p1
10200000-1023ffff : 0000:00:03.0
10240000-10240fff : 0000:00:03.0
10241000-10241fff : 0000:00:02.0
10242000-10242fff : 0000:00:01.0
20000000-381fffff : PCI Bus 0000:0c
20000000-2fffffff : PCI Bus 0000:0d
20000000-2fffffff : PCI Bus 0000:0e
20000000-23ffffff : PCI Bus 0000:12
24000000-27ffffff : PCI Bus 0000:11
28000000-2bffffff : PCI Bus 0000:10
2c000000-2fffffff : PCI Bus 0000:0f
30000000-381fffff : PCI Bus 0000:0d
30000000-380fffff : PCI Bus 0000:0e
30000000-31ffffff : PCI Bus 0000:12
32000000-33ffffff : PCI Bus 0000:11
34000000-35ffffff : PCI Bus 0000:10
34000000-3403ffff : 0000:10:00.0
34000080-34000087 : 0000:10:00.0
34000088-340008a7 : 0000:10:00.0
340008a8-340008af : 0000:10:00.0
34010000-34010dff : 0000:10:00.0
34020000-34020dff : 0000:10:00.0
34040000-3404ffff : 0000:10:00.0
34041080-340410d7 : 0000:10:00.0
34041128-340411b7 : endpoint4
34050000-34050fff : 0000:10:00.0
36000000-37ffffff : PCI Bus 0000:0f
36000000-3603ffff : 0000:0f:00.0
36000080-36000087 : 0000:0f:00.0
36000088-360008a7 : 0000:0f:00.0
360008a8-360008af : 0000:0f:00.0
36010000-36010dff : 0000:0f:00.0
36020000-36020dff : 0000:0f:00.0
36040000-3604ffff : 0000:0f:00.0
36041080-360410d7 : 0000:0f:00.0
36041128-360411b7 : endpoint3
36050000-36050fff : 0000:0f:00.0
38000000-3801ffff : 0000:0e:00.0
38001080-380010d7 : mem0
38020000-3803ffff : 0000:0e:01.0
38021080-380210d7 : mem1
38040000-3805ffff : 0000:0e:02.0
38060000-3807ffff : 0000:0e:03.0
38100000-3811ffff : 0000:0d:00.0
38101128-381011b7 : port2
38200000-3efeffff : PCI Bus 0000:00
40000000-b9d7ffff : System RAM
With the kernel rerunning.
10000000-1fffffff : PCI Bus 0000:00
10000000-101fffff : PCI Bus 0000:01
10200000-1023ffff : 0000:00:03.0
10240000-10240fff : 0000:00:01.0
10241000-10241fff : 0000:00:02.0
10242000-10242fff : 0000:00:03.0
20000000-381fffff : PCI Bus 0000:0c
20000000-20bfffff : PCI Bus 0000:0d
20000000-20afffff : PCI Bus 0000:0e
20000000-2003ffff : PCI Bus 0000:0f
20000000-2003ffff : 0000:0f:00.0
20000080-20000087 : 0000:0f:00.0
20000088-200008a7 : 0000:0f:00.0
200008a8-200008af : 0000:0f:00.0
20010000-20010dff : 0000:0f:00.0
20020000-20020dff : 0000:0f:00.0
20040000-2005ffff : 0000:0e:00.0
20060000-2007ffff : 0000:0e:01.0
20061080-200610d7 : mem1
20080000-2009ffff : 0000:0e:02.0
200a0000-200bffff : 0000:0e:03.0
20100000-2017ffff : PCI Bus 0000:10
20100000-2013ffff : 0000:10:00.0
20100080-20100087 : 0000:10:00.0
20100088-201008a7 : 0000:10:00.0
201008a8-201008af : 0000:10:00.0
20110000-20110dff : 0000:10:00.0
20120000-20120dff : 0000:10:00.0
20140000-2014ffff : 0000:10:00.0
20141080-201410d7 : 0000:10:00.0
20141128-201411b7 : endpoint3
20150000-20150fff : 0000:10:00.0
20200000-203fffff : PCI Bus 0000:11
20400000-205fffff : PCI Bus 0000:12
20b00000-20b1ffff : 0000:0d:00.0
20b01128-20b011b7 : port2
20c00000-217fffff : PCI Bus 0000:0d
20c00000-217fffff : PCI Bus 0000:0e
20c00000-20cfffff : PCI Bus 0000:0f
20d00000-20dfffff : PCI Bus 0000:10
20e00000-20efffff : PCI Bus 0000:11
20f00000-20ffffff : PCI Bus 0000:12
All suggestions welcome. I've tried to figure out what is going on but
beyond thinking that the
20000000-381fffff : PCI Bus 0000:0c
entry isn't being distributed, I'm drawing a blank.
Simpler case (no extra padding from QEMU / EDK2)
EDK2 gives:
10000000-103fffff : PCI Bus 0000:00
10000000-101fffff : PCI Bus 0000:01
10200000-1023ffff : 0000:00:03.0
10240000-10240fff : 0000:00:03.0
10241000-10241fff : 0000:00:02.0
10242000-10242fff : 0000:00:01.0
10400000-10dfffff : PCI Bus 0000:0c
10400000-10dfffff : PCI Bus 0000:0d
10400000-10cfffff : PCI Bus 0000:0e
10400000-105fffff : PCI Bus 0000:12
10600000-107fffff : PCI Bus 0000:11
10800000-109fffff : PCI Bus 0000:10
10800000-1083ffff : 0000:10:00.0
10800080-10800087 : 0000:10:00.0
10800088-108008a7 : 0000:10:00.0
108008a8-108008af : 0000:10:00.0
10810000-10810dff : 0000:10:00.0
10820000-10820dff : 0000:10:00.0
10840000-1084ffff : 0000:10:00.0
10841080-108410d7 : 0000:10:00.0
10841128-108411b7 : endpoint4
10850000-10850fff : 0000:10:00.0
10a00000-10bfffff : PCI Bus 0000:0f
10a00000-10a3ffff : 0000:0f:00.0
10a00080-10a00087 : 0000:0f:00.0
10a00088-10a008a7 : 0000:0f:00.0
10a008a8-10a008af : 0000:0f:00.0
10a10000-10a10dff : 0000:0f:00.0
10a20000-10a20dff : 0000:0f:00.0
10a40000-10a4ffff : 0000:0f:00.0
10a41080-10a410d7 : 0000:0f:00.0
10a41128-10a411b7 : endpoint3
10a50000-10a50fff : 0000:0f:00.0
10c00000-10c1ffff : 0000:0e:00.0
10c01080-10c010d7 : mem1
10c20000-10c3ffff : 0000:0e:01.0
10c21080-10c210d7 : mem0
10c40000-10c5ffff : 0000:0e:02.0
10c60000-10c7ffff : 0000:0e:03.0
10d00000-10d1ffff : 0000:0d:00.0
10d01128-10d011b7 : port2
10e00000-3efeffff : PCI Bus 0000:00
And the kernel enumeration (resulting in missing BARS on 0f:00.0)
10000000-103fffff : PCI Bus 0000:00
10000000-101fffff : PCI Bus 0000:01
10200000-1023ffff : 0000:00:03.0
10240000-10240fff : 0000:00:01.0
10241000-10241fff : 0000:00:02.0
10242000-10242fff : 0000:00:03.0
10400000-10dfffff : PCI Bus 0000:0c
10400000-109fffff : PCI Bus 0000:0d
10400000-108fffff : PCI Bus 0000:0e
10400000-1043ffff : PCI Bus 0000:0f
10400000-1043ffff : 0000:0f:00.0
10400080-10400087 : 0000:0f:00.0
10400088-104008a7 : 0000:0f:00.0
104008a8-104008af : 0000:0f:00.0
10410000-10410dff : 0000:0f:00.0
10420000-10420dff : 0000:0f:00.0
10440000-1045ffff : 0000:0e:00.0
10460000-1047ffff : 0000:0e:01.0
10461080-104610d7 : mem1
10480000-1049ffff : 0000:0e:02.0
104a0000-104bffff : 0000:0e:03.0
10500000-1057ffff : PCI Bus 0000:10
10500000-1053ffff : 0000:10:00.0
10500080-10500087 : 0000:10:00.0
10500088-105008a7 : 0000:10:00.0
105008a8-105008af : 0000:10:00.0
10510000-10510dff : 0000:10:00.0
10520000-10520dff : 0000:10:00.0
10540000-1054ffff : 0000:10:00.0
10541080-105410d7 : 0000:10:00.0
10541128-105411b7 : endpoint3
10550000-10550fff : 0000:10:00.0
10600000-107fffff : PCI Bus 0000:12
10900000-1091ffff : 0000:0d:00.0
10901128-109011b7 : port2
10a00000-10dfffff : PCI Bus 0000:0d
10a00000-10dfffff : PCI Bus 0000:0e
10a00000-10afffff : PCI Bus 0000:0f
10b00000-10bfffff : PCI Bus 0000:10
10c00000-10cfffff : PCI Bus 0000:11
10d00000-10dfffff : PCI Bus 0000:12
10e00000-3efeffff : PCI Bus 0000:00
Thanks,
Jonathan
> v2:
> - Add "typedef" to kerneldoc to get correct formatting
> - Use RESOURCE_SIZE_MAX instead of literal
> - Remove unnecessary checks for io{port/mem}_resource
> - Apply a few style tweaks from Andy
>
> Ilpo Järvinen (7):
> PCI: Fix resource double counting on remove & rescan
> resource: Rename find_resource() to find_empty_resource_slot()
> resource: Document find_empty_resource_slot() and resource_constraint
> resource: Use typedef for alignf callback
> resource: Handle simple alignment inside __find_empty_resource_slot()
> resource: Export find_empty_resource_slot()
> PCI: Relax bridge window tail sizing rules
>
> drivers/pci/bus.c | 10 ++----
> drivers/pci/setup-bus.c | 80 +++++++++++++++++++++++++++++++++++++----
> include/linux/ioport.h | 44 ++++++++++++++++++++---
> include/linux/pci.h | 5 +--
> kernel/resource.c | 68 ++++++++++++++++-------------------
> 5 files changed, 148 insertions(+), 59 deletions(-)
>
On Tue, 9 Apr 2024, Jonathan Cameron wrote:
> On Thu, 28 Dec 2023 18:57:00 +0200
> Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
>
> > Here's a series that contains two fixes to PCI bridge window sizing
> > algorithm. Together, they should enable remove & rescan cycle to work
> > for a PCI bus that has PCI devices with optional resources and/or
> > disparity in BAR sizes.
> >
> > For the second fix, I chose to expose find_empty_resource_slot() from
> > kernel/resource.c because it should increase accuracy of the cannot-fit
> > decision (currently that function is called find_resource()). In order
> > to do that sensibly, a few improvements seemed in order to make its
> > interface and name of the function sane before exposing it. Thus, the
> > few extra patches on resource side.
> >
> > Unfortunately I don't have a reason to suspect these would help with
> > the issues related to the currently ongoing resource regression
> > thread [1].
> >
> > [1] https://lore.kernel.org/linux-pci/ZXpaNCLiDM+Kv38H@marvin.atrad.com.au/
> >
>
> Hi Ilpo
>
> I've hit what looks to be a similar issue to this (not fixed by this series :()
>
> QEMU setup with a CXL PCI Expander bridge a single RP and a 4 port
> port switch with CXL Type 3 devices below ports 0 and 1. (2 and 3 are empty but
> each has a single BAR). RP and DSP on the switch all hotplug capable bridges.
>
> Note that if I touch almost anything about the configuration it all works, but
> this particular combination doesn't. I can add a 3rd empty port or remove 1
> or add or remove an EP below the switch and it all succeeds.
>
> arm64 setup and I'd rather not set the DSM to not reenumerate (because
> there should be no problem doing so.
> Note that arm64 support in general isn't upstream in qemu yet (at least partly
> because we haven't figure out how to do PXB bus enumeration on DT) but can
> be found at gitlab.com/jic23/qemu (not including the vsec additions to expand
> the available CRS entries for the root bridge)
>
> I've added EDK2 support and the vsec structures to expand the windows massively
> so there 'should' be no issue with tight fits. However, the large CRS
> entries for the root bridge don't seem to get picked up.
> I did some digging and the 0c bus has those windows, but not the 0c:00.0
> (the root port). I can't work out how extra space is supposed to get distributed
> to root ports.
>
> Problem chunk with the kernel enumeration is that the first CXL type3 device
> has 3 bars, but the range has been shrunken to the point where only one of them
> fits.
Hi Jonathan,
I'm not entirely sure about the scenario here, because you didn't exactly
explain which steps you used before you the no space for BAR problem
triggers.
The main goal of this patchset is to help in cases where something is
first working as expected, then removed and rescanned, it is not expected
to help on initial enumeration/cases where there is no "first working"
condition (it might help even there, under some scenarios but that's just
"bonus", not the original goal I had in mind).
That being said, there is still caveats even when rescanning. In
multiple children/multilevel cases, the allocation still greedily follows
the current policy and only tries to use the minimal strategy when the
resource can no longer fit. Because the greedy strategy is the default,
the first allocations can consume the space that would be needed later and
all allocations should have been done with minimal strategy but due to
online nature of the resource allocation algorithm it's not able to
correct its mistake at that point.
(The above by no means implies I wouldn't have interest in looking into
and addressing other problems besides what this patchset tries to solve.)
> pci 0000:0f:00.0: BAR 2 [mem 0x20000000-0x2003ffff 64bit]: assigned
> pci 0000:0f:00.0: BAR 0 [mem size 0x00010000 64bit]: can't assign; no space
> pci 0000:0f:00.0: BAR 0 [mem size 0x00010000 64bit]: failed to assign
> pci 0000:0f:00.0: BAR 4 [mem size 0x00001000]: can't assign; no space
> pci 0000:0f:00.0: BAR 4 [mem size 0x00001000]: failed to assign
> pci 0000:0e:00.0: PCI bridge to [bus 0f]
> pci 0000:0e:00.0: bridge window [mem 0x20000000-0x2003ffff]
> pci 0000:0e:00.0: bridge window [mem 0x20c00000-0x20cfffff 64bit pref]
> pci 0000:10:00.0: BAR 2 [mem 0x20100000-0x2013ffff 64bit]: assigned
> pci 0000:10:00.0: BAR 0 [mem 0x20140000-0x2014ffff 64bit]: assigned
> pci 0000:10:00.0: BAR 4 [mem 0x20150000-0x20150fff]: assigned
> pci 0000:0e:01.0: PCI bridge to [bus 10]
> pci 0000:0e:01.0: bridge window [mem 0x20100000-0x2017ffff]
> pci 0000:0e:01.0: bridge window [mem 0x20d00000-0x20dfffff 64bit pref]
>
> AS you can see the window for that first device is simply too small.
The challenge in tracking the allocations is that the sizes are calculated
long before the allocations occurs so it's pretty hard to track things
back into the root cause. And there's also the intermediate step too which
tries to fit the optional (add) sizes.
> EDK2 ends up with a /proc/iomap of
> (kernel hacked as if the DSM was there to prevent reenumeration.
>
> 0b000080-0b0000ff : PRP0001:00
> 10000000-1fffffff : PCI Bus 0000:00
> 10000000-101fffff : PCI Bus 0000:0p1
> 10200000-1023ffff : 0000:00:03.0
> 10240000-10240fff : 0000:00:03.0
> 10241000-10241fff : 0000:00:02.0
> 10242000-10242fff : 0000:00:01.0
> 20000000-381fffff : PCI Bus 0000:0c
> 20000000-2fffffff : PCI Bus 0000:0d
> 20000000-2fffffff : PCI Bus 0000:0e
> 20000000-23ffffff : PCI Bus 0000:12
> 24000000-27ffffff : PCI Bus 0000:11
> 28000000-2bffffff : PCI Bus 0000:10
> 2c000000-2fffffff : PCI Bus 0000:0f
> 30000000-381fffff : PCI Bus 0000:0d
> 30000000-380fffff : PCI Bus 0000:0e
> 30000000-31ffffff : PCI Bus 0000:12
> 32000000-33ffffff : PCI Bus 0000:11
> 34000000-35ffffff : PCI Bus 0000:10
> 34000000-3403ffff : 0000:10:00.0
> 34000080-34000087 : 0000:10:00.0
> 34000088-340008a7 : 0000:10:00.0
> 340008a8-340008af : 0000:10:00.0
> 34010000-34010dff : 0000:10:00.0
> 34020000-34020dff : 0000:10:00.0
> 34040000-3404ffff : 0000:10:00.0
> 34041080-340410d7 : 0000:10:00.0
> 34041128-340411b7 : endpoint4
> 34050000-34050fff : 0000:10:00.0
> 36000000-37ffffff : PCI Bus 0000:0f
> 36000000-3603ffff : 0000:0f:00.0
> 36000080-36000087 : 0000:0f:00.0
> 36000088-360008a7 : 0000:0f:00.0
> 360008a8-360008af : 0000:0f:00.0
> 36010000-36010dff : 0000:0f:00.0
> 36020000-36020dff : 0000:0f:00.0
> 36040000-3604ffff : 0000:0f:00.0
> 36041080-360410d7 : 0000:0f:00.0
> 36041128-360411b7 : endpoint3
> 36050000-36050fff : 0000:0f:00.0
> 38000000-3801ffff : 0000:0e:00.0
> 38001080-380010d7 : mem0
> 38020000-3803ffff : 0000:0e:01.0
> 38021080-380210d7 : mem1
> 38040000-3805ffff : 0000:0e:02.0
> 38060000-3807ffff : 0000:0e:03.0
> 38100000-3811ffff : 0000:0d:00.0
> 38101128-381011b7 : port2
> 38200000-3efeffff : PCI Bus 0000:00
> 40000000-b9d7ffff : System RAM
>
> With the kernel rerunning.
>
> 10000000-1fffffff : PCI Bus 0000:00
> 10000000-101fffff : PCI Bus 0000:01
> 10200000-1023ffff : 0000:00:03.0
> 10240000-10240fff : 0000:00:01.0
> 10241000-10241fff : 0000:00:02.0
> 10242000-10242fff : 0000:00:03.0
> 20000000-381fffff : PCI Bus 0000:0c
> 20000000-20bfffff : PCI Bus 0000:0d
> 20000000-20afffff : PCI Bus 0000:0e
> 20000000-2003ffff : PCI Bus 0000:0f
> 20000000-2003ffff : 0000:0f:00.0
> 20000080-20000087 : 0000:0f:00.0
> 20000088-200008a7 : 0000:0f:00.0
> 200008a8-200008af : 0000:0f:00.0
> 20010000-20010dff : 0000:0f:00.0
> 20020000-20020dff : 0000:0f:00.0
> 20040000-2005ffff : 0000:0e:00.0
> 20060000-2007ffff : 0000:0e:01.0
> 20061080-200610d7 : mem1
> 20080000-2009ffff : 0000:0e:02.0
> 200a0000-200bffff : 0000:0e:03.0
> 20100000-2017ffff : PCI Bus 0000:10
> 20100000-2013ffff : 0000:10:00.0
> 20100080-20100087 : 0000:10:00.0
> 20100088-201008a7 : 0000:10:00.0
> 201008a8-201008af : 0000:10:00.0
> 20110000-20110dff : 0000:10:00.0
> 20120000-20120dff : 0000:10:00.0
> 20140000-2014ffff : 0000:10:00.0
> 20141080-201410d7 : 0000:10:00.0
> 20141128-201411b7 : endpoint3
> 20150000-20150fff : 0000:10:00.0
> 20200000-203fffff : PCI Bus 0000:11
> 20400000-205fffff : PCI Bus 0000:12
> 20b00000-20b1ffff : 0000:0d:00.0
> 20b01128-20b011b7 : port2
> 20c00000-217fffff : PCI Bus 0000:0d
> 20c00000-217fffff : PCI Bus 0000:0e
> 20c00000-20cfffff : PCI Bus 0000:0f
> 20d00000-20dfffff : PCI Bus 0000:10
> 20e00000-20efffff : PCI Bus 0000:11
> 20f00000-20ffffff : PCI Bus 0000:12
>
> All suggestions welcome. I've tried to figure out what is going on but
> beyond thinking that the
> 20000000-381fffff : PCI Bus 0000:0c
> entry isn't being distributed, I'm drawing a blank.
I think it would be generally useful in tracking these problems to have
something actually logged about these resource decisions. E.g., there are
zero pci_dbg()s in pci_bus_distribute_available_resources(). So unless the
window is adjusted, we have zero information on what's going on so no
surprise why everyone is "drawing a blank". :-(
What often comes into play in some scenarios is that once a bridge
window is assigned (res->parent is non-zero), it won't be changed. This
leads to some code not working as per what likely was the original intent.
Also adjust_bridge_window() seems to check for res->parent as the first
thing, maybe check if that prevents distributing the window?
Below is a patch which adds a bit of logging into
pci_bus_distribute_available_resources().
--
i.
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Subject: [PATCH] PCI: Log bridge window distribute value
pci_bus_distribute_available_resources() is currently very silent, add
debugging print about the distribute decision.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 909e6a7c3cc3..7a455d75e657 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1917,6 +1917,11 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
normal_bridges);
}
+ pci_dbg(bridge, "distribute to bridges: %u (hp) + %u, io: %llx mem: %llx mem pref: %llx\n",
+ hotplug_bridges, normal_bridges,
+ (unsigned long long)io_per_b, (unsigned long long)mmio_per_b,
+ (unsigned long long)mmio_pref_per_b);
+
for_each_pci_bridge(dev, bus) {
struct resource *res;
struct pci_bus *b;
--
tg: (4cece7649650..) log/distribute (depends on: main)
On Thu, Apr 11, 2024 at 01:41:10PM +0300, Ilpo Järvinen wrote: > On Tue, 9 Apr 2024, Jonathan Cameron wrote: > > On Thu, 28 Dec 2023 18:57:00 +0200 > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: ... > E.g., there are > zero pci_dbg()s in pci_bus_distribute_available_resources(). So unless the > window is adjusted, we have zero information on what's going on so no > surprise why everyone is "drawing a blank". :-( Perhaps it's a good time to start trace events / points for PCI (if not yet)? Just my 2c. -- With Best Regards, Andy Shevchenko
On Thu, 28 Dec 2023, Ilpo Järvinen wrote:
> Here's a series that contains two fixes to PCI bridge window sizing
> algorithm. Together, they should enable remove & rescan cycle to work
> for a PCI bus that has PCI devices with optional resources and/or
> disparity in BAR sizes.
>
> For the second fix, I chose to expose find_empty_resource_slot() from
> kernel/resource.c because it should increase accuracy of the cannot-fit
> decision (currently that function is called find_resource()). In order
> to do that sensibly, a few improvements seemed in order to make its
> interface and name of the function sane before exposing it. Thus, the
> few extra patches on resource side.
>
> Unfortunately I don't have a reason to suspect these would help with
> the issues related to the currently ongoing resource regression
> thread [1].
>
> [1] https://lore.kernel.org/linux-pci/ZXpaNCLiDM+Kv38H@marvin.atrad.com.au/
>
> v2:
> - Add "typedef" to kerneldoc to get correct formatting
> - Use RESOURCE_SIZE_MAX instead of literal
> - Remove unnecessary checks for io{port/mem}_resource
> - Apply a few style tweaks from Andy
>
> Ilpo Järvinen (7):
> PCI: Fix resource double counting on remove & rescan
> resource: Rename find_resource() to find_empty_resource_slot()
> resource: Document find_empty_resource_slot() and resource_constraint
> resource: Use typedef for alignf callback
> resource: Handle simple alignment inside __find_empty_resource_slot()
> resource: Export find_empty_resource_slot()
> PCI: Relax bridge window tail sizing rules
I finally managed to get the group of people who reported this initially
here to go and test to confirm these did solve the issues they're seeing,
so for all the patches:
Tested-by: Lidong Wang <lidong.wang@intel.com>
(If needed, I can send v3 with that tag).
--
i.
ps. Bjorn, I realized I pointed you earlier to v1 of this patchset, not
this v2 one. I'm sorry about that confusion (it was too far back I didn't
immediately even remember I did v2).
On Fri, Mar 15, 2024 at 12:33:43PM +0200, Ilpo Järvinen wrote: > On Thu, 28 Dec 2023, Ilpo Järvinen wrote: ... > (If needed, I can send v3 with that tag). Dunno what's Bjorn's workflow, but `b4 am` has parameter to accept tags given against a cover letter and propagate them to all patches in the series. I.o.w. no need to send a new version in such cases. -- With Best Regards, Andy Shevchenko
On Thu, 28 Dec 2023 18:57:00 +0200
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> Hi all,
>
> Here's a series that contains two fixes to PCI bridge window sizing
> algorithm. Together, they should enable remove & rescan cycle to work
> for a PCI bus that has PCI devices with optional resources and/or
> disparity in BAR sizes.
>
> For the second fix, I chose to expose find_empty_resource_slot() from
> kernel/resource.c because it should increase accuracy of the cannot-fit
> decision (currently that function is called find_resource()). In order
> to do that sensibly, a few improvements seemed in order to make its
> interface and name of the function sane before exposing it. Thus, the
> few extra patches on resource side.
>
> Unfortunately I don't have a reason to suspect these would help with
> the issues related to the currently ongoing resource regression
> thread [1].
Jonathan,
can you test this series on affected machine with broken kernel to see if
it's of any help in your case?
>
> [1] https://lore.kernel.org/linux-pci/ZXpaNCLiDM+Kv38H@marvin.atrad.com.au/
>
> v2:
> - Add "typedef" to kerneldoc to get correct formatting
> - Use RESOURCE_SIZE_MAX instead of literal
> - Remove unnecessary checks for io{port/mem}_resource
> - Apply a few style tweaks from Andy
>
> Ilpo Järvinen (7):
> PCI: Fix resource double counting on remove & rescan
> resource: Rename find_resource() to find_empty_resource_slot()
> resource: Document find_empty_resource_slot() and resource_constraint
> resource: Use typedef for alignf callback
> resource: Handle simple alignment inside __find_empty_resource_slot()
> resource: Export find_empty_resource_slot()
> PCI: Relax bridge window tail sizing rules
>
> drivers/pci/bus.c | 10 ++----
> drivers/pci/setup-bus.c | 80 +++++++++++++++++++++++++++++++++++++----
> include/linux/ioport.h | 44 ++++++++++++++++++++---
> include/linux/pci.h | 5 +--
> kernel/resource.c | 68 ++++++++++++++++-------------------
> 5 files changed, 148 insertions(+), 59 deletions(-)
>
On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote: > On Thu, 28 Dec 2023 18:57:00 +0200 > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > Hi all, > > > > Here's a series that contains two fixes to PCI bridge window sizing > > algorithm. Together, they should enable remove & rescan cycle to work > > for a PCI bus that has PCI devices with optional resources and/or > > disparity in BAR sizes. > > > > For the second fix, I chose to expose find_empty_resource_slot() from > > kernel/resource.c because it should increase accuracy of the cannot-fit > > decision (currently that function is called find_resource()). In order > > to do that sensibly, a few improvements seemed in order to make its > > interface and name of the function sane before exposing it. Thus, the > > few extra patches on resource side. > > > > Unfortunately I don't have a reason to suspect these would help with > > the issues related to the currently ongoing resource regression > > thread [1]. > > Jonathan, > can you test this series on affected machine with broken kernel to see if > it's of any help in your case? Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm still on leave this week, and when at work I only have physical access to the machine concerned on Thursdays at present. Which kernel would you prefer I apply the series to? Regards jonathan
On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote: > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote: > > On Thu, 28 Dec 2023 18:57:00 +0200 > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > Hi all, > > > > > > Here's a series that contains two fixes to PCI bridge window sizing > > > algorithm. Together, they should enable remove & rescan cycle to work > > > for a PCI bus that has PCI devices with optional resources and/or > > > disparity in BAR sizes. > > > > > > For the second fix, I chose to expose find_empty_resource_slot() from > > > kernel/resource.c because it should increase accuracy of the cannot-fit > > > decision (currently that function is called find_resource()). In order > > > to do that sensibly, a few improvements seemed in order to make its > > > interface and name of the function sane before exposing it. Thus, the > > > few extra patches on resource side. > > > > > > Unfortunately I don't have a reason to suspect these would help with > > > the issues related to the currently ongoing resource regression > > > thread [1]. > > > > Jonathan, > > can you test this series on affected machine with broken kernel to see if > > it's of any help in your case? > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm > still on leave this week, and when at work I only have physical access to > the machine concerned on Thursdays at present. > > Which kernel would you prefer I apply the series to? I was very short of time today but I did apply the above series to the 5.15.y branch (since I had this source available), resulting in version 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the bisect reset, so the resulting kernel was not correctly built. It booted but thought it was a different version and therefore none of the modules could be found. As a result, the test is invalid. I will try again in a week when I next have physical access to the system. Apologies for the delay. In the meantime, if there's a specific kernel I should apply the patch series against please let me know. As I understand it, you want it applied to one of the kernels which failed, making 5.15.y (for y < 145) a reasonable choice. Regards jonathan
On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote: > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote: > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote: > > > On Thu, 28 Dec 2023 18:57:00 +0200 > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > > > Hi all, > > > > > > > > Here's a series that contains two fixes to PCI bridge window sizing > > > > algorithm. Together, they should enable remove & rescan cycle to work > > > > for a PCI bus that has PCI devices with optional resources and/or > > > > disparity in BAR sizes. > > > > > > > > For the second fix, I chose to expose find_empty_resource_slot() from > > > > kernel/resource.c because it should increase accuracy of the cannot-fit > > > > decision (currently that function is called find_resource()). In order > > > > to do that sensibly, a few improvements seemed in order to make its > > > > interface and name of the function sane before exposing it. Thus, the > > > > few extra patches on resource side. > > > > > > > > Unfortunately I don't have a reason to suspect these would help with > > > > the issues related to the currently ongoing resource regression > > > > thread [1]. > > > > > > Jonathan, > > > can you test this series on affected machine with broken kernel to see if > > > it's of any help in your case? > > > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm > > still on leave this week, and when at work I only have physical access to > > the machine concerned on Thursdays at present. > > > > Which kernel would you prefer I apply the series to? > > I was very short of time today but I did apply the above series to the > 5.15.y branch (since I had this source available), resulting in version > 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the > bisect reset, so the resulting kernel was not correctly built. It booted > but thought it was a different version and therefore none of the modules > could be found. As a result, the test is invalid. > > I will try again in a week when I next have physical access to the system. > Apologies for the delay. In the meantime, if there's a specific kernel I > should apply the patch series against please let me know. As I understand > it, you want it applied to one of the kernels which failed, making 5.15.y > (for y < 145) a reasonable choice. I did a "make clean" to reset the source tree and recompiled. However, it errored out: drivers/pci/setup-bus.c:988:24: error: ‘RESOURCE_SIZE_MAX’ undeclared drivers/pci/setup-bus.c:998:17: error: ‘pci_bus_for_each_resource’ undeclared This was with the patch series applied against 5.15.141. It seems the patch targets a kernel that's too far removed from 5.15.x. Which kernel would you like me to apply the patch series to and test? Regards jonathan
On Thu, Jan 18, 2024 at 05:18:45PM +1030, Jonathan Woithe wrote: > On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote: > > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote: > > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote: > > > > On Thu, 28 Dec 2023 18:57:00 +0200 > > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > > > > > Hi all, > > > > > > > > > > Here's a series that contains two fixes to PCI bridge window sizing > > > > > algorithm. Together, they should enable remove & rescan cycle to work > > > > > for a PCI bus that has PCI devices with optional resources and/or > > > > > disparity in BAR sizes. > > > > > > > > > > For the second fix, I chose to expose find_empty_resource_slot() from > > > > > kernel/resource.c because it should increase accuracy of the cannot-fit > > > > > decision (currently that function is called find_resource()). In order > > > > > to do that sensibly, a few improvements seemed in order to make its > > > > > interface and name of the function sane before exposing it. Thus, the > > > > > few extra patches on resource side. > > > > > > > > > > Unfortunately I don't have a reason to suspect these would help with > > > > > the issues related to the currently ongoing resource regression > > > > > thread [1]. > > > > > > > > Jonathan, > > > > can you test this series on affected machine with broken kernel to see if > > > > it's of any help in your case? > > > > > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm > > > still on leave this week, and when at work I only have physical access to > > > the machine concerned on Thursdays at present. > > > > > > Which kernel would you prefer I apply the series to? > > > > I was very short of time today but I did apply the above series to the > > 5.15.y branch (since I had this source available), resulting in version > > 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the > > bisect reset, so the resulting kernel was not correctly built. It booted > > but thought it was a different version and therefore none of the modules > > could be found. As a result, the test is invalid. > > > > I will try again in a week when I next have physical access to the system. > > Apologies for the delay. In the meantime, if there's a specific kernel I > > should apply the patch series against please let me know. As I understand > > it, you want it applied to one of the kernels which failed, making 5.15.y > > (for y < 145) a reasonable choice. > > I did a "make clean" to reset the source tree and recompiled. However, it > errored out: > > drivers/pci/setup-bus.c:988:24: error: ‘RESOURCE_SIZE_MAX’ undeclared > drivers/pci/setup-bus.c:998:17: error: ‘pci_bus_for_each_resource’ undeclared > > This was with the patch series applied against 5.15.141. It seems the patch > targets a kernel that's too far removed from 5.15.x. > > Which kernel would you like me to apply the patch series to and test? The rule of thumb is to test against latest vanilla (as of today v6.7). Also makes sense to test against Linux Next. The v5.15 is way too old for a new code. -- With Best Regards, Andy Shevchenko
On Sun, Jan 21, 2024 at 02:54:22PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 18, 2024 at 05:18:45PM +1030, Jonathan Woithe wrote:
> > On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote:
> > > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote:
> > > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote:
> > > > > On Thu, 28 Dec 2023 18:57:00 +0200
> > > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > Here's a series that contains two fixes to PCI bridge window sizing
> > > > > > algorithm. Together, they should enable remove & rescan cycle to work
> > > > > > for a PCI bus that has PCI devices with optional resources and/or
> > > > > > disparity in BAR sizes.
> > > > > >
> > > > > > For the second fix, I chose to expose find_empty_resource_slot() from
> > > > > > kernel/resource.c because it should increase accuracy of the cannot-fit
> > > > > > decision (currently that function is called find_resource()). In order
> > > > > > to do that sensibly, a few improvements seemed in order to make its
> > > > > > interface and name of the function sane before exposing it. Thus, the
> > > > > > few extra patches on resource side.
> > > > > >
> > > > > > Unfortunately I don't have a reason to suspect these would help with
> > > > > > the issues related to the currently ongoing resource regression
> > > > > > thread [1].
> > > > >
> > > > > Jonathan,
> > > > > can you test this series on affected machine with broken kernel to see if
> > > > > it's of any help in your case?
> > > >
> > > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm
> > > > still on leave this week, and when at work I only have physical access to
> > > > the machine concerned on Thursdays at present.
> > > >
> > > > Which kernel would you prefer I apply the series to?
> > >
> > > I was very short of time today but I did apply the above series to the
> > > 5.15.y branch (since I had this source available), resulting in version
> > > 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the
> > > bisect reset, so the resulting kernel was not correctly built. It booted
> > > but thought it was a different version and therefore none of the modules
> > > could be found. As a result, the test is invalid.
> > >
> > > I will try again in a week when I next have physical access to the system.
> > > Apologies for the delay. In the meantime, if there's a specific kernel I
> > > should apply the patch series against please let me know. As I understand
> > > it, you want it applied to one of the kernels which failed, making 5.15.y
> > > (for y < 145) a reasonable choice.
> >
> > I did a "make clean" to reset the source tree and recompiled. However, it
> > errored out:
> >
> > drivers/pci/setup-bus.c:988:24: error: ‘RESOURCE_SIZE_MAX’ undeclared
> > drivers/pci/setup-bus.c:998:17: error: ‘pci_bus_for_each_resource’ undeclared
> >
> > This was with the patch series applied against 5.15.141. It seems the patch
> > targets a kernel that's too far removed from 5.15.x.
> >
> > Which kernel would you like me to apply the patch series to and test?
>
> The rule of thumb is to test against latest vanilla (as of today v6.7).
> Also makes sense to test against Linux Next. The v5.15 is way too old for
> a new code.
Thanks, and understood. In this case the request from Igor was
can you test this series on affected machine with broken kernel to see if
it's of any help in your case?
The latest vanilla kernel (6.7) has (AFAIK) had the offending commit
reverted, so it's not a "broken" kernel in this respect. Therefore, if I've
understood the request correctly, working with that kernel won't produce the
desired test.
Regards
jonathan
On Mon, 22 Jan 2024, Jonathan Woithe wrote: > On Sun, Jan 21, 2024 at 02:54:22PM +0200, Andy Shevchenko wrote: > > On Thu, Jan 18, 2024 at 05:18:45PM +1030, Jonathan Woithe wrote: > > > On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote: > > > > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote: > > > > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote: > > > > > > On Thu, 28 Dec 2023 18:57:00 +0200 > > > > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > Here's a series that contains two fixes to PCI bridge window sizing > > > > > > > algorithm. Together, they should enable remove & rescan cycle to work > > > > > > > for a PCI bus that has PCI devices with optional resources and/or > > > > > > > disparity in BAR sizes. > > > > > > > > > > > > > > For the second fix, I chose to expose find_empty_resource_slot() from > > > > > > > kernel/resource.c because it should increase accuracy of the cannot-fit > > > > > > > decision (currently that function is called find_resource()). In order > > > > > > > to do that sensibly, a few improvements seemed in order to make its > > > > > > > interface and name of the function sane before exposing it. Thus, the > > > > > > > few extra patches on resource side. > > > > > > > > > > > > > > Unfortunately I don't have a reason to suspect these would help with > > > > > > > the issues related to the currently ongoing resource regression > > > > > > > thread [1]. > > > > > > > > > > > > Jonathan, > > > > > > can you test this series on affected machine with broken kernel to see if > > > > > > it's of any help in your case? > > > > > > > > > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm > > > > > still on leave this week, and when at work I only have physical access to > > > > > the machine concerned on Thursdays at present. > > > > > > > > > > Which kernel would you prefer I apply the series to? > > > > > > > > I was very short of time today but I did apply the above series to the > > > > 5.15.y branch (since I had this source available), resulting in version > > > > 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the > > > > bisect reset, so the resulting kernel was not correctly built. It booted > > > > but thought it was a different version and therefore none of the modules > > > > could be found. As a result, the test is invalid. > > > > > > > > I will try again in a week when I next have physical access to the system. > > > > Apologies for the delay. In the meantime, if there's a specific kernel I > > > > should apply the patch series against please let me know. As I understand > > > > it, you want it applied to one of the kernels which failed, making 5.15.y > > > > (for y < 145) a reasonable choice. > > > > > > I did a "make clean" to reset the source tree and recompiled. However, it > > > errored out: > > > > > > drivers/pci/setup-bus.c:988:24: error: ‘RESOURCE_SIZE_MAX’ undeclared > > > drivers/pci/setup-bus.c:998:17: error: ‘pci_bus_for_each_resource’ undeclared > > > > > > This was with the patch series applied against 5.15.141. It seems the patch > > > targets a kernel that's too far removed from 5.15.x. > > > > > > Which kernel would you like me to apply the patch series to and test? > > > > The rule of thumb is to test against latest vanilla (as of today v6.7). > > Also makes sense to test against Linux Next. The v5.15 is way too old for > > a new code. > > Thanks, and understood. In this case the request from Igor was > > can you test this series on affected machine with broken kernel to see if > it's of any help in your case? > > The latest vanilla kernel (6.7) has (AFAIK) had the offending commit > reverted, so it's not a "broken" kernel in this respect. Therefore, if I've > understood the request correctly, working with that kernel won't produce the > desired test. Well, you can revert the revert again to get back to the broken state. -- i.
On Mon, 22 Jan 2024 14:37:32 +0200 (EET) Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > On Mon, 22 Jan 2024, Jonathan Woithe wrote: > > > On Sun, Jan 21, 2024 at 02:54:22PM +0200, Andy Shevchenko wrote: > > > On Thu, Jan 18, 2024 at 05:18:45PM +1030, Jonathan Woithe wrote: > > > > On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote: > > > > > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote: > > > > > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote: > > > > > > > On Thu, 28 Dec 2023 18:57:00 +0200 > > > > > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > Here's a series that contains two fixes to PCI bridge window sizing > > > > > > > > algorithm. Together, they should enable remove & rescan cycle to work > > > > > > > > for a PCI bus that has PCI devices with optional resources and/or > > > > > > > > disparity in BAR sizes. > > > > > > > > > > > > > > > > For the second fix, I chose to expose find_empty_resource_slot() from > > > > > > > > kernel/resource.c because it should increase accuracy of the cannot-fit > > > > > > > > decision (currently that function is called find_resource()). In order > > > > > > > > to do that sensibly, a few improvements seemed in order to make its > > > > > > > > interface and name of the function sane before exposing it. Thus, the > > > > > > > > few extra patches on resource side. > > > > > > > > > > > > > > > > Unfortunately I don't have a reason to suspect these would help with > > > > > > > > the issues related to the currently ongoing resource regression > > > > > > > > thread [1]. > > > > > > > > > > > > > > Jonathan, > > > > > > > can you test this series on affected machine with broken kernel to see if > > > > > > > it's of any help in your case? > > > > > > > > > > > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm > > > > > > still on leave this week, and when at work I only have physical access to > > > > > > the machine concerned on Thursdays at present. > > > > > > > > > > > > Which kernel would you prefer I apply the series to? > > > > > > > > > > I was very short of time today but I did apply the above series to the > > > > > 5.15.y branch (since I had this source available), resulting in version > > > > > 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the > > > > > bisect reset, so the resulting kernel was not correctly built. It booted > > > > > but thought it was a different version and therefore none of the modules > > > > > could be found. As a result, the test is invalid. > > > > > > > > > > I will try again in a week when I next have physical access to the system. > > > > > Apologies for the delay. In the meantime, if there's a specific kernel I > > > > > should apply the patch series against please let me know. As I understand > > > > > it, you want it applied to one of the kernels which failed, making 5.15.y > > > > > (for y < 145) a reasonable choice. > > > > > > > > I did a "make clean" to reset the source tree and recompiled. However, it > > > > errored out: > > > > > > > > drivers/pci/setup-bus.c:988:24: error: ‘RESOURCE_SIZE_MAX’ undeclared > > > > drivers/pci/setup-bus.c:998:17: error: ‘pci_bus_for_each_resource’ undeclared > > > > > > > > This was with the patch series applied against 5.15.141. It seems the patch > > > > targets a kernel that's too far removed from 5.15.x. > > > > > > > > Which kernel would you like me to apply the patch series to and test? > > > > > > The rule of thumb is to test against latest vanilla (as of today v6.7). > > > Also makes sense to test against Linux Next. The v5.15 is way too old for > > > a new code. > > > > Thanks, and understood. In this case the request from Igor was > > > > can you test this series on affected machine with broken kernel to see if > > it's of any help in your case? > > > > The latest vanilla kernel (6.7) has (AFAIK) had the offending commit > > reverted, so it's not a "broken" kernel in this respect. Therefore, if I've > > understood the request correctly, working with that kernel won't produce the > > desired test. > > Well, you can revert the revert again to get back to the broken state. either this or just a hand patching as Ilpo has suggested earlier would do. There is non zero chance that this series might fix issues Jonathan is facing. i.e. failed resource reallocation which offending patches trigger. There are 2 different issues here, * 1st unwanted reallocation - it should happen but well that how current code works * 2nd failed reallocation - seemingly matches what this series is trying to fix and if it doesn't help we would need to dig some more in this direction as well to figure out why it fails.
On Mon, Jan 22, 2024 at 02:45:20PM +0100, Igor Mammedov wrote:
> On Mon, 22 Jan 2024 14:37:32 +0200 (EET)
> Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
>
> > On Mon, 22 Jan 2024, Jonathan Woithe wrote:
> >
> > > On Sun, Jan 21, 2024 at 02:54:22PM +0200, Andy Shevchenko wrote:
> > > > On Thu, Jan 18, 2024 at 05:18:45PM +1030, Jonathan Woithe wrote:
> > > > > On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote:
> > > > > > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote:
> > > > > > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote:
> > > > > > > > On Thu, 28 Dec 2023 18:57:00 +0200
> > > > > > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > > > > > > >
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > Here's a series that contains two fixes to PCI bridge window sizing
> > > > > > > > > algorithm. Together, they should enable remove & rescan cycle to work
> > > > > > > > > for a PCI bus that has PCI devices with optional resources and/or
> > > > > > > > > disparity in BAR sizes.
> > > > > > > > >
> > > > > > > > > For the second fix, I chose to expose find_empty_resource_slot() from
> > > > > > > > > kernel/resource.c because it should increase accuracy of the cannot-fit
> > > > > > > > > decision (currently that function is called find_resource()). In order
> > > > > > > > > to do that sensibly, a few improvements seemed in order to make its
> > > > > > > > > interface and name of the function sane before exposing it. Thus, the
> > > > > > > > > few extra patches on resource side.
> > > > > > > > >
> > > > > > > > > Unfortunately I don't have a reason to suspect these would help with
> > > > > > > > > the issues related to the currently ongoing resource regression
> > > > > > > > > thread [1].
> > > > > > > >
> > > > > > > > Jonathan,
> > > > > > > > can you test this series on affected machine with broken kernel to see if
> > > > > > > > it's of any help in your case?
> > > > > > >
> > > > > > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm
> > > > > > > still on leave this week, and when at work I only have physical access to
> > > > > > > the machine concerned on Thursdays at present.
> > > > > > >
> > > > > > > Which kernel would you prefer I apply the series to?
> > > > > >
> > > > > > I was very short of time today but I did apply the above series to the
> > > > > > 5.15.y branch (since I had this source available), resulting in version
> > > > > > 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the
> > > > > > bisect reset, so the resulting kernel was not correctly built. It booted
> > > > > > but thought it was a different version and therefore none of the modules
> > > > > > could be found. As a result, the test is invalid.
> > > > > >
> > > > > > I will try again in a week when I next have physical access to the system.
> > > > > > Apologies for the delay. In the meantime, if there's a specific kernel I
> > > > > > should apply the patch series against please let me know. As I understand
> > > > > > it, you want it applied to one of the kernels which failed, making 5.15.y
> > > > > > (for y < 145) a reasonable choice.
> > > > >
> > > > > I did a "make clean" to reset the source tree and recompiled. However, it
> > > > > errored out:
> > > > >
> > > > > drivers/pci/setup-bus.c:988:24: error: ‘RESOURCE_SIZE_MAX’ undeclared
> > > > > drivers/pci/setup-bus.c:998:17: error: ‘pci_bus_for_each_resource’ undeclared
> > > > >
> > > > > This was with the patch series applied against 5.15.141. It seems the patch
> > > > > targets a kernel that's too far removed from 5.15.x.
> > > > >
> > > > > Which kernel would you like me to apply the patch series to and test?
> > > >
> > > > The rule of thumb is to test against latest vanilla (as of today v6.7).
> > > > Also makes sense to test against Linux Next. The v5.15 is way too old for
> > > > a new code.
> > >
> > > Thanks, and understood. In this case the request from Igor was
> > >
> > > can you test this series on affected machine with broken kernel to see if
> > > it's of any help in your case?
> > >
> > > The latest vanilla kernel (6.7) has (AFAIK) had the offending commit
> > > reverted, so it's not a "broken" kernel in this respect. Therefore, if I've
> > > understood the request correctly, working with that kernel won't produce the
> > > desired test.
> >
> > Well, you can revert the revert again to get back to the broken state.
>
> either this or just a hand patching as Ilpo has suggested earlier
> would do.
No problem. This was the easiest approach for me and I have now done this.
Apologies for the delay in getting to this: I ran out of time last Thursday.
> There is non zero chance that this series might fix issues
> Jonathan is facing. i.e. failed resource reallocation which
> offending patches trigger.
I can confirm that as expected, this patch series has had no effect on the
system which experiences the failed resource reallocation. From syslog,
running a 5.15.141+ kernel[1]:
kernel: radeon 0000:4b:00.0: Fatal error during GPU init
kernel: radeon: probe of 0000:4b:00.0 failed with error -12
This is unchanged from what is seen with the unaltered 5.15.141 kernel.
In case it's important, can also confirm that the errors related to the
thunderbolt device are are also still present in the patched 5.15.141+
kernel:
thunderbolt 0000:04:00.0: interrupt for TX ring 0 is already enabled
:
thunderbolt 0000:04:00.0: interrupt for RX ring 0 is already enabled
:
Like the GPU failure, they do not appear in the working kernels on this
system.
Let me know if you would like to me to run further tests.
Regards
jonathan
[1] This is 5.15.141, patched with the series of interest here and the hand
patch from Ilpo.
On Thu, 1 Feb 2024, Jonathan Woithe wrote: > On Mon, Jan 22, 2024 at 02:45:20PM +0100, Igor Mammedov wrote: > > On Mon, 22 Jan 2024 14:37:32 +0200 (EET) > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > On Mon, 22 Jan 2024, Jonathan Woithe wrote: > > > > > > > On Sun, Jan 21, 2024 at 02:54:22PM +0200, Andy Shevchenko wrote: > > > > > On Thu, Jan 18, 2024 at 05:18:45PM +1030, Jonathan Woithe wrote: > > > > > > On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote: > > > > > > > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote: > > > > > > > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote: > > > > > > > > > On Thu, 28 Dec 2023 18:57:00 +0200 > > > > > > > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > Here's a series that contains two fixes to PCI bridge window sizing > > > > > > > > > > algorithm. Together, they should enable remove & rescan cycle to work > > > > > > > > > > for a PCI bus that has PCI devices with optional resources and/or > > > > > > > > > > disparity in BAR sizes. > > > > > > > > > > > > > > > > > > > > For the second fix, I chose to expose find_empty_resource_slot() from > > > > > > > > > > kernel/resource.c because it should increase accuracy of the cannot-fit > > > > > > > > > > decision (currently that function is called find_resource()). In order > > > > > > > > > > to do that sensibly, a few improvements seemed in order to make its > > > > > > > > > > interface and name of the function sane before exposing it. Thus, the > > > > > > > > > > few extra patches on resource side. > > > > > > > > > > > > > > > > > > > > Unfortunately I don't have a reason to suspect these would help with > > > > > > > > > > the issues related to the currently ongoing resource regression > > > > > > > > > > thread [1]. > > > > Thanks, and understood. In this case the request from Igor was > > > > > > > > can you test this series on affected machine with broken kernel to see if > > > > it's of any help in your case? > > > > > > > > The latest vanilla kernel (6.7) has (AFAIK) had the offending commit > > > > reverted, so it's not a "broken" kernel in this respect. Therefore, if I've > > > > understood the request correctly, working with that kernel won't produce the > > > > desired test. > > > > > > Well, you can revert the revert again to get back to the broken state. > > > > either this or just a hand patching as Ilpo has suggested earlier > > would do. > > No problem. This was the easiest approach for me and I have now done this. > Apologies for the delay in getting to this: I ran out of time last Thursday. > > > There is non zero chance that this series might fix issues > > Jonathan is facing. i.e. failed resource reallocation which > > offending patches trigger. > > I can confirm that as expected, this patch series has had no effect on the > system which experiences the failed resource reallocation. From syslog, > running a 5.15.141+ kernel[1]: > > kernel: radeon 0000:4b:00.0: Fatal error during GPU init > kernel: radeon: probe of 0000:4b:00.0 failed with error -12 > > This is unchanged from what is seen with the unaltered 5.15.141 kernel. > > In case it's important, can also confirm that the errors related to the > thunderbolt device are are also still present in the patched 5.15.141+ > kernel: > > thunderbolt 0000:04:00.0: interrupt for TX ring 0 is already enabled > : > thunderbolt 0000:04:00.0: interrupt for RX ring 0 is already enabled > : > > Like the GPU failure, they do not appear in the working kernels on this > system. > > Let me know if you would like to me to run further tests. > > Regards > jonathan > > [1] This is 5.15.141, patched with the series of interest here and the hand > patch from Ilpo. Hi Jonathan, Thanks a lot for testing it regardless. The end result was not a big surprise given how it looked like based on the logs but was certainly worth a test like Igor mentioned. The resource allocation code isn't among the easiest to track. -- i.
On Thu, 18 Jan 2024, Jonathan Woithe wrote:
> On Thu, Jan 11, 2024 at 06:30:22PM +1030, Jonathan Woithe wrote:
> > On Thu, Jan 04, 2024 at 10:48:53PM +1030, Jonathan Woithe wrote:
> > > On Thu, Jan 04, 2024 at 01:12:10PM +0100, Igor Mammedov wrote:
> > > > On Thu, 28 Dec 2023 18:57:00 +0200
> > > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > Here's a series that contains two fixes to PCI bridge window sizing
> > > > > algorithm. Together, they should enable remove & rescan cycle to work
> > > > > for a PCI bus that has PCI devices with optional resources and/or
> > > > > disparity in BAR sizes.
> > > > >
> > > > > For the second fix, I chose to expose find_empty_resource_slot() from
> > > > > kernel/resource.c because it should increase accuracy of the cannot-fit
> > > > > decision (currently that function is called find_resource()). In order
> > > > > to do that sensibly, a few improvements seemed in order to make its
> > > > > interface and name of the function sane before exposing it. Thus, the
> > > > > few extra patches on resource side.
> > > > >
> > > > > Unfortunately I don't have a reason to suspect these would help with
> > > > > the issues related to the currently ongoing resource regression
> > > > > thread [1].
> > > >
> > > > Jonathan,
> > > > can you test this series on affected machine with broken kernel to see if
> > > > it's of any help in your case?
> > >
> > > Certainly, but it will have to wait until next Thursday (11 Jan 2024). I'm
> > > still on leave this week, and when at work I only have physical access to
> > > the machine concerned on Thursdays at present.
> > >
> > > Which kernel would you prefer I apply the series to?
> >
> > I was very short of time today but I did apply the above series to the
> > 5.15.y branch (since I had this source available), resulting in version
> > 5.15.141+. Unfortunately, in the rush I forgot to do a clean after the
> > bisect reset, so the resulting kernel was not correctly built. It booted
> > but thought it was a different version and therefore none of the modules
> > could be found. As a result, the test is invalid.
> >
> > I will try again in a week when I next have physical access to the system.
> > Apologies for the delay. In the meantime, if there's a specific kernel I
> > should apply the patch series against please let me know. As I understand
> > it, you want it applied to one of the kernels which failed, making 5.15.y
> > (for y < 145) a reasonable choice.
>
> I did a "make clean" to reset the source tree and recompiled. However, it
> errored out:
>
> drivers/pci/setup-bus.c:988:24: error: ‘RESOURCE_SIZE_MAX’ undeclared
> drivers/pci/setup-bus.c:998:17: error: ‘pci_bus_for_each_resource’ undeclared
>
> This was with the patch series applied against 5.15.141. It seems the patch
> targets a kernel that's too far removed from 5.15.x.
>
> Which kernel would you like me to apply the patch series to and test?
Two argument version of pci_bus_for_each_resource() is quite new (so
either 6.6 or 6.7). If want to attempt to compile in 5.15.x, you need
this:
include/linux/limits.h:#define RESOURCE_SIZE_MAX ((resource_size_t)~0)
And to add one extra argument into pci_bus_for_each_resource(bus, r) in
pbus_upstream_assigned_limit():
...
while ((bus = bus->parent)) {
+ unsigned int i;
if (pci_is_root_bus(bus))
break;
- pci_bus_for_each_resource(bus, r) {
+ pci_bus_for_each_resource(bus, r, i) {
Note I've written this "patch" by hand inline so patch command cannot
apply it but you need to edit those in.
--
i.
Hi Ilpo,
On Thu, Dec 28, 2023 at 06:57:00PM +0200, Ilpo Järvinen wrote:
> Hi all,
>
> Here's a series that contains two fixes to PCI bridge window sizing
> algorithm. Together, they should enable remove & rescan cycle to work
> for a PCI bus that has PCI devices with optional resources and/or
> disparity in BAR sizes.
>
> For the second fix, I chose to expose find_empty_resource_slot() from
> kernel/resource.c because it should increase accuracy of the cannot-fit
> decision (currently that function is called find_resource()). In order
> to do that sensibly, a few improvements seemed in order to make its
> interface and name of the function sane before exposing it. Thus, the
> few extra patches on resource side.
>
> Unfortunately I don't have a reason to suspect these would help with
> the issues related to the currently ongoing resource regression
> thread [1].
>
> [1] https://lore.kernel.org/linux-pci/ZXpaNCLiDM+Kv38H@marvin.atrad.com.au/
>
> v2:
> - Add "typedef" to kerneldoc to get correct formatting
> - Use RESOURCE_SIZE_MAX instead of literal
> - Remove unnecessary checks for io{port/mem}_resource
> - Apply a few style tweaks from Andy
>
> Ilpo Järvinen (7):
> PCI: Fix resource double counting on remove & rescan
> resource: Rename find_resource() to find_empty_resource_slot()
> resource: Document find_empty_resource_slot() and resource_constraint
> resource: Use typedef for alignf callback
> resource: Handle simple alignment inside __find_empty_resource_slot()
> resource: Export find_empty_resource_slot()
> PCI: Relax bridge window tail sizing rules
Thanks for doing this! :)
All look good to me,
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
© 2016 - 2025 Red Hat, Inc.