From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Dutile Subject: Re: [net-next 08/14] pci: Add SRIOV helper function to determine if VFs are assigned to guest Date: Tue, 23 Apr 2013 17:16:43 -0400 Message-ID: <5176FA3B.8040003@redhat.com> References: <1366451353-24714-1-git-send-email-jeffrey.t.kirsher@intel.com> <1366451353-24714-9-git-send-email-jeffrey.t.kirsher@intel.com> <1366515088.2209.15.camel@jtkirshe-mobl> <5175B0A9.7070100@intel.com> <20130423125101.00005bb7@unknown> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alexander Duyck , Bjorn Helgaas , "Kirsher, Jeffrey T" , David Miller , netdev , gospo@redhat.com, sassmann@redhat.com, "linux-pci@vger.kernel.org" , Chris Wright To: Greg Rose Return-path: In-Reply-To: <20130423125101.00005bb7@unknown> Sender: linux-pci-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 04/23/2013 03:51 PM, Greg Rose wrote: > On Mon, 22 Apr 2013 14:50:33 -0700 > Alexander Duyck wrote: > >> On 04/22/2013 01:09 PM, Bjorn Helgaas wrote: >>> On Sat, Apr 20, 2013 at 9:31 PM, Jeff Kirsher >>> wrote: >>>> On Sat, 2013-04-20 at 02:49 -0700, Jeff Kirsher wrote: >>>>> From: Alexander Duyck >>>>> >>>>> This function is meant to add a helper function that will >>>>> determine if a PF has any VFs that are currently assigned to a >>>>> guest. We currently have been implementing this function per >>>>> driver, and going forward I would like to avoid that by making >>>>> this function generic and using this helper. >>>>> >>>>> Signed-off-by: Alexander Duyck >>>>> Signed-off-by: Jeff Kirsher >>>> Adding linux-pci mailing list and Bjorn to the CC. >>>> >>>> Bjorn- David Miller needs a signoff by PCI maintainer. >>>> >>>>> --- >>>>> drivers/pci/iov.c | 41 >>>>> +++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | >>>>> 5 +++++ 2 files changed, 46 insertions(+) >>>>> >>>>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c >>>>> index ee599f2..fd99720 100644 >>>>> --- a/drivers/pci/iov.c >>>>> +++ b/drivers/pci/iov.c >>>>> @@ -729,6 +729,47 @@ int pci_num_vf(struct pci_dev *dev) >>>>> EXPORT_SYMBOL_GPL(pci_num_vf); >>>>> >>>>> /** >>>>> + * pci_vfs_assigned - returns number of VFs are assigned to a >>>>> guest >>>>> + * @dev: the PCI device >>>>> + * >>>>> + * Returns number of VFs belonging to this device that are >>>>> assigned to a guest. >>>>> + * If device is not a physical function returns -ENODEV. >>>>> + */ >>>>> +int pci_vfs_assigned(struct pci_dev *dev) >>> I guess the idea here is to replace be_find_vfs(), >>> igb_vfs_are_assigned(), ixgbe_vfs_are_assigned(), etc. It does seem >>> good to reduce duplicated code. >> >> The general idea was just to remove duplicate code. As is we have a >> couple more drivers on the way that would end up needing a similar >> function. >> >>> I'm trying to figure out why this is safe -- there's no explicit >>> synchronization between the iteration through PCI devices looking >>> for matching VFs and the device assignment/deassignment paths that >>> set or clear PCI_DEV_FLAGS_ASSIGNED, so on the face of it, it looks >>> like things could change between calling pci_vfs_assigned() and >>> using the result to make a decision. >>> >>> Most of the calls would be in .remove() functions, so maybe there's >>> some sort of synchronization in that path that makes this safe. >>> >>> Bjorn >> >> I'm assuming this will be used in regions that are somehow protected >> since the main spots where this might be called would be probe, >> remove, or when updating the number of VFs. From what I can tell in >> the Xen case there is a driver stub that is loaded that sets the flag >> so that is covered by probe/remove. I don't know about the KVM case. > > KVM should be fine. Setting/clearing the flag occurs while a device is > being assigned to or removed from a VM - presumably device assignment > is already safe against race conditions. I'd find it hard to believe > that it's not. Code is in ../virt/assigned_dev.c and ../virt/iommu.c. > > - Greg > Added Chris to this thread since he has history here... Q: Why not link-list the VF pci-dev's off the PF's pci-dev when they are created? (yes, voices in my head have asked me this question in the past! ;-) ) -- that way, finding them is easy, and locked down when doing a get on the PF pci-dev. a backptr of the VF's pci-dev to it's PF pci-dev would be handy too! --> it'd also simplify that rmmod PF driver while VF drivers loaded (& assigned) case as well. >> >> Thanks, >> >> Alex >> >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html