Tim Kelly <hockey@dialectronics.com> writes:

> According to the commit message, your patch doesn't enable interrupts if
> they were already off.

Correct.

> As I see it, by preserving interrupt status, your patch would
> highlight problems in which PSL_EE was off after traps when it
> wasn't supposed to be, rather than eliminate the problem outright
> (but determining where PSL_EE is off incorrectly is highly needed).

"The problem" was interrupts happening in trap() before the rest of
the kernel had enabled PSL_EE for mainstream use, and do_pending_int()
checking for that condition.

> It seems to me that this code:
> 
> _C_LABEL(extint_call):
>         bl      _C_LABEL(extint_call)   /* to be filled in later */
> 
> intr_exit:
> /* Disable interrupts (should already be disabled) and MMU here: */
>         mfmsr   %r3
>         andi.   %r3,%r3,~(PSL_EE|PSL_ME|PSL_RI|PSL_DR|PSL_IR)@l
>         mtmsr   %r3
>         isync
> 
> 
> in trap_subr.S could leave PSL_EE off, if where rfi returns to doesn't
> enable it. Should this code also preserve interrupt status before it exits?

The rfi itself will re-enable interrupts if they were enabled in the
trapping frame, because rfi loads the MSR with the contents of
srr1. The explicit disabling of all interrupts here is only on between
the mtmsr and the rfi, and it's necessary to prevent another interrupt
from stomping on srr0 and srr1 while we're trying to return.

        - Nathan