Re: [pci PATCH v6 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources

From: Christoph Hellwig
Date: Wed Mar 14 2018 - 04:54:29 EST


On Tue, Mar 13, 2018 at 02:28:49PM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@xxxxxxxxx>
>
> This patch adds a common configuration function called
> pci_sriov_configure_simple that will allow for managing VFs on devices
> where the PF is not capable of managing VF resources.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@xxxxxxxxx>
> ---
>
> v5: New patch replacing pci_sriov_configure_unmanaged with
> pci_sriov_configure_simple
> Dropped bits related to autoprobe changes
> v6: Defined pci_sriov_configure_simple as NULL if IOV is disabled
>
> drivers/pci/iov.c | 32 ++++++++++++++++++++++++++++++++
> include/linux/pci.h | 3 +++
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 677924ae0350..bd7021491fdb 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -807,3 +807,35 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)
> return dev->sriov->total_VFs;
> }
> EXPORT_SYMBOL_GPL(pci_sriov_get_totalvfs);
> +
> +/**
> + * pci_sriov_configure_simple - helper to configure unmanaged SR-IOV
> + * @dev: the PCI device
> + * @nr_virtfn: number of virtual functions to enable, 0 to disable
> + *
> + * Used to provide generic enable/disable SR-IOV option for devices
> + * that do not manage the VFs generated by their driver
> + */
> +int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
> +{
> + int err = -EINVAL;

This assignment seems like it is never used..

> +
> + might_sleep();
> +
> + if (!dev->is_physfn)
> + return -ENODEV;
> +
> + if (pci_vfs_assigned(dev)) {
> + pci_warn(dev,
> + "Cannot modify SR-IOV while VFs are assigned\n");
> + err = -EPERM;

Why not:

if (pci_vfs_assigned(dev)) {
pci_warn(dev,
"Cannot modify SR-IOV while VFs are assigned\n");
return -EPERM;
}

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>