All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sinan Kaya <okaya@kernel.org>
To: linux-pci@vger.kernel.org
Cc: Sinan Kaya <okaya@kernel.org>,
	Derek Chickles <derek.chickles@caviumnetworks.com>,
	Satanand Burla <satananda.burla@caviumnetworks.com>,
	Felix Manlunas <felix.manlunas@caviumnetworks.com>,
	Raghu Vatsavayi <raghu.vatsavayi@caviumnetworks.com>,
	"David S. Miller" <davem@davemloft.net>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Juergen Gross <jgross@suse.com>,
	Jia-Ju Bai <baijiaju1990@gmail.com>
Subject: [PATCH v6 1/7] PCI: Expose reset_type to users of __pci_reset_function_locked()
Date: Fri, 19 Oct 2018 02:11:21 +0000	[thread overview]
Message-ID: <20181019021132.14743-1-okaya@kernel.org> (raw)

We need a contract between the reset API users and the PCI core about the
types of reset that a user needs vs. what PCI core can do internally.
If a platform supports hotplug, we need to do hotplug reset as an example.

Expose the reset types to the drivers and try different reset types based
on the new reset_type parameter.

Most users are expected to use PCI_RESET_ANY, PCI_RESET_FUNC or
PCI_RESET_LINK parameters.

Link: https://www.spinics.net/lists/linux-pci/msg75828.html
Suggested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 .../net/ethernet/cavium/liquidio/lio_main.c   |  2 +-
 drivers/pci/pci.c                             | 59 ++++++++++++-------
 drivers/xen/xen-pciback/pci_stub.c            |  6 +-
 include/linux/pci.h                           | 58 +++++++++++++++++-
 4 files changed, 100 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 6fb13fa73b27..0ff76722734d 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -989,7 +989,7 @@ static void octeon_pci_flr(struct octeon_device *oct)
 	pci_write_config_word(oct->pci_dev, PCI_COMMAND,
 			      PCI_COMMAND_INTX_DISABLE);
 
-	rc = __pci_reset_function_locked(oct->pci_dev);
+	rc = __pci_reset_function_locked(oct->pci_dev, PCI_RESET_ANY);
 
 	if (rc != 0)
 		dev_err(&oct->pci_dev->dev, "Error %d resetting PCI function %d\n",
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1835f3a7aa8d..e292ea589d3e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4673,6 +4673,7 @@ static void pci_dev_restore(struct pci_dev *dev)
  * __pci_reset_function_locked - reset a PCI device function while holding
  * the @dev mutex lock.
  * @dev: PCI device to reset
+ * @reset_type: reset mask to try
  *
  * Some devices allow an individual function to be reset without affecting
  * other functions in the same device.  The PCI device must be responsive
@@ -4688,9 +4689,9 @@ static void pci_dev_restore(struct pci_dev *dev)
  * Returns 0 if the device function was successfully reset or negative if the
  * device doesn't support resetting a single function.
  */
-int __pci_reset_function_locked(struct pci_dev *dev)
+int __pci_reset_function_locked(struct pci_dev *dev, u32 reset_type)
 {
-	int rc;
+	int rc = -EINVAL;
 
 	might_sleep();
 
@@ -4702,24 +4703,42 @@ int __pci_reset_function_locked(struct pci_dev *dev)
 	 * other error, we're also finished: this indicates that further
 	 * reset mechanisms might be broken on the device.
 	 */
-	rc = pci_dev_specific_reset(dev, 0);
-	if (rc != -ENOTTY)
-		return rc;
-	if (pcie_has_flr(dev)) {
-		rc = pcie_flr(dev);
+	if (reset_type & PCI_RESET_DEV_SPECIFIC) {
+		rc = pci_dev_specific_reset(dev, 0);
 		if (rc != -ENOTTY)
 			return rc;
 	}
-	rc = pci_af_flr(dev, 0);
-	if (rc != -ENOTTY)
-		return rc;
-	rc = pci_pm_reset(dev, 0);
-	if (rc != -ENOTTY)
-		return rc;
-	rc = pci_dev_reset_slot_function(dev, 0);
-	if (rc != -ENOTTY)
-		return rc;
-	return pci_parent_bus_reset(dev, 0);
+
+	if (reset_type & PCI_RESET_FLR) {
+		if (pcie_has_flr(dev)) {
+			rc = pcie_flr(dev);
+			if (rc != -ENOTTY)
+				return rc;
+		}
+		rc = pci_af_flr(dev, 0);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	if (reset_type & PCI_RESET_PM) {
+		rc = pci_pm_reset(dev, 0);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	if (reset_type & PCI_RESET_SLOT) {
+		rc = pci_dev_reset_slot_function(dev, 0);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	if (reset_type & PCI_RESET_BUS) {
+		rc = pci_parent_bus_reset(dev, 0);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	return rc;
 }
 EXPORT_SYMBOL_GPL(__pci_reset_function_locked);
 
@@ -4784,7 +4803,7 @@ int pci_reset_function(struct pci_dev *dev)
 	pci_dev_lock(dev);
 	pci_dev_save_and_disable(dev);
 
-	rc = __pci_reset_function_locked(dev);
+	rc = __pci_reset_function_locked(dev, PCI_RESET_ANY);
 
 	pci_dev_restore(dev);
 	pci_dev_unlock(dev);
@@ -4819,7 +4838,7 @@ int pci_reset_function_locked(struct pci_dev *dev)
 
 	pci_dev_save_and_disable(dev);
 
-	rc = __pci_reset_function_locked(dev);
+	rc = __pci_reset_function_locked(dev, PCI_RESET_ANY);
 
 	pci_dev_restore(dev);
 
@@ -4844,7 +4863,7 @@ int pci_try_reset_function(struct pci_dev *dev)
 		return -EAGAIN;
 
 	pci_dev_save_and_disable(dev);
-	rc = __pci_reset_function_locked(dev);
+	rc = __pci_reset_function_locked(dev, PCI_RESET_ANY);
 	pci_dev_restore(dev);
 	pci_dev_unlock(dev);
 
diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index 59661db144e5..6dfb805bcb19 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -105,7 +105,7 @@ static void pcistub_device_release(struct kref *kref)
 	/* Call the reset function which does not take lock as this
 	 * is called from "unbind" which takes a device_lock mutex.
 	 */
-	__pci_reset_function_locked(dev);
+	__pci_reset_function_locked(dev, PCI_RESET_ANY);
 	if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state))
 		dev_info(&dev->dev, "Could not reload PCI state\n");
 	else
@@ -283,7 +283,7 @@ void pcistub_put_pci_dev(struct pci_dev *dev)
 	 * (so it's ready for the next domain)
 	 */
 	device_lock_assert(&dev->dev);
-	__pci_reset_function_locked(dev);
+	__pci_reset_function_locked(dev, PCI_RESET_ANY);
 
 	dev_data = pci_get_drvdata(dev);
 	ret = pci_load_saved_state(dev, dev_data->pci_saved_state);
@@ -417,7 +417,7 @@ static int pcistub_init_device(struct pci_dev *dev)
 		dev_err(&dev->dev, "Could not store PCI conf saved state!\n");
 	else {
 		dev_dbg(&dev->dev, "resetting (FLR, D3, etc) the device\n");
-		__pci_reset_function_locked(dev);
+		__pci_reset_function_locked(dev, PCI_RESET_ANY);
 		pci_restore_state(dev);
 	}
 	/* Now disable the device (this also ensures some private device
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 6925828f9f25..7ace46b3e479 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -849,6 +849,62 @@ enum {
 	PCI_SCAN_ALL_PCIE_DEVS	= 0x00000040,	/* Scan all, not just dev 0 */
 };
 
+/*
+ * A common theme here is that the driver wants some degree of
+ * control of the type of reset used, vfio specifically wants to specify a
+ * bus or slot reset while hfi1 just wants to specify that the link is
+ * reset and doesn't care if it's a bus or slot reset that accomplishes
+ * that.  The right "unified" interface is one that takes a parameter
+ * allowing the caller to specify the scope or type of reset
+ * with aliases so drivers can ignore the hotplug interface if they wish
+ * for special cases.
+ *
+ * PCI_RESET_ANY tries all reset type one by one until device is successfully
+ * reset. Under normal circumstances, most drivers are expected to use
+ * PCI_RESET_ANY as they don't usually care about the type of reset as long
+ * as device is reset.
+ *
+ * PCI_RESET_FUNC is useful when you want to reset one particular PCI device
+ * but you don't want to impact other devices or cause a temporary service
+ * outage.
+ *
+ * PCI_RESET_LINK can be used to cause a link retraining. This operation will
+ * cause service outage for the PCI bus if there are other devices on the same
+ * bus. PCI_RESET_LINK determines if a platform supports hotplug or not and
+ * suppresses hotplug interrupts during secondary bus reset.
+ *
+ * PCI_RESET_DEV_SPECIFIC can be used to reset a device by help from the
+ * device driver. Not all device drivers support this option.
+ *
+ * PCI_RESET_FLR can be used to issue a Function Level Reset to a device. This
+ * option will fail if FLR is not supported.
+ *
+ * PCI_RESET_PM can be used to reset the device via D3->D0 and D0->D3 sleep
+ * transition. This assumes that device supports PM based reset.
+ *
+ * PCI_RESET_SLOT forces a slot/hotplug reset. This will not work on platforms
+ * without hotplug capability. This option should only be used for advanced
+ * use-cases where driver developer absolutely knows that the device will never
+ * be used on non-hotplug environments.
+ * Not recommended for scalability. Please refer to PCI_RESET_LINK and let the
+ * PCI core do the hotplug detection.
+ *
+ * PCI_RESET_BUS performs a secondary bus reset on the link and causes a link
+ * recovery. Using this option directly and bypassing hotplug driver may
+ * cause a deadlock if platform supports hotplug. Please refer to
+ * PCI_RESET_LINK and let the PCI core do the hotplug detection.
+ */
+#define PCI_RESET_DEV_SPECIFIC	(1 << 0)
+#define PCI_RESET_FLR		(1 << 1)
+#define PCI_RESET_PM		(1 << 2)
+#define PCI_RESET_SLOT		(1 << 3)
+#define PCI_RESET_BUS		(1 << 4)
+
+#define PCI_RESET_ANY		(~0)
+#define PCI_RESET_FUNC		(PCI_RESET_DEV_SPECIFIC | \
+				 PCI_RESET_FLR | PCI_RESET_PM)
+#define PCI_RESET_LINK		(PCI_RESET_SLOT | PCI_RESET_BUS)
+
 /* These external functions are only available when PCI support is enabled */
 #ifdef CONFIG_PCI
 
@@ -1111,7 +1167,7 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
 void pcie_print_link_status(struct pci_dev *dev);
 bool pcie_has_flr(struct pci_dev *dev);
 int pcie_flr(struct pci_dev *dev);
-int __pci_reset_function_locked(struct pci_dev *dev);
+int __pci_reset_function_locked(struct pci_dev *dev, u32 reset_type);
 int pci_reset_function(struct pci_dev *dev);
 int pci_reset_function_locked(struct pci_dev *dev);
 int pci_try_reset_function(struct pci_dev *dev);
-- 
2.19.0


             reply	other threads:[~2018-10-19  2:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-19  2:11 Sinan Kaya [this message]
2018-10-19  2:11 ` [PATCH v6 2/7] PCI: Expose reset_type to users of pci_reset_function() Sinan Kaya
2018-10-19  2:11 ` [PATCH v6 3/7] PCI: Expose reset_type to users of pci_reset_function_locked() Sinan Kaya
2018-10-19 20:20   ` Bjorn Helgaas
2018-10-19 22:18     ` Sinan Kaya
2018-10-19  2:11 ` [PATCH v6 4/7] PCI: Expose reset type to users of pci_try_reset_function() Sinan Kaya
2018-10-19 20:14   ` Bjorn Helgaas
2018-10-19 20:21     ` Brian Norris
2018-10-19  2:11 ` [PATCH v6 5/7] PCI: Expose reset type to users of pci_probe_reset_function() Sinan Kaya
2018-10-19  2:11 ` [PATCH v6 6/7] PCI: Expose reset type to users of pci_reset_bus() Sinan Kaya
2018-10-19  2:11 ` [PATCH v6 7/7] IB/hfi1,PCI: switch to __pci_function_locked() for reset request Sinan Kaya
2018-10-19 13:10   ` Doug Ledford
2018-10-20  2:09 ` [PATCH v6 1/7] PCI: Expose reset_type to users of __pci_reset_function_locked() Bjorn Helgaas
2018-10-20  2:58   ` Sinan Kaya
2018-10-20 15:03     ` Bjorn Helgaas
2018-10-20 16:21       ` Sinan Kaya
2018-11-08 20:31       ` Alex Williamson
2018-11-08 21:13         ` Bjorn Helgaas

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=20181019021132.14743-1-okaya@kernel.org \
    --to=okaya@kernel.org \
    --cc=baijiaju1990@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=davem@davemloft.net \
    --cc=derek.chickles@caviumnetworks.com \
    --cc=felix.manlunas@caviumnetworks.com \
    --cc=jgross@suse.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=raghu.vatsavayi@caviumnetworks.com \
    --cc=satananda.burla@caviumnetworks.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.