Re: [PATCH v7 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

Re: [PATCH v7 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

[Guilherme G. Piccoli]

Hi Kuppuswamy Sathyanarayanan (and all involved here), thanks for the
patch! I'd like to ask what is the status of this patchset - I just
"parachuted" in the issue, and by tracking the linux-pci ML, I found
this V7 (and all previous versions since V2). Also, noticed that Jay's
email might have gotten lost in translation (he's not CCed in latest
versions of the patchset).

I was able to find even another interesting thread that might be
related, Ethan's patchset. So, if any of the developers can clarify the
current status of this patchset or if the functionality hereby proposed
ended-up being implemented in another patch, I appreciate a lot.

Thanks in advance! Below, some references to lore archives.
Cheers,


Guilherme


References:

This V7 link:
https://lore.kernel.org/linux-pci/546d346644654915877365b19ea534378db0894d.1602788209.git.sathyanarayanan.kuppuswamy@linux.intel.com/

V6:
https://lore.kernel.org/linux-pci/546d346644654915877365b19ea534378db0894d.1602663397.git.sathyanarayanan.kuppuswamy@linux.intel.com/#t

V5:
https://lore.kernel.org/linux-pci/162495c76c391de6e021919e2b69c5cd2dbbc22a.1602632140.git.sathyanarayanan.kuppuswamy@linux.intel.com/

V4:
https://lore.kernel.org/linux-pci/5c5bca0bdb958e456176fe6ede10ba8f838fbafc.1602263264.git.sathyanarayanan.kuppuswamy@linux.intel.com/

V3:
https://lore.kernel.org/linux-pci/cbba08a5e9ca62778c8937f44eda2192a2045da7.1595617529.git.sathyanarayanan.kuppuswamy@linux.intel.com/

V2:
https://lore.kernel.org/linux-pci/ce417fbf81a8a46a89535f44b9224ee9fbb55a29.1591307288.git.sathyanarayanan.kuppuswamy@linux.intel.com/#t

Ethan's related(?) patchset, V8 :
https://lore.kernel.org/linux-pci/20201007113158.48933-1-haifeng.zhao@intel.com/#t

Re: [PATCH v7 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

[Kuppuswamy, Sathyanarayanan]

Hi Guilherme,

On 11/24/20 10:45 AM, Guilherme G. Piccoli wrote:
> Hi Kuppuswamy Sathyanarayanan (and all involved here), thanks for the
> patch! I'd like to ask what is the status of this patchset - I just
> "parachuted" in the issue, and by tracking the linux-pci ML, I found
> this V7 (and all previous versions since V2). Also, noticed that Jay's
> email might have gotten lost in translation (he's not CCed in latest
> versions of the patchset).
> 
> I was able to find even another interesting thread that might be
> related, Ethan's patchset. So, if any of the developers can clarify the
> current status of this patchset or if the functionality hereby proposed
> ended-up being implemented in another patch, I appreciate a lot.
Thanks for bringing this up. Its waiting for Bjorn's comments/approval.

Bjorn, any comments ? Some of our customers also looking for this issue
fix. Please let me know.
> 
> Thanks in advance! Below, some references to lore archives.
> Cheers,
> 
> 

> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v7 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

[Ethan Zhao]

On Wed, Nov 25, 2020 at 2:47 AM Guilherme G. Piccoli
<gpiccoli@canonical.com> wrote:
>
> Hi Kuppuswamy Sathyanarayanan (and all involved here), thanks for the
> patch! I'd like to ask what is the status of this patchset - I just
> "parachuted" in the issue, and by tracking the linux-pci ML, I found
> this V7 (and all previous versions since V2). Also, noticed that Jay's
> email might have gotten lost in translation (he's not CCed in latest
> versions of the patchset).
>
> I was able to find even another interesting thread that might be
> related, Ethan's patchset. So, if any of the developers can clarify the

Thank you for asking.

> current status of this patchset or if the functionality hereby proposed
> ended-up being implemented in another patch, I appreciate a lot.
>
> Thanks in advance! Below, some references to lore archives.
> Cheers,
>
>
> Guilherme
>
>
> References:
>
> This V7 link:
> https://lore.kernel.org/linux-pci/546d346644654915877365b19ea534378db0894d.1602788209.git.sathyanarayanan.kuppuswamy@linux.intel.com/
>
> V6:
> https://lore.kernel.org/linux-pci/546d346644654915877365b19ea534378db0894d.1602663397.git.sathyanarayanan.kuppuswamy@linux.intel.com/#t
>
> V5:
> https://lore.kernel.org/linux-pci/162495c76c391de6e021919e2b69c5cd2dbbc22a.1602632140.git.sathyanarayanan.kuppuswamy@linux.intel.com/
>
> V4:
> https://lore.kernel.org/linux-pci/5c5bca0bdb958e456176fe6ede10ba8f838fbafc.1602263264.git.sathyanarayanan.kuppuswamy@linux.intel.com/
>
> V3:
> https://lore.kernel.org/linux-pci/cbba08a5e9ca62778c8937f44eda2192a2045da7.1595617529.git.sathyanarayanan.kuppuswamy@linux.intel.com/
>
> V2:
> https://lore.kernel.org/linux-pci/ce417fbf81a8a46a89535f44b9224ee9fbb55a29.1591307288.git.sathyanarayanan.kuppuswamy@linux.intel.com/#t
>
> Ethan's related(?) patchset, V8 :
> https://lore.kernel.org/linux-pci/20201007113158.48933-1-haifeng.zhao@intel.com/#t

This patchset I think need more discussion and maintainer's (Bjorn's)
suggestion or advice.
You are welcome to try it and give me the feedback.

Thanks,
Ethan

>

[PATCH v7 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

[Kuppuswamy Sathyanarayanan]

Currently if report_error_detected() or report_mmio_enabled()
functions requests PCI_ERS_RESULT_NEED_RESET, current
pcie_do_recovery() implementation does not do the requested
explicit device reset, but instead just calls the
report_slot_reset() on all affected devices. Notifying about the
reset via report_slot_reset() without doing the actual device
reset is incorrect. So call pci_bus_reset() before triggering
->slot_reset() callback.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Sinan Kaya <okaya@kernel.org>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
---
 Changes since v6:
  * None.

 Changes since v5:
  * Added Ashok's Reviewed-by tag.

 Changes since v4:
  * Added check for pci_reset_bus() return value.

 drivers/pci/pcie/err.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index c543f419d8f9..315a4d559c4c 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -152,6 +152,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 {
 	pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
 	struct pci_bus *bus;
+	int ret;
 
 	/*
 	 * Error recovery runs on all subordinates of the first downstream port.
@@ -181,11 +182,12 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 	}
 
 	if (status == PCI_ERS_RESULT_NEED_RESET) {
-		/*
-		 * TODO: Should call platform-specific
-		 * functions to reset slot before calling
-		 * drivers' slot_reset callbacks?
-		 */
+		ret = pci_reset_bus(dev);
+		if (ret < 0) {
+			pci_err(dev, "Failed to reset %d\n", ret);
+			status = PCI_ERS_RESULT_DISCONNECT;
+			goto failed;
+		}
 		status = PCI_ERS_RESULT_RECOVERED;
 		pci_dbg(dev, "broadcast slot_reset message\n");
 		pci_walk_bus(bus, report_slot_reset, &status);
-- 
2.17.1

[PATCH v7 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

[Kuppuswamy Sathyanarayanan]

Currently if report_error_detected() or report_mmio_enabled()
functions requests PCI_ERS_RESULT_NEED_RESET, current
pcie_do_recovery() implementation does not do the requested
explicit device reset, but instead just calls the
report_slot_reset() on all affected devices. Notifying about the
reset via report_slot_reset() without doing the actual device
reset is incorrect. So call pci_bus_reset() before triggering
->slot_reset() callback.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Sinan Kaya <okaya@kernel.org>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
---
 Changes since v6:
  * None.

 Changes since v5:
  * Added Ashok's Reviewed-by tag.

 Changes since v4:
  * Added check for pci_reset_bus() return value.

 drivers/pci/pcie/err.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index c543f419d8f9..315a4d559c4c 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -152,6 +152,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 {
 	pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
 	struct pci_bus *bus;
+	int ret;
 
 	/*
 	 * Error recovery runs on all subordinates of the first downstream port.
@@ -181,11 +182,12 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 	}
 
 	if (status == PCI_ERS_RESULT_NEED_RESET) {
-		/*
-		 * TODO: Should call platform-specific
-		 * functions to reset slot before calling
-		 * drivers' slot_reset callbacks?
-		 */
+		ret = pci_reset_bus(dev);
+		if (ret < 0) {
+			pci_err(dev, "Failed to reset %d\n", ret);
+			status = PCI_ERS_RESULT_DISCONNECT;
+			goto failed;
+		}
 		status = PCI_ERS_RESULT_RECOVERED;
 		pci_dbg(dev, "broadcast slot_reset message\n");
 		pci_walk_bus(bus, report_slot_reset, &status);
-- 
2.17.1