[PATCH] cxl: use pcibios_free_controller_deferred() when removing vPHBs

[PATCH] cxl: use pcibios_free_controller_deferred() when removing vPHBs

[Andrew Donnellan]

When cxl removes a vPHB, it's possible that the pci_controller may be freed
before all references to the devices on the vPHB have been released. This
in turn causes an invalid memory access when the devices are eventually
released, as pcibios_release_device() attempts to call the phb's
release_device hook.

In cxl_pci_vphb_remove(), remove the existing call to
pcibios_free_controller(). Instead, use
pcibios_free_controller_deferred() to free the pci_controller after all
devices have been released. Export pci_set_host_bridge_release() so we can
do this.

Cc: stable@vger.kernel.org
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

This patch requires http://patchwork.ozlabs.org/patch/658324/. It should go
through the powerpc tree.
---
 drivers/misc/cxl/vphb.c   | 10 +++++++++-
 drivers/pci/host-bridge.c |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c
index 7ada5f1..3519ace 100644
--- a/drivers/misc/cxl/vphb.c
+++ b/drivers/misc/cxl/vphb.c
@@ -230,6 +230,11 @@ int cxl_pci_vphb_add(struct cxl_afu *afu)
 	if (phb->bus == NULL)
 		return -ENXIO;
 
+	/* Set release hook on root bus */
+	pci_set_host_bridge_release(to_pci_host_bridge(phb->bus->bridge),
+				    pcibios_free_controller_deferred,
+				    (void *) phb);
+
 	/* Claim resources. This might need some rework as well depending
 	 * whether we are doing probe-only or not, like assigning unassigned
 	 * resources etc...
@@ -256,7 +261,10 @@ void cxl_pci_vphb_remove(struct cxl_afu *afu)
 	afu->phb = NULL;
 
 	pci_remove_root_bus(phb->bus);
-	pcibios_free_controller(phb);
+	/*
+	 * We don't free phb here - that's handled by
+	 * pcibios_free_controller_deferred()
+	 */
 }
 
 static bool _cxl_pci_is_vphb_device(struct pci_controller *phb)
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 5f4a2e0..add6623 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -44,6 +44,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
 	bridge->release_fn = release_fn;
 	bridge->release_data = release_data;
 }
+EXPORT_SYMBOL_GPL(pci_set_host_bridge_release);
 
 void pcibios_resource_to_bus(struct pci_bus *bus, struct pci_bus_region *region,
 			     struct resource *res)
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] cxl: use pcibios_free_controller_deferred() when removing vPHBs

[Matthew R. Ochs]

> On Aug 18, 2016, at 2:35 AM, Andrew Donnellan =
<andrew.donnellan@au1.ibm.com> wrote:
>=20
> When cxl removes a vPHB, it's possible that the pci_controller may be =
freed
> before all references to the devices on the vPHB have been released. =
This
> in turn causes an invalid memory access when the devices are =
eventually
> released, as pcibios_release_device() attempts to call the phb's
> release_device hook.
>=20
> In cxl_pci_vphb_remove(), remove the existing call to
> pcibios_free_controller(). Instead, use
> pcibios_free_controller_deferred() to free the pci_controller after =
all
> devices have been released. Export pci_set_host_bridge_release() so we =
can
> do this.
>=20
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Re: [PATCH] cxl: use pcibios_free_controller_deferred() when removing vPHBs

[Ian Munsie]

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: [PATCH] cxl: use pcibios_free_controller_deferred() when removing vPHBs

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

Hi Andrew,

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.8-rc3 next-20160822]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Andrew-Donnellan/cxl-use-pcibios_free_controller_deferred-when-removing-vPHBs/20160818-154040
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   drivers/misc/cxl/vphb.c: In function 'cxl_pci_vphb_add':
>> drivers/misc/cxl/vphb.c:235:9: error: 'pcibios_free_controller_deferred' undeclared (first use in this function)
            pcibios_free_controller_deferred,
            ^
   drivers/misc/cxl/vphb.c:235:9: note: each undeclared identifier is reported only once for each function it appears in

vim +/pcibios_free_controller_deferred +235 drivers/misc/cxl/vphb.c

   229		pcibios_scan_phb(phb);
   230		if (phb->bus == NULL)
   231			return -ENXIO;
   232	
   233		/* Set release hook on root bus */
   234		pci_set_host_bridge_release(to_pci_host_bridge(phb->bus->bridge),
 > 235					    pcibios_free_controller_deferred,
   236					    (void *) phb);
   237	
   238		/* Claim resources. This might need some rework as well depending

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 49909 bytes --]

Re: [PATCH] cxl: use pcibios_free_controller_deferred() when removing vPHBs

[Andrew Donnellan]

Hi stable team,

The following patch, which ended up upstream as 
6f38a8b9a45833495dc878c335c5431cd98a16ed:

On 18/08/16 17:35, Andrew Donnellan wrote:
> When cxl removes a vPHB, it's possible that the pci_controller may be freed
> before all references to the devices on the vPHB have been released. This
> in turn causes an invalid memory access when the devices are eventually
> released, as pcibios_release_device() attempts to call the phb's
> release_device hook.
>
> In cxl_pci_vphb_remove(), remove the existing call to
> pcibios_free_controller(). Instead, use
> pcibios_free_controller_deferred() to free the pci_controller after all
> devices have been released. Export pci_set_host_bridge_release() so we can
> do this.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
> ---
>
> This patch requires http://patchwork.ozlabs.org/patch/658324/. It should go
> through the powerpc tree.

This depends on 2dd9c11b9d4dfbd6c070eab7b81197f65e82f1a0 which didn't 
end up being tagged as cc: stable. It also ended up being applied in the 
wrong order in the powerpc/fixes tree...


Thanks,
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] cxl: use pcibios_free_controller_deferred() when removing vPHBs

[Benjamin Herrenschmidt]

On Tue, 2016-08-30 at 11:58 +1000, Andrew Donnellan wrote:
> Hi stable team,
> 
> The following patch, which ended up upstream as 
> 6f38a8b9a45833495dc878c335c5431cd98a16ed:
> 
> On 18/08/16 17:35, Andrew Donnellan wrote:
> > 
> > When cxl removes a vPHB, it's possible that the pci_controller may be freed
> > before all references to the devices on the vPHB have been released. This
> > in turn causes an invalid memory access when the devices are eventually
> > released, as pcibios_release_device() attempts to call the phb's
> > release_device hook.
> > 
> > In cxl_pci_vphb_remove(), remove the existing call to
> > pcibios_free_controller(). Instead, use
> > pcibios_free_controller_deferred() to free the pci_controller after all
> > devices have been released. Export pci_set_host_bridge_release() so we can
> > do this.
> > 
> > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> > 
> > ---
> > 
> > > > This patch requires http://patchwork.ozlabs.org/patch/658324/. It should go
> > through the powerpc tree.
> 
> This depends on 2dd9c11b9d4dfbd6c070eab7b81197f65e82f1a0 which didn't 
> end up being tagged as cc: stable. It also ended up being applied in the 
> wrong order in the powerpc/fixes tree...

My fault. Best at this point is to also apply 2dd9c11b9d4dfbd6c070eab7b81197f65e82f1a0
to stable.

Cheers,
Ben.