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

[Alex Williamson <alex.williamson@nvidia.com>]

On Thu, 13 Aug 2026 23:22:33 +0000
Samiullah Khawaja <skhawaja@google.com> wrote:

> 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.

Devices are only ever opened into a user owned domain, the IOMMU
context switch happens before this and regardless of the reset.  Close
also disables bus-master regardless of reset, so there's no risk of
ongoing DMA if the device is placed into an identity domain between
close and re-open.

Actually, I think -ENOTTY is a leftover from a previous iteration where
this test was pushed into the individual reset methods.  -ENOTTY allows
continuing to the next reset method.  With the test guarding all the
reset methods in this version, we should probably use -EBUSY.  -EAGAIN
would conflate the try-lock contention error, which is actually a usage
race, versus the PF is not in a state to handle the request.

If the user owns the PF, as evidenced by them being able to get to
vfio_pci_core_enable(), and reset is blocked by the SR-IOV state of the
PF, I think there are arguments both that the user implicitly opted in
to the best-effort reset, as well as a use case that allows the PF
driver to fail and re-open the PF demands this behavior.

Further, the PF requires vf-token authentication if VFs are open within
the vfio ecosystem.  If the VFs are instead bound to in-kernel drivers,
they've already been opted out of that security barrier.  Thanks,

Alex