Message ID | 20241128084039.54972-1-kaihengf@nvidia.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | PCI: Use downstream bridges for distributing resources | expand |
On Thu, Nov 28, 2024 at 04:40:39PM +0800, Kai-Heng Feng wrote: > Commit 7180c1d08639 ("PCI: Distribute available resources for root > buses, too") breaks BAR assignment on some devcies: > [ 10.021193] pci 0006:03:00.0: BAR 0 [mem 0x6300c0000000-0x6300c1ffffff 64bit pref]: assigned > [ 10.029880] pci 0006:03:00.1: BAR 0 [mem 0x6300c2000000-0x6300c3ffffff 64bit pref]: assigned > [ 10.038561] pci 0006:03:00.2: BAR 0 [mem size 0x00800000 64bit pref]: can't assign; no space > [ 10.047191] pci 0006:03:00.2: BAR 0 [mem size 0x00800000 64bit pref]: failed to assign > [ 10.055285] pci 0006:03:00.0: VF BAR 0 [mem size 0x02000000 64bit pref]: can't assign; no space > [ 10.064180] pci 0006:03:00.0: VF BAR 0 [mem size 0x02000000 64bit pref]: failed to assign > [ 10.072543] pci 0006:03:00.1: VF BAR 0 [mem size 0x02000000 64bit pref]: can't assign; no space > [ 10.081437] pci 0006:03:00.1: VF BAR 0 [mem size 0x02000000 64bit pref]: failed to assign > > The apertures of domain 0006 before the commit: > 6300c0000000-63ffffffffff : PCI Bus 0006:00 > 6300c0000000-6300c9ffffff : PCI Bus 0006:01 > 6300c0000000-6300c9ffffff : PCI Bus 0006:02 > 6300c0000000-6300c8ffffff : PCI Bus 0006:03 > 6300c0000000-6300c1ffffff : 0006:03:00.0 > 6300c0000000-6300c1ffffff : mlx5_core > 6300c2000000-6300c3ffffff : 0006:03:00.1 > 6300c2000000-6300c3ffffff : mlx5_core > 6300c4000000-6300c47fffff : 0006:03:00.2 > 6300c4800000-6300c67fffff : 0006:03:00.0 > 6300c6800000-6300c87fffff : 0006:03:00.1 > 6300c9000000-6300c9bfffff : PCI Bus 0006:04 > 6300c9000000-6300c9bfffff : PCI Bus 0006:05 > 6300c9000000-6300c91fffff : PCI Bus 0006:06 > 6300c9200000-6300c93fffff : PCI Bus 0006:07 > 6300c9400000-6300c95fffff : PCI Bus 0006:08 > 6300c9600000-6300c97fffff : PCI Bus 0006:09 > > After the commit: > 6300c0000000-63ffffffffff : PCI Bus 0006:00 > 6300c0000000-6300c9ffffff : PCI Bus 0006:01 > 6300c0000000-6300c9ffffff : PCI Bus 0006:02 > 6300c0000000-6300c43fffff : PCI Bus 0006:03 > 6300c0000000-6300c1ffffff : 0006:03:00.0 > 6300c0000000-6300c1ffffff : mlx5_core > 6300c2000000-6300c3ffffff : 0006:03:00.1 > 6300c2000000-6300c3ffffff : mlx5_core > 6300c4400000-6300c4dfffff : PCI Bus 0006:04 > 6300c4400000-6300c4dfffff : PCI Bus 0006:05 > 6300c4400000-6300c45fffff : PCI Bus 0006:06 > 6300c4600000-6300c47fffff : PCI Bus 0006:07 > 6300c4800000-6300c49fffff : PCI Bus 0006:08 > 6300c4a00000-6300c4bfffff : PCI Bus 0006:09 > > We can see that the window of 0006:03 gets shrunken too much and 0006:04 > eats away the window for 0006:03:00.2. > > The offending commit distributes the upstream bridge's resources > multiple times to every downstream bridges, hence makes the aperture > smaller than desired because calculation of io_per_b, mmio_per_b and > mmio_pref_per_b becomes incorrect. > > Instead, distributing downstream bridges' own resources to resolve the > issue. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219540 > Cc: Carol Soto <csoto@nvidia.com> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: Chris Chiu <chris.chiu@canonical.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Reviewed-by: Matthew R. Ochs <mochs@nvidia.com> > Reviewed-by: Koba Ko <kobak@nvidia.com> > Fixes: 7180c1d08639 ("PCI: Distribute available resources for root buses, too") > Signed-off-by: Kai-Heng Feng <kaihengf@nvidia.com> > --- > drivers/pci/setup-bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 23082bc0ca37..2db19c17e824 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -2105,7 +2105,7 @@ pci_root_bus_distribute_available_resources(struct pci_bus *bus, > * in case of root bukjijs. > */ > if (bridge && pci_bridge_resources_not_assigned(dev)) > - pci_bridge_distribute_available_resources(bridge, > + pci_bridge_distribute_available_resources(dev, > add_list); I think it looks better if you put this into one line instead: pci_bridge_distribute_available_resources(dev, add_list); Otherwise looks good. I wonder if you checked that this still works with the cases 7180c1d08639 tried to solve? ;-) > else > pci_root_bus_distribute_available_resources(b, add_list); > -- > 2.47.0
On 2024/11/28 5:45 PM, Mika Westerberg wrote: > External email: Use caution opening links or attachments > > > On Thu, Nov 28, 2024 at 04:40:39PM +0800, Kai-Heng Feng wrote: >> Commit 7180c1d08639 ("PCI: Distribute available resources for root >> buses, too") breaks BAR assignment on some devcies: >> [ 10.021193] pci 0006:03:00.0: BAR 0 [mem 0x6300c0000000-0x6300c1ffffff 64bit pref]: assigned >> [ 10.029880] pci 0006:03:00.1: BAR 0 [mem 0x6300c2000000-0x6300c3ffffff 64bit pref]: assigned >> [ 10.038561] pci 0006:03:00.2: BAR 0 [mem size 0x00800000 64bit pref]: can't assign; no space >> [ 10.047191] pci 0006:03:00.2: BAR 0 [mem size 0x00800000 64bit pref]: failed to assign >> [ 10.055285] pci 0006:03:00.0: VF BAR 0 [mem size 0x02000000 64bit pref]: can't assign; no space >> [ 10.064180] pci 0006:03:00.0: VF BAR 0 [mem size 0x02000000 64bit pref]: failed to assign >> [ 10.072543] pci 0006:03:00.1: VF BAR 0 [mem size 0x02000000 64bit pref]: can't assign; no space >> [ 10.081437] pci 0006:03:00.1: VF BAR 0 [mem size 0x02000000 64bit pref]: failed to assign >> >> The apertures of domain 0006 before the commit: >> 6300c0000000-63ffffffffff : PCI Bus 0006:00 >> 6300c0000000-6300c9ffffff : PCI Bus 0006:01 >> 6300c0000000-6300c9ffffff : PCI Bus 0006:02 >> 6300c0000000-6300c8ffffff : PCI Bus 0006:03 >> 6300c0000000-6300c1ffffff : 0006:03:00.0 >> 6300c0000000-6300c1ffffff : mlx5_core >> 6300c2000000-6300c3ffffff : 0006:03:00.1 >> 6300c2000000-6300c3ffffff : mlx5_core >> 6300c4000000-6300c47fffff : 0006:03:00.2 >> 6300c4800000-6300c67fffff : 0006:03:00.0 >> 6300c6800000-6300c87fffff : 0006:03:00.1 >> 6300c9000000-6300c9bfffff : PCI Bus 0006:04 >> 6300c9000000-6300c9bfffff : PCI Bus 0006:05 >> 6300c9000000-6300c91fffff : PCI Bus 0006:06 >> 6300c9200000-6300c93fffff : PCI Bus 0006:07 >> 6300c9400000-6300c95fffff : PCI Bus 0006:08 >> 6300c9600000-6300c97fffff : PCI Bus 0006:09 >> >> After the commit: >> 6300c0000000-63ffffffffff : PCI Bus 0006:00 >> 6300c0000000-6300c9ffffff : PCI Bus 0006:01 >> 6300c0000000-6300c9ffffff : PCI Bus 0006:02 >> 6300c0000000-6300c43fffff : PCI Bus 0006:03 >> 6300c0000000-6300c1ffffff : 0006:03:00.0 >> 6300c0000000-6300c1ffffff : mlx5_core >> 6300c2000000-6300c3ffffff : 0006:03:00.1 >> 6300c2000000-6300c3ffffff : mlx5_core >> 6300c4400000-6300c4dfffff : PCI Bus 0006:04 >> 6300c4400000-6300c4dfffff : PCI Bus 0006:05 >> 6300c4400000-6300c45fffff : PCI Bus 0006:06 >> 6300c4600000-6300c47fffff : PCI Bus 0006:07 >> 6300c4800000-6300c49fffff : PCI Bus 0006:08 >> 6300c4a00000-6300c4bfffff : PCI Bus 0006:09 >> >> We can see that the window of 0006:03 gets shrunken too much and 0006:04 >> eats away the window for 0006:03:00.2. >> >> The offending commit distributes the upstream bridge's resources >> multiple times to every downstream bridges, hence makes the aperture >> smaller than desired because calculation of io_per_b, mmio_per_b and >> mmio_pref_per_b becomes incorrect. >> >> Instead, distributing downstream bridges' own resources to resolve the >> issue. >> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219540 >> Cc: Carol Soto <csoto@nvidia.com> >> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> >> Cc: Chris Chiu <chris.chiu@canonical.com> >> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> >> Reviewed-by: Matthew R. Ochs <mochs@nvidia.com> >> Reviewed-by: Koba Ko <kobak@nvidia.com> >> Fixes: 7180c1d08639 ("PCI: Distribute available resources for root buses, too") >> Signed-off-by: Kai-Heng Feng <kaihengf@nvidia.com> >> --- >> drivers/pci/setup-bus.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c >> index 23082bc0ca37..2db19c17e824 100644 >> --- a/drivers/pci/setup-bus.c >> +++ b/drivers/pci/setup-bus.c >> @@ -2105,7 +2105,7 @@ pci_root_bus_distribute_available_resources(struct pci_bus *bus, >> * in case of root bukjijs. >> */ >> if (bridge && pci_bridge_resources_not_assigned(dev)) >> - pci_bridge_distribute_available_resources(bridge, >> + pci_bridge_distribute_available_resources(dev, >> add_list); > > I think it looks better if you put this into one line instead: > > pci_bridge_distribute_available_resources(dev, add_list); Sure, will do. > > > Otherwise looks good. I wonder if you checked that this still works with > the cases 7180c1d08639 tried to solve? ;-) That's why Chris is Cc'ed. Chris, is it possible to give this patch a try to make sure this doesn't break what 7180c1d08639 solved? Will send v2 after your test result. Kai-Heng >> else >> pci_root_bus_distribute_available_resources(b, add_list); >> -- >> 2.47.0
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 23082bc0ca37..2db19c17e824 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -2105,7 +2105,7 @@ pci_root_bus_distribute_available_resources(struct pci_bus *bus, * in case of root bus. */ if (bridge && pci_bridge_resources_not_assigned(dev)) - pci_bridge_distribute_available_resources(bridge, + pci_bridge_distribute_available_resources(dev, add_list); else pci_root_bus_distribute_available_resources(b, add_list);