Skip to contentSkip to conversation

PCI: Add reset type parameter to PCI reset functions

PATCH Superseded Sinan Kaya 3 participants 14 messages Sep 15, 2018

Subsystems (11)
Network drivers PCI HID Char and misc drivers +7 more subsystems

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Sinan Kaya

author
#0 lore raw

v2 cover PCI: Add reset type parameter to PCI reset functions

We want more control over what kind of reset PCI core can apply. Have the application explicitly requested allowed reset types.

differences from v1:
Add reset type to pci_reset_bus() per feedbeck from Alex.

Sinan Kaya (6):
PCI: Expose reset_type to users of __pci_reset_function_locked()
PCI: Expose reset_type to users of pci_reset_function()
PCI: Expose reset_type to users of pci_reset_function_locked()
PCI: Expose reset type to users of pci_try_reset_function()
PCI: Expose reset type to users of pci_probe_reset_function()
PCI: Expose reset type to users of pci_reset_bus()

 drivers/hid/intel-ish-hid/ipc/ipc.c           |   2 +-
 drivers/misc/genwqe/card_base.c               |   2 +-
 .../net/ethernet/cavium/liquidio/lio_main.c   |   2 +-
 .../net/ethernet/qlogic/qlcnic/qlcnic_ctx.c   |   2 +-
 drivers/net/ethernet/sfc/mcdi.c               |   2 +-
 drivers/pci/pci-sysfs.c                       |   2 +-
 drivers/pci/pci.c                             | 134 ++++++++++++------
 drivers/pci/pci.h                             |   2 +-
 drivers/pci/probe.c                           |   2 +-
 drivers/vfio/pci/vfio_pci.c                   |  11 +-
 drivers/vfio/pci/vfio_pci_config.c            |   4 +-
 drivers/xen/xen-pciback/pci_stub.c            |   6 +-
 include/linux/pci.h                           |  21 ++-
 13 files changed, 126 insertions(+), 66 deletions(-)
Signature · 1 line
2.18.0
author
#1 lore raw

PATCH v2 1/6 PCI: Expose reset_type to users of __pci_reset_function_locked()

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.

Suggested-by Alex Williamson <[email protected]>
Signed-off-by Sinan Kaya <[email protected]>
---
 .../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                           | 13 +++-
 4 files changed, 55 insertions(+), 25 deletions(-)
drivers/net/ethernet/cavium/liquidio/lio_main.c +1 -1
index 6fb13fa73b27..0ff76722734d 100644
@@ -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",
drivers/pci/pci.c +39 -20

99 lines not on this page · raw message

drivers/xen/xen-pciback/pci_stub.c +3 -3
index 59661db144e5..6dfb805bcb19 100644
@@ -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
include/linux/pci.h +12 -1
index 6925828f9f25..d2e0a8c8177b 100644
@@ -849,6 +849,17 @@ enum {
 	PCI_SCAN_ALL_PCIE_DEVS	= 0x00000040,	/* Scan all, not just dev 0 */
 };
 
+#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 +1122,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);
Signature · 1 line
2.18.0
#2 lore raw

Re: [PATCH v2 1/6] PCI: Expose reset_type to users of __pci_reset_function_locked()

Quoted 45 lines · On Fri, 14 Sep 2018 21:18:03 +0000 Sinan Kaya <[email protected]> wrote:

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.

Suggested-by Alex Williamson <[email protected]>
Signed-off-by Sinan Kaya <[email protected]>
---
 .../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                           | 13 +++-
 4 files changed, 55 insertions(+), 25 deletions(-)
drivers/net/ethernet/cavium/liquidio/lio_main.c +1 -1
index 6fb13fa73b27..0ff76722734d 100644
@@ -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",
drivers/pci/pci.c +1 -0
index 1835f3a7aa8d..8912c8ba3b2b 100644
@@ -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

drivers/pci/pci.c static void pci_dev_restore(struct pci_dev *dev) · line 4694

Quoted 8 lines
drivers/pci/pci.c +2 -2
@@ -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 = 0;

What's the proper return value when a user passes zero for reset_type? It seems to currently be: user provided no options, therefore we tried none, therefore success. I'm torn whether that's entirely bogus to the point of that we should WARN_ON (ie. caller bug), or simply return -EINVAL, which the caller can ignore, or if there's some value to allow the behavior here. Thanks,

Alex

Quoted 150 lines
drivers/pci/pci.c +36 -18
 	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);
drivers/xen/xen-pciback/pci_stub.c +3 -3
index 59661db144e5..6dfb805bcb19 100644
@@ -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
include/linux/pci.h +12 -1
index 6925828f9f25..d2e0a8c8177b 100644
@@ -849,6 +849,17 @@ enum {
 	PCI_SCAN_ALL_PCIE_DEVS	= 0x00000040,	/* Scan all, not just dev 0 */
 };
 
+#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 +1122,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);
author
#3 lore raw

Re: [PATCH v2 1/6] PCI: Expose reset_type to users of __pci_reset_function_locked()

Quoted 10 lines · Alex Williamson

drivers/pci/pci.c static void pci_dev_restore(struct pci_dev *dev) · line 4694

Quoted 4 lines
drivers/pci/pci.c +2 -1
+int __pci_reset_function_locked(struct pci_dev *dev, u32 reset_type)
  {
-	int rc;
+	int rc = 0;

What's the proper return value when a user passes zero for reset_type? It seems to currently be: user provided no options, therefore we tried none, therefore success. I'm torn whether that's entirely bogus to the point of that we should WARN_ON (ie. caller bug), or simply return -EINVAL, which the caller can ignore, or if there's some value to allow the behavior here. Thanks,

Yeah, even I'm inconsistent in this series. I return -EINVAL on pci_reset_bus() as an example. Maybe, I'll just return -EINVAL here too.

WARN_ON would be too much IMO.

author
#4 lore raw

PATCH v2 2/6 PCI: Expose reset_type to users of pci_reset_function()

In reply to Sinan Kaya

Looking to have more control between the users of the API vs. what the API can do internally. The new reset_type tells the PCI core about the bounds of the request.

Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/hid/intel-ish-hid/ipc/ipc.c             | 2 +-
 drivers/misc/genwqe/card_base.c                 | 2 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c | 2 +-
 drivers/net/ethernet/sfc/mcdi.c                 | 2 +-
 drivers/pci/pci-sysfs.c                         | 2 +-
 drivers/pci/pci.c                               | 4 ++--
 include/linux/pci.h                             | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)
drivers/hid/intel-ish-hid/ipc/ipc.c +1 -1
index bfbca7ec54ce..18312969f1b3 100644
@@ -754,7 +754,7 @@ static int _ish_hw_reset(struct ishtp_device *dev)
 	if (!pdev)
 		return	-ENODEV;
 
-	rv = pci_reset_function(pdev);
+	rv = pci_reset_function(pdev, PCI_RESET_ANY);
 	if (!rv)
 		dev->dev_state = ISHTP_DEV_RESETTING;
drivers/misc/genwqe/card_base.c +1 -1
index c7cd3675bcd1..cc78ef28ee38 100644
@@ -201,7 +201,7 @@ static int genwqe_bus_reset(struct genwqe_dev *cd)
 	 * restored by the pci_reset_function().
 	 */
 	dev_dbg(&pci_dev->dev, "[%s] pci_reset function ...\n", __func__);
-	rc = pci_reset_function(pci_dev);
+	rc = pci_reset_function(pci_dev, PCI_RESET_ANY);
 	if (rc) {
 		dev_err(&pci_dev->dev,
 			"[%s] err: failed reset func (rc %d)\n", __func__, rc);
drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c +1 -1
index d344e9d43832..bb737725f175 100644
@@ -629,7 +629,7 @@ int qlcnic_fw_create_ctx(struct qlcnic_adapter *dev)
 	int i, err, ring;
 
 	if (dev->flags & QLCNIC_NEED_FLR) {
-		pci_reset_function(dev->pdev);
+		pci_reset_function(dev->pdev, PCI_RESET_ANY);
 		dev->flags &= ~QLCNIC_NEED_FLR;
 	}
drivers/net/ethernet/sfc/mcdi.c +1 -1
index dfad93fca0a6..7f95e17b8a48 100644
@@ -1862,7 +1862,7 @@ int efx_mcdi_reset(struct efx_nic *efx, enum reset_type method)
 
 	/* If MCDI is down, we can't handle_assertion */
 	if (method == RESET_TYPE_MCDI_TIMEOUT) {
-		rc = pci_reset_function(efx->pci_dev);
+		rc = pci_reset_function(efx->pci_dev, PCI_RESET_ANY);
 		if (rc)
 			return rc;
 		/* Re-enable polled MCDI completion */
drivers/pci/pci-sysfs.c +1 -1
index 9ecfe13157c0..9569664ec4b2 100644
@@ -1449,7 +1449,7 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 
 	pm_runtime_get_sync(dev);
-	result = pci_reset_function(pdev);
+	result = pci_reset_function(pdev, PCI_RESET_ANY);
 	pm_runtime_put(dev);
 	if (result < 0)
 		return result;
drivers/pci/pci.c +2 -2
index 8912c8ba3b2b..2b1e2b4893f3 100644
@@ -4793,7 +4793,7 @@ int pci_probe_reset_function(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(struct pci_dev *dev)
+int pci_reset_function(struct pci_dev *dev, u32 reset_type)
 {
 	int rc;
 

@@ -4803,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, PCI_RESET_ANY);
+	rc = __pci_reset_function_locked(dev, reset_type);
 
 	pci_dev_restore(dev);
 	pci_dev_unlock(dev);
include/linux/pci.h +1 -1
index d2e0a8c8177b..1a1a76130e91 100644
@@ -1123,7 +1123,7 @@ 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, u32 reset_type);
-int pci_reset_function(struct pci_dev *dev);
+int pci_reset_function(struct pci_dev *dev, u32 reset_type);
 int pci_reset_function_locked(struct pci_dev *dev);
 int pci_try_reset_function(struct pci_dev *dev);
 int pci_probe_reset_slot(struct pci_slot *slot);
Signature · 1 line
2.18.0
maintainer
#5 lore raw

Re: [PATCH v2 2/6] PCI: Expose reset_type to users of pci_reset_function()

Quoted 9 lines · Sinan Kaya

Looking to have more control between the users of the API vs. what the API
can do internally. The new reset_type tells the PCI core about the bounds
of the request.

Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/hid/intel-ish-hid/ipc/ipc.c             | 2 +-

For intel-ish-hid, change looks fine.

Thanks,
Srinivas

Quoted 123 lines
 drivers/misc/genwqe/card_base.c                 | 2 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c | 2 +-
 drivers/net/ethernet/sfc/mcdi.c                 | 2 +-
 drivers/pci/pci-sysfs.c                         | 2 +-
 drivers/pci/pci.c                               | 4 ++--
 include/linux/pci.h                             | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)
drivers/hid/intel- renamed from drivers/hid/intel-ish-hid/ipc/ipc.c +0 -0

ish-hid/ipc/ipc.c
index bfbca7ec54ce..18312969f1b3 100644

drivers/hid/intel-ish-hid/ipc/ipc.c +1 -1
@@ -754,7 +754,7 @@ static int _ish_hw_reset(struct ishtp_device
 *dev)
 	if (!pdev)
 		return	-ENODEV;
 
-	rv = pci_reset_function(pdev);
+	rv = pci_reset_function(pdev, PCI_RESET_ANY);
 	if (!rv)
 		dev->dev_state = ISHTP_DEV_RESETTING;
drivers/misc/genwqe/card_base.c +1 -1
index c7cd3675bcd1..cc78ef28ee38 100644
@@ -201,7 +201,7 @@ static int genwqe_bus_reset(struct genwqe_dev
 *cd)
 	 * restored by the pci_reset_function().
 	 */
 	dev_dbg(&pci_dev->dev, "[%s] pci_reset function ...\n",
 __func__);
-	rc = pci_reset_function(pci_dev);
+	rc = pci_reset_function(pci_dev, PCI_RESET_ANY);
 	if (rc) {
drivers/misc/genwqe/card_base.c +0 -0
 		dev_err(&pci_dev->dev,
 			"[%s] err: failed reset func (rc %d)\n",

__func__, rc);

drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c +1 -1
index d344e9d43832..bb737725f175 100644
@@ -629,7 +629,7 @@ int qlcnic_fw_create_ctx(struct qlcnic_adapter
 *dev)
 	int i, err, ring;
 
 	if (dev->flags & QLCNIC_NEED_FLR) {
-		pci_reset_function(dev->pdev);
+		pci_reset_function(dev->pdev, PCI_RESET_ANY);
 		dev->flags &= ~QLCNIC_NEED_FLR;
 	}
drivers/net/ethernet/sfc/mcdi.c +1 -1
index dfad93fca0a6..7f95e17b8a48 100644
@@ -1862,7 +1862,7 @@ int efx_mcdi_reset(struct efx_nic *efx, enum
 reset_type method)
 
 	/* If MCDI is down, we can't handle_assertion */
 	if (method == RESET_TYPE_MCDI_TIMEOUT) {
-		rc = pci_reset_function(efx->pci_dev);
+		rc = pci_reset_function(efx->pci_dev, PCI_RESET_ANY);
 		if (rc)
 			return rc;
drivers/net/ethernet/sfc/mcdi.c +0 -0
 		/* Re-enable polled MCDI completion */
drivers/pci/pci-sysfs.c +1 -1
index 9ecfe13157c0..9569664ec4b2 100644
@@ -1449,7 +1449,7 @@ static ssize_t reset_store(struct device *dev,
 struct device_attribute *attr,
 		return -EINVAL;
 
 	pm_runtime_get_sync(dev);
-	result = pci_reset_function(pdev);
+	result = pci_reset_function(pdev, PCI_RESET_ANY);
 	pm_runtime_put(dev);
 	if (result < 0)
drivers/pci/pci-sysfs.c +0 -0
 		return result;
drivers/pci/pci.c +0 -0
index 8912c8ba3b2b..2b1e2b4893f3 100644
@@ -4793,7 +4793,7 @@ int pci_probe_reset_function(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. */

drivers/pci/pci.c +2 -2
-int pci_reset_function(struct pci_dev *dev)
+int pci_reset_function(struct pci_dev *dev, u32 reset_type)
 {
 	int rc;

@@ -4803,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, PCI_RESET_ANY);
+	rc = __pci_reset_function_locked(dev, reset_type);
 
 	pci_dev_restore(dev);
 	pci_dev_unlock(dev);
include/linux/pci.h +1 -1
index d2e0a8c8177b..1a1a76130e91 100644
@@ -1123,7 +1123,7 @@ 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, u32
 reset_type);
-int pci_reset_function(struct pci_dev *dev);
+int pci_reset_function(struct pci_dev *dev, u32 reset_type);
 int pci_reset_function_locked(struct pci_dev *dev);

int pci_try_reset_function(struct pci_dev *dev);
int pci_probe_reset_slot(struct pci_slot *slot);

author
#6 lore raw

Re: [PATCH v2 2/6] PCI: Expose reset_type to users of pci_reset_function()

Quoted 4 lines · Srinivas Pandruvada
Quoted 3 lines
Signed-off-by Sinan Kaya<[email protected]>
---
  drivers/hid/intel-ish-hid/ipc/ipc.c             | 2 +-

For intel-ish-hid, change looks fine.

Is this an ACK or reviewed-by?

author
#8 lore raw

PATCH v2 3/6 PCI: Expose reset_type to users of pci_reset_function_locked()

In reply to Sinan Kaya

Looking to have more control between the users of the API vs. what the API can do internally. The new reset_type tells the PCI core about the bounds of the request.

Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/pci/pci.c   | 4 ++--
 include/linux/pci.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/pci/pci.c +2 -2
index 2b1e2b4893f3..3046c202bfea 100644
@@ -4829,7 +4829,7 @@ EXPORT_SYMBOL_GPL(pci_reset_function);
  * 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;
 

@@ -4838,7 +4838,7 @@ int pci_reset_function_locked(struct pci_dev *dev)
 
 	pci_dev_save_and_disable(dev);
 
-	rc = __pci_reset_function_locked(dev, PCI_RESET_ANY);
+	rc = __pci_reset_function_locked(dev, reset_type);
 
 	pci_dev_restore(dev);
include/linux/pci.h +1 -1
index 1a1a76130e91..09aa99dd1c1b 100644
@@ -1124,7 +1124,7 @@ bool pcie_has_flr(struct pci_dev *dev);
 int pcie_flr(struct pci_dev *dev);
 int __pci_reset_function_locked(struct pci_dev *dev, u32 reset_type);
 int pci_reset_function(struct pci_dev *dev, u32 reset_type);
-int pci_reset_function_locked(struct pci_dev *dev);
+int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type);
 int pci_try_reset_function(struct pci_dev *dev);
 int pci_probe_reset_slot(struct pci_slot *slot);
 int pci_probe_reset_bus(struct pci_bus *bus);
Signature · 1 line
2.18.0
author
#9 lore raw

PATCH v2 4/6 PCI: Expose reset type to users of pci_try_reset_function()

In reply to Sinan Kaya

Looking to have more control between the users of the API vs. what the API can do internally. The new reset_type tells the PCI core about the bounds of the request.

Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/pci/pci.c                  | 4 ++--
 drivers/vfio/pci/vfio_pci.c        | 7 ++++---
 drivers/vfio/pci/vfio_pci_config.c | 4 ++--
 include/linux/pci.h                | 2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)
drivers/pci/pci.c +2 -2
index 3046c202bfea..c216ab56dbcc 100644
@@ -4852,7 +4852,7 @@ EXPORT_SYMBOL_GPL(pci_reset_function_locked);
  *
  * Same as above, except return -EAGAIN if unable to lock device.
  */
-int pci_try_reset_function(struct pci_dev *dev)
+int pci_try_reset_function(struct pci_dev *dev, u32 reset_type)
 {
 	int rc;
 

@@ -4863,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, PCI_RESET_ANY);
+	rc = __pci_reset_function_locked(dev, reset_type);
 	pci_dev_restore(dev);
 	pci_dev_unlock(dev);
drivers/vfio/pci/vfio_pci.c +4 -3
index cddb453a1ba5..fe7ada997c51 100644
@@ -228,7 +228,7 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
 		return ret;
 
 	/* If reset fails because of the device lock, fail this path entirely */
-	ret = pci_try_reset_function(pdev);
+	ret = pci_try_reset_function(pdev, PCI_RESET_ANY);
 	if (ret == -EAGAIN) {
 		pci_disable_device(pdev);
 		return ret;

@@ -376,7 +376,7 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
 	 * Try to reset the device.  The success of this is dependent on
 	 * being able to lock the device, which is not always possible.
 	 */
-	if (vdev->reset_works && !pci_try_reset_function(pdev))
+	if (vdev->reset_works && !pci_try_reset_function(pdev, PCI_RESET_ANY))
 		vdev->needs_reset = false;
 
 	pci_restore_state(pdev);

@@ -844,7 +844,8 @@ static long vfio_pci_ioctl(void *device_data,
 
 	} else if (cmd == VFIO_DEVICE_RESET) {
 		return vdev->reset_works ?
-			pci_try_reset_function(vdev->pdev) : -EINVAL;
+			pci_try_reset_function(vdev->pdev, PCI_RESET_ANY) :
+			 -EINVAL;
 
 	} else if (cmd == VFIO_DEVICE_GET_PCI_HOT_RESET_INFO) {
 		struct vfio_pci_hot_reset_info hdr;
drivers/vfio/pci/vfio_pci_config.c +2 -2
index 115a36f6f403..0d66bac66211 100644
@@ -831,7 +831,7 @@ static int vfio_exp_config_write(struct vfio_pci_device *vdev, int pos,
 						 &cap);
 
 		if (!ret && (cap & PCI_EXP_DEVCAP_FLR))
-			pci_try_reset_function(vdev->pdev);
+			pci_try_reset_function(vdev->pdev, PCI_RESET_ANY);
 	}
 
 	/*

@@ -910,7 +910,7 @@ static int vfio_af_config_write(struct vfio_pci_device *vdev, int pos,
 						&cap);
 
 		if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP))
-			pci_try_reset_function(vdev->pdev);
+			pci_try_reset_function(vdev->pdev, PCI_RESET_ANY);
 	}
 
 	return count;
include/linux/pci.h +1 -1
index 09aa99dd1c1b..4fdddcb85066 100644
@@ -1125,7 +1125,7 @@ int pcie_flr(struct pci_dev *dev);
 int __pci_reset_function_locked(struct pci_dev *dev, u32 reset_type);
 int pci_reset_function(struct pci_dev *dev, u32 reset_type);
 int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type);
-int pci_try_reset_function(struct pci_dev *dev);
+int pci_try_reset_function(struct pci_dev *dev, u32 reset_type);
 int pci_probe_reset_slot(struct pci_slot *slot);
 int pci_probe_reset_bus(struct pci_bus *bus);
 int pci_reset_bus(struct pci_dev *dev);
Signature · 1 line
2.18.0
author
#10 lore raw

PATCH v2 5/6 PCI: Expose reset type to users of pci_probe_reset_function()

In reply to Sinan Kaya

Looking to have more control between the users of the API vs. what the API can do internally. The new reset_type tells the PCI core about the bounds of the request.

Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/pci/pci.c   | 51 ++++++++++++++++++++++++++++++---------------
 drivers/pci/pci.h   |  2 +-
 drivers/pci/probe.c |  2 +-
 3 files changed, 36 insertions(+), 19 deletions(-)
drivers/pci/pci.c +34 -17
index c216ab56dbcc..32b367937cc1 100644
@@ -4753,28 +4753,45 @@ EXPORT_SYMBOL_GPL(__pci_reset_function_locked);
  * Returns 0 if the device function can be reset or negative if the
  * device doesn't support resetting a single function.
  */
-int pci_probe_reset_function(struct pci_dev *dev)
+int pci_probe_reset_function(struct pci_dev *dev, u32 reset_type)
 {
-	int rc;
+	int rc = 0;
 
 	might_sleep();
 
-	rc = pci_dev_specific_reset(dev, 1);
-	if (rc != -ENOTTY)
-		return rc;
-	if (pcie_has_flr(dev))
-		return 0;
-	rc = pci_af_flr(dev, 1);
-	if (rc != -ENOTTY)
-		return rc;
-	rc = pci_pm_reset(dev, 1);
-	if (rc != -ENOTTY)
-		return rc;
-	rc = pci_dev_reset_slot_function(dev, 1);
-	if (rc != -ENOTTY)
-		return rc;
+	if (reset_type & PCI_RESET_DEV_SPECIFIC) {
+		rc = pci_dev_specific_reset(dev, 1);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	if (reset_type & PCI_RESET_FLR) {
+		if (pcie_has_flr(dev))
+			return 0;
+		rc = pci_af_flr(dev, 1);
+		if (rc != -ENOTTY)
+			return rc;
+	}
 
-	return pci_parent_bus_reset(dev, 1);
+	if (reset_type & PCI_RESET_PM) {
+		rc = pci_pm_reset(dev, 1);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	if (reset_type & PCI_RESET_SLOT) {
+		rc = pci_dev_reset_slot_function(dev, 1);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	if (reset_type & PCI_RESET_BUS) {
+		rc = pci_parent_bus_reset(dev, 1);
+		if (rc != -ENOTTY)
+			return rc;
+	}
+
+	return rc;
 }
 
 /**
drivers/pci/pci.h +1 -1
index 6e0d1528d471..0444bfa51b52 100644
@@ -33,7 +33,7 @@ enum pci_mmap_api {
 int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vmai,
 		  enum pci_mmap_api mmap_api);
 
-int pci_probe_reset_function(struct pci_dev *dev);
+int pci_probe_reset_function(struct pci_dev *dev, u32 reset_type);
 int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
 
 /**
drivers/pci/probe.c +1 -1
index ec784009a36b..cbce5420431b 100644
@@ -2332,7 +2332,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
 
 	pcie_report_downtraining(dev);
 
-	if (pci_probe_reset_function(dev) == 0)
+	if (pci_probe_reset_function(dev, PCI_RESET_ANY) == 0)
 		dev->reset_fn = 1;
 }
Signature · 1 line
2.18.0
author
#11 lore raw

PATCH v2 6/6 PCI: Expose reset type to users of pci_reset_bus()

In reply to Sinan Kaya

Looking to have more control between the users of the API vs. what the API can do internally. The new reset_type tells the PCI core about the bounds of the request.

Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/pci/pci.c           | 18 +++++++++++++++---
 drivers/vfio/pci/vfio_pci.c |  4 ++--
 include/linux/pci.h         |  2 +-
 3 files changed, 18 insertions(+), 6 deletions(-)
drivers/pci/pci.c +15 -3
index 32b367937cc1..7192cd460db4 100644
@@ -5232,13 +5232,25 @@ static int __pci_reset_bus(struct pci_bus *bus)
 /**
  * pci_reset_bus - Try to reset a PCI bus
  * @pdev: top level PCI device to reset via slot/bus
+ * @reset_type: resets to try
  *
  * Same as above except return -EAGAIN if the bus cannot be locked
  */
-int pci_reset_bus(struct pci_dev *pdev)
+int pci_reset_bus(struct pci_dev *pdev, u32 reset_type)
 {
-	return (!pci_probe_reset_slot(pdev->slot)) ?
-	    __pci_reset_slot(pdev->slot) : __pci_reset_bus(pdev->bus);
+	if (reset_type & PCI_RESET_LINK) {
+		return (!pci_probe_reset_slot(pdev->slot)) ?
+				__pci_reset_slot(pdev->slot) :
+				__pci_reset_bus(pdev->bus);
+	}
+
+	if (reset_type & PCI_RESET_BUS)
+		return __pci_reset_bus(pdev->bus);
+
+	if (reset_type & PCI_RESET_SLOT)
+		return __pci_reset_slot(pdev->slot);
+
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(pci_reset_bus);
drivers/vfio/pci/vfio_pci.c +2 -2
index fe7ada997c51..754f08286bb1 100644
@@ -1015,7 +1015,7 @@ static long vfio_pci_ioctl(void *device_data,
 						    &info, slot);
 		if (!ret)
 			/* User has access, do the reset */
-			ret = pci_reset_bus(vdev->pdev);
+			ret = pci_reset_bus(vdev->pdev, PCI_RESET_LINK);
 
 hot_reset_release:
 		for (i--; i >= 0; i--)

@@ -1390,7 +1390,7 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev)
 	}
 
 	if (needs_reset)
-		ret = pci_reset_bus(vdev->pdev);
+		ret = pci_reset_bus(vdev->pdev, PCI_RESET_LINK);
 
 put_devs:
 	for (i = 0; i < devs.cur_index; i++) {
include/linux/pci.h +1 -1
index 4fdddcb85066..85f48e156753 100644
@@ -1128,7 +1128,7 @@ int pci_reset_function_locked(struct pci_dev *dev, u32 reset_type);
 int pci_try_reset_function(struct pci_dev *dev, u32 reset_type);
 int pci_probe_reset_slot(struct pci_slot *slot);
 int pci_probe_reset_bus(struct pci_bus *bus);
-int pci_reset_bus(struct pci_dev *dev);
+int pci_reset_bus(struct pci_dev *dev, u32 reset_type);
 void pci_reset_secondary_bus(struct pci_dev *dev);
 void pcibios_reset_secondary_bus(struct pci_dev *dev);
 void pci_update_resource(struct pci_dev *dev, int resno);
Signature · 1 line
2.18.0
#12 lore raw

Re: [PATCH v2 6/6] PCI: Expose reset type to users of pci_reset_bus()

drivers/vfio/pci/vfio_pci.c static long vfio_pci_ioctl(void *device_data, · line 1018

Quoted 10 lines · On Fri, 14 Sep 2018 21:18:08 +0000 Sinan Kaya <[email protected]> wrote:
drivers/vfio/pci/vfio_pci.c +1 -1
index fe7ada997c51..754f08286bb1 100644
@@ -1015,7 +1015,7 @@ static long vfio_pci_ioctl(void *device_data,
 						    &info, slot);
 		if (!ret)
 			/* User has access, do the reset */
-			ret = pci_reset_bus(vdev->pdev);
+			ret = pci_reset_bus(vdev->pdev, PCI_RESET_LINK);

This should have switched back the control we had prior to 811c5cb37df4 with something like:

	ret = pci_reset_bus(vdev->pdev,
			slot ? PCI_RESET_SLOT :	PCI_RESET_BUS);

drivers/vfio/pci/vfio_pci.c static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) · line 1393

Quoted 9 lines
drivers/vfio/pci/vfio_pci.c +1 -1
 hot_reset_release:
 		for (i--; i >= 0; i--)

@@ -1390,7 +1390,7 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev)
 	}
 
 	if (needs_reset)
-		ret = pci_reset_bus(vdev->pdev);
+		ret = pci_reset_bus(vdev->pdev, PCI_RESET_LINK);

Same here. Thanks,

Alex

author
#13 lore raw

Re: [PATCH v2 6/6] PCI: Expose reset type to users of pci_reset_bus()

Quoted 18 lines · Alex Williamson

drivers/vfio/pci/vfio_pci.c static long vfio_pci_ioctl(void *device_data, · line 1018

Quoted 6 lines
drivers/vfio/pci/vfio_pci.c +1 -1
@@ -1015,7 +1015,7 @@ static long vfio_pci_ioctl(void *device_data,
  						    &info, slot);
  		if (!ret)
  			/* User has access, do the reset */
-			ret = pci_reset_bus(vdev->pdev);
+			ret = pci_reset_bus(vdev->pdev, PCI_RESET_LINK);

This should have switched back the control we had prior to 811c5cb37df4 with something like:

	ret = pci_reset_bus(vdev->pdev,
			slot ? PCI_RESET_SLOT :	PCI_RESET_BUS);
Quoted 6 lines
drivers/vfio/pci/vfio_pci.c +0 -0
  hot_reset_release:
  		for (i--; i >= 0; i--)

@@ -1390,7 +1390,7 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev)
  	}
Quoted 4 lines

drivers/vfio/pci/vfio_pci.c static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev) · line 1393

Quoted 3 lines
drivers/vfio/pci/vfio_pci.c +1 -1
  	if (needs_reset)
-		ret = pci_reset_bus(vdev->pdev);
+		ret = pci_reset_bus(vdev->pdev, PCI_RESET_LINK);

Same here.

Sure I can do that.