Re: [RFC PATCH 1/5] PCI: Refuse function reset of an SR-IOV PF with enabled VFs

[Samiullah Khawaja <skhawaja@google.com>]

On Tue, Aug 11, 2026 at 10:53:19PM -0600, Alex Williamson wrote:
>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 <alex.williamson@nvidia.com>
>---
> 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;
>+	}

I am wondering whether we should return EAGAIN from here, since this
function is used by vfio_pci_core_enable() during open and it doesn't
fail the open on ENOTTY. Basically whether we should allow the user to
reopen the device if the reset was skipped previously? In the previous
instance of open, the device was setup with vfio/iommufd and the vfio fd
was abruptly closed and the reset was skipped. But the device went back
to the IOMMU default domain and that is probably an Identity domain. If
we allow the device to be opened and re-enable busmaster without a
reset, is there a chance that device would continue to DMA based on its
previous setup/context? Probably unlikely?

Note this is different from the current vfio-pci behaviour where device
is always reset during close.

Maybe thinking with too much paranoia about it :D.

Thanks,
Sami