On Mon, Mar 11, 2019 at 04:31:21PM +0300, Sergey Miroshnichenko wrote:
> With movable BARs, adding a hotplugged device may affect all the PCIe
> domain starting from the root, so use a pci_rescan_bus() function which
> handles the rearrangement of existing BARs and bridge windows.
>
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@xxxxxxxxx>
> ---
> drivers/pci/hotplug/pciehp_pci.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
> index b9c1396db6fe..7c0871db5bae 100644
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -56,12 +56,16 @@ int pciehp_configure_device(struct controller *ctrl)
> goto out;
> }
>
> - for_each_pci_bridge(dev, parent)
> - pci_hp_add_bridge(dev);
> + if (pci_movable_bars_enabled()) {
> + pci_rescan_bus(parent);
> + } else {
> + for_each_pci_bridge(dev, parent)
> + pci_hp_add_bridge(dev);
>
> - pci_assign_unassigned_bridge_resources(bridge);
> - pcie_bus_configure_settings(parent);
> - pci_bus_add_devices(parent);
> + pci_assign_unassigned_bridge_resources(bridge);
> + pcie_bus_configure_settings(parent);
> + pci_bus_add_devices(parent);
> + }
The addition of a second path at this level, i.e., different paths
depending on whether movable BARs are enabled, seems a little
problematic because it's hard to determine whether they're equivalent
except for the movable BAR aspect. For example, you don't call
pci_hp_add_bridge() when movable BARs are enabled, and I can't tell
whether that's intentional or whether it's a problem.
This looks like the sort of change that should be made in other
hotplug paths, e.g., enable_slot() for acpiphp,
pcibios_finish_adding_to_bus() for powerpc (maybe? I can't really
tell), cpci_configure_slot() shpchp_configure_device()?
If we have or could invent some top-level interface that all these
places could use, and somewhere inside that we could do the movable
BAR magic, I think that would make it more maintainable.
> out:
> pci_unlock_rescan_remove();
> --
> 2.20.1
>
