diff mbox

[V11,02/17] PCI/IOV: add VF enable/disable hook

Message ID 1421288887-7765-3-git-send-email-weiyang@linux.vnet.ibm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Wei Yang Jan. 15, 2015, 2:27 a.m. UTC
VFs are dynamically created/released when driver enable them. On some
platforms, like PowerNV, special resources are necessary to enable VFs.

This patch adds two hooks for platform initialization before creating the
VFs.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/pci/iov.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Benjamin Herrenschmidt Feb. 10, 2015, 12:26 a.m. UTC | #1
On Thu, 2015-01-15 at 10:27 +0800, Wei Yang wrote:
> +       if ((retval = pcibios_sriov_enable(dev, initial))) {
> +               dev_err(&dev->dev, "Failure %d from
> pcibios_sriov_setup()\n",
> +                       retval);
> +               return retval;
> +       }
> +

Don't we want pcibios_sriov_enable() to be able to crop the number
of VFs or do we think any resource limits have been applied
already ?

Ben.


--
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
Wei Yang Feb. 10, 2015, 1:35 a.m. UTC | #2
On Tue, Feb 10, 2015 at 11:26:19AM +1100, Benjamin Herrenschmidt wrote:
>On Thu, 2015-01-15 at 10:27 +0800, Wei Yang wrote:
>> +       if ((retval = pcibios_sriov_enable(dev, initial))) {
>> +               dev_err(&dev->dev, "Failure %d from
>> pcibios_sriov_setup()\n",
>> +                       retval);
>> +               return retval;
>> +       }
>> +
>
>Don't we want pcibios_sriov_enable() to be able to crop the number
>of VFs or do we think any resource limits have been applied
>already ?

The second parameter "initial" is the number of VFs will be enabled. Arch
dependent function will check the resources for these number of VFs.

Do I catch your question correctly?

>
>Ben.
>
Benjamin Herrenschmidt Feb. 10, 2015, 2:13 a.m. UTC | #3
On Tue, 2015-02-10 at 09:35 +0800, Wei Yang wrote:
> >Don't we want pcibios_sriov_enable() to be able to crop the number
> >of VFs or do we think any resource limits have been applied
> >already ?
> 
> The second parameter "initial" is the number of VFs will be enabled.
> Arch
> dependent function will check the resources for these number of VFs.
> 
> Do I catch your question correctly?

I was wondering if the number of resource that can be enabled is
smaller, should the arch function be able to return that smaller
number and we would still enable that number ?

Ie, have the arch function be able to "update" the value of
"initial" (by passing it by pointer for example).

Cheers,
Ben.


--
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
Wei Yang Feb. 10, 2015, 6:18 a.m. UTC | #4
On Tue, Feb 10, 2015 at 01:13:14PM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2015-02-10 at 09:35 +0800, Wei Yang wrote:
>> >Don't we want pcibios_sriov_enable() to be able to crop the number
>> >of VFs or do we think any resource limits have been applied
>> >already ?
>> 
>> The second parameter "initial" is the number of VFs will be enabled.
>> Arch
>> dependent function will check the resources for these number of VFs.
>> 
>> Do I catch your question correctly?
>
>I was wondering if the number of resource that can be enabled is
>smaller, should the arch function be able to return that smaller
>number and we would still enable that number ?
>
>Ie, have the arch function be able to "update" the value of
>"initial" (by passing it by pointer for example).

This would increase the time to enable sriov and block others driver to enable
sriov.

On powernv platform, those resources needed are M64 BAR and PE numbers.
Currently they are acquired separately. We have a lock to protect those
resources respectively. If we want to apply the logic you mentioned, we need
to have a "bigger" lock to protect both of them and try different values,
since at the same time, other driver may want to enable their sriov too. We
have to protect this contention.

Another example is PF has two IOV BAR but just one M64 BAR left in system.
In this case, no matter how many VFs want to enable, it will fail.

>
>Cheers,
>Ben.
>
>
>--
>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
diff mbox

Patch

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index e76d1a0..933d8cc 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -213,6 +213,11 @@  static void virtfn_remove(struct pci_dev *dev, int id, int reset)
 	pci_dev_put(dev);
 }
 
+int __weak pcibios_sriov_enable(struct pci_dev *pdev, u16 vf_num)
+{
+       return 0;
+}
+
 static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 {
 	int rc;
@@ -223,6 +228,7 @@  static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 	struct pci_dev *pdev;
 	struct pci_sriov *iov = dev->sriov;
 	int bars = 0;
+	int retval;
 
 	if (!nr_virtfn)
 		return 0;
@@ -297,6 +303,12 @@  static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 	if (nr_virtfn < initial)
 		initial = nr_virtfn;
 
+	if ((retval = pcibios_sriov_enable(dev, initial))) {
+		dev_err(&dev->dev, "Failure %d from pcibios_sriov_setup()\n",
+			retval);
+		return retval;
+	}
+
 	for (i = 0; i < initial; i++) {
 		rc = virtfn_add(dev, i, 0);
 		if (rc)
@@ -325,6 +337,11 @@  failed:
 	return rc;
 }
 
+int __weak pcibios_sriov_disable(struct pci_dev *pdev, u16 vf_num)
+{
+       return 0;
+}
+
 static void sriov_disable(struct pci_dev *dev)
 {
 	int i;
@@ -336,6 +353,8 @@  static void sriov_disable(struct pci_dev *dev)
 	for (i = 0; i < iov->num_VFs; i++)
 		virtfn_remove(dev, i, 0);
 
+	pcibios_sriov_disable(dev, iov->num_VFs);
+
 	iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
 	pci_cfg_access_lock(dev);
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);