Mon Dec 30 23:32:30 2019 UTC ()
Fix a problem with intr_unmask() that can cause a forever-loop:
- When handling the source-is-masked case in the interrupt vector, set the
  interrupt bit in a new ci_imasked field and ensure the bit is cleared
  from ci_ipending.
- In intr_unmask(), transfer the bit from ci_imasked to ci_ipending for
  non-level-sensitive interrupts (the PIC does the work for us in the
  level-sensitive case), and only force pending interrupts to be processed
  in this case.  (In all cases, make sure the now-unmasked bit is cleared
  from ci_imasked.)

Before, the bit was left in ci_ipending so as not to use edge-triggered
interrupts while the source is masked, but Xspllower() relies on the
pending bits getting cleared.

Tested by forcing all wm(4) interrupts on my test system though an
intr_mask() / softint / intr_unmask() cycle and exercising the network
heavily.


(thorpej)
diff -r1.79 -r1.80 src/sys/arch/amd64/amd64/genassym.cf
diff -r1.72 -r1.73 src/sys/arch/amd64/amd64/vector.S
diff -r1.116 -r1.117 src/sys/arch/i386/i386/genassym.cf
diff -r1.84 -r1.85 src/sys/arch/i386/i386/vector.S
diff -r1.115 -r1.116 src/sys/arch/x86/include/cpu.h
diff -r1.149 -r1.150 src/sys/arch/x86/x86/intr.c

cvs diff -r1.79 -r1.80 src/sys/arch/amd64/amd64/genassym.cf (expand / switch to context diff)
--- src/sys/arch/amd64/amd64/genassym.cf 2019/12/22 15:09:39 1.79
+++ src/sys/arch/amd64/amd64/genassym.cf 2019/12/30 23:32:29 1.80
@@ -1,4 +1,4 @@
-#	$NetBSD: genassym.cf,v 1.79 2019/12/22 15:09:39 thorpej Exp $
+#	$NetBSD: genassym.cf,v 1.80 2019/12/30 23:32:29 thorpej Exp $
 
 #
 # Copyright (c) 1998, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -252,6 +252,7 @@
 define	CPU_INFO_IDEPTH		offsetof(struct cpu_info, ci_idepth)
 if !defined(XENPV)
 define	CPU_INFO_IPENDING	offsetof(struct cpu_info, ci_ipending)
+define	CPU_INFO_IMASKED	offsetof(struct cpu_info, ci_imasked)
 define	CPU_INFO_IMASK		offsetof(struct cpu_info, ci_imask)
 define	CPU_INFO_IUNMASK	offsetof(struct cpu_info, ci_iunmask)
 define	CPU_INFO_ISOURCES	offsetof(struct cpu_info, ci_isources)

cvs diff -r1.72 -r1.73 src/sys/arch/amd64/amd64/vector.S (expand / switch to context diff)
--- src/sys/arch/amd64/amd64/vector.S 2019/12/22 15:09:39 1.72
+++ src/sys/arch/amd64/amd64/vector.S 2019/12/30 23:32:29 1.73
@@ -1,4 +1,4 @@
-/*	$NetBSD: vector.S,v 1.72 2019/12/22 15:09:39 thorpej Exp $	*/
+/*	$NetBSD: vector.S,v 1.73 2019/12/30 23:32:29 thorpej Exp $	*/
 
 /*
  * Copyright (c) 1998, 2007, 2008 The NetBSD Foundation, Inc.
@@ -392,7 +392,7 @@
 	incl	CPUVAR(IDEPTH)						;\
 	movq	IS_HANDLERS(%r14),%rbx					;\
 	cmpl	$0,IS_MASK_COUNT(%r14)	/* source currently masked? */	;\
-	jne	7f			/* yes, hold it */		;\
+	jne	12f			/* yes, hold it */		;\
 6:									\
 	movl	IH_LEVEL(%rbx),%r12d					;\
 	cmpl	%r13d,%r12d						;\
@@ -406,7 +406,7 @@
 	jnz	6b							;\
 5:									\
 	cmpl	$0,IS_MASK_COUNT(%r14)	/* source now masked? */	;\
-	jne	7f			/* yes, deal */			;\
+	jne	12f			/* yes, deal */			;\
 	cli								;\
 	unmask(num)			/* unmask it in hardware */	;\
 	late_ack(num)							;\
@@ -415,10 +415,15 @@
 7:									\
 	cli								;\
 	orl	$(1 << num),CPUVAR(IPENDING)				;\
-	level_mask(num)							;\
+8:	level_mask(num)							;\
 	late_ack(num)							;\
 	sti								;\
 	jmp	_C_LABEL(Xdoreti)	/* lower spl and do ASTs */	;\
+12:									\
+	cli								;\
+	orl	$(1 << num),CPUVAR(IMASKED)				;\
+	btrl	$(num),CPUVAR(IPENDING)					;\
+	jmp	8b							;\
 10:									\
 	cli								;\
 	orl	$(1 << num),CPUVAR(IPENDING)				;\

cvs diff -r1.116 -r1.117 src/sys/arch/i386/i386/genassym.cf (expand / switch to context diff)
--- src/sys/arch/i386/i386/genassym.cf 2019/12/22 15:09:39 1.116
+++ src/sys/arch/i386/i386/genassym.cf 2019/12/30 23:32:29 1.117
@@ -1,4 +1,4 @@
-#	$NetBSD: genassym.cf,v 1.116 2019/12/22 15:09:39 thorpej Exp $
+#	$NetBSD: genassym.cf,v 1.117 2019/12/30 23:32:29 thorpej Exp $
 
 #
 # Copyright (c) 1998, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -274,6 +274,7 @@
 define	CPU_INFO_GDT		offsetof(struct cpu_info, ci_gdt)
 if !defined(XENPV)
 define	CPU_INFO_IPENDING	offsetof(struct cpu_info, ci_ipending)
+define	CPU_INFO_IMASKED	offsetof(struct cpu_info, ci_imasked)
 define	CPU_INFO_IMASK		offsetof(struct cpu_info, ci_imask)
 define	CPU_INFO_ISOURCES	offsetof(struct cpu_info, ci_isources)
 define	CPU_INFO_IUNMASK	offsetof(struct cpu_info, ci_iunmask)

cvs diff -r1.84 -r1.85 src/sys/arch/i386/i386/vector.S (expand / switch to context diff)
--- src/sys/arch/i386/i386/vector.S 2019/12/22 15:09:39 1.84
+++ src/sys/arch/i386/i386/vector.S 2019/12/30 23:32:29 1.85
@@ -1,4 +1,4 @@
-/*	$NetBSD: vector.S,v 1.84 2019/12/22 15:09:39 thorpej Exp $	*/
+/*	$NetBSD: vector.S,v 1.85 2019/12/30 23:32:29 thorpej Exp $	*/
 
 /*
  * Copyright 2002 (c) Wasabi Systems, Inc.
@@ -65,7 +65,7 @@
  */
 
 #include <machine/asm.h>
-__KERNEL_RCSID(0, "$NetBSD: vector.S,v 1.84 2019/12/22 15:09:39 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vector.S,v 1.85 2019/12/30 23:32:29 thorpej Exp $");
 
 #include "opt_ddb.h"
 #include "opt_multiprocessor.h"
@@ -409,7 +409,7 @@
 	sti								;\
 	movl	IS_HANDLERS(%ebp),%ebx					;\
 	cmpl	$0,IS_MASK_COUNT(%ebp)	/* source currently masked? */	;\
-	jne	7f			/* yes, hold it */		;\
+	jne	12f			/* yes, hold it */		;\
 6:									\
 	movl	IH_LEVEL(%ebx),%edi					;\
 	cmpl	%esi,%edi						;\
@@ -423,7 +423,7 @@
 	testl	%ebx,%ebx						;\
 	jnz	6b							;\
 	cmpl	$0,IS_MASK_COUNT(%ebp)	/* source now masked? */	;\
-	jne	7f			/* yes, deal */			;\
+	jne	12f			/* yes, deal */			;\
 	cli								;\
 	unmask(num)			/* unmask it in hardware */	;\
 	late_ack(num)							;\
@@ -431,9 +431,14 @@
 7:									\
 	cli								;\
 	orl	$(1 << num),CPUVAR(IPENDING)				;\
-	level_mask(num)							;\
+8:	level_mask(num)							;\
 	late_ack(num)							;\
 	jmp	_C_LABEL(Xdoreti)	/* lower spl and do ASTs */	;\
+12:									\
+	cli								;\
+	orl	$(1 << num),CPUVAR(IMASKED)				;\
+	btrl	$(num),CPUVAR(IPENDING)					;\
+	jmp	8b							;\
 10:									\
 	orl	$(1 << num),CPUVAR(IPENDING)				;\
 	level_mask(num)							;\

cvs diff -r1.115 -r1.116 src/sys/arch/x86/include/cpu.h (expand / switch to context diff)
--- src/sys/arch/x86/include/cpu.h 2019/12/01 15:34:46 1.115
+++ src/sys/arch/x86/include/cpu.h 2019/12/30 23:32:29 1.116
@@ -1,4 +1,4 @@
-/*	$NetBSD: cpu.h,v 1.115 2019/12/01 15:34:46 ad Exp $	*/
+/*	$NetBSD: cpu.h,v 1.116 2019/12/30 23:32:29 thorpej Exp $	*/
 
 /*
  * Copyright (c) 1990 The Regents of the University of California.
@@ -149,9 +149,11 @@
 	struct {
 		uint32_t	ipending;
 		int		ilevel;
+		uint32_t	imasked;
 	} ci_istate __aligned(8);
 #define ci_ipending	ci_istate.ipending
 #define	ci_ilevel	ci_istate.ilevel
+#define	ci_imasked	ci_istate.imasked
 	int		ci_idepth;
 	void *		ci_intrstack;
 	uint32_t	ci_imask[NIPL];

cvs diff -r1.149 -r1.150 src/sys/arch/x86/x86/intr.c (expand / switch to context diff)
--- src/sys/arch/x86/x86/intr.c 2019/12/22 16:50:03 1.149
+++ src/sys/arch/x86/x86/intr.c 2019/12/30 23:32:30 1.150
@@ -1,4 +1,4 @@
-/*	$NetBSD: intr.c,v 1.149 2019/12/22 16:50:03 ad Exp $	*/
+/*	$NetBSD: intr.c,v 1.150 2019/12/30 23:32:30 thorpej Exp $	*/
 
 /*
  * Copyright (c) 2007, 2008, 2009, 2019 The NetBSD Foundation, Inc.
@@ -133,7 +133,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: intr.c,v 1.149 2019/12/22 16:50:03 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: intr.c,v 1.150 2019/12/30 23:32:30 thorpej Exp $");
 
 #include "opt_intrdebug.h"
 #include "opt_multiprocessor.h"
@@ -1052,9 +1052,24 @@
 			 * If this interrupt source is being moved, don't
 			 * unmask it at the hw.
 			 */
-			if (! source->is_distribute_pending)
+			if (! source->is_distribute_pending) {
 				(*pic->pic_hwunmask)(pic, ih->ih_pin);
-			force_pending = true;
+			}
+
+			/*
+			 * For level-sensitive interrupts, the hardware
+			 * will let us know.  For everything else, we
+			 * need to explicitly handle interrupts that
+			 * happened when when the source was masked.
+			 */
+			const uint32_t bit = (1U << ih->ih_slot);
+			if (ci->ci_imasked & bit) {
+				ci->ci_imasked &= ~bit;
+				if (source->is_type != IST_LEVEL) {
+					ci->ci_ipending |= bit;
+					force_pending = true;
+				}
+			}
 		}
 	}