Module Name: src
Committed By: thorpej
Date: Mon Dec 30 23:32:30 UTC 2019
Modified Files:
src/sys/arch/amd64/amd64: genassym.cf vector.S
src/sys/arch/i386/i386: genassym.cf vector.S
src/sys/arch/x86/include: cpu.h
src/sys/arch/x86/x86: intr.c
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.79 -r1.80 src/sys/arch/amd64/amd64/genassym.cf
cvs rdiff -u -r1.72 -r1.73 src/sys/arch/amd64/amd64/vector.S
cvs rdiff -u -r1.116 -r1.117 src/sys/arch/i386/i386/genassym.cf
cvs rdiff -u -r1.84 -r1.85 src/sys/arch/i386/i386/vector.S
cvs rdiff -u -r1.115 -r1.116 src/sys/arch/x86/include/cpu.h
cvs rdiff -u -r1.149 -r1.150 src/sys/arch/x86/x86/intr.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/arch/amd64/amd64/genassym.cf
diff -u src/sys/arch/amd64/amd64/genassym.cf:1.79 src/sys/arch/amd64/amd64/genassym.cf:1.80
--- src/sys/arch/amd64/amd64/genassym.cf:1.79 Sun Dec 22 15:09:39 2019
+++ src/sys/arch/amd64/amd64/genassym.cf Mon Dec 30 23:32:29 2019
@@ -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_ILEVEL offsetof(struct
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)
Index: src/sys/arch/amd64/amd64/vector.S
diff -u src/sys/arch/amd64/amd64/vector.S:1.72 src/sys/arch/amd64/amd64/vector.S:1.73
--- src/sys/arch/amd64/amd64/vector.S:1.72 Sun Dec 22 15:09:39 2019
+++ src/sys/arch/amd64/amd64/vector.S Mon Dec 30 23:32:29 2019
@@ -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 @@ IDTVEC(handle_ ## name ## num) ;\
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 @@ IDTVEC(handle_ ## name ## num) ;\
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 @@ IDTVEC(handle_ ## name ## num) ;\
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) ;\
Index: src/sys/arch/i386/i386/genassym.cf
diff -u src/sys/arch/i386/i386/genassym.cf:1.116 src/sys/arch/i386/i386/genassym.cf:1.117
--- src/sys/arch/i386/i386/genassym.cf:1.116 Sun Dec 22 15:09:39 2019
+++ src/sys/arch/i386/i386/genassym.cf Mon Dec 30 23:32:29 2019
@@ -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_SIGNATURE offsetof(struc
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)
Index: src/sys/arch/i386/i386/vector.S
diff -u src/sys/arch/i386/i386/vector.S:1.84 src/sys/arch/i386/i386/vector.S:1.85
--- src/sys/arch/i386/i386/vector.S:1.84 Sun Dec 22 15:09:39 2019
+++ src/sys/arch/i386/i386/vector.S Mon Dec 30 23:32:29 2019
@@ -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 @@ IDTVEC(intr_ ## name ## num) ;\
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 @@ IDTVEC(intr_ ## name ## num) ;\
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 @@ IDTVEC(intr_ ## name ## num) ;\
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) ;\
Index: src/sys/arch/x86/include/cpu.h
diff -u src/sys/arch/x86/include/cpu.h:1.115 src/sys/arch/x86/include/cpu.h:1.116
--- src/sys/arch/x86/include/cpu.h:1.115 Sun Dec 1 15:34:46 2019
+++ src/sys/arch/x86/include/cpu.h Mon Dec 30 23:32:29 2019
@@ -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 cpu_info {
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];
Index: src/sys/arch/x86/x86/intr.c
diff -u src/sys/arch/x86/x86/intr.c:1.149 src/sys/arch/x86/x86/intr.c:1.150
--- src/sys/arch/x86/x86/intr.c:1.149 Sun Dec 22 16:50:03 2019
+++ src/sys/arch/x86/x86/intr.c Mon Dec 30 23:32:30 2019
@@ -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 @@ intr_mask_xcall(void *arg1, void *arg2)
* 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;
+ }
+ }
}
}
