Module Name:    src
Committed By:   msaitoh
Date:           Fri Dec 11 05:01:19 UTC 2020

Modified Files:
        src/sys/dev/pci/ixgbe: ixgbe.c ixgbe_type.h

Log Message:
 Don't use EIMC_OTHER bit because it's read only other than 82598.

 Documents say:

  82598:
     All of bit 31(OTHER bit) of EIxx are reserved. In reality, at least
    EIMS_OTHER and EIMC_OTHER exist and the OTHER interrupt doesn't work
    without EIMS_OTHER.

  Other than 82598:
     EIMS_OTHER is read only and EIMC_OTHER doesn't exist. If one of
    bit 29..16 is set, EIMS_OTHER is set to 1 (Note that bit 30(TCP timer
    isn't included)). Even if write bit 31 of EIMC to 1, it's ignored
    (EIMS_OTHER doesn't set).

 We introduced new spin mutex in ixgbe.c rev. 1.260, so it's OK to remove
EIMC_OTHER stuff. We already set EIMS_OTHER in if_init(), so keep it for
82598. No functional change other than 82598.

 Another solution is to control bit 30..16 directly to mask/unmask interrupt
instead of the mutex.

TODO:
  Some MSI-X interrupt(LSC, module insertion/removal etc.)'s mask/unmask
  code between ixgbe_msix_admin() and ixgbe_handle_admin() may be wrong.
  It'll be fixed later.


To generate a diff of this commit:
cvs rdiff -u -r1.261 -r1.262 src/sys/dev/pci/ixgbe/ixgbe.c
cvs rdiff -u -r1.45 -r1.46 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.261 src/sys/dev/pci/ixgbe/ixgbe.c:1.262
--- src/sys/dev/pci/ixgbe/ixgbe.c:1.261	Mon Nov 30 07:53:42 2020
+++ src/sys/dev/pci/ixgbe/ixgbe.c	Fri Dec 11 05:01:19 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.c,v 1.261 2020/11/30 07:53:42 msaitoh Exp $ */
+/* $NetBSD: ixgbe.c,v 1.262 2020/12/11 05:01:19 msaitoh Exp $ */
 
 /******************************************************************************
 
@@ -3095,9 +3095,6 @@ ixgbe_msix_admin(void *arg)
 
 	++adapter->admin_irqev.ev_count;
 
-	/* Pause other interrupts */
-	IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_EIMC_OTHER);
-
 	/* First get the cause */
 	/*
 	 * The specifications of 82598, 82599, X540 and X550 say EICS register
@@ -3219,9 +3216,6 @@ ixgbe_msix_admin(void *arg)
 		adapter->task_requests |= task_requests;
 		ixgbe_schedule_admin_tasklet(adapter);
 		mutex_exit(&adapter->admin_mtx);
-	} else {
-		/* Re-enable other interrupts */
-		IXGBE_WRITE_REG(hw, IXGBE_EIMS, IXGBE_EIMS_OTHER);
 	}
 
 	return 1;
@@ -4809,7 +4803,6 @@ 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;
 
 	mutex_enter(&adapter->admin_mtx);
@@ -4848,11 +4841,12 @@ ixgbe_handle_admin(struct work *wk, void
 	}
 #endif
 	if ((task_requests & IXGBE_REQUEST_TASK_NEED_ACKINTR) != 0) {
-		if ((adapter->feat_en & IXGBE_FEATURE_MSIX) != 0) {
-			/* Re-enable other interrupts */
-			IXGBE_WRITE_REG(hw, IXGBE_EIMS, IXGBE_EIMS_OTHER);
-		} else
-			ixgbe_enable_intr(adapter);
+		/*
+		 * XXX FIXME.
+		 * ixgbe_enable_intr() enables all interrupts. It might enable
+		 * an interrupt which should not be enabled.
+		 */
+		ixgbe_enable_intr(adapter);
 	}
 
 	IXGBE_CORE_UNLOCK(adapter);

Index: src/sys/dev/pci/ixgbe/ixgbe_type.h
diff -u src/sys/dev/pci/ixgbe/ixgbe_type.h:1.45 src/sys/dev/pci/ixgbe/ixgbe_type.h:1.46
--- src/sys/dev/pci/ixgbe/ixgbe_type.h:1.45	Mon Aug 31 11:19:54 2020
+++ src/sys/dev/pci/ixgbe/ixgbe_type.h	Fri Dec 11 05:01:19 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe_type.h,v 1.45 2020/08/31 11:19:54 msaitoh Exp $ */
+/* $NetBSD: ixgbe_type.h,v 1.46 2020/12/11 05:01:19 msaitoh Exp $ */
 
 /******************************************************************************
   SPDX-License-Identifier: BSD-3-Clause
@@ -1998,6 +1998,13 @@ enum {
 #define IXGBE_EIMS_PBUR		IXGBE_EICR_PBUR /* Pkt Buf Handler Err */
 #define IXGBE_EIMS_DHER		IXGBE_EICR_DHER /* Descr Handler Error */
 #define IXGBE_EIMS_TCP_TIMER	IXGBE_EICR_TCP_TIMER /* TCP Timer */
+/*
+ * EIMS_OTHER is R/W on 82598 though the document says it's reserved.
+ * It MUST be required to set this bit to get OTHER interrupt.
+ *
+ * On other chips, it's read only. It's set if any bits of 29..16 is not zero.
+ * Bit 30 (TCP_TIMER) doesn't affect to EIMS_OTHER.
+ */
 #define IXGBE_EIMS_OTHER	IXGBE_EICR_OTHER /* INT Cause Active */
 
 /* Extended Interrupt Mask Clear */
@@ -2019,6 +2026,7 @@ enum {
 #define IXGBE_EIMC_PBUR		IXGBE_EICR_PBUR /* Pkt Buf Handler Err */
 #define IXGBE_EIMC_DHER		IXGBE_EICR_DHER /* Desc Handler Err */
 #define IXGBE_EIMC_TCP_TIMER	IXGBE_EICR_TCP_TIMER /* TCP Timer */
+/* EIMC_OTHER works only on 82598. See EIMS_OTHER's comment */
 #define IXGBE_EIMC_OTHER	IXGBE_EICR_OTHER /* INT Cause Active */
 
 #define IXGBE_EIMS_ENABLE_MASK ( \

Reply via email to