NetBSD Problem Report #26158

Received: (qmail 27975 invoked by uid 605); 4 Jul 2004 00:46:27 -0000
Message-Id: <Pine.BSF.4.51.0407030401070.49962@vegeta.city-net.com>
Date: Sat, 3 Jul 2004 20:37:33 -0400 (EDT)
From: Matthew Orgass <darkstar@city-net.com>
Sender: gnats-bugs-owner@NetBSD.org
To: gnats-bugs@gnats.netbsd.org
Subject: cpu_intr problems

>Number:         26158
>Category:       port-hpcmips
>Synopsis:       cpu_intr problems
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    port-hpcmips-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 04 00:47:00 +0000 2004
>Closed-Date:    Sun Jan 16 10:47:58 +0000 2005
>Last-Modified:  Sun Jan 16 10:47:58 +0000 2005
>Originator:     darkstar@city-net.com
>Release:        NetBSD 2.0_BETA
>Organization:
>Environment:
>Description:

  The hpcmips cpu_intr routines are incorrect.  The vr code will allow
non-clock hard interrupts to interrupt hardclock and can allow stack
overflow during high interrupt activity.  The TX code will keep hard
interrupts blocked while executing soft ISRs if a hard and soft interrupt
occur at the same time and looks like it could give soft interrupts
priority over hard interrupts.  It can also allow stack overflow.

>How-To-Repeat:
>Fix:

  This is just a fix for vr.c.  Something similar should be done in
tx39icu.c.  This patch lowers the priority in strict order and never
unmasks the lowest pending interrupt.  This will bound stack usage.

Index: vr.c
===================================================================
RCS file: /cvsroot/src/sys/arch/hpcmips/vr/vr.c,v
retrieving revision 1.44
diff -u -r1.44 vr.c
--- vr.c	25 Oct 2003 18:04:34 -0000	1.44
+++ vr.c	3 Jul 2004 07:38:54 -0000
@@ -546,33 +546,27 @@
 {
 	uvmexp.intrs++;

+	/* Deal with unneded compare interrupts occasionally so that we can
+	 * keep spllowersoftclock. */
 	if (ipending & MIPS_INT_MASK_5) {
-		/*
-		 * spl* uses MIPS_INT_MASK not MIPS3_INT_MASK. it causes
-		 * INT5 interrupt.
-		 */
-		mips3_cp0_compare_write(mips3_cp0_count_read());
+		mips3_cp0_compare_write(0);
 	}

-	/* for spllowersoftclock */
-	_splset(((status & ~cause) & MIPS_HARD_INT_MASK) | MIPS_SR_INT_IE);
-
 	if (ipending & MIPS_INT_MASK_1) {
+		_splset(MIPS_SR_INT_IE); /* for spllowersoftclock */
 		(*vr_intr_handler[1])(vr_intr_arg[1], pc, status);
-
-		cause &= ~MIPS_INT_MASK_1;
-		_splset(((status & ~cause) & MIPS_HARD_INT_MASK)
-		    | MIPS_SR_INT_IE);
 	}

 	if (ipending & MIPS_INT_MASK_0) {
+		_splset(MIPS_INT_MASK_1|MIPS_SR_INT_IE);
 		(*vr_intr_handler[0])(vr_intr_arg[0], pc, status);
-
-		cause &= ~MIPS_INT_MASK_0;
 	}
-	_splset(((status & ~cause) & MIPS_HARD_INT_MASK) | MIPS_SR_INT_IE);

-	softintr(ipending);
+	/* XXX sofintr priorities should be separated. */
+	if (ipending & (MIPS_SOFT_INT_MASK_1|MIPS_SOFT_INT_MASK_0)) {
+		_splset(MIPS_INT_MASK_1|MIPS_INT_MASK_0|MIPS_SR_INT_IE);
+		softintr(ipending);
+	}
 }

 void *
>Release-Note:
>Audit-Trail:

From: Matthew Orgass <darkstar@city-net.com>
To: gnats-bugs@netbsd.org, port-hpcmips-maintainer@netbsd.org
Cc:  
Subject: Re: port-hpcmips/26158: cpu_intr problems
Date: Sun, 4 Jul 2004 07:09:23 -0400 (EDT)

   It seems the patch to vr.c causes the display scroll (bicons/hpcfb) to
 go slow under somewhat heavy interrupts (1ms), but not heavier interrupts.
 Removing the spllowersoftclock in hardclock or setting CLKF_BASEPRI to 0
 both cause normal behavior (as does the old vr_intr).  I don't know why
 this is happening or why it didn't happen previously.

 Matthew Orgass
 darstar@city-net.com


From: Matthew Orgass <darkstar@city-net.com>
To: gnats-bugs@netbsd.org, port-hpcmips-maintainer@netbsd.org
Cc:  
Subject: Re: port-hpcmips/26158: cpu_intr problems
Date: Wed, 28 Jul 2004 17:14:02 -0400 (EDT)

   The slow scroll problem seems to appear only occasionally and seems to
 be based on kernel size or layout, particularly (perhaps) in the interrupt
 paths.  I think it is unrelated to this patch.  Furthermore, both the
 20040721 2.0_BETA GENERIC from releng and my 2.0 branch sources (modified
 in a few ways) without the patch will hang when starting userland.

   Here is a different patch that separates the soft interrupts and
 modifies the value passed to hardclock to prevent the possibility of
 spurrious interrupts (though I think other general interrupt code would
 catch this) if multiple interrupts happen at the same time with nothing
 previously masked.  I think this should be examined more later (in
 particular, checking for new soft interrupts after hard interrupts are run
 if the soft interrupts were not previously masked might be helpful in the
 common case of hard interrupt generates soft interrupt but is complicated
 by lowersoftclock;  also, softintr should be changed to softintr_dispatch,
 IMO), however hopfully this patch or something like it can make 2.0.

 Index: vr.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/hpcmips/vr/vr.c,v
 retrieving revision 1.44
 diff -u -r1.44 vr.c
 --- vr.c	25 Oct 2003 18:04:34 -0000	1.44
 +++ vr.c	5 Jul 2004 04:20:54 -0000
 @@ -546,33 +546,36 @@
  {
  	uvmexp.intrs++;

 +	/* Deal with unneded compare interrupts occasionally so that we can
 +	 * keep spllowersoftclock. */
  	if (ipending & MIPS_INT_MASK_5) {
 -		/*
 -		 * spl* uses MIPS_INT_MASK not MIPS3_INT_MASK. it causes
 -		 * INT5 interrupt.
 -		 */
 -		mips3_cp0_compare_write(mips3_cp0_count_read());
 +		mips3_cp0_compare_write(0);
  	}

 -	/* for spllowersoftclock */
 -	_splset(((status & ~cause) & MIPS_HARD_INT_MASK) | MIPS_SR_INT_IE);
 -
  	if (ipending & MIPS_INT_MASK_1) {
 -		(*vr_intr_handler[1])(vr_intr_arg[1], pc, status);
 -
 -		cause &= ~MIPS_INT_MASK_1;
 -		_splset(((status & ~cause) & MIPS_HARD_INT_MASK)
 -		    | MIPS_SR_INT_IE);
 +		_splset(MIPS_SR_INT_IE); /* for spllowersoftclock */
 +		/* Remove the lower priority pending bits from status so that
 +		 * spllowersoftclock will not happen if other interrupts are
 +		 * pending. */
 +		(*vr_intr_handler[1])(vr_intr_arg[1], pc, status & ~(ipending
 +		& (MIPS_INT_MASK_0|MIPS_SOFT_INT_MASK_0|MIPS_SOFT_INT_MASK_1)));
  	}

  	if (ipending & MIPS_INT_MASK_0) {
 +		_splset(MIPS_INT_MASK_1|MIPS_SR_INT_IE);
  		(*vr_intr_handler[0])(vr_intr_arg[0], pc, status);
 +	}

 -		cause &= ~MIPS_INT_MASK_0;
 +	if (ipending & MIPS_SOFT_INT_MASK_1) {
 +		_splset(MIPS_INT_MASK_1|MIPS_INT_MASK_0|MIPS_SR_INT_IE);
 +		softintr(MIPS_SOFT_INT_MASK_1);
  	}
 -	_splset(((status & ~cause) & MIPS_HARD_INT_MASK) | MIPS_SR_INT_IE);

 -	softintr(ipending);
 +	if (ipending & MIPS_SOFT_INT_MASK_0) {
 +		_splset(MIPS_SOFT_INT_MASK_1|MIPS_INT_MASK_1|MIPS_INT_MASK_0|
 +		    MIPS_SR_INT_IE);
 +		softintr(MIPS_SOFT_INT_MASK_0);
 +	}
  }

  void *


From: Matthew Orgass <darkstar@city-net.com>
To: gnats-bugs@netbsd.org, port-hpcmips-maintainer@netbsd.org
Cc:  
Subject: Re: port-hpcmips/26158: cpu_intr problems
Date: Wed, 28 Jul 2004 23:10:24 -0400 (EDT)

   I just thought to break into DDB during the slow scroll and see what was
 happening.  I frequently saw stacked interrupts, so it seems likely that
 the slow scroll problem is the same as the stack smashing problem that
 caused me to look at cpu_intr in the first place (and not due in any way
 to the cpu_intr changes).

 Matthew Orgass
 darkstar@city-net.com
State-Changed-From-To: open->closed
State-Changed-By: hamajima@netbsd.org
State-Changed-When: Sun, 16 Jan 2005 10:47:58 +0000
State-Changed-Why:
I commited this patch. thanks.
(vr.c:1.45)


>Unformatted:

Home	PR Database Search