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