On Tue, Apr 12, 2022 at 12:25:44PM -0600, Alex Williamson wrote: > On Mon, 11 Apr 2022 10:56:31 -0300 > Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > > @@ -1732,10 +1705,28 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, > > static int vfio_pci_vf_init(struct vfio_pci_core_device *vdev) > > { > > struct pci_dev *pdev = vdev->pdev; > > + struct vfio_pci_core_device *cur; > > + struct pci_dev *physfn; > > int ret; > > > > - if (!pdev->is_physfn) > > + if (!pdev->is_physfn) { > > + /* > > + * If this VF was created by our vfio_pci_core_sriov_configure() > > + * then we can find the PF vfio_pci_core_device now, and due to > > + * the locking in pci_disable_sriov() it cannot change until > > + * this VF device driver is removed. > > + */ > > + physfn = pci_physfn(vdev->pdev); > > + mutex_lock(&vfio_pci_sriov_pfs_mutex); > > + list_for_each_entry (cur, &vfio_pci_sriov_pfs, sriov_pfs_item) { > > + if (cur->pdev == physfn) { > > + vdev->sriov_pf_core_dev = cur; > > + break; > > + } > > + } > > + mutex_unlock(&vfio_pci_sriov_pfs_mutex); > > return 0; > > + } > > > > vdev->vf_token = kzalloc(sizeof(*vdev->vf_token), GFP_KERNEL); > > if (!vdev->vf_token) > > One more comment on final review; are we equating !is_physfn to > is_virtfn above? This branch was originally meant to kick out both VFs > and non-SRIOV PFs. Calling pci_physfn() on a !is_virtfn device will > return itself, so we should never find a list match, but we also don't > need to look for a match for !is_virtfn, so it's a bit confusing and > slightly inefficient. Should the new code be added in a separate > is_virtfn branch above the existing !is_physfn test? Thanks, I started at it for a while and came the same conclusion, I misunderstood that is_physfn is really trying to be is_sriov_physfn.. So not a bug, but not really clear code. I added this, I'll repost it. diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 8bf0f18e668a32..3c6493957abe19 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1709,7 +1709,7 @@ static int vfio_pci_vf_init(struct vfio_pci_core_device *vdev) struct pci_dev *physfn; int ret; - if (!pdev->is_physfn) { + if (pdev->is_virtfn) { /* * If this VF was created by our vfio_pci_core_sriov_configure() * then we can find the PF vfio_pci_core_device now, and due to @@ -1728,6 +1728,10 @@ static int vfio_pci_vf_init(struct vfio_pci_core_device *vdev) return 0; } + /* Not a SRIOV PF */ + if (!pdev->is_physfn) + return 0; + vdev->vf_token = kzalloc(sizeof(*vdev->vf_token), GFP_KERNEL); if (!vdev->vf_token) return -ENOMEM; Thanks, Jason