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

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Jul 02, 2025 at 11:50:32AM -0600, Alex Williamson wrote:
> 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.

The PF could have returned non-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 thought we can turn on SRIOV within VFIO and then attach VFIO to the
VFs which would have included the whole lot in the same dev sset?

> 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 think it will disable SRIOV and that will leave something of a
mess. Arguably we should be blocking resets that disable SRIOV inside
vfio?

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

No, it makes sense, it is just not entirely obvious on some of these
details.

I also did not find any locking concerns, so

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason