[PATCH 15/15] HiFive1: disable/enable interrupts during context switch

[Gedare Bloom]

On Thu, Aug 17, 2017 at 4:17 PM, Denis Obrezkov <denisobrezkov at gmail.com> wrote:
> 2017-08-17 22:07 GMT+02:00 Gedare Bloom <gedare at rtems.org>:
>>
>> On Thu, Aug 17, 2017 at 1:48 PM, Denis Obrezkov <denisobrezkov at gmail.com>
>> wrote:
>> > 2017-08-17 17:25 GMT+02:00 Gedare Bloom <gedare at rtems.org>:
>> >>
>> >> On Wed, Aug 16, 2017 at 11:13 AM, Denis Obrezkov
>> >> <denisobrezkov at gmail.com> wrote:
>> >> > ---
>> >> >  cpukit/score/cpu/riscv32/riscv-context-switch.S | 12 ++++++++++--
>> >> >  1 file changed, 10 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/cpukit/score/cpu/riscv32/riscv-context-switch.S
>> >> > b/cpukit/score/cpu/riscv32/riscv-context-switch.S
>> >> > index a199596..bcdfe0e 100644
>> >> > --- a/cpukit/score/cpu/riscv32/riscv-context-switch.S
>> >> > +++ b/cpukit/score/cpu/riscv32/riscv-context-switch.S
>> >> > @@ -46,6 +46,7 @@ PUBLIC(restore)
>> >> >
>> >> >  SYM(_CPU_Context_switch):
>> >> >    /* Disable interrupts and store all registers */
>> >> > +  csrci mstatus, 0x8
>> >> Why is this necessary?
>> >>
>> >> >    SREG x1, 4(a0)
>> >> >    SREG x2, 8(a0)
>> >> >    SREG x3, 12(a0)
>> >> > @@ -78,8 +79,9 @@ SYM(_CPU_Context_switch):
>> >> >    SREG x30, 120(a0)
>> >> >    SREG x31, 124(a0)
>> >> >
>> >> > -SYM(restore):
>> >> >
>> >> > +SYM(restore):
>> >> > +
>> >> >    LREG x1, 4(a1)
>> >> >    LREG x2, 8(a1)
>> >> >    LREG x3, 12(a1)
>> >> > @@ -111,9 +113,15 @@ SYM(restore):
>> >> >    LREG x29, 116(a1)
>> >> >    LREG x30, 120(a1)
>> >> >    LREG x31, 124(a1)
>> >> > -       ret
>> >> > +
>> >> > +
>> >> > +  csrsi mstatus, 0x8
>> >> > +  nop
>> >> > +  nop
>> >> Why the nops?
>> >>
>> >> > +  ret
>> >> >
>> >> >  SYM(_CPU_Context_restore):
>> >> > +  csrci mstatus, 0x8
>> >> >    mv     a1, a0
>> >> >    j       restore
>> >> >    nop
>> >> > --
>> >> > 2.1.4
>> >> >
>> >> > _______________________________________________
>> >> > devel mailing list
>> >> > devel at rtems.org
>> >> > http://lists.rtems.org/mailman/listinfo/devel
>> >
>> > So, don't we turn off interrupts during the context switch?
>>
>> Nope, and turning them back on unconditionally is wrong too.
>>
>> > Yes, nops are unnecessary.
>> >
>> >
>> > --
>> > Regards, Denis Obrezkov
>
> Ok, I was confused by this obsolete comment:
> /* Disable interrupts and store all registers */
> Will remove all that enabling/disabling.
>
> Is the same true for start.S file with interrupted task's stack saving?
>
I'm not sure exactly what you're referring to, but usually start.S
will, among other things, turn off and ack pending interrupts to
quiesce the hardware before handing off the rest of system init to
boot_card. Eventually RTEMS will enable interrupts again using the
cpukit's interrupt_enable layer.

>
>
> --
> Regards, Denis Obrezkov