Hi Sebastian,
On Tue, Dec 18, 2018 at 11:16:49AM +0100, Sebastian Ott wrote:
> Provide a flag to skip scanning for new VFs after SRIOV enablement.
> This can be set by implementations for which the VFs are already
> reported by other means.
>
> Signed-off-by: Sebastian Ott <seb...@linux.ibm.com>
> ---
> drivers/pci/iov.c | 48 ++++++++++++++++++++++++++++++++++++------------
> include/linux/pci.h | 1 +
> 2 files changed, 37 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 9616eca3182f..3aa115ed3a65 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -252,6 +252,27 @@ int __weak pcibios_sriov_disable(struct pci_dev *pdev)
> return 0;
> }
>
> +static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
> +{
> + unsigned int i;
> + int rc;
> +
> + if (dev->no_vf_scan)
> + return 0;
> +
> + for (i = 0; i < num_vfs; i++) {
> + rc = pci_iov_add_virtfn(dev, i);
> + if (rc)
> + goto failed;
> + }
> + return 0;
> +failed:
> + while (i--)
> + pci_iov_remove_virtfn(dev, i);
> +
> + return rc;
> +}
I think the strategy is fine, but can you restructure the patches
like this:
1) Factor out sriov_add_vfs() and sriov_dev_vfs(). This makes no
functional change at all.
2) Add dev->no_vf_scan, set it in the s390 pcibios_add_device(), and
test it in sriov_add_vfs(), and sriov_del_vfs().
I think both pieces will be easier to review that way.
> static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> {
> int rc;
> @@ -337,21 +358,15 @@ static int sriov_enable(struct pci_dev *dev, int
> nr_virtfn)
> msleep(100);
> pci_cfg_access_unlock(dev);
>
> - for (i = 0; i < initial; i++) {
> - rc = pci_iov_add_virtfn(dev, i);
> - if (rc)
> - goto failed;
> - }
> + rc = sriov_add_vfs(dev, initial);
> + if (rc)
> + goto err_pcibios;
>
> kobject_uevent(&dev->dev.kobj, KOBJ_CHANGE);
> iov->num_VFs = nr_virtfn;
>
> return 0;
>
> -failed:
> - while (i--)
> - pci_iov_remove_virtfn(dev, i);
> -
> err_pcibios:
> iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
> pci_cfg_access_lock(dev);
> @@ -368,17 +383,26 @@ static int sriov_enable(struct pci_dev *dev, int
> nr_virtfn)
> return rc;
> }
>
> -static void sriov_disable(struct pci_dev *dev)
> +static void sriov_del_vfs(struct pci_dev *dev)
> {
> - int i;
> struct pci_sriov *iov = dev->sriov;
> + int i;
>
> - if (!iov->num_VFs)
> + if (dev->no_vf_scan)
> return;
>
> for (i = 0; i < iov->num_VFs; i++)
> pci_iov_remove_virtfn(dev, i);
> +}
> +
> +static void sriov_disable(struct pci_dev *dev)
> +{
> + struct pci_sriov *iov = dev->sriov;
> +
> + if (!iov->num_VFs)
> + return;
>
> + sriov_del_vfs(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);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 11c71c4ecf75..f70b9ccd3e86 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -405,6 +405,7 @@ struct pci_dev {
> unsigned int non_compliant_bars:1; /* Broken BARs; ignore them */
> unsigned int is_probed:1; /* Device probing in progress */
> unsigned int link_active_reporting:1;/* Device capable of reporting
> link active */
> + unsigned int no_vf_scan:1; /* Don't scan for VF's after VF
> enablement */
> pci_dev_flags_t dev_flags;
> atomic_t enable_cnt; /* pci_enable_device has been called */
>
> --
> 2.13.4
>
