[PATCH v2] PCI: defer bridge cfg lock in pci_try_reset_function()
Runyu Xiao <[email protected]>
| Newsgroups | org.kernel.vger.linux-pci,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
pci_try_reset_function() may need upstream bridge config serialization when __pci_reset_function_locked() falls back to the "bus" or "cxl_bus" reset methods. Unlike pci_reset_function(), it currently only locks the target device, so those fallback paths can still issue an unlocked secondary-bus reset. A naive fix is to trylock the shared upstream bridge before locking the target device, mirroring pci_reset_function(). But pci_try_reset_function() is used in vfio_pci_core_enable(), which treats -EAGAIN as a fatal open failure. Taking the bridge trylock up front therefore broadens -EAGAIN to all try-reset callers, including sibling devices that would otherwise reset independently via FLR. Keep the existing device trylock semantics, but defer bridge serialization until __pci_reset_function_locked() is actually about to run a bus-level reset method. For pci_try_reset_function(), trylock only the bridge config access needed by the bus/cxl_bus paths and return -EAGAIN only if that serialization is really required and contended. This preserves bridge config serialization for the secondary-bus reset fallback without introducing spurious -EAGAIN failures for concurrent resets of sibling devices. Link: https://lore.kernel.org/r/[email protected] Fixes: 61cf16d8bd38 ("PCI: Add pci_try_reset_function(), pci_try_reset_slot(), pci_try_reset_bus()") Cc: [email protected] Signed-off-by: Runyu Xiao <[email protected]> --- v2: - Rework v1 to defer bridge cfg serialization until the `bus` or `cxl_bus` fallback is actually selected. - Avoid broadening `-EAGAIN` to sibling devices that can reset independently via FLR or other function-level methods. - Keep the upstream bridge serialization limited to the path that actually needs secondary-bus reset protection. drivers/pci/pci.c | 75 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 23 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e128696..0aa770c 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5166,28 +5166,17 @@ const struct pci_reset_fn_method pci_reset_fn_methods[] = { { cxl_reset_bus_function, .name = "cxl_bus" }, }; -/** - * __pci_reset_function_locked - reset a PCI device function while holding - * the @dev mutex lock. - * @dev: PCI device to reset - * - * Some devices allow an individual function to be reset without affecting - * other functions in the same device. The PCI device must be responsive - * to PCI config space in order to use this function. - * - * The device function is presumed to be unused and the caller is holding - * the device mutex lock when this function is called. - * - * Resetting the device will make the contents of PCI configuration space - * random, so any caller of this must be prepared to reinitialise the - * device including MSI, bus mastering, BARs, decoding IO and memory spaces, - * etc. - * - * Returns 0 if the device function was successfully reset or negative if the - * device doesn't support resetting a single function. - */ -int __pci_reset_function_locked(struct pci_dev *dev) +static bool +pci_reset_fn_needs_bridge_cfg_access(const struct pci_reset_fn_method *method) { + return method->reset_fn == pci_reset_bus_function || + method->reset_fn == cxl_reset_bus_function; +} + +static int pci_reset_function_locked_internal(struct pci_dev *dev, + bool trylock_bridge_cfg) +{ + struct pci_dev *bridge; int i, m, rc; const struct pci_reset_fn_method *method; @@ -5208,7 +5197,21 @@ int __pci_reset_function_locked(struct pci_dev *dev) method = &pci_reset_fn_methods[m]; pci_dbg(dev, "reset via %s\n", method->name); + + bridge = NULL; + if (trylock_bridge_cfg && + pci_reset_fn_needs_bridge_cfg_access(method)) { + bridge = pci_upstream_bridge(dev); + if (bridge && !pci_cfg_access_trylock(bridge)) { + rc = -EAGAIN; + goto done; + } + } + rc = method->reset_fn(dev, PCI_RESET_DO_RESET); + if (bridge) + pci_cfg_access_unlock(bridge); +done: if (!rc) return 0; @@ -5219,6 +5222,31 @@ int __pci_reset_function_locked(struct pci_dev *dev) return -ENOTTY; } + +/** + * __pci_reset_function_locked - reset a PCI device function while holding + * the @dev mutex lock. + * @dev: PCI device to reset + * + * Some devices allow an individual function to be reset without affecting + * other functions in the same device. The PCI device must be responsive + * to PCI config space in order to use this function. + * + * The device function is presumed to be unused and the caller is holding + * the device mutex lock when this function is called. + * + * Resetting the device will make the contents of PCI configuration space + * random, so any caller of this must be prepared to reinitialise the + * device including MSI, bus mastering, BARs, decoding IO and memory spaces, + * etc. + * + * Returns 0 if the device function was successfully reset or negative if the + * device doesn't support resetting a single function. + */ +int __pci_reset_function_locked(struct pci_dev *dev) +{ + return pci_reset_function_locked_internal(dev, false); +} EXPORT_SYMBOL_GPL(__pci_reset_function_locked); /** @@ -5338,7 +5366,8 @@ EXPORT_SYMBOL_GPL(pci_reset_function_locked); * pci_try_reset_function - quiesce and reset a PCI device function * @dev: PCI device to reset * - * Same as above, except return -EAGAIN if unable to lock device. + * Same as above, except return -EAGAIN if unable to lock device or the + * upstream bridge config access needed for a bus-level reset fallback. */ int pci_try_reset_function(struct pci_dev *dev) { @@ -5351,7 +5380,7 @@ int pci_try_reset_function(struct pci_dev *dev) return -EAGAIN; pci_dev_save_and_disable(dev); - rc = __pci_reset_function_locked(dev); + rc = pci_reset_function_locked_internal(dev, true); pci_dev_restore(dev); pci_dev_unlock(dev); -- 2.34.1