pci_reset_function() and its locked and try variants are intended to provide a function-scoped reset. The bus and slot methods supporting this interface refuse when sibling or subordinate devices are present. SR-IOV VFs however, are not currently considered in this scope. Correct this oversight by testing for non-zero VF count in calls through the pci_reset_function() interfaces. This test needs to occur under device_lock to avoid races with .sriov_configure. It should also occur before pci_dev_save_and_disable() to avoid calling potentially destructive reset hooks. Tests are therefore added to each of pci_reset_function(), pci_reset_function_locked(), and pci_try_reset_function(). The __pci_reset_function_locked() interface remains a low-level primitive depending on the caller to perform such tests as necessary. The vfio_pci_core use case of __pci_reset_function_locked() is pulled through with this test. Other use cases, such as xen-pciback, that don't obviously support or prevent binding to SR-IOV enabled PFs will need to decide whether VFs are possible and can be preserved. Additionally, direct callers of sriov_enable() that do not hold device_lock (lpfc) are considered a preexisting, non-compliance issue. Fixes: dd7cc44d0bce ("PCI: add SR-IOV API for Physical Function driver") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Alex Williamson --- drivers/pci/pci.c | 19 +++++++++++++++++++ drivers/vfio/pci/vfio_pci_core.c | 4 +++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 77b17b13ee61..b40b00c0c0c9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5222,11 +5222,22 @@ int pci_reset_function(struct pci_dev *dev) pci_dev_lock(bridge); pci_dev_lock(dev); + + /* + * Reset of an SR-IOV PF necessarily resets any active VFs. Such resets are + * beyond the scope advertised for pci_reset_function() and variants, refuse. + */ + if (pci_num_vf(dev) > 0) { + rc = -ENOTTY; + goto unlock; + } + pci_dev_save_and_disable(dev); rc = __pci_reset_function_locked(dev); pci_dev_restore(dev); +unlock: pci_dev_unlock(dev); if (bridge) @@ -5264,6 +5275,9 @@ int pci_reset_function_locked(struct pci_dev *dev) if (!pci_reset_supported(dev)) return -ENOTTY; + if (pci_num_vf(dev) > 0) + return -ENOTTY; + pci_dev_save_and_disable(dev); rc = __pci_reset_function_locked(dev); @@ -5290,6 +5304,11 @@ int pci_try_reset_function(struct pci_dev *dev) if (!pci_dev_trylock(dev)) return -EAGAIN; + if (pci_num_vf(dev) > 0) { + pci_dev_unlock(dev); + return -ENOTTY; + } + pci_dev_save_and_disable(dev); rc = __pci_reset_function_locked(dev); pci_dev_restore(dev); diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 3f11a9624b9c..9757b171791c 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -790,7 +790,9 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev) if (bridge && !pci_dev_trylock(bridge)) goto out_restore_state; if (pci_dev_trylock(pdev)) { - if (!__pci_reset_function_locked(pdev)) + /* Enforce function scope under lock for SR-IOV PFs */ + if (!pci_num_vf(pdev) && + !__pci_reset_function_locked(pdev)) vdev->needs_reset = false; pci_dev_unlock(pdev); } -- 2.53.0 pci_reset_bus() locks every device affected by a slot or bus reset, performs the reset, and unlocks the devices, all internally. A caller has no way to evaluate what the reset would actually touch. Add pci_reset_bus_cond(), which takes a callback invoked on each affected device once they are all locked. A nonzero return value from the callback aborts the reset and the value is returned to the caller. This allows, for instance, the caller to validate that SR-IOV is not enabled on any affected device under device_lock, where the value is known stable across the reset. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Alex Williamson --- drivers/pci/pci.c | 97 ++++++++++++++++++++++++++++++++++++++------- include/linux/pci.h | 3 ++ 2 files changed, 85 insertions(+), 15 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b40b00c0c0c9..06728137c407 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5567,9 +5567,36 @@ int pci_probe_reset_slot(struct pci_slot *slot) } EXPORT_SYMBOL_GPL(pci_probe_reset_slot); +/* Call @cb on every device a slot or bus reset affects, stopping on error. */ +static int pci_walk_reset_check(struct pci_bus *bus, struct pci_slot *slot, + int (*cb)(struct pci_dev *dev, void *data), + void *data) +{ + struct pci_dev *dev; + int rc; + + list_for_each_entry(dev, &bus->devices, bus_list) { + if (slot && (!dev->slot || dev->slot != slot)) + continue; + rc = cb(dev, data); + if (rc) + return rc; + if (dev->subordinate) { + rc = pci_walk_reset_check(dev->subordinate, + NULL, cb, data); + if (rc) + return rc; + } + } + + return 0; +} + /** * pci_try_reset_slot - Try to reset a PCI slot * @slot: PCI slot to reset + * @check: optional per-device callback that can abort the reset + * @data: opaque argument for @check * * A PCI bus may host multiple slots, each slot may support a reset mechanism * independent of other slots. For instance, some slots may support slot power @@ -5582,7 +5609,9 @@ EXPORT_SYMBOL_GPL(pci_probe_reset_slot); * * Same as above except return -EAGAIN if the slot cannot be locked */ -static int pci_try_reset_slot(struct pci_slot *slot) +static int pci_try_reset_slot(struct pci_slot *slot, + int (*check)(struct pci_dev *dev, void *data), + void *data) { int rc; @@ -5591,10 +5620,14 @@ static int pci_try_reset_slot(struct pci_slot *slot) return rc; if (pci_slot_trylock(slot)) { - pci_slot_save_and_disable_locked(slot); - might_sleep(); - rc = pci_reset_hotplug_slot(slot->hotplug, PCI_RESET_DO_RESET); - pci_slot_restore_locked(slot); + rc = check ? pci_walk_reset_check(slot->bus, slot, check, data) : 0; + if (!rc) { + pci_slot_save_and_disable_locked(slot); + might_sleep(); + rc = pci_reset_hotplug_slot(slot->hotplug, + PCI_RESET_DO_RESET); + pci_slot_restore_locked(slot); + } pci_slot_unlock(slot); } else rc = -EAGAIN; @@ -5626,10 +5659,14 @@ static int pci_bus_reset(struct pci_bus *bus, bool probe) /** * pci_try_reset_bus - Try to reset a PCI bus * @bus: top level PCI bus to reset + * @check: optional per-device callback that can abort the reset + * @data: opaque argument for @check * * Same as above except return -EAGAIN if the bus cannot be locked */ -static int pci_try_reset_bus(struct pci_bus *bus) +static int pci_try_reset_bus(struct pci_bus *bus, + int (*check)(struct pci_dev *dev, void *data), + void *data) { int rc; @@ -5638,10 +5675,13 @@ static int pci_try_reset_bus(struct pci_bus *bus) return rc; if (pci_bus_trylock(bus)) { - pci_bus_save_and_disable_locked(bus); - might_sleep(); - rc = pci_bridge_secondary_bus_reset(bus->self); - pci_bus_restore_locked(bus); + rc = check ? pci_walk_reset_check(bus, NULL, check, data) : 0; + if (!rc) { + pci_bus_save_and_disable_locked(bus); + might_sleep(); + rc = pci_bridge_secondary_bus_reset(bus->self); + pci_bus_restore_locked(bus); + } pci_bus_unlock(bus); } else rc = -EAGAIN; @@ -5680,7 +5720,7 @@ static int pci_reset_bridge(struct pci_dev *bridge, bool restore) list_for_each_entry(slot, &bus->slots, list) { if (restore) - ret = pci_try_reset_slot(slot); + ret = pci_try_reset_slot(slot, NULL, NULL); else ret = pci_slot_reset(slot, PCI_RESET_DO_RESET); @@ -5694,7 +5734,7 @@ static int pci_reset_bridge(struct pci_dev *bridge, bool restore) mutex_unlock(&pci_slot_mutex); if (restore) - return pci_try_reset_bus(bus); + return pci_try_reset_bus(bus, NULL, NULL); return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET); } @@ -5724,16 +5764,43 @@ int pci_probe_reset_bus(struct pci_bus *bus) } EXPORT_SYMBOL_GPL(pci_probe_reset_bus); +/** + * pci_reset_bus_cond - conditionally reset the slot or bus containing a device + * @pdev: top level PCI device to reset via slot/bus + * @check: optional callback invoked on each affected device before the reset + * @data: opaque argument passed to @check + * + * Reset the slot or bus containing @pdev. Once the entire physical bus/slot + * hierarchy is locked, @check (if not NULL) is called on each of those locked + * devices. A nonzero return aborts the reset and is returned to the caller, + * otherwise the reset proceeds. + * + * NB. @check runs with the full set of device_locks noted above held; callbacks + * must take these locking semantics into account. Use NULL to perform an + * unconditional reset. + * + * Return: 0 on success, -ENOTTY if @pdev is not resettable, -EAGAIN if the + * devices cannot be locked, or the value returned by @check. + */ +int pci_reset_bus_cond(struct pci_dev *pdev, + int (*check)(struct pci_dev *dev, void *data), + void *data) +{ + return !pci_probe_reset_slot(pdev->slot) ? + pci_try_reset_slot(pdev->slot, check, data) : + pci_try_reset_bus(pdev->bus, check, data); +} +EXPORT_SYMBOL_GPL(pci_reset_bus_cond); + /** * pci_reset_bus - Try to reset a PCI bus * @pdev: top level PCI device to reset via slot/bus * - * Same as above except return -EAGAIN if the bus cannot be locked + * Same as above without the conditional check. */ int pci_reset_bus(struct pci_dev *pdev) { - return (!pci_probe_reset_slot(pdev->slot)) ? - pci_try_reset_slot(pdev->slot) : pci_try_reset_bus(pdev->bus); + return pci_reset_bus_cond(pdev, NULL, NULL); } EXPORT_SYMBOL_GPL(pci_reset_bus); diff --git a/include/linux/pci.h b/include/linux/pci.h index 64b308b6e61c..ab41a49f4eb7 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1480,6 +1480,9 @@ int pci_try_reset_function(struct pci_dev *dev); int pci_probe_reset_slot(struct pci_slot *slot); int pci_probe_reset_bus(struct pci_bus *bus); int pci_reset_bus(struct pci_dev *dev); +int pci_reset_bus_cond(struct pci_dev *pdev, + int (*check)(struct pci_dev *dev, void *data), + void *data); void pci_reset_secondary_bus(struct pci_dev *dev); void pcibios_reset_secondary_bus(struct pci_dev *dev); void pci_update_resource(struct pci_dev *dev, int resno); -- 2.53.0 The vfio-pci hot-reset interface VFIO_DEVICE_PCI_HOT_RESET does not take into account whether an SR-IOV PF has active VFs. The VFs appear on a virtual bus, which is not enumerated in collecting affected devices. Move the burden to the user when active VFs are present, require that there are no VFs present on SR-IOV capable PFs in order to conduct a hot reset. Fixes: 137e5531351d ("vfio/pci: Add sriov_configure support") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_core.c | 15 +++++++++++++-- include/uapi/linux/vfio.h | 3 +++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 9757b171791c..956a05ca12e5 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -894,6 +894,17 @@ static int vfio_pci_count_devs(struct pci_dev *pdev, void *data) return 0; } +/* + * PCI walk callback to check for SR-IOV PFs with active VFs. VFs are not + * enumerated when determining affected devices and may be owned by separate + * userspace processes from the PF. It's therefore the user's responsibility + * to teardown VFs for any affected PF before performing a hot-reset. + */ +static int vfio_pci_dev_has_vfs(struct pci_dev *pdev, void *data) +{ + return pci_num_vf(pdev) ? -EBUSY : 0; +} + struct vfio_pci_fill_info { struct vfio_device *vdev; struct vfio_pci_dependent_device *devices; @@ -2598,7 +2609,7 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list) vfio_pci_set_power_state(vdev, PCI_D0); - ret = pci_reset_bus(pdev); + ret = pci_reset_bus_cond(pdev, vfio_pci_dev_has_vfs, NULL); vdev = list_last_entry(&dev_set->device_list, struct vfio_pci_core_device, vdev.dev_set_list); @@ -2661,7 +2672,7 @@ static void vfio_pci_dev_set_try_reset(struct vfio_device_set *dev_set) if (vfio_pci_dev_set_pm_runtime_get(dev_set)) return; - if (!pci_reset_bus(pdev)) + if (!pci_reset_bus_cond(pdev, vfio_pci_dev_has_vfs, NULL)) reset_done = true; list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list) { diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 5de618a3a5ee..8603959f1735 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -772,6 +772,9 @@ struct vfio_pci_hot_reset_info { * Mixed usage of legacy groups and cdevs across the set of affected * devices is not supported. * + * Hot reset of SR-IOV PFs with active VFs is not supported, SR-IOV + * should first be disabled on any affected PF. + * * Return: 0 on success, -errno on failure. */ struct vfio_pci_hot_reset { -- 2.53.0 The success or failure of pci_reset_function() is conditional on the state of the device, ex. VFs enabled on an SR-IOV PF. Therefore a driver can no longer infer through executing pci_reset_function() whether a device has supported reset methods. The reset_methods array indicates whether the device has any available reset mechanisms and is not gated on dynamic blockers, like SR-IOV enablement. Export pci_reset_supported() for drivers to use for an instant snapshot of reset support. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Alex Williamson --- drivers/pci/pci.c | 1 + drivers/pci/pci.h | 1 - include/linux/pci.h | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 06728137c407..0b4f13397da2 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -98,6 +98,7 @@ bool pci_reset_supported(struct pci_dev *dev) { return dev->reset_methods[0] != 0; } +EXPORT_SYMBOL_GPL(pci_reset_supported); #ifdef CONFIG_PCI_DOMAINS int pci_domains_supported = 1; diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 4469e1a77f3c..416fd6b74abe 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -230,7 +230,6 @@ enum pci_mmap_api { int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vmai, enum pci_mmap_api mmap_api); -bool pci_reset_supported(struct pci_dev *dev); void pci_init_reset_methods(struct pci_dev *dev); int pci_bridge_secondary_bus_reset(struct pci_dev *dev); int pci_bus_error_reset(struct pci_dev *dev); diff --git a/include/linux/pci.h b/include/linux/pci.h index ab41a49f4eb7..4de21e6ac538 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1474,6 +1474,7 @@ void pcie_print_link_status(struct pci_dev *dev); int pcie_reset_flr(struct pci_dev *dev, bool probe); int pcie_flr(struct pci_dev *dev); int __pci_reset_function_locked(struct pci_dev *dev); +bool pci_reset_supported(struct pci_dev *dev); int pci_reset_function(struct pci_dev *dev); int pci_reset_function_locked(struct pci_dev *dev); int pci_try_reset_function(struct pci_dev *dev); -- 2.53.0 vfio-pci latches whether pci_reset_function() works at open device and makes decisions based on this latched value at runtime. With the introduction of the reset_method pci-sysfs attribute, this flag can be made stale at runtime by administrative action. Further, with the SR-IOV active VFs gating of pci_reset_function(), the flag can be made stale via more subtle dependencies. Drop the latched flag and rely on pci_reset_supported() to indicate whether reset methods exist for the device. This is no guarantee that those reset methods work, nor has the RESET flag in struct vfio_device_info ever been a guarantee of VFIO_DEVICE_RESET success. It's only a guarantee that there are reset methods that are applicable to the device at the instant it's called. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_core.c | 9 ++++----- include/linux/vfio_pci_core.h | 1 - 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 956a05ca12e5..8e42342d56d3 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -609,7 +609,6 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev) if (ret == -EAGAIN) goto out_disable_device; - vdev->reset_works = !ret; pci_save_state(pdev); vdev->pci_saved_state = pci_store_saved_state(pdev); if (!vdev->pci_saved_state) @@ -766,7 +765,7 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev) if (pci_load_and_free_saved_state(pdev, &vdev->pci_saved_state)) { pci_info(pdev, "%s: Couldn't reload saved state\n", __func__); - if (!vdev->reset_works) + if (!pci_reset_supported(pdev)) goto out; pci_save_state(pdev); @@ -785,7 +784,7 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev) * We can not use the "try" reset interface here, which will * overwrite the previously restored configuration information. */ - if (vdev->reset_works) { + if (pci_reset_supported(pdev)) { bridge = pci_upstream_bridge(pdev); if (bridge && !pci_dev_trylock(bridge)) goto out_restore_state; @@ -1098,7 +1097,7 @@ static int vfio_pci_ioctl_get_info(struct vfio_pci_core_device *vdev, info.flags = VFIO_DEVICE_FLAGS_PCI; - if (vdev->reset_works) + if (pci_reset_supported(vdev->pdev)) info.flags |= VFIO_DEVICE_FLAGS_RESET; info.num_regions = VFIO_PCI_NUM_REGIONS + vdev->num_regions; @@ -1330,7 +1329,7 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, { int ret; - if (!vdev->reset_works) + if (!pci_reset_supported(vdev->pdev)) return -EINVAL; vfio_pci_zap_and_down_write_memory_lock(vdev); diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index 9a1674c152aa..6fb599ec2919 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -121,7 +121,6 @@ struct vfio_pci_core_device { /* Flags only modified on setup/release - bitfield ok */ bool has_dyn_msix:1; bool pci_2_3:1; - bool reset_works:1; bool extended_caps:1; bool has_vga:1; bool nointx:1; -- 2.53.0