Skip to content

Commit d316a8a

Browse files
Mauricio Faria de Oliveirartg-canonical
authored andcommitted
powerpc/pseries: use pci_host_bridge.release_fn() to kfree(phb)
BugLink: http://bugs.launchpad.net/bugs/1618151 This patch leverages 'struct pci_host_bridge' from the PCI subsystem in order to free the pci_controller only after the last reference to its devices is dropped (avoiding an oops in pcibios_release_device() if the last reference is dropped after pcibios_free_controller()). The patch relies on pci_host_bridge.release_fn() (and .release_data), which is called automatically by the PCI subsystem when the root bus is released (i.e., the last reference is dropped). Those fields are set via pci_set_host_bridge_release() (e.g. in the platform-specific implementation of pcibios_root_bridge_prepare()). It introduces the 'pcibios_free_controller_deferred()' .release_fn() and it expects .release_data to hold a pointer to the pci_controller. The function implictly calls 'pcibios_free_controller()', so an user must *NOT* explicitly call it if using the new _deferred() callback. The functionality is enabled for pseries (although it isn't platform specific, and may be used by cxl). Details on not-so-elegant design choices: - Use 'pci_host_bridge.release_data' field as pointer to associated 'struct pci_controller' so *not* to 'pci_bus_to_host(bridge->bus)' in pcibios_free_controller_deferred(). That's because pci_remove_root_bus() sets 'host_bridge->bus = NULL' (so, if the last reference is released after pci_remove_root_bus() runs, which eventually reaches pcibios_free_controller_deferred(), that would hit a null pointer dereference). The cxl/vphb.c code calls pci_remove_root_bus(), and the cxl folks are interested in this fix. Test-case #1 (hold references) # ls -ld /sys/block/sd* | grep -m1 0021:01:00.0 <...> /sys/block/sdaa -> ../devices/pci0021:01/0021:01:00.0/<...> # ls -ld /sys/block/sd* | grep -m1 0021:01:00.1 <...> /sys/block/sdab -> ../devices/pci0021:01/0021:01:00.1/<...> # cat >/dev/sdaa & pid1=$! # cat >/dev/sdab & pid2=$! # drmgr -w 5 -d 1 -c phb -s 'PHB 33' -r Validating PHB DLPAR capability...yes. [ 594.306719] pci_hp_remove_devices: PCI: Removing devices on bus 0021:01 [ 594.306738] pci_hp_remove_devices: Removing 0021:01:00.0... ... [ 598.236381] pci_hp_remove_devices: Removing 0021:01:00.1... ... [ 611.972077] pci_bus 0021:01: busn_res: [bus 01-ff] is released [ 611.972140] rpadlpar_io: slot PHB 33 removed # kill -9 $pid1 # kill -9 $pid2 [ 632.918088] pcibios_free_controller_deferred: domain 33, dynamic 1 Test-case #2 (don't hold references) # drmgr -w 5 -d 1 -c phb -s 'PHB 33' -r Validating PHB DLPAR capability...yes. [ 916.357363] pci_hp_remove_devices: PCI: Removing devices on bus 0021:01 [ 916.357386] pci_hp_remove_devices: Removing 0021:01:00.0... ... [ 920.566527] pci_hp_remove_devices: Removing 0021:01:00.1... ... [ 933.955873] pci_bus 0021:01: busn_res: [bus 01-ff] is released [ 933.955977] pcibios_free_controller_deferred: domain 33, dynamic 1 [ 933.955999] rpadlpar_io: slot PHB 33 removed Suggested-By: Gavin Shan <[email protected]> Signed-off-by: Mauricio Faria de Oliveira <[email protected]> Reviewed-by: Gavin Shan <[email protected]> Reviewed-by: Andrew Donnellan <[email protected]> Tested-by: Andrew Donnellan <[email protected]> # cxl Signed-off-by: Benjamin Herrenschmidt <[email protected]> (cherry picked from commit 2dd9c11) Signed-off-by: Tim Gardner <[email protected]> Acked-by: Kamal Mostafa <[email protected]> Acked-by: Chris J Arges <[email protected]>
1 parent 880c817 commit d316a8a

File tree

4 files changed

+46
-2
lines changed

4 files changed

+46
-2
lines changed

arch/powerpc/include/asm/pci-bridge.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ extern void pci_process_bridge_OF_ranges(struct pci_controller *hose,
296296
/* Allocate & free a PCI host bridge structure */
297297
extern struct pci_controller *pcibios_alloc_controller(struct device_node *dev);
298298
extern void pcibios_free_controller(struct pci_controller *phb);
299+
extern void pcibios_free_controller_deferred(struct pci_host_bridge *bridge);
299300

300301
#ifdef CONFIG_PCI
301302
extern int pcibios_vaddr_is_ioport(void __iomem *address);

arch/powerpc/kernel/pci-common.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,42 @@ void pcibios_free_controller(struct pci_controller *phb)
150150
}
151151
EXPORT_SYMBOL_GPL(pcibios_free_controller);
152152

153+
/*
154+
* This function is used to call pcibios_free_controller()
155+
* in a deferred manner: a callback from the PCI subsystem.
156+
*
157+
* _*DO NOT*_ call pcibios_free_controller() explicitly if
158+
* this is used (or it may access an invalid *phb pointer).
159+
*
160+
* The callback occurs when all references to the root bus
161+
* are dropped (e.g., child buses/devices and their users).
162+
*
163+
* It's called as .release_fn() of 'struct pci_host_bridge'
164+
* which is associated with the 'struct pci_controller.bus'
165+
* (root bus) - it expects .release_data to hold a pointer
166+
* to 'struct pci_controller'.
167+
*
168+
* In order to use it, register .release_fn()/release_data
169+
* like this:
170+
*
171+
* pci_set_host_bridge_release(bridge,
172+
* pcibios_free_controller_deferred
173+
* (void *) phb);
174+
*
175+
* e.g. in the pcibios_root_bridge_prepare() callback from
176+
* pci_create_root_bus().
177+
*/
178+
void pcibios_free_controller_deferred(struct pci_host_bridge *bridge)
179+
{
180+
struct pci_controller *phb = (struct pci_controller *)
181+
bridge->release_data;
182+
183+
pr_debug("domain %d, dynamic %d\n", phb->global_number, phb->is_dynamic);
184+
185+
pcibios_free_controller(phb);
186+
}
187+
EXPORT_SYMBOL_GPL(pcibios_free_controller_deferred);
188+
153189
/*
154190
* The function is used to return the minimal alignment
155191
* for memory or I/O windows of the associated P2P bridge.

arch/powerpc/platforms/pseries/pci.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ int pseries_root_bridge_prepare(struct pci_host_bridge *bridge)
119119

120120
bus = bridge->bus;
121121

122+
/* Rely on the pcibios_free_controller_deferred() callback. */
123+
pci_set_host_bridge_release(bridge, pcibios_free_controller_deferred,
124+
(void *) pci_bus_to_host(bus));
125+
122126
dn = pcibios_get_phb_of_node(bus);
123127
if (!dn)
124128
return 0;

arch/powerpc/platforms/pseries/pci_dlpar.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,11 @@ int remove_phb_dynamic(struct pci_controller *phb)
138138
release_resource(res);
139139
}
140140

141-
/* Free pci_controller data structure */
142-
pcibios_free_controller(phb);
141+
/*
142+
* The pci_controller data structure is freed by
143+
* the pcibios_free_controller_deferred() callback;
144+
* see pseries_root_bridge_prepare().
145+
*/
143146

144147
return 0;
145148
}

0 commit comments

Comments
 (0)