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; + } + } } }