Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/pci/ixgbe Disable/enable the OTHER interrupts correc...
details: https://anonhg.NetBSD.org/src/rev/ebb63b1270cd
branches: trunk
changeset: 958133:ebb63b1270cd
user: msaitoh <msaitoh%NetBSD.org@localhost>
date: Sat Dec 26 06:07:16 2020 +0000
description:
Disable/enable the OTHER interrupts correctly.
The OTHER interrupt was not blocked correctly when MSI-X is used.
ixgbe.c rev. 1.260 added new mutex to avoid the race but it didn't
disable the interrupt itself.
Calling ixgbe_enable_intr() enables all interrupts, so it's not good to
call it when some interrupt sources should not be enabled (e.g.:
calling ixgbe_enable_intr() in ixgbe_handle_admin() enables queue
interrupt).
IXGBE_REQUEST_TASK_NEED_ACKINTR doesn't work as expected because
ixgbe_handle_admin() can't know which task is enqueued from the
interrupt context and can't re-enable a specific EIMS bit.
Solve the above three problems by the following two changes:
- MSI-X: Disable the OTHER interrupts in the biginning of
ixgbe_msix_admin().
- Set mask bits correctly at the end of ixgbe_legacy_irq() and
ixgbe_msix_admin() using with eim_orig, eims_enable and eims_disable.
- Remove IXGBE_REQUEST_TASK_NEED_ACKINTR and add
IXGBE_REQUEST_TASK_{MOD,MSF}_WOI.
diffstat:
sys/dev/pci/ixgbe/ixgbe.c | 121 +++++++++++++++++++++++++---------------
sys/dev/pci/ixgbe/ixgbe.h | 15 ++--
sys/dev/pci/ixgbe/ixgbe_type.h | 5 +-
3 files changed, 87 insertions(+), 54 deletions(-)
diffs (truncated from 380 to 300 lines):
diff -r e91efb916958 -r ebb63b1270cd sys/dev/pci/ixgbe/ixgbe.c
--- a/sys/dev/pci/ixgbe/ixgbe.c Sat Dec 26 06:02:42 2020 +0000
+++ b/sys/dev/pci/ixgbe/ixgbe.c Sat Dec 26 06:07:16 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.c,v 1.272 2020/12/26 06:02:42 msaitoh Exp $ */
+/* $NetBSD: ixgbe.c,v 1.273 2020/12/26 06:07:16 msaitoh Exp $ */
/******************************************************************************
@@ -269,7 +269,7 @@
static void ixgbe_handle_que(void *);
static void ixgbe_handle_link(void *);
static void ixgbe_handle_msf(void *);
-static void ixgbe_handle_mod(void *);
+static void ixgbe_handle_mod(void *, bool);
static void ixgbe_handle_phy(void *);
/* Deferred workqueue handlers */
@@ -1566,9 +1566,9 @@
if (sfp) {
if (hw->phy.multispeed_fiber) {
ixgbe_enable_tx_laser(hw);
- task_requests |= IXGBE_REQUEST_TASK_MSF;
+ task_requests |= IXGBE_REQUEST_TASK_MSF_WOI;
}
- task_requests |= IXGBE_REQUEST_TASK_MOD;
+ task_requests |= IXGBE_REQUEST_TASK_MOD_WOI;
mutex_enter(&adapter->admin_mtx);
adapter->task_requests |= task_requests;
@@ -3090,17 +3090,24 @@
struct adapter *adapter = arg;
struct ixgbe_hw *hw = &adapter->hw;
u32 eicr, eicr_mask;
+ u32 eims_orig;
+ u32 eims_disable = 0;
u32 task_requests = 0;
s32 retval;
++adapter->admin_irqev.ev_count;
- /* First get the cause */
+ eims_orig = IXGBE_READ_REG(hw, IXGBE_EIMS);
+ /* Pause other interrupts */
+ IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_MSIX_OTHER_CLEAR_MASK);
+
/*
+ * First get the cause.
+ *
* The specifications of 82598, 82599, X540 and X550 say EICS register
* is write only. However, Linux says it is a workaround for silicon
- * errata to read EICS instead of EICR to get interrupt cause. It seems
- * there is a problem about read clear mechanism for EICR register.
+ * errata to read EICS instead of EICR to get interrupt cause.
+ * At least, reading EICR clears lower 16bits of EIMS on 82598.
*/
eicr = IXGBE_READ_REG(hw, IXGBE_EICS);
/* Be sure the queue bits are not cleared */
@@ -3110,8 +3117,8 @@
/* Link status change */
if (eicr & IXGBE_EICR_LSC) {
- IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_EIMC_LSC);
task_requests |= IXGBE_REQUEST_TASK_LSC;
+ eims_disable |= IXGBE_EIMS_LSC;
}
if (ixgbe_is_sfp(hw)) {
@@ -3131,11 +3138,13 @@
|| ((hw->phy.sfp_type == ixgbe_sfp_type_not_present)
&& (eicr & IXGBE_EICR_LSC))) {
task_requests |= IXGBE_REQUEST_TASK_MOD;
+ eims_disable |= IXGBE_EIMS_LSC;
}
if ((hw->mac.type == ixgbe_mac_82599EB) &&
(eicr & IXGBE_EICR_GPI_SDP1_BY_MAC(hw))) {
task_requests |= IXGBE_REQUEST_TASK_MSF;
+ eims_disable |= IXGBE_EIMS_GPI_SDP1_BY_MAC(hw);
}
}
@@ -3145,9 +3154,9 @@
/* This is probably overkill :) */
if (!atomic_cas_uint(&adapter->fdir_reinit, 0, 1))
return 1;
+ task_requests |= IXGBE_REQUEST_TASK_FDIR;
/* Disable the interrupt */
- IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_EIMC_FLOW_DIR);
- task_requests |= IXGBE_REQUEST_TASK_FDIR;
+ eims_disable |= IXGBE_EIMS_FLOW_DIR;
}
if (eicr & IXGBE_EICR_ECC) {
@@ -3161,8 +3170,7 @@
case ixgbe_mac_X550EM_a:
if (!(eicr & IXGBE_EICR_GPI_SDP0_X550EM_a))
break;
- IXGBE_WRITE_REG(hw, IXGBE_EIMC,
- IXGBE_EICR_GPI_SDP0_X550EM_a);
+
retval = hw->phy.ops.check_overtemp(hw);
if (retval != IXGBE_ERR_OVERTEMP)
break;
@@ -3172,6 +3180,7 @@
default:
if (!(eicr & IXGBE_EICR_TS))
break;
+
retval = hw->phy.ops.check_overtemp(hw);
if (retval != IXGBE_ERR_OVERTEMP)
break;
@@ -3185,30 +3194,32 @@
if ((adapter->feat_en & IXGBE_FEATURE_SRIOV) &&
(eicr & IXGBE_EICR_MAILBOX)) {
task_requests |= IXGBE_REQUEST_TASK_MBX;
+ eims_disable |= IXGBE_EIMS_MAILBOX;
}
}
/* Check for fan failure */
if (adapter->feat_en & IXGBE_FEATURE_FAN_FAIL) {
- ixgbe_check_fan_failure(adapter, eicr, TRUE);
+ ixgbe_check_fan_failure(adapter, eicr, true);
}
/* External PHY interrupt */
if ((hw->phy.type == ixgbe_phy_x550em_ext_t) &&
(eicr & IXGBE_EICR_GPI_SDP0_X540)) {
task_requests |= IXGBE_REQUEST_TASK_PHY;
+ eims_disable |= IXGBE_EICR_GPI_SDP0_X540;
}
if (task_requests != 0) {
- /* Re-enabling other interrupts is done in the admin task */
- task_requests |= IXGBE_REQUEST_TASK_NEED_ACKINTR;
-
mutex_enter(&adapter->admin_mtx);
adapter->task_requests |= task_requests;
ixgbe_schedule_admin_tasklet(adapter);
mutex_exit(&adapter->admin_mtx);
}
+ /* Re-enable some OTHER interrupts */
+ IXGBE_WRITE_REG(hw, IXGBE_EIMS, eims_orig & ~eims_disable);
+
return 1;
} /* ixgbe_msix_admin */
@@ -4520,7 +4531,7 @@
}
if (sched_mod_task) {
mutex_enter(&adapter->admin_mtx);
- adapter->task_requests |= IXGBE_REQUEST_TASK_MOD;
+ adapter->task_requests |= IXGBE_REQUEST_TASK_MOD_WOI;
ixgbe_schedule_admin_tasklet(adapter);
mutex_exit(&adapter->admin_mtx);
}
@@ -4658,9 +4669,10 @@
/************************************************************************
* ixgbe_handle_mod - Tasklet for SFP module interrupts
+ * bool int_en: true if it's called when the interrupt is enabled.
************************************************************************/
static void
-ixgbe_handle_mod(void *context)
+ixgbe_handle_mod(void *context, bool int_en)
{
struct adapter *adapter = context;
struct ixgbe_hw *hw = &adapter->hw;
@@ -4733,7 +4745,10 @@
*/
if (hw->mac.type != ixgbe_mac_82598EB) {
mutex_enter(&adapter->admin_mtx);
- adapter->task_requests |= IXGBE_REQUEST_TASK_MSF;
+ if (int_en)
+ adapter->task_requests |= IXGBE_REQUEST_TASK_MSF;
+ else
+ adapter->task_requests |= IXGBE_REQUEST_TASK_MSF_WOI;
mutex_exit(&adapter->admin_mtx);
}
@@ -4794,7 +4809,9 @@
{
struct adapter *adapter = context;
struct ifnet *ifp = adapter->ifp;
+ struct ixgbe_hw *hw = &adapter->hw;
u32 task_requests;
+ u32 eims_enable = 0;
mutex_enter(&adapter->admin_mtx);
adapter->admin_pending = 0;
@@ -4813,32 +4830,40 @@
IXGBE_CORE_LOCK(adapter);
if ((task_requests & IXGBE_REQUEST_TASK_LSC) != 0) {
ixgbe_handle_link(adapter);
+ eims_enable |= IXGBE_EIMS_LSC;
+ }
+ if ((task_requests & IXGBE_REQUEST_TASK_MOD_WOI) != 0) {
+ ixgbe_handle_mod(adapter, false);
}
if ((task_requests & IXGBE_REQUEST_TASK_MOD) != 0) {
- ixgbe_handle_mod(adapter);
- }
- if ((task_requests & IXGBE_REQUEST_TASK_MSF) != 0) {
+ ixgbe_handle_mod(adapter, true);
+ if (hw->mac.type >= ixgbe_mac_X540)
+ eims_enable |= IXGBE_EICR_GPI_SDP0_X540;
+ else
+ eims_enable |= IXGBE_EICR_GPI_SDP2_BY_MAC(hw);
+ }
+ if ((task_requests
+ & (IXGBE_REQUEST_TASK_MSF_WOI | IXGBE_REQUEST_TASK_MSF)) != 0) {
ixgbe_handle_msf(adapter);
+ if (((task_requests & IXGBE_REQUEST_TASK_MSF) != 0) &&
+ (hw->mac.type == ixgbe_mac_82599EB))
+ eims_enable |= IXGBE_EIMS_GPI_SDP1_BY_MAC(hw);
}
if ((task_requests & IXGBE_REQUEST_TASK_PHY) != 0) {
ixgbe_handle_phy(adapter);
+ eims_enable |= IXGBE_EICR_GPI_SDP0_X540;
}
if ((task_requests & IXGBE_REQUEST_TASK_FDIR) != 0) {
ixgbe_reinit_fdir(adapter);
+ eims_enable |= IXGBE_EIMS_FLOW_DIR;
}
#if 0 /* notyet */
if ((task_requests & IXGBE_REQUEST_TASK_MBX) != 0) {
ixgbe_handle_mbx(adapter);
+ eims_enable |= IXGBE_EIMS_MAILBOX;
}
#endif
- if ((task_requests & IXGBE_REQUEST_TASK_NEED_ACKINTR) != 0) {
- /*
- * XXX FIXME.
- * ixgbe_enable_intr() enables all interrupts. It might enable
- * an interrupt which should not be enabled.
- */
- ixgbe_enable_intr(adapter);
- }
+ IXGBE_WRITE_REG(hw, IXGBE_EIMS, eims_enable);
IXGBE_CORE_UNLOCK(adapter);
IFNET_UNLOCK(ifp);
@@ -5155,9 +5180,10 @@
struct adapter *adapter = que->adapter;
struct ixgbe_hw *hw = &adapter->hw;
struct tx_ring *txr = adapter->tx_rings;
- bool reenable_intr = true;
u32 eicr, eicr_mask;
u32 eims_orig;
+ u32 eims_enable = 0;
+ u32 eims_disable = 0;
u32 task_requests = 0;
eims_orig = IXGBE_READ_REG(hw, IXGBE_EIMS);
@@ -5197,12 +5223,17 @@
que->req.ev_count++;
ixgbe_sched_handle_que(adapter, que);
- reenable_intr = false;
- }
+ /* Disable queue 0 interrupt */
+ eims_disable |= 1UL << 0;
+
+ } else
+ eims_enable |= IXGBE_EIMC_RTX_QUEUE;
/* Link status change */
- if (eicr & IXGBE_EICR_LSC)
+ if (eicr & IXGBE_EICR_LSC) {
task_requests |= IXGBE_REQUEST_TASK_LSC;
+ eims_disable |= IXGBE_EIMS_LSC;
+ }
if (ixgbe_is_sfp(hw)) {
/* Pluggable optics-related interrupt */
@@ -5221,11 +5252,13 @@
|| ((hw->phy.sfp_type == ixgbe_sfp_type_not_present)
&& (eicr & IXGBE_EICR_LSC))) {
task_requests |= IXGBE_REQUEST_TASK_MOD;
+ eims_disable |= IXGBE_EIMS_LSC;
}
if ((hw->mac.type == ixgbe_mac_82599EB) &&
(eicr & IXGBE_EICR_GPI_SDP1_BY_MAC(hw))) {
task_requests |= IXGBE_REQUEST_TASK_MSF;
+ eims_disable |= IXGBE_EIMS_GPI_SDP1_BY_MAC(hw);
}
}
@@ -5236,23 +5269,21 @@
/* External PHY interrupt */
if ((hw->phy.type == ixgbe_phy_x550em_ext_t) &&
- (eicr & IXGBE_EICR_GPI_SDP0_X540))
+ (eicr & IXGBE_EICR_GPI_SDP0_X540)) {
task_requests |= IXGBE_REQUEST_TASK_PHY;
+ eims_disable |= IXGBE_EICR_GPI_SDP0_X540;
+ }
Home |
Main Index |
Thread Index |
Old Index