PCI: Remove spurious error for sriov_numvfs store and simplify flow

If we request "num_vfs" and the driver's sriov_configure() method enables
exactly that number ("num_vfs_enabled"), we complain "Invalid value for
number of VFs to enable" and return an error.  We should silently return
success instead.

Also, use kstrtou16() since numVFs is defined to be a 16-bit field and
rework to simplify control flow.

Reported-by: Greg Rose <gregory.v.rose@intel.com>
Reference: http://lkml.kernel.org/r/20121214101911.00002f59@unknown
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Donald Dutile <ddutile@redhat.com>

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 05b78b16d20bdca1acb822e7654360102fb09072..9c6e9bb674ec54835d40e09b6c77d0683cd9265d 100644 (file)
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -422,77 +422,60 @@ static ssize_t sriov_numvfs_show(struct device *dev,
 }
 
 /*
- * num_vfs > 0; number of vfs to enable
- * num_vfs = 0; disable all vfs
+ * num_vfs > 0; number of VFs to enable
+ * num_vfs = 0; disable all VFs
  *
  * Note: SRIOV spec doesn't allow partial VF
- *       disable, so its all or none.
+ *       disable, so it's all or none.
  */
 static ssize_t sriov_numvfs_store(struct device *dev,
                                  struct device_attribute *attr,
                                  const char *buf, size_t count)
 {
        struct pci_dev *pdev = to_pci_dev(dev);
-       int num_vfs_enabled = 0;
-       int num_vfs;
-       int ret = 0;
-       u16 total;
+       int ret;
+       u16 num_vfs;
 
-       if (kstrtoint(buf, 0, &num_vfs) < 0)
-               return -EINVAL;
+       ret = kstrtou16(buf, 0, &num_vfs);
+       if (ret < 0)
+               return ret;
+
+       if (num_vfs > pci_sriov_get_totalvfs(pdev))
+               return -ERANGE;
+
+       if (num_vfs == pdev->sriov->num_VFs)
+               return count;           /* no change */
 
        /* is PF driver loaded w/callback */
        if (!pdev->driver || !pdev->driver->sriov_configure) {
-               dev_info(&pdev->dev,
-                        "Driver doesn't support SRIOV configuration via sysfs\n");
+               dev_info(&pdev->dev, "Driver doesn't support SRIOV configuration via sysfs\n");
                return -ENOSYS;
        }
 
-       /* if enabling vf's ... */
-       total = pci_sriov_get_totalvfs(pdev);
-       /* Requested VFs to enable < totalvfs and none enabled already */
-       if ((num_vfs > 0) && (num_vfs <= total)) {
-               if (pdev->sriov->num_VFs == 0) {
-                       num_vfs_enabled =
-                               pdev->driver->sriov_configure(pdev, num_vfs);
-                       if ((num_vfs_enabled >= 0) &&
-                           (num_vfs_enabled != num_vfs)) {
-                               dev_warn(&pdev->dev,
-                                        "Only %d VFs enabled\n",
-                                        num_vfs_enabled);
-                               return count;
-                       } else if (num_vfs_enabled < 0)
-                               /* error code from driver callback */
-                               return num_vfs_enabled;
-               } else if (num_vfs == pdev->sriov->num_VFs) {
-                       dev_warn(&pdev->dev,
-                                "%d VFs already enabled; no enable action taken\n",
-                                num_vfs);
-                       return count;
-               } else {
-                       dev_warn(&pdev->dev,
-                                "%d VFs already enabled. Disable before enabling %d VFs\n",
-                                pdev->sriov->num_VFs, num_vfs);
-                       return -EINVAL;
-               }
+       if (num_vfs == 0) {
+               /* disable VFs */
+               ret = pdev->driver->sriov_configure(pdev, 0);
+               if (ret < 0)
+                       return ret;
+               return count;
        }
 
-       /* disable vfs */
-       if (num_vfs == 0) {
-               if (pdev->sriov->num_VFs != 0) {
-                       ret = pdev->driver->sriov_configure(pdev, 0);
-                       return ret ? ret : count;
-               } else {
-                       dev_warn(&pdev->dev,
-                                "All VFs disabled; no disable action taken\n");
-                       return count;
-               }
+       /* enable VFs */
+       if (pdev->sriov->num_VFs) {
+               dev_warn(&pdev->dev, "%d VFs already enabled. Disable before enabling %d VFs\n",
+                        pdev->sriov->num_VFs, num_vfs);
+               return -EBUSY;
        }
 
-       dev_err(&pdev->dev,
-               "Invalid value for number of VFs to enable: %d\n", num_vfs);
+       ret = pdev->driver->sriov_configure(pdev, num_vfs);
+       if (ret < 0)
+               return ret;
 
-       return -EINVAL;
+       if (ret != num_vfs)
+               dev_warn(&pdev->dev, "%d VFs requested; only %d enabled\n",
+                        num_vfs, ret);
+
+       return count;
 }
 
 static struct device_attribute sriov_totalvfs_attr = __ATTR_RO(sriov_totalvfs);