ACK/Cmnt: [SRU B][PATCH 1/1] powerpc/powernv/pci: Work around races in PCI bridge enabling

[Stefan Bader]

On 26.11.18 23:08, Daniel Axtens wrote:
> From: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/1805245
> 
> The generic code is racy when multiple children of a PCI bridge try to
> enable it simultaneously.
> 
> This leads to drivers trying to access a device through a
> not-yet-enabled bridge, and this EEH errors under various
> circumstances when using parallel driver probing.
> 
> There is work going on to fix that properly in the PCI core but it
> will take some time.
> 
> x86 gets away with it because (outside of hotplug), the BIOS enables
> all the bridges at boot time.
> 
> This patch does the same thing on powernv by enabling all bridges that
> have child devices at boot time, thus avoiding subsequent races. It's
> suitable for backporting to stable and distros, while the proper PCI
> fix will probably be significantly more invasive.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> Cc: stable at vger.kernel.org
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> (backported from commit db2173198b9513f7add8009f225afa1f1c79bcc6
>  - changed pci_err() -> dev_err())
> Signed-off-by: Michael Neuling <mikey at neuling.org>
> Signed-off-by: Daniel Axtens <daniel.axtens at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---

With changes suggested by Kleber.

>  arch/powerpc/platforms/powernv/pci-ioda.c | 37 +++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index d5af700820f3..ea4630f17f74 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -3312,12 +3312,49 @@ static void pnv_pci_ioda_create_dbgfs(void)
>  #endif /* CONFIG_DEBUG_FS */
>  }
>  
> +static void pnv_pci_enable_bridge(struct pci_bus *bus)
> +{
> +	struct pci_dev *dev = bus->self;
> +	struct pci_bus *child;
> +
> +	/* Empty bus ? bail */
> +	if (list_empty(&bus->devices))
> +		return;
> +
> +	/*
> +	 * If there's a bridge associated with that bus enable it. This works
> +	 * around races in the generic code if the enabling is done during
> +	 * parallel probing. This can be removed once those races have been
> +	 * fixed.
> +	 */
> +	if (dev) {
> +		int rc = pci_enable_device(dev);
> +		if (rc)
> +			dev_err(&dev->dev, "Error enabling bridge (%d)\n", rc);
> +		pci_set_master(dev);
> +	}
> +
> +	/* Perform the same to child busses */
> +	list_for_each_entry(child, &bus->children, node)
> +		pnv_pci_enable_bridge(child);
> +}
> +
> +static void pnv_pci_enable_bridges(void)
> +{
> +	struct pci_controller *hose;
> +
> +	list_for_each_entry(hose, &hose_list, list_node)
> +		pnv_pci_enable_bridge(hose->bus);
> +}
> +
>  static void pnv_pci_ioda_fixup(void)
>  {
>  	pnv_pci_ioda_setup_PEs();
>  	pnv_pci_ioda_setup_iommu_api();
>  	pnv_pci_ioda_create_dbgfs();
>  
> +	pnv_pci_enable_bridges();
> +
>  #ifdef CONFIG_EEH
>  	pnv_eeh_post_init();
>  #endif
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190107/e2137a8d/attachment-0001.sig>