All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] vfio/pci: Disable sriov on PF device close
@ 2026-08-05  0:33 Samiullah Khawaja
  2026-08-05  0:33 ` [RFC PATCH 1/1] " Samiullah Khawaja
  0 siblings, 1 reply; 10+ messages in thread
From: Samiullah Khawaja @ 2026-08-05  0:33 UTC (permalink / raw)
  To: Alex Williamson, Jason Gunthorpe, bhelgaas
  Cc: Samiullah Khawaja, dmatlack, Kevin Tian, Leon Romanovsky, kvm,
	linux-kernel

When userspace closes a VFIO device file descriptor, the vfio driver
performs a hardware reset on the PCIe device to ensure it is returned to
a clean state. However, if the closed device is an SR-IOV Physical
Function (PF), it may have instantiated Virtual Functions (VFs) that are
actively bound to host kernel drivers (or other vfio instances).

When the PF is hardware-reset via VFIO, it implicitly disrupts SR-IOV
operations at the device level. Because this reset happens without
notifying the core PCI driver model, the kernel drivers bound to the VFs
remain loaded and operate under the assumption that the VF hardware is
still functional.

This creates a state mismatch between the kernel's view of the hardware
and the actual device state. Subsequent attempts by the host OS or bound
drivers to interact with the VFs will fail, leading to unexpected
errors.

Disabling SRIOV before resetting the device avoids this issue by tearing
down the VFs and removing the VF devices.

The fix in this patch only disables SRIOV on PF reset for devices bound
to vfio-pci drivers. But this probably needs a fix for kernel drivers
also, maybe return -EBUSY when SRIOV is enabled?

Looking forward to your feedback on this.

---

CC: bhelgaas@google.com
CC: dmatlack@google.com

Samiullah Khawaja (1):
  vfio/pci: Disable sriov on PF device close

 drivers/vfio/pci/vfio_pci_core.c | 7 +++++++
 1 file changed, 7 insertions(+)


base-commit: 0f6da28aab51b16762ed82e8fdeaa5042da45b08
-- 
2.55.0.629.g250fe7f194-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-05  0:33 [RFC PATCH 0/1] vfio/pci: Disable sriov on PF device close Samiullah Khawaja
@ 2026-08-05  0:33 ` Samiullah Khawaja
  2026-08-05  1:03   ` sashiko-bot
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Samiullah Khawaja @ 2026-08-05  0:33 UTC (permalink / raw)
  To: Alex Williamson, Jason Gunthorpe, bhelgaas
  Cc: Samiullah Khawaja, Kevin Tian, Leon Romanovsky, dmatlack, kvm,
	linux-kernel

When userspace closes a VFIO device file descriptor, the vfio driver
performs a hardware reset on the PCIe device to ensure it is returned to
a clean state. However, if the closed device is an SR-IOV Physical
Function (PF), it may have instantiated Virtual Functions (VFs) that are
actively bound to host kernel drivers (or other vfio instances).

When the PF is hardware-reset via VFIO, it implicitly disrupts SR-IOV
operations at the device level. Because this reset happens without notifying
the core PCI driver model, the kernel drivers bound to the VFs remain loaded
and operate under the assumption that the VF hardware is still functional.

This creates a state mismatch between the kernel's view of the hardware
and the actual device state. Subsequent attempts by the host OS or bound
drivers to interact with the VFs will fail, leading to unexpected
errors.

Disable SR-IOV on the device prior to issuing the PF reset so that the
VFs can teardown and remove at the software level also.

Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
---
 drivers/vfio/pci/vfio_pci_core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index a113c55845e1..b36c77eb3c40 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -826,6 +826,13 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
 #if IS_ENABLED(CONFIG_EEH)
 	eeh_dev_release(vdev->pdev);
 #endif
+
+	if (pci_num_vf(vdev->pdev)) {
+		device_lock(&vdev->pdev->dev);
+		vfio_pci_core_sriov_configure(vdev, 0);
+		device_unlock(&vdev->pdev->dev);
+	}
+
 	vfio_pci_dma_buf_cleanup(vdev);
 
 	vfio_pci_core_disable(vdev);
-- 
2.55.0.629.g250fe7f194-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-05  0:33 ` [RFC PATCH 1/1] " Samiullah Khawaja
@ 2026-08-05  1:03   ` sashiko-bot
  2026-08-05  9:34   ` Tian, Kevin
  2026-08-05 15:03   ` Alex Williamson
  2 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-05  1:03 UTC (permalink / raw)
  To: Samiullah Khawaja; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Acquiring `device_lock()` inside `vfio_pci_core_close_device()` introduces a guaranteed AB-BA deadlock if the device is unbound from the driver while still in use by userspace.
- [High] Synchronously tearing down SR-IOV VFs from the PF's `close_device` path deadlocks if any VF is actively in use by VFIO.
- [High] Calling `pci_disable_sriov()` without holding `pci_rescan_remove_lock` causes an immediate lockdep splat and races with PCI bus enumeration.
- [Medium] Time-of-Check to Time-of-Use (TOCTOU) races around `device_lock()` allow VFs to be enabled immediately before hardware reset, defeating the purpose of the patch.
--

--- Patch [1]: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close ---
commit 996eca93815cca91725d215b78730f993e057c49
Author: Samiullah Khawaja <skhawaja@google.com>

vfio/pci: Disable sriov on PF device close

When a user closes a VFIO device file descriptor for an SR-IOV Physical
Function, the device is reset. This reset disrupts the VFs without notifying
the core PCI driver model. This commit disables SR-IOV prior to issuing the
PF reset so VFs are torn down gracefully at the software level.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index a113c55845e10..b36c77eb3c407 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -826,6 +826,13 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
>  #if IS_ENABLED(CONFIG_EEH)
>  	eeh_dev_release(vdev->pdev);
>  #endif
> +
> +	if (pci_num_vf(vdev->pdev)) {

[Severity: Medium]
Is it possible for VFs to be enabled concurrently right after this check?

Since pci_num_vf() is evaluated locklessly before acquiring the device lock, a
concurrent sysfs write to sriov_numvfs could enable VFs immediately after this
returns 0, causing the teardown to be skipped.

Additionally, because the device lock is dropped before the hardware reset in
vfio_pci_core_disable(), a concurrent sysfs write can acquire the lock,
successfully enable SR-IOV, and release the lock before the reset occurs.

Could this recreate the exact hardware state mismatch the patch attempts to fix?

> +		device_lock(&vdev->pdev->dev);

[Severity: High]
Does acquiring the device lock here introduce an AB-BA deadlock during device
unbind?

When a device is unbound, the PCI core acquires the device lock in
device_release_driver() and invokes the driver's remove callback. For
vfio-pci, this calls vfio_unregister_group_dev(), which blocks indefinitely
waiting for userspace to close all active file descriptors.

When userspace closes the device fd, vfio_pci_core_close_device() is invoked.
Because this patch tries to acquire the device lock, the fd release thread
will block forever waiting for the unbind thread, and the unbind thread will
wait forever for the fd release thread to finish.

> +		vfio_pci_core_sriov_configure(vdev, 0);

[Severity: High]
Can tearing down VFs from the PF's close path deadlock if a VF is actively in
use by VFIO?

When vfio_pci_core_close_device() calls pci_disable_sriov(), the PCI core
synchronously removes all VF devices from the bus. If a VF is also bound to
vfio-pci and currently opened by userspace, removing the VF triggers
vfio_pci_remove(), which blocks indefinitely until the VF file descriptor is
closed.

If a single process holds both the PF and VF open and exits, exit_files()
schedules the fput() for both file descriptors sequentially. If the PF is
closed first, the thread hangs here waiting for the VF to close, preventing
the VF's fput() from ever running.

[Severity: High]
Does this code trigger a lockdep warning by manipulating the PCI bus topology
without the required lock?

Calling vfio_pci_core_sriov_configure() here ultimately calls
pci_stop_and_remove_bus_device() to remove VFs from the PCI bus. That function
enforces a strict locking requirement via
lockdep_assert_held(&pci_rescan_remove_lock).

Because this global mutex is omitted, closing a PF with active VFs could race
against concurrent PCI hotplug or sysfs rescan operations.

> +		device_unlock(&vdev->pdev->dev);
> +	}
> +
>  	vfio_pci_dma_buf_cleanup(vdev);
>  
>  	vfio_pci_core_disable(vdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805003355.728299-2-skhawaja@google.com?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-05  0:33 ` [RFC PATCH 1/1] " Samiullah Khawaja
  2026-08-05  1:03   ` sashiko-bot
@ 2026-08-05  9:34   ` Tian, Kevin
  2026-08-10 21:47     ` Samiullah Khawaja
  2026-08-05 15:03   ` Alex Williamson
  2 siblings, 1 reply; 10+ messages in thread
From: Tian, Kevin @ 2026-08-05  9:34 UTC (permalink / raw)
  To: Samiullah Khawaja, Alex Williamson, Jason Gunthorpe,
	bhelgaas@google.com
  Cc: Leon Romanovsky, dmatlack@google.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org

> From: Samiullah Khawaja <skhawaja@google.com>
> Sent: Wednesday, August 5, 2026 8:34 AM
> 
> When userspace closes a VFIO device file descriptor, the vfio driver
> performs a hardware reset on the PCIe device to ensure it is returned to
> a clean state. However, if the closed device is an SR-IOV Physical
> Function (PF), it may have instantiated Virtual Functions (VFs) that are
> actively bound to host kernel drivers (or other vfio instances).
> 
> When the PF is hardware-reset via VFIO, it implicitly disrupts SR-IOV
> operations at the device level. Because this reset happens without notifying
> the core PCI driver model, the kernel drivers bound to the VFs remain loaded
> and operate under the assumption that the VF hardware is still functional.
> 
> This creates a state mismatch between the kernel's view of the hardware
> and the actual device state. Subsequent attempts by the host OS or bound
> drivers to interact with the VFs will fail, leading to unexpected
> errors.
> 
> Disable SR-IOV on the device prior to issuing the PF reset so that the
> VFs can teardown and remove at the software level also.
> 
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index a113c55845e1..b36c77eb3c40 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -826,6 +826,13 @@ void vfio_pci_core_close_device(struct vfio_device
> *core_vdev)
>  #if IS_ENABLED(CONFIG_EEH)
>  	eeh_dev_release(vdev->pdev);
>  #endif
> +
> +	if (pci_num_vf(vdev->pdev)) {
> +		device_lock(&vdev->pdev->dev);
> +		vfio_pci_core_sriov_configure(vdev, 0);
> +		device_unlock(&vdev->pdev->dev);
> +	}
> +

Sashiko reported several locking issues with this change:

https://sashiko.dev/#/patchset/20260805003355.728299-2-skhawaja%40google.com

Actually it is a discouraged usage allowing kernel drivers bound to VFs
which belongs to a PF owned by vfio userspace. 

vfio_pci_bus_notifier prints a warning message upon 
BUS_NOTIFY_BOUND_DRIVER:

	"VF %s bound to driver %s while PF bound to driver %s\n"

Fixing this alone is insufficient to remove that warning...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-05  0:33 ` [RFC PATCH 1/1] " Samiullah Khawaja
  2026-08-05  1:03   ` sashiko-bot
  2026-08-05  9:34   ` Tian, Kevin
@ 2026-08-05 15:03   ` Alex Williamson
  2026-08-06 19:47     ` Jason Gunthorpe
  2 siblings, 1 reply; 10+ messages in thread
From: Alex Williamson @ 2026-08-05 15:03 UTC (permalink / raw)
  To: Samiullah Khawaja
  Cc: Jason Gunthorpe, bhelgaas, Kevin Tian, Leon Romanovsky, dmatlack,
	kvm, linux-kernel, alex

On Wed,  5 Aug 2026 00:33:55 +0000
Samiullah Khawaja <skhawaja@google.com> wrote:

> When userspace closes a VFIO device file descriptor, the vfio driver
> performs a hardware reset on the PCIe device to ensure it is returned to
> a clean state. However, if the closed device is an SR-IOV Physical
> Function (PF), it may have instantiated Virtual Functions (VFs) that are
> actively bound to host kernel drivers (or other vfio instances).

Wait, what?  We actively try to prevent VFs from a vfio-pci owned PF
from being bound to host drivers other than vfio-pci.  You need to
overwrite the imposed driver_override to make this happen and you're in
a very precarious security model to have the VF owned by a trusted
in-kernel driver while the PF is owned by userspace.
 
> When the PF is hardware-reset via VFIO, it implicitly disrupts SR-IOV
> operations at the device level. Because this reset happens without notifying
> the core PCI driver model, the kernel drivers bound to the VFs remain loaded
> and operate under the assumption that the VF hardware is still functional.

And this is a) why we actively try to prevent VFs from being bound to
native host kernel drivers and b) part of the vf_token negotiation of
trust opt-in that must be performed by userspace drivers of the VFs.
 
> This creates a state mismatch between the kernel's view of the hardware
> and the actual device state. Subsequent attempts by the host OS or bound
> drivers to interact with the VFs will fail, leading to unexpected
> errors.

The described model is not a supported use case, there are fundamental
security issues with a userspace vfio-pci driver owning the PF with VFs
bound to in-kernel drivers.

> Disable SR-IOV on the device prior to issuing the PF reset so that the
> VFs can teardown and remove at the software level also.

The below only accounts for the close reset path, not reset in general.
Also note that the SR-IOV model in vfio-pci does have a path by which a
PF can be re-opened with existing VFs.  Whether that's actually used is
a separate question, but this fundamentally disallows that design.

It's possible there are gaps that closing the PF can interrupt the VFs
and we need to defer a reset until the VFs are closed, but the behavior
here seems counter to the original design, and the use case of
in-kernel VF drivers is certainly counter to that design and guardrails
installed to prevent it.  Thanks,

Alex

> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index a113c55845e1..b36c77eb3c40 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -826,6 +826,13 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
>  #if IS_ENABLED(CONFIG_EEH)
>  	eeh_dev_release(vdev->pdev);
>  #endif
> +
> +	if (pci_num_vf(vdev->pdev)) {
> +		device_lock(&vdev->pdev->dev);
> +		vfio_pci_core_sriov_configure(vdev, 0);
> +		device_unlock(&vdev->pdev->dev);
> +	}
> +
>  	vfio_pci_dma_buf_cleanup(vdev);
>  
>  	vfio_pci_core_disable(vdev);


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-05 15:03   ` Alex Williamson
@ 2026-08-06 19:47     ` Jason Gunthorpe
  2026-08-10 15:32       ` Alex Williamson
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2026-08-06 19:47 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Samiullah Khawaja, bhelgaas, Kevin Tian, Leon Romanovsky,
	dmatlack, kvm, linux-kernel

On Wed, Aug 05, 2026 at 09:03:23AM -0600, Alex Williamson wrote:
> On Wed,  5 Aug 2026 00:33:55 +0000
> Samiullah Khawaja <skhawaja@google.com> wrote:
> 
> > When userspace closes a VFIO device file descriptor, the vfio driver
> > performs a hardware reset on the PCIe device to ensure it is returned to
> > a clean state. However, if the closed device is an SR-IOV Physical
> > Function (PF), it may have instantiated Virtual Functions (VFs) that are
> > actively bound to host kernel drivers (or other vfio instances).
> 
> Wait, what?  We actively try to prevent VFs from a vfio-pci owned PF
> from being bound to host drivers other than vfio-pci.  You need to
> overwrite the imposed driver_override to make this happen and you're in
> a very precarious security model to have the VF owned by a trusted
> in-kernel driver while the PF is owned by userspace.

Maybe, it really depends on the device. I can easially see someone
using a device where this would be safe. mlx5 for instance is pretty
OK.

So I don't really mind someone doing this, we should block it and warn
it and so on, but like noiommu and the other vfio insecure modes, why
not give an opt in?

> It's possible there are gaps that closing the PF can interrupt the VFs
> and we need to defer a reset until the VFs are closed, 

Oh definately, when running in a SRIOV mode it is really problematic
for the PF to reset while there are any active VFs. The PF controls a
number of shared items (MMIO, ATS, etc) and when it blips everyone is
at risk of unexpected fairly catastrophic system crashing errors
related to the shared items going away.

So resetting the PF device unconditionally when vfio closes is
definately wrong in principal. I can see it maybe working for simple
systems, especially ones that don't MCE..

I don't think we can skip the PF reset on close because of dev_set
reasons and leave a rouge device for the next user, so the thing looks
somewhat troubled?

Adding the sriov disable here at least makes it defensibly safe, that
we do still clear the PF on close, and we don't take risks that the
active VFs will crash the system during the FLR blip.

Though I understand it was not the original intention, I feel we have
ended up in a strange place with the SRIOV PF feature ..

Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-06 19:47     ` Jason Gunthorpe
@ 2026-08-10 15:32       ` Alex Williamson
  2026-08-11  9:52         ` Tian, Kevin
  0 siblings, 1 reply; 10+ messages in thread
From: Alex Williamson @ 2026-08-10 15:32 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Samiullah Khawaja, bhelgaas, Kevin Tian, Leon Romanovsky,
	dmatlack, kvm, linux-kernel, alex

On Thu, 6 Aug 2026 16:47:39 -0300
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Wed, Aug 05, 2026 at 09:03:23AM -0600, Alex Williamson wrote:
> > On Wed,  5 Aug 2026 00:33:55 +0000
> > Samiullah Khawaja <skhawaja@google.com> wrote:
> >   
> > > When userspace closes a VFIO device file descriptor, the vfio driver
> > > performs a hardware reset on the PCIe device to ensure it is returned to
> > > a clean state. However, if the closed device is an SR-IOV Physical
> > > Function (PF), it may have instantiated Virtual Functions (VFs) that are
> > > actively bound to host kernel drivers (or other vfio instances).  
> > 
> > Wait, what?  We actively try to prevent VFs from a vfio-pci owned PF
> > from being bound to host drivers other than vfio-pci.  You need to
> > overwrite the imposed driver_override to make this happen and you're in
> > a very precarious security model to have the VF owned by a trusted
> > in-kernel driver while the PF is owned by userspace.  
> 
> Maybe, it really depends on the device. I can easially see someone
> using a device where this would be safe. mlx5 for instance is pretty
> OK.
> 
> So I don't really mind someone doing this, we should block it and warn
> it and so on, but like noiommu and the other vfio insecure modes, why
> not give an opt in?

An opt-in to what currently?  We generate a warning, but nothing
actually prevents the re-bind to an in-kernel driver.  Whether that's
sustainable in eliminating this reset gap is yet to be seen.

Binding a VF from a userspace owned PF flips our model on its head in
two important ways.  First is the security inversion.  An in-kernel PF
driver is considered trusted, we absolve ourselves of issues related to
whether the PF has access to VF data or interrupts VF operations and
state through reset.  Second, the vf_token model explicitly defines the
trust and coordination boundaries for the userspace SR-IOV ecosystem.
An in-kernel bound VF lives entirely outside of that boundary.  For
example, one mechanism we might use to prevent an MCE around PF reset
would be to zap VF mappings and invalidate dmabufs.  Reinstatement of
those mappings would require userspace coordination, which the vf_token
model would define as an implementation detail in userspace relative to
vf_token coordination and trust.

So we haven't done anything that explicitly blocks or taints this mode,
yet, but correctly handling reset could be a tipping point.

> > It's possible there are gaps that closing the PF can interrupt the VFs
> > and we need to defer a reset until the VFs are closed,   
> 
> Oh definately, when running in a SRIOV mode it is really problematic
> for the PF to reset while there are any active VFs. The PF controls a
> number of shared items (MMIO, ATS, etc) and when it blips everyone is
> at risk of unexpected fairly catastrophic system crashing errors
> related to the shared items going away.
> 
> So resetting the PF device unconditionally when vfio closes is
> definately wrong in principal. I can see it maybe working for simple
> systems, especially ones that don't MCE..
> 
> I don't think we can skip the PF reset on close because of dev_set
> reasons and leave a rouge device for the next user, so the thing looks
> somewhat troubled?

This is what our vf_token model would handle, if we could use it.
Re-open with VFs in use through vfio-pci requires the vf_token
proof/opt-in.  Re-open with no VFs in use could reset the PF, but we
can't account for VFs bound to in-kernel drivers.
 
> Adding the sriov disable here at least makes it defensibly safe, that
> we do still clear the PF on close, and we don't take risks that the
> active VFs will crash the system during the FLR blip.
> 
> Though I understand it was not the original intention, I feel we have
> ended up in a strange place with the SRIOV PF feature ..

I think we have 3 potential options for PF reset:

 a) All resets are blocked or deferred while SR-IOV is active.

 b) VF mappings are zapped/invalidated at PF reset and require
    coordination to be reinstated.

 c) VFs are torn down on all PF resets.

Option (a) is complex and doesn't look like stable material.  A deferred
reset on .close needs to materialize on .remove or .sriov_configure
(and of course .open), but to allow in-kernel VF drivers we can't rely
on vfio usage counts, or even our vf-token trust boundaries, which is
what leads to the .sriov_configure hook since we need to take advantage
of every opportunity to issue a deferred reset when SR-IOV is not
active when we can only rely on the PF state.

Option (b) is potentially more simple, it implements the blocking of
access on reset in the kernel for enforcement, but defers the
coordination of reinstantiating mappings to userspace, which is really
how the vf_token model is intended to work.  This is incompatible with
in-kernel VF drivers, which I think means tainting on VF unbind from
vfio-pci and those working outside the model would tread carefully.

Option (c) is the more heavy handed approach, it means that a PF driver
cannot fail or exit and re-attach.  We have existing logic that handles
a PF driver re-opening the PF device with active VFs and authenticating
the vf_token (logic also broken by in-kernel VF drivers that bypass the
vf_token mechanics).  There also appears to be significant locking
challenges in this approach, ex. the inversion of getting the device
lock for SR-IOV teardown from the ioctl, config space, and .close
contexts.

I'm open to suggestions, PF resets with live VFs is very much a gap
that I'd like to close in the vfio-pci SR-IOV model.  Thanks,

Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-05  9:34   ` Tian, Kevin
@ 2026-08-10 21:47     ` Samiullah Khawaja
  0 siblings, 0 replies; 10+ messages in thread
From: Samiullah Khawaja @ 2026-08-10 21:47 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Alex Williamson, Jason Gunthorpe, bhelgaas@google.com,
	Leon Romanovsky, dmatlack@google.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Wed, Aug 05, 2026 at 09:34:32AM +0000, Tian, Kevin wrote:
>> From: Samiullah Khawaja <skhawaja@google.com>
>> Sent: Wednesday, August 5, 2026 8:34 AM
>>
>> When userspace closes a VFIO device file descriptor, the vfio driver
>> performs a hardware reset on the PCIe device to ensure it is returned to
>> a clean state. However, if the closed device is an SR-IOV Physical
>> Function (PF), it may have instantiated Virtual Functions (VFs) that are
>> actively bound to host kernel drivers (or other vfio instances).
>>
>> When the PF is hardware-reset via VFIO, it implicitly disrupts SR-IOV
>> operations at the device level. Because this reset happens without notifying
>> the core PCI driver model, the kernel drivers bound to the VFs remain loaded
>> and operate under the assumption that the VF hardware is still functional.
>>
>> This creates a state mismatch between the kernel's view of the hardware
>> and the actual device state. Subsequent attempts by the host OS or bound
>> drivers to interact with the VFs will fail, leading to unexpected
>> errors.
>>
>> Disable SR-IOV on the device prior to issuing the PF reset so that the
>> VFs can teardown and remove at the software level also.
>>
>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>> ---
>>  drivers/vfio/pci/vfio_pci_core.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index a113c55845e1..b36c77eb3c40 100644
>> --- a/drivers/vfio/pci/vfio_pci_core.c
>> +++ b/drivers/vfio/pci/vfio_pci_core.c
>> @@ -826,6 +826,13 @@ void vfio_pci_core_close_device(struct vfio_device
>> *core_vdev)
>>  #if IS_ENABLED(CONFIG_EEH)
>>  	eeh_dev_release(vdev->pdev);
>>  #endif
>> +
>> +	if (pci_num_vf(vdev->pdev)) {
>> +		device_lock(&vdev->pdev->dev);
>> +		vfio_pci_core_sriov_configure(vdev, 0);
>> +		device_unlock(&vdev->pdev->dev);
>> +	}
>> +
>
>Sashiko reported several locking issues with this change:
>
>https://sashiko.dev/#/patchset/20260805003355.728299-2-skhawaja%40google.com
>
>Actually it is a discouraged usage allowing kernel drivers bound to VFs
>which belongs to a PF owned by vfio userspace.

Thanks for the feedback Kevin.

I sent this as an RFC specifically to discuss the right approach for
resolving this issue before fixing the locking issues that sashiko
raised. I will get back to this once the discussion to fix this the
right way concludes in the other thread.

Thanks,
Sami


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-10 15:32       ` Alex Williamson
@ 2026-08-11  9:52         ` Tian, Kevin
  2026-08-11 13:59           ` Jason Gunthorpe
  0 siblings, 1 reply; 10+ messages in thread
From: Tian, Kevin @ 2026-08-11  9:52 UTC (permalink / raw)
  To: Alex Williamson, Jason Gunthorpe
  Cc: Samiullah Khawaja, bhelgaas@google.com, Leon Romanovsky,
	dmatlack@google.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org

> From: Alex Williamson <alex@shazbot.org>
> Sent: Monday, August 10, 2026 11:32 PM
> 
> On Thu, 6 Aug 2026 16:47:39 -0300
> Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
> > Adding the sriov disable here at least makes it defensibly safe, that
> > we do still clear the PF on close, and we don't take risks that the
> > active VFs will crash the system during the FLR blip.
> >
> > Though I understand it was not the original intention, I feel we have
> > ended up in a strange place with the SRIOV PF feature ..
> 
> I think we have 3 potential options for PF reset:

Is there any coordination between in-kernel PF driver and in-kernel VF
drivers regarding to reset? At a glance looks that requesting FLR on PF
via sysfs would silently affect active VFs anyway...

> 
>  a) All resets are blocked or deferred while SR-IOV is active.
> 
>  b) VF mappings are zapped/invalidated at PF reset and require
>     coordination to be reinstated.
> 
>  c) VFs are torn down on all PF resets.
> 
> Option (a) is complex and doesn't look like stable material.  A deferred
> reset on .close needs to materialize on .remove or .sriov_configure
> (and of course .open), but to allow in-kernel VF drivers we can't rely
> on vfio usage counts, or even our vf-token trust boundaries, which is
> what leads to the .sriov_configure hook since we need to take advantage
> of every opportunity to issue a deferred reset when SR-IOV is not
> active when we can only rely on the PF state.
> 
> Option (b) is potentially more simple, it implements the blocking of
> access on reset in the kernel for enforcement, but defers the
> coordination of reinstantiating mappings to userspace, which is really
> how the vf_token model is intended to work.  This is incompatible with
> in-kernel VF drivers, which I think means tainting on VF unbind from
> vfio-pci and those working outside the model would tread carefully.
> 
> Option (c) is the more heavy handed approach, it means that a PF driver
> cannot fail or exit and re-attach.  We have existing logic that handles
> a PF driver re-opening the PF device with active VFs and authenticating
> the vf_token (logic also broken by in-kernel VF drivers that bypass the
> vf_token mechanics).  There also appears to be significant locking
> challenges in this approach, ex. the inversion of getting the device
> lock for SR-IOV teardown from the ioctl, config space, and .close
> contexts.
> 
> I'm open to suggestions, PF resets with live VFs is very much a gap
> that I'd like to close in the vfio-pci SR-IOV model.  Thanks,
> 

Before closing the open on reset, does it make sense to first fit it into
the coming trust infrastructure [1]? e.g. initially set to TRUST_NONE
for any VF with a PF owned by vfio-pci, preventing any bind to
in-kernel VF drivers. Then opt-in is allowed to promote the trust of
such VFs to TRUST_ADVERSARY, allowing driver binding but also put
it in precaution with IOMMU protection. So a malicious userspace
PF driver cannot indirectly affect VFs to do dma-based attack.

somehow VFs in this scenario feel akin to Thunderbolt devices...

[1] https://lore.kernel.org/linux-coco/20260705220819.2472765-10-djbw@kernel.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close
  2026-08-11  9:52         ` Tian, Kevin
@ 2026-08-11 13:59           ` Jason Gunthorpe
  0 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2026-08-11 13:59 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Alex Williamson, Samiullah Khawaja, bhelgaas@google.com,
	Leon Romanovsky, dmatlack@google.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Tue, Aug 11, 2026 at 09:52:37AM +0000, Tian, Kevin wrote:

> Before closing the open on reset, does it make sense to first fit it into
> the coming trust infrastructure [1]? e.g. initially set to TRUST_NONE
> for any VF with a PF owned by vfio-pci, preventing any bind to
> in-kernel VF drivers. Then opt-in is allowed to promote the trust of
> such VFs to TRUST_ADVERSARY, allowing driver binding but also put
> it in precaution with IOMMU protection. So a malicious userspace
> PF driver cannot indirectly affect VFs to do dma-based attack.
> 
> somehow VFs in this scenario feel akin to Thunderbolt devices...

That's certainly a novel idea that could make sense for drivers that
support the trust level

Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-11 13:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  0:33 [RFC PATCH 0/1] vfio/pci: Disable sriov on PF device close Samiullah Khawaja
2026-08-05  0:33 ` [RFC PATCH 1/1] " Samiullah Khawaja
2026-08-05  1:03   ` sashiko-bot
2026-08-05  9:34   ` Tian, Kevin
2026-08-10 21:47     ` Samiullah Khawaja
2026-08-05 15:03   ` Alex Williamson
2026-08-06 19:47     ` Jason Gunthorpe
2026-08-10 15:32       ` Alex Williamson
2026-08-11  9:52         ` Tian, Kevin
2026-08-11 13:59           ` Jason Gunthorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.