Skip to contentSkip to conversation

e1000e i219 fix unit hang on reset and runtime D3

PATCH New Yanir Lubetkin 4 participants 5 messages Apr 13, 2015 – Apr 22, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v5 1/1 e1000e i219 fix unit hang on reset and runtime D3

unit hang may occur if multiple descriptors are available in the rings during reset or runtime suspend. This state can be detected by testing bit 8 in the FEXTNVM7 register. if this bit is set and there are pending descriptors in one of the rings, we must flush them prior to reset. same applies entering runtime suspend.

---
 drivers/net/ethernet/intel/e1000e/ich8lan.h |   5 ++
 drivers/net/ethernet/intel/e1000e/netdev.c  | 102 ++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/e1000e/regs.h    |   1 +
 3 files changed, 108 insertions(+)
drivers/net/ethernet/intel/e1000e/ich8lan.h +5 -0
index 770a573..69c4dbe 100644
@@ -100,6 +100,11 @@
 #define E1000_FEXTNVM7_DISABLE_PB_READ	0x00040000
 
 #define E1000_FEXTNVM7_DISABLE_SMB_PERST	0x00000020
+#define E1000_FEXTNVM7_NEED_DESCRING_FLUSH     0x00000100
+#define E1000_FEXTNVM11_DISABLE_MULR_FIX       0x00002000
+
+/* bit24: RXDCTL thresholds granularity: 0 - cache lines, 1 - descriptors */
+#define E1000_RXDCTL_THRESH_UNIT_DESC 0x01000000
 
 #define K1_ENTRY_LATENCY	0
 #define K1_MIN_TIME		1
drivers/net/ethernet/intel/e1000e/netdev.c +102 -0

114 lines not on this page · raw message

drivers/net/ethernet/intel/e1000e/regs.h +1 -0
index 85eefc4..fdaac8e 100644
@@ -38,6 +38,7 @@
 #define E1000_FEXTNVM4	0x00024	/* Future Extended NVM 4 - RW */
 #define E1000_FEXTNVM6	0x00010	/* Future Extended NVM 6 - RW */
 #define E1000_FEXTNVM7	0x000E4	/* Future Extended NVM 7 - RW */
+#define E1000_FEXTNVM11        0x5BBC  /* Future Extended NVM 11 - RW */
 #define E1000_PCIEANACFG	0x00F18	/* PCIE Analog Config */
 #define E1000_FCT	0x00030	/* Flow Control Type - RW */
 #define E1000_VET	0x00038	/* VLAN Ether Type - RW */
Signature · 9 lines
1.9.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
#1 lore raw

PATCH v5 1/1 e1000e i219 fix unit hang on reset and runtime D3

Quoted 6 lines · Yanir Lubetkin

unit hang may occur if multiple descriptors are available in the rings during reset or runtime suspend. This state can be detected by testing bit 8 in the FEXTNVM7 register. if this bit is set and there are pending descriptors in one of the rings, we must flush them prior to reset. same applies entering runtime suspend.

Quoted 7 lines
---
  drivers/net/ethernet/intel/e1000e/ich8lan.h |   5 ++
  drivers/net/ethernet/intel/e1000e/netdev.c  | 102 ++++++++++++++++++++++++++++
  drivers/net/ethernet/intel/e1000e/regs.h    |   1 +
  3 files changed, 108 insertions(+)

This looks okay to me. The BUG_ON seems a bit aggressive but if you think you need to do the workaround rather than just avoid it in the case of TDT != next_to_use I suppose that works. I would just suggest doing testing with heavy transmit and a link partner flooding flow control to see what happens. That should be the test that would be most likely to trigger something like that.

#2 lore raw

PATCH v5 1/1 e1000e i219 fix unit hang on reset and runtime D3

In reply to Yanir Lubetkin
Quoted 10 lines · Yanir Lubetkin

unit hang may occur if multiple descriptors are available in the rings during
reset or runtime suspend. This state can be detected by testing bit 8 in the
FEXTNVM7 register. if this bit is set and there are pending descriptors in one
of the rings, we must flush them prior to reset. same applies entering runtime
suspend.

Quoted 7 lines
---
 drivers/net/ethernet/intel/e1000e/ich8lan.h |   5 ++
 drivers/net/ethernet/intel/e1000e/netdev.c  | 102
++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/e1000e/regs.h    |   1 +
 3 files changed, 108 insertions(+)

Thanks Yanir, I have applied your patch to my queue.

Signature · 9 lines
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git
dev-queue
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150414/e5c476fc/attachment.asc>
#3 lore raw

PATCH v5 1/1 e1000e i219 fix unit hang on reset and runtime D3

In reply to Yanir Lubetkin
Quoted 130 lines · Yanir Lubetkin

unit hang may occur if multiple descriptors are available in the rings during reset or runtime suspend. This state can be detected by testing bit 8 in the FEXTNVM7 register. if this bit is set and there are pending descriptors in one of the rings, we must flush them prior to reset. same applies entering runtime suspend.

---
  drivers/net/ethernet/intel/e1000e/ich8lan.h |   5 ++
  drivers/net/ethernet/intel/e1000e/netdev.c  | 102 ++++++++++++++++++++++++++++
  drivers/net/ethernet/intel/e1000e/regs.h    |   1 +
  3 files changed, 108 insertions(+)
drivers/net/ethernet/intel/e1000e/ich8lan.h +5 -0
index 770a573..69c4dbe 100644
@@ -100,6 +100,11 @@
  #define E1000_FEXTNVM7_DISABLE_PB_READ	0x00040000
 
  #define E1000_FEXTNVM7_DISABLE_SMB_PERST	0x00000020
+#define E1000_FEXTNVM7_NEED_DESCRING_FLUSH     0x00000100
+#define E1000_FEXTNVM11_DISABLE_MULR_FIX       0x00002000
+
+/* bit24: RXDCTL thresholds granularity: 0 - cache lines, 1 - descriptors */
+#define E1000_RXDCTL_THRESH_UNIT_DESC 0x01000000
 
  #define K1_ENTRY_LATENCY	0
  #define K1_MIN_TIME		1
drivers/net/ethernet/intel/e1000e/netdev.c +93 -0
index 74ec185..0dfdb52 100644
@@ -3788,6 +3788,106 @@ static void e1000_power_down_phy(struct e1000_adapter *adapter)
  }
 
  /**
+ * e1000_flush_tx_ring - remove all descriptors from the tx_ring
+ *
+ * We want to clear all pending descriptors from the TX ring.
+ * zeroing happens when the HW reads the regs. We  assign the ring itself as
+ * the data of the next descriptor. We don't care about the data we are about
+ * to reset the HW.
+ */
+static void e1000_flush_tx_ring(struct e1000_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	struct e1000_ring *tx_ring = adapter->tx_ring;
+	struct e1000_tx_desc *tx_desc = NULL;
+	u32 tdt, tctl, txd_lower = E1000_TXD_CMD_IFCS;
+	u16 size = 512;
+
+	tctl = er32(TCTL);
+	ew32(TCTL, tctl | E1000_TCTL_EN);
+	tdt = er32(TDT(0));
+	BUG_ON(tdt != tx_ring->next_to_use);
+	tx_desc =  E1000_TX_DESC(*tx_ring, tx_ring->next_to_use);
+	tx_desc->buffer_addr = tx_ring->dma;
+
+	tx_desc->lower.data = cpu_to_le32(txd_lower | size);
+	tx_desc->upper.data = 0;
+	/* flush descriptors to memory before notifying the HW */
+	wmb();
+	tx_ring->next_to_use++;
+	if (tx_ring->next_to_use == tx_ring->count)
+		tx_ring->next_to_use = 0;
+	ew32(TDT(0), tx_ring->next_to_use);
+	mmiowb();
+	usleep_range(200, 250);
+}
+
+/**
+ * e1000_flush_rx_ring - remove all descriptors from the rx_ring
+ *
+ * Mark all descriptors in the RX ring as consumed and disable the rx ring
+ */
+static void e1000_flush_rx_ring(struct e1000_adapter *adapter)
+{
+	u32 rctl, rxdctl;
+	struct e1000_hw *hw = &adapter->hw;
+
+	rctl = er32(RCTL);
+	ew32(RCTL, rctl & ~E1000_RCTL_EN);
+	e1e_flush();
+	usleep_range(100, 150);
+
+	rxdctl = er32(RXDCTL(0));
+	/* zero the lower 14 bits (prefetch and host thresholds) */
+	rxdctl &= 0xffffc000;
+
+	/* update thresholds: prefetch threshold to 31, host threshold to 1
+	 * and make sure the granularity is "descriptors" and not "cache lines"
+	 */
+	rxdctl |= (0x1F | (1 << 8) | E1000_RXDCTL_THRESH_UNIT_DESC);
+
+	ew32(RXDCTL(0), rxdctl);
+	/* momentarily enable the RX ring for the changes to take effect */
+	ew32(RCTL, rctl | E1000_RCTL_EN);
+	e1e_flush();
+	usleep_range(100, 150);
+	ew32(RCTL, rctl & ~E1000_RCTL_EN);
+}
+
+/**
+ * e1000_flush_desc_rings - remove all descriptors from the descriptor rings
+ *
+ * In i219, the descriptor rings must be emptied before resetting the HW
+ * or before changing the device state to D3 during runtime (runtime PM).
+ *
+ * Failure to do this will cause the HW to enter a unit hang state which can
+ * only be released by PCI reset on the device
+ *
+ */
+
+static void e1000_flush_desc_rings(struct e1000_adapter *adapter)
+{
+	u32 hang_state;
+	u32 fext_nvm11, tdlen;
+	struct e1000_hw *hw = &adapter->hw;
+
+	/* First, disable MULR fix in FEXTNVM11 */
+	fext_nvm11 = er32(FEXTNVM11);
+	fext_nvm11 |= E1000_FEXTNVM11_DISABLE_MULR_FIX;
+	ew32(FEXTNVM11, fext_nvm11);
+	/* do nothing if we're not in faulty state, or if the queue is empty */
+	tdlen = er32(TDLEN(0));
+	hang_state = er32(FEXTNVM7);
+	if ((hang_state & E1000_FEXTNVM7_NEED_DESCRING_FLUSH) || tdlen)
+		return;
+	e1000_flush_tx_ring(adapter);

drivers/net/ethernet/intel/e1000e/netdev.c static void e1000_power_down_phy(struct e1000_adapter *adapter) · line 3893

Quoted 10 lines
drivers/net/ethernet/intel/e1000e/netdev.c +7 -0
+	/* recheck, maybe the fault is caused by the rx ring */
+	hang_state = er32(FEXTNVM7);
+	if (hang_state & E1000_FEXTNVM7_NEED_DESCRING_FLUSH)
+		e1000_flush_rx_ring(adapter);
+}
+
+/**
   * e1000e_reset - bring the hardware into a known good state
   *
   * This function boots the hardware and enables some settings that

I was just looking at this and I was wondering, why is it you are calling "return" on the Tx check? Seems like you should be calling e1000_flush_tx_ring shouldn't you?

Quoted 21 lines
drivers/net/ethernet/intel/e1000e/netdev.c +2 -0
@@ -4115,6 +4215,8 @@ void e1000e_down(struct e1000_adapter *adapter, bool reset)
  	spin_unlock(&adapter->stats64_lock);
 
  	e1000e_flush_descriptors(adapter);
+	if (hw->mac.type == e1000_pch_spt)
+		e1000_flush_desc_rings(adapter);
  	e1000_clean_tx_ring(adapter->tx_ring);
  	e1000_clean_rx_ring(adapter->rx_ring);
 
drivers/net/ethernet/intel/e1000e/regs.h +1 -0
index 85eefc4..fdaac8e 100644
@@ -38,6 +38,7 @@
  #define E1000_FEXTNVM4	0x00024	/* Future Extended NVM 4 - RW */
  #define E1000_FEXTNVM6	0x00010	/* Future Extended NVM 6 - RW */
  #define E1000_FEXTNVM7	0x000E4	/* Future Extended NVM 7 - RW */
+#define E1000_FEXTNVM11        0x5BBC  /* Future Extended NVM 11 - RW */
  #define E1000_PCIEANACFG	0x00F18	/* PCIE Analog Config */
  #define E1000_FCT	0x00030	/* Flow Control Type - RW */
  #define E1000_VET	0x00038	/* VLAN Ether Type - RW */
#4 lore raw

PATCH v5 1/1 e1000e i219 fix unit hang on reset and runtime D3

Quoted 148 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.h.duyck at redhat.com]
Sent: Wednesday, April 22, 2015 17:41
To: Lubetkin, YanirX; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH v5] e1000e i219 fix unit hang on reset and runtime D3

Quoted 11 lines · Yanir Lubetkin

unit hang may occur if multiple descriptors are available in the rings during reset or runtime suspend. This state can be detected by testing bit 8 in the
FEXTNVM7 register. if this bit is set and there are pending descriptors in one of the rings, we must flush them prior to reset. same applies entering runtime suspend.

---
  drivers/net/ethernet/intel/e1000e/ich8lan.h |   5 ++
  drivers/net/ethernet/intel/e1000e/netdev.c  | 102

++++++++++++++++++++++++++++

drivers/net/ethernet/intel/e1000e/netdev.c static void e1000_power_down_phy(struct

Quoted 26 lines
  drivers/net/ethernet/intel/e1000e/regs.h    |   1 +
  3 files changed, 108 insertions(+)
drivers/net/ethernet/intel/e1000e/ich8lan.h +5 -0
index 770a573..69c4dbe 100644
@@ -100,6 +100,11 @@
  #define E1000_FEXTNVM7_DISABLE_PB_READ	0x00040000
 
  #define E1000_FEXTNVM7_DISABLE_SMB_PERST	0x00000020
+#define E1000_FEXTNVM7_NEED_DESCRING_FLUSH     0x00000100
+#define E1000_FEXTNVM11_DISABLE_MULR_FIX       0x00002000
+
+/* bit24: RXDCTL thresholds granularity: 0 - cache lines, 1 -
+descriptors */ #define E1000_RXDCTL_THRESH_UNIT_DESC 0x01000000
 
  #define K1_ENTRY_LATENCY	0
  #define K1_MIN_TIME		1
drivers/net/ethernet/intel/e1000e/netdev.c +0 -0
index 74ec185..0dfdb52 100644
@@ -3788,6 +3788,106 @@ static void e1000_power_down_phy(struct

e1000_adapter *adapter)

Quoted 86 lines
drivers/net/ethernet/intel/e1000e/netdev.c +83 -0
  }
 
  /**
+ * e1000_flush_tx_ring - remove all descriptors from the tx_ring
+ *
+ * We want to clear all pending descriptors from the TX ring.
+ * zeroing happens when the HW reads the regs. We  assign the ring
+itself as
+ * the data of the next descriptor. We don't care about the data we
+are about
+ * to reset the HW.
+ */
+static void e1000_flush_tx_ring(struct e1000_adapter *adapter) {
+	struct e1000_hw *hw = &adapter->hw;
+	struct e1000_ring *tx_ring = adapter->tx_ring;
+	struct e1000_tx_desc *tx_desc = NULL;
+	u32 tdt, tctl, txd_lower = E1000_TXD_CMD_IFCS;
+	u16 size = 512;
+
+	tctl = er32(TCTL);
+	ew32(TCTL, tctl | E1000_TCTL_EN);
+	tdt = er32(TDT(0));
+	BUG_ON(tdt != tx_ring->next_to_use);
+	tx_desc =  E1000_TX_DESC(*tx_ring, tx_ring->next_to_use);
+	tx_desc->buffer_addr = tx_ring->dma;
+
+	tx_desc->lower.data = cpu_to_le32(txd_lower | size);
+	tx_desc->upper.data = 0;
+	/* flush descriptors to memory before notifying the HW */
+	wmb();
+	tx_ring->next_to_use++;
+	if (tx_ring->next_to_use == tx_ring->count)
+		tx_ring->next_to_use = 0;
+	ew32(TDT(0), tx_ring->next_to_use);
+	mmiowb();
+	usleep_range(200, 250);
+}
+
+/**
+ * e1000_flush_rx_ring - remove all descriptors from the rx_ring
+ *
+ * Mark all descriptors in the RX ring as consumed and disable the rx
+ring  */ static void e1000_flush_rx_ring(struct e1000_adapter
+*adapter) {
+	u32 rctl, rxdctl;
+	struct e1000_hw *hw = &adapter->hw;
+
+	rctl = er32(RCTL);
+	ew32(RCTL, rctl & ~E1000_RCTL_EN);
+	e1e_flush();
+	usleep_range(100, 150);
+
+	rxdctl = er32(RXDCTL(0));
+	/* zero the lower 14 bits (prefetch and host thresholds) */
+	rxdctl &= 0xffffc000;
+
+	/* update thresholds: prefetch threshold to 31, host threshold to 1
+	 * and make sure the granularity is "descriptors" and not "cache lines"
+	 */
+	rxdctl |= (0x1F | (1 << 8) | E1000_RXDCTL_THRESH_UNIT_DESC);
+
+	ew32(RXDCTL(0), rxdctl);
+	/* momentarily enable the RX ring for the changes to take effect */
+	ew32(RCTL, rctl | E1000_RCTL_EN);
+	e1e_flush();
+	usleep_range(100, 150);
+	ew32(RCTL, rctl & ~E1000_RCTL_EN);
+}
+
+/**
+ * e1000_flush_desc_rings - remove all descriptors from the
+descriptor rings
+ *
+ * In i219, the descriptor rings must be emptied before resetting the
+HW
+ * or before changing the device state to D3 during runtime (runtime PM).
+ *
+ * Failure to do this will cause the HW to enter a unit hang state
+which can
+ * only be released by PCI reset on the device
+ *
+ */
+
+static void e1000_flush_desc_rings(struct e1000_adapter *adapter) {
+	u32 hang_state;
+	u32 fext_nvm11, tdlen;

drivers/net/ethernet/intel/e1000e/netdev.c static void e1000_power_down_phy(struct e1000_adapter *adapter)

Quoted 10 lines
drivers/net/ethernet/intel/e1000e/netdev.c +10 -0
+	struct e1000_hw *hw = &adapter->hw;
+
+	/* First, disable MULR fix in FEXTNVM11 */
+	fext_nvm11 = er32(FEXTNVM11);
+	fext_nvm11 |= E1000_FEXTNVM11_DISABLE_MULR_FIX;
+	ew32(FEXTNVM11, fext_nvm11);
+	/* do nothing if we're not in faulty state, or if the queue is empty */
+	tdlen = er32(TDLEN(0));
+	hang_state = er32(FEXTNVM7);
+	if ((hang_state & E1000_FEXTNVM7_NEED_DESCRING_FLUSH) ||

tdlen)

Quoted 16 lines

drivers/net/ethernet/intel/e1000e/netdev.c static void e1000_power_down_phy(struct e1000_adapter *adapter) · line 3893

Quoted 12 lines
drivers/net/ethernet/intel/e1000e/netdev.c +9 -0
+		return;
+	e1000_flush_tx_ring(adapter);
+	/* recheck, maybe the fault is caused by the rx ring */
+	hang_state = er32(FEXTNVM7);
+	if (hang_state & E1000_FEXTNVM7_NEED_DESCRING_FLUSH)
+		e1000_flush_rx_ring(adapter);
+}
+
+/**
   * e1000e_reset - bring the hardware into a known good state
   *
   * This function boots the hardware and enables some settings that

I was just looking at this and I was wondering, why is it you are calling "return" on the Tx check? Seems like you should be calling e1000_flush_tx_ring shouldn't you?

The return should occur if any of them is zero.
I will send a fix shortly

Quoted 19 lines

drivers/net/ethernet/intel/e1000e/netdev.c void e1000e_down(struct e1000_adapter

Quoted 2 lines
drivers/net/ethernet/intel/e1000e/netdev.c +0 -0
@@ -4115,6 +4215,8 @@ void e1000e_down(struct e1000_adapter

*adapter, bool reset)

Quoted 14 lines
drivers/net/ethernet/intel/e1000e/netdev.c +2 -0
  	spin_unlock(&adapter->stats64_lock);
 
  	e1000e_flush_descriptors(adapter);
+	if (hw->mac.type == e1000_pch_spt)
+		e1000_flush_desc_rings(adapter);
  	e1000_clean_tx_ring(adapter->tx_ring);
  	e1000_clean_rx_ring(adapter->rx_ring);
 
drivers/net/ethernet/intel/e1000e/regs.h +0 -0
index 85eefc4..fdaac8e 100644

drivers/net/ethernet/intel/e1000e/regs.h line 38

Quoted 1 line
drivers/net/ethernet/intel/e1000e/regs.h +0 -0
@@ -38,6 +38,7 @@
  #define E1000_FEXTNVM4	0x00024	/* Future Extended NVM 4 -

RW */

Quoted 10 lines

drivers/net/ethernet/intel/e1000e/regs.h line 39

Quoted 1 line
drivers/net/ethernet/intel/e1000e/regs.h +0 -0
  #define E1000_FEXTNVM6	0x00010	/* Future Extended NVM 6 -

RW */

drivers/net/ethernet/intel/e1000e/regs.h line 40

Quoted 1 line
drivers/net/ethernet/intel/e1000e/regs.h +0 -0
  #define E1000_FEXTNVM7	0x000E4	/* Future Extended NVM 7 -

RW */

drivers/net/ethernet/intel/e1000e/regs.h line 41

Quoted 1 line
drivers/net/ethernet/intel/e1000e/regs.h +1 -0
+#define E1000_FEXTNVM11        0x5BBC  /* Future Extended NVM 11 - RW

*/

drivers/net/ethernet/intel/e1000e/regs.h line 42

Quoted 1 line
drivers/net/ethernet/intel/e1000e/regs.h +0 -0
  #define E1000_PCIEANACFG	0x00F18	/* PCIE Analog Config

*/

Quoted 2 lines
drivers/net/ethernet/intel/e1000e/regs.h +0 -0
  #define E1000_FCT	0x00030	/* Flow Control Type - RW */
  #define E1000_VET	0x00038	/* VLAN Ether Type - RW */

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.