e1000e: i219 execute unit hang fix on every reset or power state transition
Subsystems (2)
PATCH 1/1 e1000e: i219 execute unit hang fix on every reset or power state transition
after testing various cases, the conclusion is that the fix MUST be executed BEFORE any event that the HW is reset or transition to D3. to fix that I moved the execution to the relevant places but per Alexander Duyck's review, ensure now that the DMA is valid and was not freed before manipulating the ring.
--- drivers/net/ethernet/intel/e1000e/netdev.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/net/ethernet/intel/e1000e/netdev.c +10 -6
@@ -4043,6 +4043,8 @@ void e1000e_reset(struct e1000_adapter *adapter) } } +if (hw->mac.type == e1000_pch_spt) +e1000_flush_desc_rings (adapter);/* Allow time for pending master requests to run */ mac->ops.reset_hw (hw);
@@ -4215,10 +4217,6 @@ 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); adapter->link_speed =0 ; adapter->link_duplex =0 ;
@@ -4229,8 +4227,14 @@ void e1000e_down(struct e1000_adapter *adapter, bool reset) e1000_lv_jumbo_workaround_ich8lan(hw, false))e_dbg ("failed to disable jumbo frame workaround mode\n" ); -if (reset && !pci_channel_offline (adapter->pdev)) -e1000e_reset (adapter); +if (!pci_channel_offline (adapter->pdev)) { +if (reset) +e1000e_reset (adapter); +else 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); }void e1000e_reinit_locked (struct e1000_adapter *adapter)
Signature · 9 lines
2.1.0 --------------------------------------------------------------------- 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.
PATCH 1/1 e1000e: i219 execute unit hang fix on every reset or power state transition
Quoted 42 lines · Yanir Lubetkin
after testing various cases, the conclusion is that the fix MUST be executed BEFORE any event that the HW is reset or transition to D3. to fix that I moved the execution to the relevant places but per Alexander Duyck's review, ensure now that the DMA is valid and was not freed before manipulating the ring.
Signed-off-by Yanir Lubetkin <[email protected]>--- drivers/net/ethernet/intel/e1000e/netdev.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)drivers/net/ethernet/intel/e1000e/netdev.c +3
-6@@ -4043,6 +4043,8 @@ void e1000e_reset(struct e1000_adapter *adapter) } } +if (hw->mac.type == e1000_pch_spt) +e1000_flush_desc_rings (adapter);/* Allow time for pending master requests to run */ mac->ops.reset_hw (hw);@@ -4215,10 +4217,6 @@ 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); adapter->link_speed =0 ; adapter->link_duplex =0 ;@@ -4229,8 +4227,14 @@ void e1000e_down(struct e1000_adapter *adapter, bool reset) e1000_lv_jumbo_workaround_ich8lan(hw, false))e_dbg ("failed to disable jumbo frame workaround mode\n" ); -if (reset && !pci_channel_offline (adapter->pdev)) -e1000e_reset (adapter); +if (!pci_channel_offline (adapter->pdev)) {
drivers/net/ethernet/intel/e1000e/netdev.c void e1000e_down(struct e1000_adapter *adapter, bool reset) · line 4240
Quoted 10 lines
drivers/net/ethernet/intel/e1000e/netdev.c +7
-0+if (reset) +e1000e_reset (adapter); +else 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); }void e1000e_reinit_locked (struct e1000_adapter *adapter)
I'm really not a fan of this as it really seems to confuse things. Do you know if you need the full workaround or only part of it? For example if the descriptor ring is empty flushing the descriptor rings shouldn't be necessary, so I suspect you probably only need to move the bits where you were setting E1000_FEXTNVM11_DISABLE_MULR_FIX into the e1000_reset_hw_ich8lan call itself and then you could probably just leave the rest of the workaround as is.
- Alex
Lubetkin
PATCH 1/1 e1000e: i219 execute unit hang fix on every reset or power state transition
Quoted 66 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.h.duyck at redhat.com]
Sent: Wednesday, April 22, 2015 17:39
To: Lubetkin, YanirX; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH] e1000e: i219 execute unit hang fix on every reset or power state transitionQuoted 4 lines · Yanir Lubetkin
after testing various cases, the conclusion is that the fix MUST be executed BEFORE any event that the HW is reset or transition to D3. to fix that I moved the execution to the relevant places but per Alexander Duyck's review, ensure now that the DMA is valid and was not
freed before manipulating the ring.
drivers/net/ethernet/intel/e1000e/netdev.c void e1000e_reset(struct e1000_adapter
Quoted 12 lines
Signed-off-by Yanir Lubetkin <[email protected]>--- drivers/net/ethernet/intel/e1000e/netdev.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)drivers/net/ethernet/intel/e1000e/netdev.c +0
-0@@ -4043,6 +4043,8 @@ void e1000e_reset(struct e1000_adapter*adapter)
drivers/net/ethernet/intel/e1000e/netdev.c void e1000e_down(struct e1000_adapter
Quoted 9 lines
drivers/net/ethernet/intel/e1000e/netdev.c +2
-0} } +if (hw->mac.type == e1000_pch_spt) +e1000_flush_desc_rings (adapter);/* Allow time for pending master requests to run */ mac->ops.reset_hw (hw);@@ -4215,10 +4217,6 @@ void e1000e_down(struct e1000_adapter*adapter, bool reset)
drivers/net/ethernet/intel/e1000e/netdev.c void e1000e_down(struct e1000_adapter
Quoted 11 lines
drivers/net/ethernet/intel/e1000e/netdev.c +0
-4spin_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); adapter->link_speed =0 ; adapter->link_duplex =0 ;@@ -4229,8 +4227,14 @@ void e1000e_down(struct e1000_adapter*adapter, bool reset)
drivers/net/ethernet/intel/e1000e/netdev.c void e1000e_down(struct e1000_adapter · line 4228
Quoted 2 lines
drivers/net/ethernet/intel/e1000e/netdev.c +0
-0e1000_lv_jumbo_workaround_ich8lan(hw, false)) e_dbg("failed to disable jumbo frame workaroundmode\n");
Quoted 13 lines
drivers/net/ethernet/intel/e1000e/netdev.c +8
-2-if (reset && !pci_channel_offline (adapter->pdev)) -e1000e_reset (adapter); +if (!pci_channel_offline (adapter->pdev)) { +if (reset) +e1000e_reset (adapter); +else 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); }
Quoted 10 lines
drivers/net/ethernet/intel/e1000e/netdev.c void e1000e_down(struct e1000_adapter · line 4240
Quoted 2 lines
drivers/net/ethernet/intel/e1000e/netdev.c +0
-0void e1000e_reinit_locked (struct e1000_adapter *adapter)I'm really not a fan of this as it really seems to confuse things. Do you know if you need the full workaround or only part of it? For example if the descriptor ring is empty flushing the descriptor rings shouldn't be necessary, so I suspect you probably only need to move the bits where you were setting
E1000_FEXTNVM11_DISABLE_MULR_FIXinto thee1000_reset_hw_ich8lan
call itself and then you could probably just leave the rest of the workaround as is.
The logic is:
If the bit is SET and TDLEN > 0
Clear E1000_FEXTNVM11_DISABLE_MULR_FIX Execute TX ring flush
If the bit is still set
Execute the RX flush
This logic should be executed before any attempt to reset the hardware (unless resetting the bus), and before any state transition out of D0.
the E1000_FEXTNVM11_DISABLE_MULR_FIX bit will return SET from reset. It is set when reloading the NVM.
Quoted 2 lines
- Alex
---------------------------------------------------------------------
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.
Brown
PATCH 1/1 e1000e: i219 execute unit hang fix on every reset or power state transition
In reply to Yanir LubetkinQuoted 15 lines
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Yanir Lubetkin
Sent: Tuesday, April 21, 2015 6:15 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [PATCH] e1000e: i219 execute unit hang fix on every reset or power state transitionafter testing various cases, the conclusion is that the fix MUST be executed BEFORE
any event that the HW is reset or transition to D3. to fix that I moved the execution
to the relevant places but per Alexander Duyck's review, ensure now that the DMA is
valid and was not freed before manipulating the ring.
Quoted 4 lines
Signed-off-by Yanir Lubetkin <[email protected]>--- drivers/net/ethernet/intel/e1000e/netdev.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)