linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: linux-pci@vger.kernel.org
Cc: bhelgaas@google.com, linux-kernel@vger.kernel.org,
	myron.stowe@redhat.com, bodong@mellanox.com, eli@mellanox.com,
	laine@redhat.com
Subject: [PATCH] PCI: Return error if cannot probe VF
Date: Wed, 01 May 2019 11:00:16 -0600	[thread overview]
Message-ID: <155672991496.20698.4279330795743262888.stgit@gimli.home> (raw)

Commit 0e7df22401a3 ("PCI: Add sysfs sriov_drivers_autoprobe to control
VF driver binding") allows the user to specify that drivers for VFs of
a PF should not be probed, but it actually causes pci_device_probe() to
return success back to the driver core in this case.  Therefore by all
sysfs appearances the device is bound to a driver, the driver link from
the device exists as does the device link back from the driver, yet the
driver's probe function is never called on the device.  We also fail to
do any sort of cleanup when we're prohibited from probing the device,
the irq setup remains in place and we even hold a device reference.

Instead, abort with errno before any setup or references are taken when
pci_device_can_probe() prevents us from trying to probe the device.

Fixes: 0e7df22401a3 ("PCI: Add sysfs sriov_drivers_autoprobe to control VF driver binding")
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

This issue is easily tested by disabling sriov_drivers_autoprobe and
creating VFs:

# echo 0 > sriov_drivers_autoprobe
# echo 3 > sriov_numvfs
# readlink -f virtfn*/driver
/sys/bus/pci/drivers/iavf
/sys/bus/pci/drivers/iavf
/sys/bus/pci/drivers/iavf
(yet no netdevs exist for these VFs)

The semantics of this autoprobe disabling are a bit strange for the
user as well, I suppose it works if we force a bind through a driver's
bind attribute, but tools like libvirt and driverctl expect to bind
devices by setting the driver_override and then pushing the device
through driver_probe on the bus.  Is the intention of disabling
"autoprobe" that a driver_override should still work?  Otherwise the
user needs to set the driver_override for each VF, re-enable
sriov_drivers_autoprobe on the PF, and then probe the VFs.  Thus maybe
pci_device_can_probe() should allow probes of the driver_override
driver?  Thanks,

Alex

 drivers/pci/pci-driver.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 71853befd435..da7b82e56c83 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -414,6 +414,9 @@ static int pci_device_probe(struct device *dev)
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	struct pci_driver *drv = to_pci_driver(dev->driver);
 
+	if (!pci_device_can_probe(pci_dev))
+		return -ENODEV;
+
 	pci_assign_irq(pci_dev);
 
 	error = pcibios_alloc_irq(pci_dev);
@@ -421,12 +424,10 @@ static int pci_device_probe(struct device *dev)
 		return error;
 
 	pci_dev_get(pci_dev);
-	if (pci_device_can_probe(pci_dev)) {
-		error = __pci_device_probe(drv, pci_dev);
-		if (error) {
-			pcibios_free_irq(pci_dev);
-			pci_dev_put(pci_dev);
-		}
+	error = __pci_device_probe(drv, pci_dev);
+	if (error) {
+		pcibios_free_irq(pci_dev);
+		pci_dev_put(pci_dev);
 	}
 
 	return error;


             reply	other threads:[~2019-05-01 17:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-01 17:00 Alex Williamson [this message]
2019-05-02  0:58 ` [PATCH] PCI: Return error if cannot probe VF Laine Stump
2019-05-30 13:47 ` Bjorn Helgaas
2019-05-30 14:33   ` Alex Williamson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=155672991496.20698.4279330795743262888.stgit@gimli.home \
    --to=alex.williamson@redhat.com \
    --cc=bhelgaas@google.com \
    --cc=bodong@mellanox.com \
    --cc=eli@mellanox.com \
    --cc=laine@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=myron.stowe@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).