NetBSD Problem Report #41372

From bouyer@antioche.eu.org  Wed May  6 20:29:10 2009
Return-Path: <bouyer@antioche.eu.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id D351763B8DF
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  6 May 2009 20:29:10 +0000 (UTC)
Message-Id: <200905062029.n46KT7qs006850@rochebonne.antioche.eu.org>
Date: Wed, 6 May 2009 22:29:07 +0200 (CEST)
From: Manuel Bouyer <bouyer@antioche.eu.org>
Reply-To: bouyer@antioche.eu.org
To: gnats-bugs@gnats.NetBSD.org
Subject: inline functions writing psr needs __insn_barrier()
X-Send-Pr-Version: 3.95

>Number:         41372
>Category:       port-sparc
>Synopsis:       inline functions writing psr needs __insn_barrier()
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    martin
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed May 06 20:30:01 +0000 2009
>Closed-Date:    Mon Aug 03 19:53:09 +0000 2009
>Last-Modified:  Mon Aug 03 19:53:09 +0000 2009
>Originator:     Manuel Bouyer
>Release:        NetBSD 5.0
>Organization:
>Environment:
5.0_STABLE sources
Architecture: sparc
Machine: sparc
>Description:
	if splfoo() are inline or macro, the compiler may optimise things
	wrongly around it. An example is softint_schedule(), where the
	second check for SOFTINT_PENDING is optimised out. See PR kern/38637
	for details.
	To avoid this, splx(), splraiseipl(), spl0() and I guess
	setpsr() should be a barrier for the compiler;
	a __insn_barrier() will do it.
>How-To-Repeat:
	code inspection
	dissasembly of softint_schedule(). See also kern/38637.
>Fix:
	The patch below should fix it (I confirm that the softint_schedule()
	dissasembly is correct with this patch). A better way may be to
	use memory clobber instruction in the inline assembly, but I'm
	not familiar enough with sparc assembly to fix it this way.

Index: include/psl.h
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc/include/psl.h,v
retrieving revision 1.44
diff -u -p -r1.44 psl.h
--- include/psl.h	19 Feb 2007 02:57:40 -0000	1.44
+++ include/psl.h	6 May 2009 20:23:24 -0000
@@ -254,6 +254,7 @@ setpsr(int newpsr)
 {
 	__asm volatile("wr %0,0,%%psr" : : "r" (newpsr));
 	__asm volatile("nop; nop; nop");
+	__insn_barrier();
 }

 static __inline void
@@ -261,6 +262,7 @@ spl0(void)
 {
 	int psr, oldipl;

+	__insn_barrier();
 	/*
 	 * wrpsr xors two values: we choose old psr and old ipl here,
 	 * which gives us the same value as the old psr but with all
@@ -286,11 +288,13 @@ spl0(void)
 static __inline void name(void) \
 { \
 	int psr; \
+	__insn_barrier(); \
 	__asm volatile("rd %%psr,%0" : "=r" (psr)); \
 	psr &= ~PSR_PIL; \
 	__asm volatile("wr %0,%1,%%psr" : : \
 	    "r" (psr), "n" ((newipl) << 8)); \
 	__asm volatile("nop; nop; nop"); \
+	__insn_barrier(); \
 }

 _SPLSET(spllowerschedclock, IPL_SCHED)
@@ -318,14 +322,16 @@ splraiseipl(ipl_cookie_t icookie)

 	oldipl = psr & PSR_PIL;
 	newipl <<= 8;
-	if (newipl <= oldipl)
+	if (newipl <= oldipl) {
 		return (oldipl);
+	}

 	psr = (psr & ~oldipl) | newipl;

 	__asm volatile("wr %0,0,%%psr" : : "r" (psr));
 	__asm volatile("nop; nop; nop");

+	__insn_barrier();
 	return (oldipl);
 }

@@ -344,7 +350,8 @@ static __inline void
 splx(int newipl)
 {
 	int psr;
-
+	
+	__insn_barrier();
 	__asm volatile("rd %%psr,%0" : "=r" (psr));
 	__asm volatile("wr %0,%1,%%psr" : : \
 	    "r" (psr & ~PSR_PIL), "rn" (newipl));

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: port-sparc-maintainer->martin
Responsible-Changed-By: martin@NetBSD.org
Responsible-Changed-When: Sat, 16 May 2009 17:18:42 +0000
Responsible-Changed-Why:
I think I fixed it


State-Changed-From-To: open->feedback
State-Changed-By: martin@NetBSD.org
State-Changed-When: Sat, 16 May 2009 17:18:42 +0000
State-Changed-Why:
I commited a slightly different change, can you please verify?
If it's ok, please also request pullup.


From: Martin Husemann <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41372 CVS commit: src/sys/arch/sparc/include
Date: Sat, 16 May 2009 17:16:12 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sat May 16 17:16:12 UTC 2009

 Modified Files:
 	src/sys/arch/sparc/include: psl.h

 Log Message:
 Add memory clobbers to the inline assembler modifying/testing the %psr
 register, to avoid the compiler reordering instructions out of critical
 sections. Should fix PR port-sparc/41372.


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.45 src/sys/arch/sparc/include/psl.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Manuel Bouyer <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41372 CVS commit: [netbsd-4] src/sys/arch/sparc/include
Date: Mon, 18 May 2009 18:23:13 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Mon May 18 18:23:13 UTC 2009

 Modified Files:
 	src/sys/arch/sparc/include [netbsd-4]: psl.h

 Log Message:
 Pull up following revision(s) (requested by martin in ticket #1317):
 	sys/arch/sparc/include/psl.h: revision 1.45
 Add memory clobbers to the inline assembler modifying/testing the %psr
 register, to avoid the compiler reordering instructions out of critical
 sections. Should fix PR port-sparc/41372.


 To generate a diff of this commit:
 cvs rdiff -u -r1.40 -r1.40.12.1 src/sys/arch/sparc/include/psl.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Manuel Bouyer <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41372 CVS commit: [netbsd-4-0] src/sys/arch/sparc/include
Date: Mon, 18 May 2009 18:23:47 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Mon May 18 18:23:47 UTC 2009

 Modified Files:
 	src/sys/arch/sparc/include [netbsd-4-0]: psl.h

 Log Message:
 Pull up following revision(s) (requested by martin in ticket #1317):
 	sys/arch/sparc/include/psl.h: revision 1.45
 Add memory clobbers to the inline assembler modifying/testing the %psr
 register, to avoid the compiler reordering instructions out of critical
 sections. Should fix PR port-sparc/41372.


 To generate a diff of this commit:
 cvs rdiff -u -r1.40 -r1.40.18.1 src/sys/arch/sparc/include/psl.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Manuel Bouyer <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41372 CVS commit: [netbsd-5] src/sys/arch/sparc/include
Date: Mon, 18 May 2009 19:54:56 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Mon May 18 19:54:56 UTC 2009

 Modified Files:
 	src/sys/arch/sparc/include [netbsd-5]: psl.h

 Log Message:
 Pull up following revision(s) (requested by martin in ticket #764):
 	sys/arch/sparc/include/psl.h: revision 1.45
 Add memory clobbers to the inline assembler modifying/testing the %psr
 register, to avoid the compiler reordering instructions out of critical
 sections. Should fix PR port-sparc/41372.


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.44.56.1 src/sys/arch/sparc/include/psl.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Manuel Bouyer <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41372 CVS commit: [netbsd-5-0] src/sys/arch/sparc/include
Date: Mon, 18 May 2009 19:55:35 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Mon May 18 19:55:35 UTC 2009

 Modified Files:
 	src/sys/arch/sparc/include [netbsd-5-0]: psl.h

 Log Message:
 Pull up following revision(s) (requested by martin in ticket #764):
 	sys/arch/sparc/include/psl.h: revision 1.45
 Add memory clobbers to the inline assembler modifying/testing the %psr
 register, to avoid the compiler reordering instructions out of critical
 sections. Should fix PR port-sparc/41372.


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.44.62.1 src/sys/arch/sparc/include/psl.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: feedback->closed
State-Changed-By: dsl@NetBSD.org
State-Changed-When: Mon, 03 Aug 2009 19:53:09 +0000
State-Changed-Why:
seems to have been pulled up, so I guess it is ok!


>Unformatted:

Home	PR Database Search