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 05b78b1..9c6e9bb 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -422,77 +422,60 @@
}
/*
- * 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;
- }
- }
-
- /* 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;
- }
+ /* disable VFs */
+ ret = pdev->driver->sriov_configure(pdev, 0);
+ if (ret < 0)
+ return ret;
+ return count;
}
- dev_err(&pdev->dev,
- "Invalid value for number of VFs to enable: %d\n", num_vfs);
+ /* 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;
+ }
- return -EINVAL;
+ ret = pdev->driver->sriov_configure(pdev, num_vfs);
+ if (ret < 0)
+ return ret;
+
+ 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);