Re: [PATCH] vfio/pci: Separate SR-IOV VF dev_set

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

On Wed, 2 Jul 2025 13:00:31 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Thu, Jun 26, 2025 at 04:56:18PM -0600, Alex Williamson wrote:
> > @@ -2149,7 +2149,7 @@ int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
> >  		return -EBUSY;
> >  	}
> >  
> > -	if (pci_is_root_bus(pdev->bus)) {
> > +	if (pci_is_root_bus(pdev->bus) || pdev->is_virtfn) {
> >  		ret = vfio_assign_device_set(&vdev->vdev, vdev);
> >  	} else if (!pci_probe_reset_slot(pdev->slot)) {
> >  		ret = vfio_assign_device_set(&vdev->vdev, pdev->slot);  
> 
> What about the logic in vfio_pci_dev_set_resettable()?

IIRC, VFs are going to fail the pci_probe_reset_slot() and
pci_probe_reset_bus() tests because they don't have a pdev->slot or
pdev->bus->self respectively.

> I guess in most cases vfio_pci_dev_set_resettable() == NULL already
> for VFs since it would be rare that the PFs and VFs are all under
> VFIO.

Regardless, the VF will return NULL.

> But it could happen and this would permanently block hot reset? Maybe
> just mention it in the commit?

There is no bridge from which to initiate an SBR for a VF, there would
only be the bridge above the PF.  We require SR-IOV is disabled when
the PF is opened, so the dev_set of the PF would never have included the
VFs.  I haven't tried it, but it may be possible to trigger a hot reset
on a user owned PF while there are open VFs.  If that is possible, I
wonder if it isn't just a userspace problem though, it doesn't seem
there's anything fundamentally wrong with it from a vfio perspective.
The vf-token already indicates at the kernel level that there is
collaboration between PF and VF userspace drivers.

> I guess the commit message is also trying to say that we don't use
> VFIO_DEVICE_PCI_HOT_RESET if VFs are present on either the VF or
> PF - and this change will block that.

The hot reset ioctl has never been available to VFs, there's no bridge
from which to initiate an SBR without traversing above the PF.
 
> All VF resets should go through VFIO_DEVICE_RESETF. If you want to
> slot/bus reset the PF then you have to disable SRIOV first.

I'm not positive on the latter, but AFAIK it's always been the case
that only FLR is available on VFs and this doesn't change that.  Am I
still overlooking something that concerns you?  Thanks,

Alex