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.miroshniche...@yadro.com>
> ---
>  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
> 

Reply via email to