Hi Jakub,
On Mon, Apr 02, 2018 at 03:46:52PM -0700, Jakub Kicinski wrote:
> Some user space depends on enabling sriov_totalvfs number of VFs
> to not fail, e.g.:
>
> $ cat .../sriov_totalvfs > .../sriov_numvfs
>
> For devices which VF support depends on loaded FW we have the
> pci_sriov_{g,s}et_totalvfs() API. However, this API uses 0 as
> a special "unset" value, meaning drivers can't limit sriov_totalvfs
> to 0. Remove the special values completely and simply initialize
> driver_max_VFs to total_VFs. Then always use driver_max_VFs.
> Add a helper for drivers to reset the VF limit back to total.
I still can't really make sense out of the changelog.
I think part of the reason it's confusing is because there are two
things going on:
1) You want this:
pci_sriov_set_totalvfs(dev, 0);
x = pci_sriov_get_totalvfs(dev)
to return 0 instead of total_VFs. That seems to connect with
your subject line. It means "sriov_totalvfs" in sysfs could be
0, but I don't know how that is useful (I'm sure it is; just
educate me :))
2) You're adding the pci_sriov_reset_totalvfs() interface. I'm not
sure what you intend for this. Is *every* driver supposed to
call it in .remove()? Could/should this be done in the core
somehow instead of depending on every driver?
I'm also having a hard time connecting your user-space command example
with the rest of this. Maybe it will make more sense to me tomorrow
after some coffee.
> Signed-off-by: Jakub Kicinski <jakub.kicinski@xxxxxxxxxxxxx>
> ---
> drivers/net/ethernet/netronome/nfp/nfp_main.c | 6 +++---
> drivers/pci/iov.c | 27 +++++++++++++++++++++------
> include/linux/pci.h | 2 ++
> 3 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
> index c4b1f344b4da..a76d177e40dd 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
> @@ -123,7 +123,7 @@ static int nfp_pcie_sriov_read_nfd_limit(struct nfp_pf *pf)
> return pci_sriov_set_totalvfs(pf->pdev, pf->limit_vfs);
>
> pf->limit_vfs = ~0;
> - pci_sriov_set_totalvfs(pf->pdev, 0); /* 0 is unset */
> + pci_sriov_reset_totalvfs(pf->pdev);
> /* Allow any setting for backwards compatibility if symbol not found */
> if (err == -ENOENT)
> return 0;
> @@ -537,7 +537,7 @@ static int nfp_pci_probe(struct pci_dev *pdev,
> err_net_remove:
> nfp_net_pci_remove(pf);
> err_sriov_unlimit:
> - pci_sriov_set_totalvfs(pf->pdev, 0);
> + pci_sriov_reset_totalvfs(pf->pdev);
> err_fw_unload:
> kfree(pf->rtbl);
> nfp_mip_close(pf->mip);
> @@ -570,7 +570,7 @@ static void nfp_pci_remove(struct pci_dev *pdev)
> nfp_hwmon_unregister(pf);
>
> nfp_pcie_sriov_disable(pdev);
> - pci_sriov_set_totalvfs(pf->pdev, 0);
> + pci_sriov_reset_totalvfs(pf->pdev);
>
> nfp_net_pci_remove(pf);
>
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 677924ae0350..c63ea870d8be 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -443,6 +443,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
> iov->nres = nres;
> iov->ctrl = ctrl;
> iov->total_VFs = total;
> + iov->driver_max_VFs = total;
> pci_read_config_word(dev, pos + PCI_SRIOV_VF_DID, &iov->vf_device);
> iov->pgsz = pgsz;
> iov->self = dev;
> @@ -788,12 +789,29 @@ int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
> }
> EXPORT_SYMBOL_GPL(pci_sriov_set_totalvfs);
>
> +/**
> + * pci_sriov_reset_totalvfs -- return the TotalVFs value to the default
> + * @dev: the PCI PF device
> + *
> + * Should be called from PF driver's remove routine with
> + * device's mutex held.
> + */
> +void pci_sriov_reset_totalvfs(struct pci_dev *dev)
> +{
> + /* Shouldn't change if VFs already enabled */
> + if (!dev->is_physfn || dev->sriov->ctrl & PCI_SRIOV_CTRL_VFE)
> + return;
> +
> + dev->sriov->driver_max_VFs = dev->sriov->total_VFs;
> +}
> +EXPORT_SYMBOL_GPL(pci_sriov_reset_totalvfs);
> +
> /**
> * pci_sriov_get_totalvfs -- get total VFs supported on this device
> * @dev: the PCI PF device
> *
> - * For a PCIe device with SRIOV support, return the PCIe
> - * SRIOV capability value of TotalVFs or the value of driver_max_VFs
> + * For a PCIe device with SRIOV support, return the value of driver_max_VFs
> + * which can be equal to the PCIe SRIOV capability value of TotalVFs or lower
> * if the driver reduced it. Otherwise 0.
> */
> int pci_sriov_get_totalvfs(struct pci_dev *dev)
> @@ -801,9 +819,6 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)
> if (!dev->is_physfn)
> return 0;
>
> - if (dev->sriov->driver_max_VFs)
> - return dev->sriov->driver_max_VFs;
> -
> - return dev->sriov->total_VFs;
> + return dev->sriov->driver_max_VFs;
> }
> EXPORT_SYMBOL_GPL(pci_sriov_get_totalvfs);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 024a1beda008..95fde8850393 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1952,6 +1952,7 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id);
> int pci_num_vf(struct pci_dev *dev);
> int pci_vfs_assigned(struct pci_dev *dev);
> int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
> +void pci_sriov_reset_totalvfs(struct pci_dev *dev);
> int pci_sriov_get_totalvfs(struct pci_dev *dev);
> resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
> void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe);
> @@ -1978,6 +1979,7 @@ static inline int pci_vfs_assigned(struct pci_dev *dev)
> { return 0; }
> static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
> { return 0; }
> +static inline void pci_sriov_reset_totalvfs(struct pci_dev *dev) { }
> static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
> { return 0; }
> static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
> --
> 2.16.2
>
