| Message ID | 20150224083352.32124.63952.stgit@bhelgaas-glaptop2.roam.corp.google.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Feb 24, 2015 at 02:33:52AM -0600, Bjorn Helgaas wrote: > From: Wei Yang <weiyang@linux.vnet.ibm.com> > > VFs are dynamically created when a driver enables them. On some platforms, > like PowerNV, special resources are necessary to enable VFs. > > Add platform hooks for enabling and disabling VFs. > > Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/pci/iov.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > index 5643a1011e23..cc6fedf4a1b9 100644 > --- a/drivers/pci/iov.c > +++ b/drivers/pci/iov.c > @@ -220,6 +220,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) I think this "vf_num" parameter should be renamed to something like "num_vfs" instead. It's subtle, but "vf_num" suggests that we're talking about one of several VFs, e.g., VF1 or VF 2. But here we really mean the total number of VFs that we're enabling. There's similar code in the powerpc implementation that should be renamed the same way. > +{ > + return 0; > +} > + > static int sriov_enable(struct pci_dev *dev, int nr_virtfn) > { > int rc; > @@ -231,6 +236,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) > struct pci_sriov *iov = dev->sriov; > int bars = 0; > int bus; > + int retval; > > if (!nr_virtfn) > return 0; > @@ -307,6 +313,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_enable()\n", > + retval); > + return retval; > + } > + > for (i = 0; i < initial; i++) { > rc = virtfn_add(dev, i, 0); > if (rc) > @@ -335,6 +347,11 @@ failed: > return rc; > } > > +int __weak pcibios_sriov_disable(struct pci_dev *pdev) > +{ > + return 0; > +} > + > static void sriov_disable(struct pci_dev *dev) > { > int i; > @@ -346,6 +363,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->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); > -- 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
On Tue, Feb 24, 2015 at 02:39:21AM -0600, Bjorn Helgaas wrote: >On Tue, Feb 24, 2015 at 02:33:52AM -0600, Bjorn Helgaas wrote: >> From: Wei Yang <weiyang@linux.vnet.ibm.com> >> >> VFs are dynamically created when a driver enables them. On some platforms, >> like PowerNV, special resources are necessary to enable VFs. >> >> Add platform hooks for enabling and disabling VFs. >> >> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> >> --- >> drivers/pci/iov.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c >> index 5643a1011e23..cc6fedf4a1b9 100644 >> --- a/drivers/pci/iov.c >> +++ b/drivers/pci/iov.c >> @@ -220,6 +220,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) > >I think this "vf_num" parameter should be renamed to something like >"num_vfs" instead. It's subtle, but "vf_num" suggests that we're talking >about one of several VFs, e.g., VF1 or VF 2. But here we really mean the >total number of VFs that we're enabling. > >There's similar code in the powerpc implementation that should be >renamed the same way. > Agree, would take this suggestion in consideration. >> +{ >> + return 0; >> +} >> + >> static int sriov_enable(struct pci_dev *dev, int nr_virtfn) >> { >> int rc; >> @@ -231,6 +236,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) >> struct pci_sriov *iov = dev->sriov; >> int bars = 0; >> int bus; >> + int retval; >> >> if (!nr_virtfn) >> return 0; >> @@ -307,6 +313,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_enable()\n", >> + retval); >> + return retval; >> + } >> + >> for (i = 0; i < initial; i++) { >> rc = virtfn_add(dev, i, 0); >> if (rc) >> @@ -335,6 +347,11 @@ failed: >> return rc; >> } >> >> +int __weak pcibios_sriov_disable(struct pci_dev *pdev) >> +{ >> + return 0; >> +} >> + >> static void sriov_disable(struct pci_dev *dev) >> { >> int i; >> @@ -346,6 +363,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->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); >> >-- >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 --git a/drivers/pci/iov.c b/drivers/pci/iov.c index 5643a1011e23..cc6fedf4a1b9 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -220,6 +220,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; @@ -231,6 +236,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) struct pci_sriov *iov = dev->sriov; int bars = 0; int bus; + int retval; if (!nr_virtfn) return 0; @@ -307,6 +313,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_enable()\n", + retval); + return retval; + } + for (i = 0; i < initial; i++) { rc = virtfn_add(dev, i, 0); if (rc) @@ -335,6 +347,11 @@ failed: return rc; } +int __weak pcibios_sriov_disable(struct pci_dev *pdev) +{ + return 0; +} + static void sriov_disable(struct pci_dev *dev) { int i; @@ -346,6 +363,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->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);