[1/2] PCI/IOV: provide flag to skip VF scanning

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 <sebott@linux.ibm.com>
---
 drivers/pci/iov.c   | 48 ++++++++++++++++++++++++++++++++++++------------
 include/linux/pci.h |  1 +
 2 files changed, 37 insertions(+), 12 deletions(-)

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 <sebott@linux.ibm.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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 <sebott@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
>

Hello Bjorn,

On Thu, 20 Dec 2018, Bjorn Helgaas wrote:
> 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.

Done. I took the liberty to add Christoph's R-b to the first two patches
since it's just a split of the patch he gave the R-b to.

Thanks!
Sebastian

On Fri, Dec 21, 2018 at 03:19:49PM +0100, Sebastian Ott wrote:
> Hello Bjorn,
> 
> On Thu, 20 Dec 2018, Bjorn Helgaas wrote:
> > 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.
> 
> Done. I took the liberty to add Christoph's R-b to the first two patches
> since it's just a split of the patch he gave the R-b to.

Thanks.

It's really way too late to do this, but they're pretty trivial, and
I've been out longer than expected for vacation and illness, so I applied
these to pci/virtualization for v4.21.

Bjorn