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

Samiullah Khawaja <[email protected]>
Newsgroups org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci
Message-ID <[email protected]>
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: [email protected]
>Assisted-by: Claude:claude-opus-4-8
>Signed-off-by: Alex Williamson <[email protected]>
>---
> drivers/pci/pci.c                | 19 +++++++++++++++++++
> drivers/vfio/pci/vfio_pci_core.c |  4 +++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c

[snip]

>index 3f11a9624b9c..9757b171791c 100644
>--- a/drivers/vfio/pci/vfio_pci_core.c
>+++ b/drivers/vfio/pci/vfio_pci_core.c

nit: This probably needs to be a different patch.
>@@ -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);
> 		}

I was worried that this would fall into pci_restore_state() and disable
sriov, but it doesn't since there is a check in pci_restore_iov_state()
that skips disabling sriov if VFs are enabled.

Do you think we should skip the pci_restore_state() in general?
>-- 
>2.53.0
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.