<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2017-08-19 14:33 GMT+02:00 Gedare Bloom <span dir="ltr"><<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Aug 18, 2017 at 11:30 AM, Denis Obrezkov<br>
<div><div class="h5"><<a href="mailto:denisobrezkov@gmail.com">denisobrezkov@gmail.com</a>> wrote:<br>
> 2017-08-17 23:58 GMT+02:00 Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>>:<br>
>><br>
>> On Thu, Aug 17, 2017 at 4:17 PM, Denis Obrezkov <<a href="mailto:denisobrezkov@gmail.com">denisobrezkov@gmail.com</a>><br>
>> wrote:<br>
>> > 2017-08-17 22:07 GMT+02:00 Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>>:<br>
>> >><br>
>> >> On Thu, Aug 17, 2017 at 1:48 PM, Denis Obrezkov<br>
>> >> <<a href="mailto:denisobrezkov@gmail.com">denisobrezkov@gmail.com</a>><br>
>> >> wrote:<br>
>> >> > 2017-08-17 17:25 GMT+02:00 Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>>:<br>
>> >> >><br>
>> >> >> On Wed, Aug 16, 2017 at 11:13 AM, Denis Obrezkov<br>
>> >> >> <<a href="mailto:denisobrezkov@gmail.com">denisobrezkov@gmail.com</a>> wrote:<br>
>> >> >> > ---<br>
>> >> >> >  cpukit/score/cpu/riscv32/<wbr>riscv-context-switch.S | 12 ++++++++++--<br>
>> >> >> >  1 file changed, 10 insertions(+), 2 deletions(-)<br>
>> >> >> ><br>
>> >> >> > diff --git a/cpukit/score/cpu/riscv32/<wbr>riscv-context-switch.S<br>
>> >> >> > b/cpukit/score/cpu/riscv32/<wbr>riscv-context-switch.S<br>
>> >> >> > index a199596..bcdfe0e 100644<br>
>> >> >> > --- a/cpukit/score/cpu/riscv32/<wbr>riscv-context-switch.S<br>
>> >> >> > +++ b/cpukit/score/cpu/riscv32/<wbr>riscv-context-switch.S<br>
>> >> >> > @@ -46,6 +46,7 @@ PUBLIC(restore)<br>
>> >> >> ><br>
>> >> >> >  SYM(_CPU_Context_switch):<br>
>> >> >> >    /* Disable interrupts and store all registers */<br>
>> >> >> > +  csrci mstatus, 0x8<br>
>> >> >> Why is this necessary?<br>
>> >> >><br>
>> >> >> >    SREG x1, 4(a0)<br>
>> >> >> >    SREG x2, 8(a0)<br>
>> >> >> >    SREG x3, 12(a0)<br>
>> >> >> > @@ -78,8 +79,9 @@ SYM(_CPU_Context_switch):<br>
>> >> >> >    SREG x30, 120(a0)<br>
>> >> >> >    SREG x31, 124(a0)<br>
>> >> >> ><br>
>> >> >> > -SYM(restore):<br>
>> >> >> ><br>
>> >> >> > +SYM(restore):<br>
>> >> >> > +<br>
>> >> >> >    LREG x1, 4(a1)<br>
>> >> >> >    LREG x2, 8(a1)<br>
>> >> >> >    LREG x3, 12(a1)<br>
>> >> >> > @@ -111,9 +113,15 @@ SYM(restore):<br>
>> >> >> >    LREG x29, 116(a1)<br>
>> >> >> >    LREG x30, 120(a1)<br>
>> >> >> >    LREG x31, 124(a1)<br>
>> >> >> > -       ret<br>
>> >> >> > +<br>
>> >> >> > +<br>
>> >> >> > +  csrsi mstatus, 0x8<br>
>> >> >> > +  nop<br>
>> >> >> > +  nop<br>
>> >> >> Why the nops?<br>
>> >> >><br>
>> >> >> > +  ret<br>
>> >> >> ><br>
>> >> >> >  SYM(_CPU_Context_restore):<br>
>> >> >> > +  csrci mstatus, 0x8<br>
>> >> >> >    mv     a1, a0<br>
>> >> >> >    j       restore<br>
>> >> >> >    nop<br>
>> >> >> > --<br>
>> >> >> > 2.1.4<br>
>> >> >> ><br>
>> >> >> > ______________________________<wbr>_________________<br>
>> >> >> > devel mailing list<br>
>> >> >> > <a href="mailto:devel@rtems.org">devel@rtems.org</a><br>
>> >> >> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/<wbr>mailman/listinfo/devel</a><br>
>> >> ><br>
>> >> > So, don't we turn off interrupts during the context switch?<br>
>> >><br>
>> >> Nope, and turning them back on unconditionally is wrong too.<br>
>> >><br>
>> >> > Yes, nops are unnecessary.<br>
>> >> ><br>
>> >> ><br>
>> >> > --<br>
>> >> > Regards, Denis Obrezkov<br>
>> ><br>
>> > Ok, I was confused by this obsolete comment:<br>
>> > /* Disable interrupts and store all registers */<br>
>> > Will remove all that enabling/disabling.<br>
>> ><br>
>> > Is the same true for start.S file with interrupted task's stack saving?<br>
>> ><br>
>> I'm not sure exactly what you're referring to, but usually start.S<br>
>> will, among other things, turn off and ack pending interrupts to<br>
>> quiesce the hardware before handing off the rest of system init to<br>
>> boot_card. Eventually RTEMS will enable interrupts again using the<br>
>> cpukit's interrupt_enable layer.<br>
>><br>
>> ><br>
>> ><br>
>> > --<br>
>> > Regards, Denis Obrezkov<br>
><br>
> I mean when an interrupt occurs, the execution flow jumps to the trap<br>
> address.<br>
> Then, in my case all process'es registers are saved in a common stack<br>
> (I know that dedicated interrupt stack should be implemented, but I haven't<br>
> time to do that)<br>
> and then we jump to IRQ dispatching routine. After interrupt handling we<br>
> restore saved<br>
> registers. So, my question is - should we disable/enable interrupts during<br>
> interrupt handling?<br>
> Disabling:<br>
> <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/c/src/lib/libbsp/riscv32/hifive1/start/start.S#L201" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/c/src/lib/libbsp/<wbr>riscv32/hifive1/start/start.S#<wbr>L201</a><br>
> Jump to dispatching routine:<br>
> <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/c/src/lib/libbsp/riscv32/hifive1/start/start.S#L241" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/c/src/lib/libbsp/<wbr>riscv32/hifive1/start/start.S#<wbr>L241</a><br>
> Enabling interrupts back:<br>
> <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/c/src/lib/libbsp/riscv32/hifive1/start/start.S#L285" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/c/src/lib/libbsp/<wbr>riscv32/hifive1/start/start.S#<wbr>L285</a><br>
><br>
</div></div>This interrupt handling is incomplete/broken I think. Keeping the<br>
interrupts turned off for the duration of RTEMS + User ISR handling is<br>
not "real-time". What other CPU did you look at to understand how to<br>
deal with interrupts?<br></blockquote><div>I'v kept Hasham's RISC-V generic interrupt code. I thought that we don't</div><div>want to have nested interrupts. I also tried to turn off interrupts only during </div><div>register saving/restoring. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Usually, the BSP will support installing interrupt handlers that jump<br>
to ISR_Handler(), which then sets up a safe "C" environment for the<br>
user handler to use. The ISR_Handler() should enable interrupts before<br>
invoking the user code. From what I can tell, your implementation of<br>
ISR_Handler doesn't do anything.<br></blockquote><div>Ok, I am going to figure out how to implement it better. </div></div><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Regards, Denis Obrezkov</div>
</div></div>