Re: powerpc/pic/intr.c IPL_HIGH vs IPL_NONE

[KIYOHARA Takashi <kiyohara%kk.iij4u.or.jp@localhost>]

Hi! all,


May I do revert of this change for bebox, ofppc (Openpic+8259), and
macppc (Ohare)?

Thanks,
--
kiyohara

From: KIYOHARA Takashi <kiyohara%kk.iij4u.or.jp@localhost>
Date: Wed, 03 Oct 2012 22:46:06 +0900 (JST)

> Hi! macallan@ and all,
> 
> 
> I apologize multi-post.
> I add more descriptions.
> -------
> 
> My bebox hangs up at boot time since change v 1.23 pic/intr.c.
> 
>   
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/powerpc/pic/intr.c.diff?r1=1.22&r2=1.23&only_with_tag=MAIN&f=h
> 
> 
> The hangup reason is changing imask[IPL_HIGH] to 0x3ffe0000 from 0x7ffe0000.
> The bit30 indicate establish PIC I8259 with IPL_NONE on bebox.
> 
> sys/arch/bebox/bebox/machdep.c:cpu_startup() ---
> 
>         /*
>          * set up i8259 as a cascade on BeInterruptController irq 26.
>          */
>         intr_establish(16 + 26, IST_LEVEL, IPL_NONE, pic_handle_intr, 
> isa_pic);
> 
> 
> intr_establish() sets IPL_HIGH to is->is_ipl in powerpc/pic/intr.c.
> However it sets IPL_NONE to ih->ih_ipl.
> 
> sys/arch/powerpc/pic/intr.c:intr_establish() ---
> 
>       int maxipl = ipl;
> 
>         if (maxipl == IPL_NONE)
>                 maxipl = IPL_HIGH;
>         :
> 
>         ih->ih_ipl = ipl;
> 
>         :
> 
>         is->is_ipl = maxipl;
> 
> 
> 
> Index: intr.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/powerpc/pic/intr.c,v
> retrieving revision 1.22
> retrieving revision 1.23
> diff -u -r1.22 -r1.23
> --- intr.c      7 Jun 2012 04:37:09 -0000       1.22
> +++ intr.c      31 Aug 2012 13:12:52 -0000      1.23
> 
> @@ -362,7 +363,9 @@
>  
>         /* First, figure out which ipl each IRQ uses. */
>         for (irq = 0, is = intrsources; irq < NVIRQ; irq++, is++) {
> -               newmask[is->is_ipl] |= PIC_VIRQ_TO_MASK(irq);
> +               for (ih = is->is_hand; ih != NULL; ih = ih->ih_next) {
> +                       newmask[ih->ih_ipl] |= PIC_VIRQ_TO_MASK(irq);
> +               }
>         }
>  
>         /*
> 
> Shall we call intr_establish() with IPL_HIGH?
> Or more better idea?
> 
> Also I think, this problem happen on sandpoint(ENCPP1, SANDPOINT).
> 
> 
> In the first place, I don't understand this commit log by macallon@.
> 
>   
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/powerpc/pic/intr.c.diff?r1=1.22&r2=1.23&only_with_tag=MAIN&f=h
> 
>   when calculating per-IPL virq masks, take into account that shared IRQs may
>   request different IPLs.
>   fixes vr(4)-related problems seen on ofppc
> 
> 
> The is->is_ipl is always set maxipl for shared IRQs.  What problem is this? 
> 
> Thanks,
> --
> kiyohara