Re: [PATCH 2/2] PCI: Skip resource distribution when no hotplug bridges

[Mika Westerberg <mika.westerberg@linux.intel.com>]

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.