Module Name: src
Committed By: msaitoh
Date: Sat Dec 26 06:07:16 UTC 2020
Modified Files:
src/sys/dev/pci/ixgbe: ixgbe.c ixgbe.h ixgbe_type.h
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.272 -r1.273 src/sys/dev/pci/ixgbe/ixgbe.c
cvs rdiff -u -r1.73 -r1.74 src/sys/dev/pci/ixgbe/ixgbe.h
cvs rdiff -u -r1.46 -r1.47 src/sys/dev/pci/ixgbe/ixgbe_type.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/pci/ixgbe/ixgbe.c
diff -u src/sys/dev/pci/ixgbe/ixgbe.c:1.272 src/sys/dev/pci/ixgbe/ixgbe.c:1.273
--- src/sys/dev/pci/ixgbe/ixgbe.c:1.272 Sat Dec 26 06:02:42 2020
+++ src/sys/dev/pci/ixgbe/ixgbe.c Sat Dec 26 06:07:16 2020
@@ -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 int ixgbe_msix_admin(void *);
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 @@ ixgbe_config_link(struct adapter *adapte
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 @@ ixgbe_msix_admin(void *arg)
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 @@ ixgbe_msix_admin(void *arg)
/* 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 @@ ixgbe_msix_admin(void *arg)
|| ((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 @@ ixgbe_msix_admin(void *arg)
/* This is probably overkill :) */
if (!atomic_cas_uint(&adapter->fdir_reinit, 0, 1))
return 1;
- /* Disable the interrupt */
- IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_EIMC_FLOW_DIR);
task_requests |= IXGBE_REQUEST_TASK_FDIR;
+ /* Disable the interrupt */
+ eims_disable |= IXGBE_EIMS_FLOW_DIR;
}
if (eicr & IXGBE_EICR_ECC) {
@@ -3161,8 +3170,7 @@ ixgbe_msix_admin(void *arg)
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 @@ ixgbe_msix_admin(void *arg)
default:
if (!(eicr & IXGBE_EICR_TS))
break;
+
retval = hw->phy.ops.check_overtemp(hw);
if (retval != IXGBE_ERR_OVERTEMP)
break;
@@ -3185,30 +3194,32 @@ ixgbe_msix_admin(void *arg)
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 @@ ixgbe_handle_timer(struct work *wk, void
}
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_recovery_mode_timer(struct
/************************************************************************
* 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 @@ out:
*/
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 @@ ixgbe_handle_admin(struct work *wk, void
{
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_handle_admin(struct work *wk, void
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);
+ 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) != 0) {
+ 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 @@ ixgbe_legacy_irq(void *arg)
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 @@ ixgbe_legacy_irq(void *arg)
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 @@ ixgbe_legacy_irq(void *arg)
|| ((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 @@ ixgbe_legacy_irq(void *arg)
/* 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;
+ }
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);
-
- reenable_intr = false;
}
- if (reenable_intr == true)
- ixgbe_enable_intr(adapter);
+ /* Re-enable some interrupts */
+ IXGBE_WRITE_REG(hw, IXGBE_EIMS,
+ (eims_orig & ~eims_disable) | eims_enable);
return 1;
} /* ixgbe_legacy_irq */
@@ -6557,7 +6588,7 @@ ixgbe_handle_que(void *context)
ixgbe_enable_queue(adapter, que->msix);
} else {
/* INTx or MSI */
- ixgbe_enable_intr(adapter);
+ ixgbe_enable_queue(adapter, 0);
}
return;
Index: src/sys/dev/pci/ixgbe/ixgbe.h
diff -u src/sys/dev/pci/ixgbe/ixgbe.h:1.73 src/sys/dev/pci/ixgbe/ixgbe.h:1.74
--- src/sys/dev/pci/ixgbe/ixgbe.h:1.73 Thu Nov 19 02:23:24 2020
+++ src/sys/dev/pci/ixgbe/ixgbe.h Sat Dec 26 06:07:16 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.h,v 1.73 2020/11/19 02:23:24 msaitoh Exp $ */
+/* $NetBSD: ixgbe.h,v 1.74 2020/12/26 06:07:16 msaitoh Exp $ */
/******************************************************************************
SPDX-License-Identifier: BSD-3-Clause
@@ -769,12 +769,13 @@ bool ixgbe_txeof(struct tx_ring *);
bool ixgbe_rxeof(struct ix_queue *);
#define IXGBE_REQUEST_TASK_MOD 0x01
-#define IXGBE_REQUEST_TASK_MSF 0x02
-#define IXGBE_REQUEST_TASK_MBX 0x04
-#define IXGBE_REQUEST_TASK_FDIR 0x08
-#define IXGBE_REQUEST_TASK_PHY 0x10
-#define IXGBE_REQUEST_TASK_LSC 0x20
-#define IXGBE_REQUEST_TASK_NEED_ACKINTR 0x80
+#define IXGBE_REQUEST_TASK_MOD_WOI 0x02
+#define IXGBE_REQUEST_TASK_MSF 0x04
+#define IXGBE_REQUEST_TASK_MSF_WOI 0x08
+#define IXGBE_REQUEST_TASK_MBX 0x10
+#define IXGBE_REQUEST_TASK_FDIR 0x20
+#define IXGBE_REQUEST_TASK_PHY 0x40
+#define IXGBE_REQUEST_TASK_LSC 0x80
/* For NetBSD */
const struct sysctlnode *ixgbe_sysctl_instance(struct adapter *);
Index: src/sys/dev/pci/ixgbe/ixgbe_type.h
diff -u src/sys/dev/pci/ixgbe/ixgbe_type.h:1.46 src/sys/dev/pci/ixgbe/ixgbe_type.h:1.47
--- src/sys/dev/pci/ixgbe/ixgbe_type.h:1.46 Fri Dec 11 05:01:19 2020
+++ src/sys/dev/pci/ixgbe/ixgbe_type.h Sat Dec 26 06:07:16 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe_type.h,v 1.46 2020/12/11 05:01:19 msaitoh Exp $ */
+/* $NetBSD: ixgbe_type.h,v 1.47 2020/12/26 06:07:16 msaitoh Exp $ */
/******************************************************************************
SPDX-License-Identifier: BSD-3-Clause
@@ -2080,7 +2080,8 @@ enum {
#define IXGBE_FTQF_QUEUE_ENABLE 0x80000000
/* Interrupt clear mask */
-#define IXGBE_IRQ_CLEAR_MASK 0xFFFFFFFF
+#define IXGBE_IRQ_CLEAR_MASK 0xFFFFFFFF
+#define IXGBE_MSIX_OTHER_CLEAR_MASK 0xFFFF0000
/* Interrupt Vector Allocation Registers */
#define IXGBE_IVAR_REG_NUM 25
