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.
This commit is contained in:
thorpej 2019-12-30 23:32:29 +00:00
parent 0e79d963e9
commit 9473b3b786
6 changed files with 45 additions and 16 deletions

View File

@ -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 cpu_info, ci_ilevel)
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)

View File

@ -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) ;\

View File

@ -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(struct cpu_info, ci_signature)
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)

View File

@ -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) ;\

View File

@ -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];

View File

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