[PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Valentine Barshak]

We really need to ack interrupts at mpic_teardown, since
not all platforms reset mpic at kernel start-up. For example,
kexec'ed kernel hangs on P.A. Semi if mpic_eoi() isn't called.

Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
---
 arch/powerpc/sysdev/mpic.c |    6 +-----
 1 files changed, 1 insertion(+), 5 deletions(-)

--- powerpc/arch/powerpc/sysdev/mpic.c	2008-02-26 15:56:57.000000000 +0300
+++ linux-2.6.new/arch/powerpc/sysdev/mpic.c	2008-04-03 23:00:45.000000000 +0400
@@ -1410,11 +1410,6 @@ void mpic_cpu_set_priority(int prio)
 	mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), prio);
 }
 
-/*
- * XXX: someone who knows mpic should check this.
- * do we need to eoi the ipi including for kexec cpu here (see xics comments)?
- * or can we reset the mpic in the new kernel?
- */
 void mpic_teardown_this_cpu(int secondary)
 {
 	struct mpic *mpic = mpic_primary;
@@ -1434,6 +1429,7 @@ void mpic_teardown_this_cpu(int secondar
 
 	/* Set current processor priority to max */
 	mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf);
+	mpic_eoi(mpic);
 
 	spin_unlock_irqrestore(&mpic_lock, flags);
 }

Re: [PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Benjamin Herrenschmidt]

On Thu, 2008-04-03 at 23:09 +0400, Valentine Barshak wrote:
> We really need to ack interrupts at mpic_teardown, since
> not all platforms reset mpic at kernel start-up. For example,
> kexec'ed kernel hangs on P.A. Semi if mpic_eoi() isn't called.
> 
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
> ---

Would be interesting to find out why it hangs tho... it shouldn't .

Ben.

>  arch/powerpc/sysdev/mpic.c |    6 +-----
>  1 files changed, 1 insertion(+), 5 deletions(-)
> 
> --- powerpc/arch/powerpc/sysdev/mpic.c	2008-02-26 15:56:57.000000000 +0300
> +++ linux-2.6.new/arch/powerpc/sysdev/mpic.c	2008-04-03 23:00:45.000000000 +0400
> @@ -1410,11 +1410,6 @@ void mpic_cpu_set_priority(int prio)
>  	mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), prio);
>  }
>  
> -/*
> - * XXX: someone who knows mpic should check this.
> - * do we need to eoi the ipi including for kexec cpu here (see xics comments)?
> - * or can we reset the mpic in the new kernel?
> - */
>  void mpic_teardown_this_cpu(int secondary)
>  {
>  	struct mpic *mpic = mpic_primary;
> @@ -1434,6 +1429,7 @@ void mpic_teardown_this_cpu(int secondar
>  
>  	/* Set current processor priority to max */
>  	mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf);
> +	mpic_eoi(mpic);
>  
>  	spin_unlock_irqrestore(&mpic_lock, flags);
>  }
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Olof Johansson]

On Fri, Apr 04, 2008 at 08:50:36AM +1100, Benjamin Herrenschmidt wrote:
> 
> On Thu, 2008-04-03 at 23:09 +0400, Valentine Barshak wrote:
> > We really need to ack interrupts at mpic_teardown, since
> > not all platforms reset mpic at kernel start-up. For example,
> > kexec'ed kernel hangs on P.A. Semi if mpic_eoi() isn't called.
> > 
> > Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
> > ---
> 
> Would be interesting to find out why it hangs tho... it shouldn't .

I haven't reproduced the problem and looked at hardware state yet, but I
would expect it to be because the openpic won't send another interrupt
until the previous is EOI'd, and the IPI is never EOI'd as far as I
can tell.

The XICS code does it explicitly already. I'm surprised it doesn't break
on the 970-based platforms actually...


-Olof

Re: [PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Segher Boessenkool]

>>> We really need to ack interrupts at mpic_teardown, since
>>> not all platforms reset mpic at kernel start-up. For example,
>>> kexec'ed kernel hangs on P.A. Semi if mpic_eoi() isn't called.
>>>
>>> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
>>> ---
>>
>> Would be interesting to find out why it hangs tho... it shouldn't .
>
> I haven't reproduced the problem and looked at hardware state yet, but 
> I
> would expect it to be because the openpic won't send another interrupt
> until the previous is EOI'd, and the IPI is never EOI'd as far as I
> can tell.
>
> The XICS code does it explicitly already. I'm surprised it doesn't 
> break
> on the 970-based platforms actually...

platforms/maple/setup.c and platforms/powermac/pic.c use 
MPIC_WANTS_RESET.
Is there a reason why we don't do that on every MPIC?  If there is such
a reason, the default should be to reset, only pseries and chrp and cell
and now pasemi do not use it.  It's the only sane way to get an MPIC 
into
a sane known state starting from <whatever> state.


Segher

Re: [PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Olof Johansson]

On Fri, Apr 04, 2008 at 07:38:27PM +0200, Segher Boessenkool wrote:
>>>> We really need to ack interrupts at mpic_teardown, since
>>>> not all platforms reset mpic at kernel start-up. For example,
>>>> kexec'ed kernel hangs on P.A. Semi if mpic_eoi() isn't called.
>>>>
>>>> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
>>>> ---
>>>
>>> Would be interesting to find out why it hangs tho... it shouldn't .
>>
>> I haven't reproduced the problem and looked at hardware state yet, but  
>> I
>> would expect it to be because the openpic won't send another interrupt
>> until the previous is EOI'd, and the IPI is never EOI'd as far as I
>> can tell.
>>
>> The XICS code does it explicitly already. I'm surprised it doesn't  
>> break
>> on the 970-based platforms actually...
>
> platforms/maple/setup.c and platforms/powermac/pic.c use  
> MPIC_WANTS_RESET.
> Is there a reason why we don't do that on every MPIC?  If there is such
> a reason, the default should be to reset, only pseries and chrp and cell
> and now pasemi do not use it.  It's the only sane way to get an MPIC  
> into
> a sane known state starting from <whatever> state.

I used to have the reset there on pasemi but due to some other issues I
ended up taking it out.

It's not like doing a final EOI is a big deal, I really don't see why
there's hassle about this patch.


-Olof

Re: [PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Segher Boessenkool]

>> platforms/maple/setup.c and platforms/powermac/pic.c use
>> MPIC_WANTS_RESET.
>> Is there a reason why we don't do that on every MPIC?  If there is 
>> such
>> a reason, the default should be to reset, only pseries and chrp and 
>> cell
>> and now pasemi do not use it.  It's the only sane way to get an MPIC
>> into
>> a sane known state starting from <whatever> state.
>
> I used to have the reset there on pasemi but due to some other issues I
> ended up taking it out.
>
> It's not like doing a final EOI is a big deal, I really don't see why
> there's hassle about this patch.

The patch is fine, don't get me wrong.  I'm just saying MPIC_WANTS_RESET
should be the default since pretty much everything wants it.  The few
platforms that don't can document why; with the current code, we cannot
see if you just forgot it or not.


Segher

Re: [PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Olof Johansson]

On Fri, Apr 04, 2008 at 08:55:01PM +0200, Segher Boessenkool wrote:
>>> platforms/maple/setup.c and platforms/powermac/pic.c use
>>> MPIC_WANTS_RESET.
>>> Is there a reason why we don't do that on every MPIC?  If there is  
>>> such
>>> a reason, the default should be to reset, only pseries and chrp and  
>>> cell
>>> and now pasemi do not use it.  It's the only sane way to get an MPIC
>>> into
>>> a sane known state starting from <whatever> state.
>>
>> I used to have the reset there on pasemi but due to some other issues I
>> ended up taking it out.
>>
>> It's not like doing a final EOI is a big deal, I really don't see why
>> there's hassle about this patch.
>
> The patch is fine, don't get me wrong.  I'm just saying MPIC_WANTS_RESET
> should be the default since pretty much everything wants it.  The few
> platforms that don't can document why; with the current code, we cannot
> see if you just forgot it or not.

It was documented in the changeset, see de0c74e7f98d8a06b784e120103eb388da8194f6.

But yeah, that's hard to tell at some point later in time, a short
comment by the call could be useful. I'll add one next time I touch that
code.


-Olof

Re: [PATCH] PowerPC: MPIC ack interrupts at mpic_teardown_this_cpu()

[Olof Johansson]

On Thu, Apr 03, 2008 at 11:09:43PM +0400, Valentine Barshak wrote:
> We really need to ack interrupts at mpic_teardown, since
> not all platforms reset mpic at kernel start-up. For example,
> kexec'ed kernel hangs on P.A. Semi if mpic_eoi() isn't called.
> 
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>

I have applied this to pasemi.git for-2.6.26 with the following comment
added before the mpic_eoi call:

        /* We need to EOI the IPI since not all platforms reset the MPIC
         * on boot and new interrupts wouldn't get delivered otherwise.
         */
	mpic_eoi(mpic);


Thanks,

-Olof