On Tue, Jun 25, 2019 at 09:45:04AM +1000, Benjamin Herrenschmidt wrote: > On Mon, 2019-06-24 at 14:24 +0300, Mika Westerberg wrote: > > > > > > I'm pretty sure this patch preserves the previous behavior of > > > pci_bus_distribute_available_resources(), but I'm not sure that > > > behavior is what we want. > > > > > > For example, in the following topology, when we process bus 10, we > > > find two non-hotplug bridges and no hotplug bridges, so IIUC we > > > return > > > without distributing any resources to them. But I would think we > > > should try to give 10:1c.0 more space if possible because it has a > > > hotplug bridge below it. > > > > > > 00:1c.0: hotplug bridge to [bus 10-2f] > > > 10:1c.0: non-hotplug bridge to [bus 11-2e] > > > 11:00.0: hotplug bridge to [bus 12-2e] > > > 10:1c.1: non-hotplug bridge to [bus 2f] > > > > Yes, I agree in this case we want to preserve more space for 10:1c.0. > > I sitll can't make sense of any of this stuff though. > > We only every distribute resources when using > pci_assign_unassigned_bridge_resources which we only use some cases, > and it's completely non obvious why we would use it there and not in > other places. We added it only for native PCIe hotplug path with the assumption that the boot firmware takes care of the initial resource allocation. I don't see any particular reason why it could not be called for other paths as well, though. > We also don't distribute during the initial root survey meaning afaik > that we get toast for any hotplug bridge that has stuff already there > at boot. The boot firmware obviously needs to follow the same logic. AFAICT recent PCs and Macs using native PCIe hotplug handle it. > Also, distributing the "available" space means we leave nothing for > potential SR-IOV siblings... have we ended up bloting the very PCIe- > centric assumption that it's "unlikely" that a hotplug bridge has an > SR-IOV sibling ? Looking at the code, I'm not sure we reserved any additional resource space for the SR-IOV even before pci_bus_distribute_available_resources() was introduced. We do reserve extra bus numbers for SR-IOV in pci_scan_child_bus_extend() so maybe we can add something similar to resource allocation path.
