Take the kernel lock and call the actual interrupt handler from a

single c function. This will hopefully make easier to stop taking
the kernel lock when running "mp safe" interrupt handlers.

help from ok kettenis

6 files changed, 66 insertions, 84 deletions

diff --git a/sys/arch/amd64/amd64/intr.c b/sys/arch/amd64/amd64/intr.c
index a49d7828b58..6175d7de464 100644
--- a/sys/arch/amd64/amd64/intr.c
+++ b/sys/arch/amd64/amd64/intr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: intr.c,v 1.32 2012/12/05 23:20:10 deraadt Exp $	*/
+/*	$OpenBSD: intr.c,v 1.33 2013/05/12 14:15:31 ratchov Exp $	*/
 /*	$NetBSD: intr.c,v 1.3 2003/03/03 22:16:20 fvdl Exp $	*/
 
 /*
@@ -523,6 +523,26 @@ intr_disestablish(struct intrhand *ih)
 	simple_unlock(&ci->ci_slock);
 }
 
+int
+intr_handler(struct intrframe *frame, struct intrhand *ih)
+{
+	int rc;
+#ifdef MULTIPROCESSOR
+	int need_lock;
+
+	need_lock = frame->if_ppl < IPL_SCHED;
+
+	if (need_lock)
+		__mp_lock(&kernel_lock);
+#endif
+	rc = (*ih->ih_fun)(ih->ih_arg ? ih->ih_arg : frame);
+#ifdef MULTIPROCESSOR
+	if (need_lock)
+		__mp_unlock(&kernel_lock);
+#endif
+	return rc;
+}
+
 #define CONCAT(x,y)	__CONCAT(x,y)
 
 /*
@@ -608,20 +628,6 @@ cpu_intr_init(struct cpu_info *ci)
 
 #ifdef MULTIPROCESSOR
 void
-x86_intlock(struct intrframe iframe)
-{
-	if (iframe.if_ppl < IPL_SCHED)
-		__mp_lock(&kernel_lock);
-}
-
-void
-x86_intunlock(struct intrframe iframe)
-{
-	if (iframe.if_ppl < IPL_SCHED)
-		__mp_unlock(&kernel_lock);
-}
-
-void
 x86_softintlock(void)
 {
 	__mp_lock(&kernel_lock);
diff --git a/sys/arch/amd64/amd64/vector.S b/sys/arch/amd64/amd64/vector.S
index c4ddc234f83..128620b00ae 100644
--- a/sys/arch/amd64/amd64/vector.S
+++ b/sys/arch/amd64/amd64/vector.S
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vector.S,v 1.32 2012/07/09 16:01:16 deraadt Exp $	*/
+/*	$OpenBSD: vector.S,v 1.33 2013/05/12 14:15:31 ratchov Exp $	*/
 /*	$NetBSD: vector.S,v 1.5 2004/06/28 09:13:11 fvdl Exp $	*/
 
 /*
@@ -423,13 +423,6 @@ IDTVEC(resume_lapic_ltimer)
 	INTRFASTEXIT
 #endif /* NLAPIC > 0 */
 
-#ifdef MULTIPROCESSOR
-#define LOCK_KERNEL	movq %rsp, %rdi; call _C_LABEL(x86_intlock)
-#define UNLOCK_KERNEL	movq %rsp, %rdi; call _C_LABEL(x86_intunlock)
-#else
-#define LOCK_KERNEL
-#define UNLOCK_KERNEL
-#endif
 
 #define voidop(num)
 
@@ -471,17 +464,14 @@ IDTVEC(intr_##name##num)						;\
 	sti								;\
 	incl	CPUVAR(IDEPTH)						;\
 	movq	IS_HANDLERS(%r14),%rbx					;\
-	LOCK_KERNEL							;\
 6:									\
 	movl	IH_LEVEL(%rbx),%r12d					;\
 	cmpl	%r13d,%r12d						;\
 	jle	7f							;\
-	movq	IH_ARG(%rbx),%rdi					;\
-	testq	%rdi, %rdi						;\
-	jnz	8f							;\
+	movl	%r12d,CPUVAR(ILEVEL)					;\
+	movq	%rbx, %rsi						;\
 	movq	%rsp, %rdi						;\
-8:	movl	%r12d,CPUVAR(ILEVEL)					;\
-	call	*IH_FUN(%rbx)		/* call it */			;\
+	call	_C_LABEL(intr_handler)	/* call it */			;\
 	orl	%eax,%eax		/* should it be counted? */	;\
 	jz	4f			/* no, skip it */		;\
 	incq	IH_COUNT(%rbx)		/* count the intrs */		;\
@@ -493,14 +483,12 @@ IDTVEC(intr_##name##num)						;\
 	testq	%rbx,%rbx						;\
 	jnz	6b							;\
 5:									\
-	UNLOCK_KERNEL							;\
 	cli								;\
 	unmask(num)			/* unmask it in hardware */	;\
 	late_ack(num)							;\
 	sti								;\
 	jmp	_C_LABEL(Xdoreti)	/* lower spl and do ASTs */	;\
 7:									\
-	UNLOCK_KERNEL							;\
 	cli								;\
 	movq	$(1 << num),%rax					;\
 	orq     %rax,CPUVAR(IPENDING)					;\
diff --git a/sys/arch/amd64/include/intr.h b/sys/arch/amd64/include/intr.h
index fdb6d236b44..a4f6674f12e 100644
--- a/sys/arch/amd64/include/intr.h
+++ b/sys/arch/amd64/include/intr.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: intr.h,v 1.23 2011/04/16 00:40:58 deraadt Exp $	*/
+/*	$OpenBSD: intr.h,v 1.24 2013/05/12 14:15:31 ratchov Exp $	*/
 /*	$NetBSD: intr.h,v 1.2 2003/05/04 22:01:56 fvdl Exp $	*/
 
 /*-
@@ -204,6 +204,7 @@ int intr_allocate_slot(struct pic *, int, int, int, struct cpu_info **, int *,
 void *intr_establish(int, struct pic *, int, int, int, int (*)(void *),
 	    void *, const char *);
 void intr_disestablish(struct intrhand *);
+int intr_handler(struct intrframe *, struct intrhand *);
 void cpu_intr_init(struct cpu_info *);
 int intr_find_mpmapping(int bus, int pin, int *handle);
 void intr_printconfig(void);
@@ -213,8 +214,6 @@ int x86_send_ipi(struct cpu_info *, int);
 int x86_fast_ipi(struct cpu_info *, int);
 void x86_broadcast_ipi(int);
 void x86_ipi_handler(void);
-void x86_intlock(struct intrframe);
-void x86_intunlock(struct intrframe);
 void x86_softintlock(void);
 void x86_softintunlock(void);
 void x86_setperf_ipi(struct cpu_info *);
diff --git a/sys/arch/i386/i386/apicvec.s b/sys/arch/i386/i386/apicvec.s
index 020e499f4c6..6d191a9c193 100644
--- a/sys/arch/i386/i386/apicvec.s
+++ b/sys/arch/i386/i386/apicvec.s
@@ -1,4 +1,4 @@
-/* $OpenBSD: apicvec.s,v 1.26 2012/07/09 16:09:47 deraadt Exp $ */
+/* $OpenBSD: apicvec.s,v 1.27 2013/05/12 14:15:31 ratchov Exp $ */
 /* $NetBSD: apicvec.s,v 1.1.2.2 2000/02/21 21:54:01 sommerfeld Exp $ */
 
 /*-
@@ -279,17 +279,12 @@ _C_LABEL(Xintr_##name##num):						\
 	testl	%ebx,%ebx						;\
 	jz      _C_LABEL(Xstray_##name##num)				;\
 	APIC_STRAY_INIT			/* nobody claimed it yet */	;\
-7:									 \
-	incl	CPUVAR(IDEPTH)						;\
-	LOCK_KERNEL(IF_PPL(%esp))					;\
-	movl	IH_ARG(%ebx),%eax	/* get handler arg */		;\
-	testl	%eax,%eax						;\
-	jnz	6f							;\
-	movl	%esp,%eax		/* 0 means frame pointer */	;\
-6:									 \
-	pushl	%eax							;\
-	call	*IH_FUN(%ebx)		/* call it */			;\
-	addl	$4,%esp			/* toss the arg */		;\
+7:	incl	CPUVAR(IDEPTH)						;\
+	movl	%esp, %eax		/* save frame pointer in eax */	;\
+	pushl	%ebx			/* arg 2: ih structure */	;\
+	pushl	%eax			/* arg 1: frame pointer */	;\
+	call	_C_LABEL(intr_handler)	/* call it */			;\
+	addl	$8, %esp		/* toss args */			;\
 	APIC_STRAY_INTEGRATE		/* maybe he claimed it */	;\
 	orl	%eax,%eax		/* should it be counted? */	;\
 	jz	4f							;\
@@ -299,11 +294,9 @@ _C_LABEL(Xintr_##name##num):						\
 	jne	4f			/* if no shared edges ... */	;\
 	orl	%eax,%eax		/* ... 1 means stop trying */	;\
 	js	4f							;\
-1:	UNLOCK_KERNEL(IF_PPL(%esp))					;\
-	decl	CPUVAR(IDEPTH)						;\
+1:	decl	CPUVAR(IDEPTH)						;\
 	jmp	8f							;\
-4:	UNLOCK_KERNEL(IF_PPL(%esp))					;\
-	decl	CPUVAR(IDEPTH)						;\
+4:	decl	CPUVAR(IDEPTH)						;\
 	movl	IH_NEXT(%ebx),%ebx	/* next handler in chain */	;\
 	testl	%ebx,%ebx						;\
 	jnz	7b							;\
diff --git a/sys/arch/i386/i386/machdep.c b/sys/arch/i386/i386/machdep.c
index 17e0c0cd999..2faff50ffd2 100644
--- a/sys/arch/i386/i386/machdep.c
+++ b/sys/arch/i386/i386/machdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: machdep.c,v 1.520 2013/02/13 21:21:34 martynas Exp $	*/
+/*	$OpenBSD: machdep.c,v 1.521 2013/05/12 14:15:31 ratchov Exp $	*/
 /*	$NetBSD: machdep.c,v 1.214 1996/11/10 03:16:17 thorpej Exp $	*/
 
 /*-
@@ -166,6 +166,7 @@ extern struct proc *npxproc;
 #endif /* NCOM > 0 */
 
 void	replacesmap(void);
+int     intr_handler(struct intrframe *, struct intrhand *);
 
 /* the following is used externally (sysctl_hw) */
 char machine[] = MACHINE;
@@ -3867,20 +3868,6 @@ splassert_check(int wantipl, const char *func)
 
 #ifdef MULTIPROCESSOR
 void
-i386_intlock(int ipl)
-{
-	if (ipl < IPL_SCHED)
-		__mp_lock(&kernel_lock);
-}
-
-void
-i386_intunlock(int ipl)
-{
-	if (ipl < IPL_SCHED)
-		__mp_unlock(&kernel_lock);
-}
-
-void
 i386_softintlock(void)
 {
 	__mp_lock(&kernel_lock);
@@ -3947,3 +3934,24 @@ spllower(int ncpl)
 	splx(ncpl);
 	return (ocpl);
 }
+
+int
+intr_handler(struct intrframe *frame, struct intrhand *ih)
+{
+	int rc;
+#ifdef MULTIPROCESSOR
+	int need_lock;
+
+	need_lock = frame->if_ppl < IPL_SCHED;
+
+	if (need_lock)
+		__mp_lock(&kernel_lock);
+#endif
+	rc = (*ih->ih_fun)(ih->ih_arg ? ih->ih_arg : frame);
+#ifdef MULTIPROCESSOR
+	if (need_lock)
+		__mp_unlock(&kernel_lock);
+#endif
+	return rc;
+}
+
diff --git a/sys/arch/i386/i386/vector.s b/sys/arch/i386/i386/vector.s
index f9c286fb634..926570a4be8 100644
--- a/sys/arch/i386/i386/vector.s
+++ b/sys/arch/i386/i386/vector.s
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vector.s,v 1.16 2011/09/22 12:17:04 deraadt Exp $	*/
+/*	$OpenBSD: vector.s,v 1.17 2013/05/12 14:15:31 ratchov Exp $	*/
 /*	$NetBSD: vector.s,v 1.32 1996/01/07 21:29:47 mycroft Exp $	*/
 
 /*
@@ -57,14 +57,6 @@
 
 	.globl	_C_LABEL(isa_strayintr)
 
-#ifdef MULTIPROCESSOR
-#define LOCK_KERNEL(ipl)	pushl ipl; call _C_LABEL(i386_intlock);	addl $4,%esp
-#define UNLOCK_KERNEL(ipl)	pushl ipl; call _C_LABEL(i386_intunlock); addl $4,%esp
-#else
-#define LOCK_KERNEL(ipl)
-#define UNLOCK_KERNEL(ipl)
-#endif
-
 #define voidop(num)
 
 /*
@@ -122,14 +114,11 @@ _C_LABEL(Xintr_##name##num):						;\
 	jz	_C_LABEL(Xstray_##name##num)	/* no handlers; we're stray */	;\
 	STRAY_INITIALIZE		/* nobody claimed it yet */	;\
 	incl	CPUVAR(IDEPTH)						;\
-	LOCK_KERNEL(IF_PPL(%esp))					;\
-7:	movl	IH_ARG(%ebx),%eax	/* get handler arg */		;\
-	testl	%eax,%eax						;\
-	jnz	4f							;\
-	movl	%esp,%eax		/* 0 means frame pointer */	;\
-4:	pushl	%eax							;\
-	call	*IH_FUN(%ebx)		/* call it */			;\
-	addl	$4,%esp			/* toss the arg */		;\
+7:	movl	%esp, %eax		/* save frame pointer in eax */	;\
+	pushl	%ebx			/* arg 2: ih structure */	;\
+	pushl	%eax			/* arg 1: frame pointer */	;\
+	call	_C_LABEL(intr_handler)	/* call it */			;\
+	addl	$8, %esp		/* toss args */			;\
 	STRAY_INTEGRATE			/* maybe he claimed it */	;\
 	orl	%eax,%eax		/* should it be counted? */	;\
 	jz	5f			/* no, skip it */		;\
@@ -142,8 +131,7 @@ _C_LABEL(Xintr_##name##num):						;\
 5:	movl	IH_NEXT(%ebx),%ebx	/* next handler in chain */	;\
 	testl	%ebx,%ebx						;\
 	jnz	7b							;\
-8:	UNLOCK_KERNEL(IF_PPL(%esp))					;\
-	decl	CPUVAR(IDEPTH)						;\
+8:	decl	CPUVAR(IDEPTH)						;\
 	STRAY_TEST(name,num)		/* see if it's a stray */	;\
 6:	unmask(num)			/* unmask it in hardware */	;\
 	late_ack(num)							;\