[PATCH v2 0/6] PCI: Add reset type parameter to PCI reset functions

[PATCH v2 0/6] PCI: Add reset type parameter to PCI reset functions

[Sinan Kaya]

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(-)

-- 
2.18.0

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

[Sinan Kaya]

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 <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                           | 13 +++-
 4 files changed, 55 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..8912c8ba3b2b 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 = 0;
 
 	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..d2e0a8c8177b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.18.0

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

[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 <okaya@kernel.org>
---
 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(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index bfbca7ec54ce..18312969f1b3 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -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;
 
diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index c7cd3675bcd1..cc78ef28ee38 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -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);
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
index d344e9d43832..bb737725f175 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
@@ -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;
 	}
 
diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
index dfad93fca0a6..7f95e17b8a48 100644
--- a/drivers/net/ethernet/sfc/mcdi.c
+++ b/drivers/net/ethernet/sfc/mcdi.c
@@ -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 */
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ecfe13157c0..9569664ec4b2 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -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;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8912c8ba3b2b..2b1e2b4893f3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d2e0a8c8177b..1a1a76130e91 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.18.0

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

[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 <okaya@kernel.org>
---
 drivers/pci/pci.c   | 4 ++--
 include/linux/pci.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 2b1e2b4893f3..3046c202bfea 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1a1a76130e91..09aa99dd1c1b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.18.0

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

[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 <okaya@kernel.org>
---
 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(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3046c202bfea..c216ab56dbcc 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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);
 
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index cddb453a1ba5..fe7ada997c51 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -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;
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 115a36f6f403..0d66bac66211 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -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;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 09aa99dd1c1b..4fdddcb85066 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.18.0

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

[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 <okaya@kernel.org>
---
 drivers/pci/pci.c   | 51 ++++++++++++++++++++++++++++++---------------
 drivers/pci/pci.h   |  2 +-
 drivers/pci/probe.c |  2 +-
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c216ab56dbcc..32b367937cc1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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;
 }
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6e0d1528d471..0444bfa51b52 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -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);
 
 /**
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ec784009a36b..cbce5420431b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -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;
 }
 
-- 
2.18.0

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

[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 <okaya@kernel.org>
---
 drivers/pci/pci.c           | 18 +++++++++++++++---
 drivers/vfio/pci/vfio_pci.c |  4 ++--
 include/linux/pci.h         |  2 +-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 32b367937cc1..7192cd460db4 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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);
 
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index fe7ada997c51..754f08286bb1 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -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++) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4fdddcb85066..85f48e156753 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.18.0

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

[Srinivas Pandruvada]

On Fri, 2018-09-14 at 21:18 +0000, Sinan Kaya wrote:
> 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 <okaya@kernel.org>
> ---
>  drivers/hid/intel-ish-hid/ipc/ipc.c             | 2 +-
For intel-ish-hid, change looks fine.

Thanks,
Srinivas

>  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(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-
> ish-hid/ipc/ipc.c
> index bfbca7ec54ce..18312969f1b3 100644
> --- a/drivers/hid/intel-ish-hid/ipc/ipc.c
> +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
> @@ -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;
>  
> diff --git a/drivers/misc/genwqe/card_base.c
> b/drivers/misc/genwqe/card_base.c
> index c7cd3675bcd1..cc78ef28ee38 100644
> --- a/drivers/misc/genwqe/card_base.c
> +++ b/drivers/misc/genwqe/card_base.c
> @@ -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);
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
> index d344e9d43832..bb737725f175 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
> @@ -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;
>  	}
>  
> diff --git a/drivers/net/ethernet/sfc/mcdi.c
> b/drivers/net/ethernet/sfc/mcdi.c
> index dfad93fca0a6..7f95e17b8a48 100644
> --- a/drivers/net/ethernet/sfc/mcdi.c
> +++ b/drivers/net/ethernet/sfc/mcdi.c
> @@ -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 */
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9ecfe13157c0..9569664ec4b2 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -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;
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8912c8ba3b2b..2b1e2b4893f3 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -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);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d2e0a8c8177b..1a1a76130e91 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -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);

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

[Alex Williamson]

On Fri, 14 Sep 2018 21:18:03 +0000
Sinan Kaya <okaya@kernel.org> 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 <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                           | 13 +++-
>  4 files changed, 55 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..8912c8ba3b2b 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 = 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

>  
>  	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..d2e0a8c8177b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -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);

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

[Alex Williamson]

On Fri, 14 Sep 2018 21:18:08 +0000
Sinan Kaya <okaya@kernel.org> wrote:
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index fe7ada997c51..754f08286bb1 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -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);

>  
>  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

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

[Sinan Kaya]

On 9/14/2018 6:33 PM, Alex Williamson wrote:
>> @@ -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);
> 
>>   
>>   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.

Sure I can do that.

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

[Sinan Kaya]

On 9/14/2018 6:27 PM, Alex Williamson wrote:
>> +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.

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

[Sinan Kaya]

On 9/14/2018 5:52 PM, Srinivas Pandruvada wrote:
>> Signed-off-by: Sinan Kaya<okaya@kernel.org>
>> ---
>>   drivers/hid/intel-ish-hid/ipc/ipc.c             | 2 +-
> For intel-ish-hid, change looks fine.

Is this an ACK or reviewed-by?

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

[Srinivas Pandruvada]

On Fri, 2018-09-14 at 19:20 -0400, Sinan Kaya wrote:
> On 9/14/2018 5:52 PM, Srinivas Pandruvada wrote:
> > > Signed-off-by: Sinan Kaya<okaya@kernel.org>
> > > ---
> > >   drivers/hid/intel-ish-hid/ipc/ipc.c             | 2 +-
> > 
> > For intel-ish-hid, change looks fine.
> 
> Is this an ACK or reviewed-by?
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>