Re: [RFC][PATCH][ARM32] Ctrl+C in the shell provides panic

[Chris Gilbert <chris%dokein.co.uk@localhost>]

Hi Andy,

This may solve the problem for the pic using ports, but all the others
that use irq_dispatch.S will also need updating.

I guess the issue isn't present without HAVE_SOFTINTS as an lwp is
scheduled to handle the interrupt, with the HAVE_FAST_SOFTINTS we
reusing the current thread.

Actually last time I spoke with Matt about fast soft ints, he'd talked
about moving the dispatching of them to the code that handles asts, so
the ast pending check on exit from the irq_dispatch routine would also
check for pending softints.  Although there maybe some work to do to
reenable irqs if soft ints are run from the ast dispatch.

Thanks,
Chris

Andy Shevchenko wrote:
> The fast soft interrupt handler should be executed when interrupt depth
> are equal to 0. So, we need to revert back the depth value before
> cpu_dosoftints() call. Without this we have got panic just after Ctrl+C
> pressing in the shell when __HAVE_FAST_SOFTINTS is enabled.
> 
> There is looks like hack until now.
> 
> On Fri, May 9, 2008 at 11:22 AM, Andy Shevchenko
> <andy.shevchenko%gmail.com@localhost> wrote:
>> Hello.
>>
>> I've built the NetBSD-current/H4 (for TI OMAP 2420SDP) with the soft
>> interrupts enabled.
>> When I try to press Ctrl+C in a shell, I've got kernel panic.
>>
>> Really, all *signal() functions in the kern_sig.c check ci_intr_depth and
>> panic when value great zero. The non-zero value is mean interrupt context of
>> code.
>>
>> ...
>> void
>> pgsignal(struct pgrp *pgrp, int sig, int checkctty)
>> {
>>     ksiginfo_t ksi;
>>
>>>>>>>    KASSERT(!cpu_intr_p());
>>     KASSERT(mutex_owned(proc_lock));
>> ...
>>
>> The depth value is changed only in two places in the irq_entry()
>> (irq_dispatch.S): before call  ARM_IRQ_HANDLER is incremented and after just
>> reverted back.
>>
>> If I just comment out all of those assertions the shell start to work fine.
>> However, I understand is it wrong way to fix.
>>
>> Please, provide ideas, comments and so on how to fix the issue. Thanks.
>>
>> --
>> With Best Regards,
>>  Andy Shevchenko
>>
>>
> 
> Index: sys/arch/arm/arm32/irq_dispatch.S
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/arm/arm32/irq_dispatch.S,v
> retrieving revision 1.10
> diff -u -r1.10 irq_dispatch.S
> --- sys/arch/arm/arm32/irq_dispatch.S 27 Apr 2008 18:58:44 -0000      1.10
> +++ sys/arch/arm/arm32/irq_dispatch.S 9 May 2008 09:06:13 -0000
> @@ -104,28 +104,10 @@
>       PUSHFRAMEINSVC                  /* Push an interrupt frame */
>       ENABLE_ALIGNMENT_FAULTS         /* finishes with curcpu() in r4 */
> 
> -     /*
> -      * Increment the interrupt nesting depth and call the interrupt
> -      * dispatch routine.  We've pushed a frame, so we can safely use
> -      * callee-saved regs here.  We use the following registers, which
> -      * we expect to presist:
> -      *
> -      *      r4      address of current cpu_info
> -      *      r6      old value of `ci_intr_depth'
> -      */
>       mov     r0, sp                  /* arg for dispatcher */
> -     ldr     r6, [r4, #CI_INTR_DEPTH]
> -     add     r1, r6, #1
> -     str     r1, [r4, #CI_INTR_DEPTH]
> 
>       bl      ARM_IRQ_HANDLER
> 
> -     /*
> -      * Restore the old interrupt depth value (which should be the
> -      * same as decrementing it at this point).
> -      */
> -     str     r6, [r4, #CI_INTR_DEPTH]
> -
>       LOCK_CAS_CHECK
> 
>       DO_AST_AND_RESTORE_ALIGNMENT_FAULTS
> Index: sys/arch/arm/pic/pic.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/arm/pic/pic.c,v
> retrieving revision 1.3
> diff -u -r1.3 pic.c
> --- sys/arch/arm/pic/pic.c    28 Apr 2008 20:23:14 -0000      1.3
> +++ sys/arch/arm/pic/pic.c    9 May 2008 09:06:13 -0000
> @@ -339,6 +339,9 @@
>       struct cpu_info * const ci = curcpu();
>       if (__predict_false(newipl == IPL_HIGH))
>               return;
> +
> +     ci->ci_intr_depth++;
> +
>       while ((pic_pending_ipls & ~__BIT(newipl)) > __BIT(newipl)) {
>               KASSERT(pic_pending_ipls < __BIT(NIPL));
>               for (;;) {
> @@ -352,6 +355,9 @@
>                       pic_list_unblock_irqs();
>               }
>       }
> +
> +     ci->ci_intr_depth--;
> +
>       if (ci->ci_cpl != newipl)
>               ci->ci_cpl = newipl;
>  #ifdef __HAVE_FAST_SOFTINTS
> 
> 
> 
>