| Message ID | 20230112110000.59974-2-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: distribute resources for root buses | expand |
Hi Mika, Unfortunately my system was de-racked meanwhile, so it will take few more days for me to test this. So far I only successfully built it on my 5.15.79 kernel. Meanwhile some comments below: On 12.01.2023 05:59, Mika Westerberg wrote: > After division the extra resource space per hotplug bridge may not be > aligned according to the window alignment so do that before passing it > down for further distribution. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/pci/setup-bus.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index b4096598dbcb..34a74bc581b0 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -1891,6 +1891,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, > * resource space between hotplug bridges. > */ > for_each_pci_bridge(dev, bus) { > + struct resource *res; > struct pci_bus *b; > > b = dev->subordinate; > @@ -1902,16 +1903,28 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, > * hotplug-capable downstream ports taking alignment into > * account. > */ > - io.end = io.start + io_per_hp - 1; > - mmio.end = mmio.start + mmio_per_hp - 1; > - mmio_pref.end = mmio_pref.start + mmio_pref_per_hp - 1; > + res = &dev->resource[PCI_BRIDGE_IO_WINDOW]; > + align = pci_resource_alignment(dev, res); > + io.end = align ? io.start + ALIGN_DOWN(io_per_hp, align) - 1 > + : io.start + io_per_hp - 1; Not bug probably, but if we align x_per_b down for one bridge, we could be able to increase it for other(s). > + > + res = &dev->resource[PCI_BRIDGE_MEM_WINDOW]; > + align = pci_resource_alignment(dev, res); > + mmio.end = align ? mmio.start + ALIGN_DOWN(mmio_per_hp, align) - 1 > + : mmio.start + io_per_hp - 1; Here is a typo, it should be mmio_per_hp here ^^^. > + > + res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW]; > + align = pci_resource_alignment(dev, res); > + mmio_pref.end = align ? mmio_pref.start + > + ALIGN_DOWN(mmio_pref_per_hp, align) - 1 > + : mmio_pref.start + mmio_pref_per_hp; > > pci_bus_distribute_available_resources(b, add_list, io, mmio, > mmio_pref); > > - io.start += io_per_hp; > - mmio.start += mmio_per_hp; > - mmio_pref.start += mmio_pref_per_hp; > + io.start += io.end + 1; > + mmio.start += mmio.end + 1; > + mmio_pref.start += mmio_pref.end + 1; > } > } >
Hi, On Thu, Jan 26, 2023 at 11:53:55AM -0500, Alexander Motin wrote: > Hi Mika, > > Unfortunately my system was de-racked meanwhile, so it will take few more > days for me to test this. So far I only successfully built it on my 5.15.79 > kernel. Meanwhile some comments below: Okay, take your time and thanks for reviewing! > > On 12.01.2023 05:59, Mika Westerberg wrote: > > After division the extra resource space per hotplug bridge may not be > > aligned according to the window alignment so do that before passing it > > down for further distribution. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > --- > > drivers/pci/setup-bus.c | 25 +++++++++++++++++++------ > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > index b4096598dbcb..34a74bc581b0 100644 > > --- a/drivers/pci/setup-bus.c > > +++ b/drivers/pci/setup-bus.c > > @@ -1891,6 +1891,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, > > * resource space between hotplug bridges. > > */ > > for_each_pci_bridge(dev, bus) { > > + struct resource *res; > > struct pci_bus *b; > > b = dev->subordinate; > > @@ -1902,16 +1903,28 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, > > * hotplug-capable downstream ports taking alignment into > > * account. > > */ > > - io.end = io.start + io_per_hp - 1; > > - mmio.end = mmio.start + mmio_per_hp - 1; > > - mmio_pref.end = mmio_pref.start + mmio_pref_per_hp - 1; > > + res = &dev->resource[PCI_BRIDGE_IO_WINDOW]; > > + align = pci_resource_alignment(dev, res); > > + io.end = align ? io.start + ALIGN_DOWN(io_per_hp, align) - 1 > > + : io.start + io_per_hp - 1; > > Not bug probably, but if we align x_per_b down for one bridge, we could be > able to increase it for other(s). Yeah, the down align is just to make sure we don't run over what is there because of the splitting. We could for example leave the "leftovers" to the last bridge or so but I don't think we want to do it in this patch series to avoid any additional bugs creeping in. Unless you guys think it needs to be done, of course. > > + > > + res = &dev->resource[PCI_BRIDGE_MEM_WINDOW]; > > + align = pci_resource_alignment(dev, res); > > + mmio.end = align ? mmio.start + ALIGN_DOWN(mmio_per_hp, align) - 1 > > + : mmio.start + io_per_hp - 1; > > Here is a typo, it should be mmio_per_hp here ^^^. Good catch! I went over these specifically for things like this but I still missed it :( Will fix in next version. > > + > > + res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW]; > > + align = pci_resource_alignment(dev, res); > > + mmio_pref.end = align ? mmio_pref.start + > > + ALIGN_DOWN(mmio_pref_per_hp, align) - 1 > > + : mmio_pref.start + mmio_pref_per_hp; > > pci_bus_distribute_available_resources(b, add_list, io, mmio, > > mmio_pref); > > - io.start += io_per_hp; > > - mmio.start += mmio_per_hp; > > - mmio_pref.start += mmio_pref_per_hp; > > + io.start += io.end + 1; > > + mmio.start += mmio.end + 1; > > + mmio_pref.start += mmio_pref.end + 1; > > } > > } > > -- > Alexander Motin
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index b4096598dbcb..34a74bc581b0 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -1891,6 +1891,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, * resource space between hotplug bridges. */ for_each_pci_bridge(dev, bus) { + struct resource *res; struct pci_bus *b; b = dev->subordinate; @@ -1902,16 +1903,28 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, * hotplug-capable downstream ports taking alignment into * account. */ - io.end = io.start + io_per_hp - 1; - mmio.end = mmio.start + mmio_per_hp - 1; - mmio_pref.end = mmio_pref.start + mmio_pref_per_hp - 1; + res = &dev->resource[PCI_BRIDGE_IO_WINDOW]; + align = pci_resource_alignment(dev, res); + io.end = align ? io.start + ALIGN_DOWN(io_per_hp, align) - 1 + : io.start + io_per_hp - 1; + + res = &dev->resource[PCI_BRIDGE_MEM_WINDOW]; + align = pci_resource_alignment(dev, res); + mmio.end = align ? mmio.start + ALIGN_DOWN(mmio_per_hp, align) - 1 + : mmio.start + io_per_hp - 1; + + res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW]; + align = pci_resource_alignment(dev, res); + mmio_pref.end = align ? mmio_pref.start + + ALIGN_DOWN(mmio_pref_per_hp, align) - 1 + : mmio_pref.start + mmio_pref_per_hp; pci_bus_distribute_available_resources(b, add_list, io, mmio, mmio_pref); - io.start += io_per_hp; - mmio.start += mmio_per_hp; - mmio_pref.start += mmio_pref_per_hp; + io.start += io.end + 1; + mmio.start += mmio.end + 1; + mmio_pref.start += mmio_pref.end + 1; } }
After division the extra resource space per hotplug bridge may not be aligned according to the window alignment so do that before passing it down for further distribution. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/pci/setup-bus.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)