Patchwork [NET-NEXT] ixgbe: Implement PCIe AER support

login
register
mail settings
Submitter Jeff Kirsher
Date Dec. 2, 2008, 12:41 a.m.
Message ID <20081202004115.12058.64881.stgit@gitlost.lost>
Download mbox | patch
Permalink /patch/11693/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Jeff Kirsher - Dec. 2, 2008, 12:41 a.m.
From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>

This patch implements the PCIe Advanced Error Reporting callbacks in
ixgbe.  The 82598 hardware supports AER, so we enable it.

Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe.h      |    1 +
 drivers/net/ixgbe/ixgbe_main.c |   26 ++++++++++++++++++--------
 2 files changed, 19 insertions(+), 8 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stephen hemminger - Dec. 2, 2008, 1:47 a.m.
On Mon, 01 Dec 2008 16:41:15 -0800
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> 
> This patch implements the PCIe Advanced Error Reporting callbacks in
> ixgbe.  The 82598 hardware supports AER, so we enable it.
> 
> Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---

Once again: pcie-aer is not checking for mmconfig working!
If you turn it on, you may get screaming errors.

pci_enable_pcie_error_reporting should be checking for pci_read/pci_write
config errors and does not.

Your driver should be checking for pci_enable_pcie_error_reporting failing.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Dec. 2, 2008, 7:25 a.m.
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 1 Dec 2008 17:47:54 -0800

> pci_enable_pcie_error_reporting should be checking for pci_read/pci_write
> config errors and does not.
> 
> Your driver should be checking for pci_enable_pcie_error_reporting failing.

Agreed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Waskiewicz Jr, Peter P - Dec. 2, 2008, 8:19 a.m.
On Mon, 1 Dec 2008, Stephen Hemminger wrote:

> On Mon, 01 Dec 2008 16:41:15 -0800
> Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> 
> > From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> > 
> > This patch implements the PCIe Advanced Error Reporting callbacks in
> > ixgbe.  The 82598 hardware supports AER, so we enable it.
> > 
> > Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> 
> Once again: pcie-aer is not checking for mmconfig working!
> If you turn it on, you may get screaming errors.
> 
> pci_enable_pcie_error_reporting should be checking for pci_read/pci_write
> config errors and does not.
> 
> Your driver should be checking for pci_enable_pcie_error_reporting failing.

I'll respin and have a new patch sent for this.  I just finally saw the 
igb thread, so my apologies for missing this earlier.

Cheers,
-PJ
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Waskiewicz Jr, Peter P - Dec. 9, 2008, 7:13 a.m.
On Mon, 1 Dec 2008, David Miller wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Mon, 1 Dec 2008 17:47:54 -0800
> 
> > pci_enable_pcie_error_reporting should be checking for pci_read/pci_write
> > config errors and does not.
> >
> > Your driver should be checking for pci_enable_pcie_error_reporting failing.
> 
> Agreed.

I've finally been able to get back to this.  I've looked at any other 
prior use of the PCIe AER routines, and the only driver using it to date 
is the QLogic QLA2xxx driver in SCSI-land.  I've also looked at the return 
codes, with and without Stephen's recent AER patch, and there's not much 
the driver can do.  If it's catching the errors, the best I can do is 
print an error message, then continue along.

I've put that patch together, and will have it sent along shortly.  If 
there's something additional I'm missing here, please let me know.

Cheers,
-PJ
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h
index 6cbf26e..e112008 100644
--- a/drivers/net/ixgbe/ixgbe.h
+++ b/drivers/net/ixgbe/ixgbe.h
@@ -32,6 +32,7 @@ 
 #include <linux/pci.h>
 #include <linux/netdevice.h>
 #include <linux/inet_lro.h>
+#include <linux/aer.h>
 
 #include "ixgbe_type.h"
 #include "ixgbe_common.h"
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index e014a73..e9cb430 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -4021,6 +4021,8 @@  static int __devinit ixgbe_probe(struct pci_dev *pdev,
 		goto err_pci_reg;
 	}
 
+	pci_enable_pcie_error_reporting(pdev);
+
 	pci_set_master(pdev);
 	pci_save_state(pdev);
 
@@ -4296,6 +4298,8 @@  static void __devexit ixgbe_remove(struct pci_dev *pdev)
 
 	free_netdev(netdev);
 
+	pci_disable_pcie_error_reporting(pdev);
+
 	pci_disable_device(pdev);
 }
 
@@ -4333,21 +4337,27 @@  static pci_ers_result_t ixgbe_io_slot_reset(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
+	pci_ers_result_t result;
 
 	if (pci_enable_device(pdev)) {
 		DPRINTK(PROBE, ERR,
 		        "Cannot re-enable PCI device after reset.\n");
-		return PCI_ERS_RESULT_DISCONNECT;
-	}
-	pci_set_master(pdev);
-	pci_restore_state(pdev);
+		result = PCI_ERS_RESULT_DISCONNECT;
+	} else {
+		pci_set_master(pdev);
+		pci_restore_state(pdev);
 
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
+		pci_enable_wake(pdev, PCI_D3hot, 0);
+		pci_enable_wake(pdev, PCI_D3cold, 0);
 
-	ixgbe_reset(adapter);
+		ixgbe_reset(adapter);
+
+		result = PCI_ERS_RESULT_RECOVERED;
+	}
+
+	pci_cleanup_aer_uncorrect_error_status(pdev);
 
-	return PCI_ERS_RESULT_RECOVERED;
+	return result;
 }
 
 /**