Subject: Re: args of mips3_clockintr()
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
From: Garrett D'Amore <garrett_damore@tadpole.com>
List: port-mips
Date: 09/09/2006 21:31:55
Izumi Tsutsui wrote:
> Currently common mips3_clockintr() takes two args (pc and status),
> but I'd like to change it to take (struct clockframe *) directly.
>   

I don't have a problem with that change.  Frankly, it would probably be
cleaner.
But more below.

> Background:
>
> I'm cleaning up cobalt port after MI mips3_clockintr() migration,
> then I just remember a problem which might happen on calling
> hardclock(9) on MIPS, as mentioned in sys/arch/arc/TODO:
>
> ---
> 	also, current MIPS interrupt handler has overblocking and
> 	other problems as follows:
>
> 	-   SR_INT_IE should be enabled before calling hardclock().
> 	    Since this is not done currently, spllowersoftclock()
> 	    on hardclock() doesn't have effect, and softclock() is
> 	    handled with all interrupt disabled in this case.
> 		-> overblocking, possibly causes missing hardclock()
> ---
>
> As noted above, we should set SR_INT_IE bit before calling hardclock()
> if all interrupts were enabled (in status) and there is no other
> pending interrupt (in ipending).
>   

Hmm.... this sounds like maybe a port that isn't using the separate
interrupts for soft interrupts?  Or am I misunderstanding something.

I guess I should take a good hard look at hardlock(9) implementation.

    -- Garrett
> Furthermore, if there is any other pending interrupt on calling
> hardclock(9), we have to prevent hardclock(9) from calling
> spllowersoftclock(9) (it uses CLKF_BASEPRI() macro, which checks
> status (SR) in clockframe) otherwise other interrupt service handlers
> won't be protected properly.
>
> Some of mips ports have had some code to handle these issue
> by tweaking SR bit or SR in struct clockframe:
> ---
>  :
> 	cf.pc = pc;
> 	cf.sr = status;
>
> 	if ((status & MIPS_INT_MASK) == MIPS_INT_MASK) {
> 		if ((ipending & MIPS_INT_MASK &
> 		     ~MIPS_INT_MASK_0) == 0) {
> 			/*
> 			 * If all interrupts were enabled and
> 			 * there is no pending interrupts,
> 			 * set MIPS_SR_INT_IE so that
> 			 * spllowerclock() in hardclock()
> 			 * works properly.
> 			 */
> 			_splset(MIPS_SR_INT_IE);
> 		} else {
> 			/*
> 			 * If there are any pending interrputs,
> 			 * clear MIPS_SR_INT_IE in cf.sr so that
> 			 * spllowerclock() in hardclock() will
> 			 * not happen.
> 			 */
> 			cf.sr &= ~MIPS_SR_INT_IE;
> 		}
> 	}
> 	hardclock(&cf);
>  :
> ---
>
> I admit that this is a hack, so I'd appreciate any other suggestions.
>
>
> BTW, there are still several ports which don't set MIPS_SR_INT_IE
> bit in SR before calling softintr_dispatch() if there is no other
> hardware intterrupt with the softintr together.
> (algor, evbmips, pmax etc?)
>
> Is it better to enable it explicitly?
> ---
> Izumi Tsutsui
>   


-- 
Garrett D'Amore, Principal Software Engineer
Tadpole Computer / Computing Technologies Division,
General Dynamics C4 Systems
http://www.tadpolecomputer.com/
Phone: 951 325-2134  Fax: 951 325-2191