<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2017-08-21 19:39 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 Mon, Aug 21, 2017 at 11:33 AM, Denis Obrezkov<br>
<div><div class="m_-1215688957641959535h5"><<a href="mailto:denisobrezkov@gmail.com" target="_blank">denisobrezkov@gmail.com</a>> wrote:<br>
> 2017-08-20 5:18 GMT+02:00 Hesham Almatary <<a href="mailto:heshamelmatary@gmail.com" target="_blank">heshamelmatary@gmail.com</a>>:<br>
>><br>
>> On Thu, Aug 17, 2017 at 1:13 AM, Denis Obrezkov <<a href="mailto:denisobrezkov@gmail.com" target="_blank">denisobrezkov@gmail.com</a>><br>
>> wrote:<br>
>> > ---<br>
>> >  cpukit/score/cpu/riscv32/rtems<wbr>/score/cpu.h | 18 ++++++++++++------<br>
>> >  1 file changed, 12 insertions(+), 6 deletions(-)<br>
>> ><br>
>> > diff --git a/cpukit/score/cpu/riscv32/rte<wbr>ms/score/cpu.h<br>
>> > b/cpukit/score/cpu/riscv32/rte<wbr>ms/score/cpu.h<br>
>> > index 8f8a864..9b484e6 100644<br>
>> > --- a/cpukit/score/cpu/riscv32/rte<wbr>ms/score/cpu.h<br>
>> > +++ b/cpukit/score/cpu/riscv32/rte<wbr>ms/score/cpu.h<br>
>> > @@ -455,7 +455,7 @@ Context_Control_fp  _CPU_Null_fp_context;<br>
>> >   *<br>
>> >   */<br>
>> ><br>
>> > -#define CPU_STACK_MINIMUM_SIZE  4096<br>
>> > +#define CPU_STACK_MINIMUM_SIZE  700<br>
>> ><br>
>> >  /*<br>
>> >   *  CPU's worst alignment requirement for data types on a byte<br>
>> > boundary.  This<br>
>> > @@ -538,7 +538,6 @@ Context_Control_fp  _CPU_Null_fp_context;<br>
>> >   *  level is returned in _level.<br>
>> >   *<br>
>> >   */<br>
>> > -<br>
>> >  static inline uint32_t riscv_interrupt_disable( void )<br>
>> >  {<br>
>> >    register uint32_t sstatus;<br>
>> > @@ -549,13 +548,15 @@ static inline uint32_t riscv_interrupt_disable(<br>
>> > void )<br>
>> >                          "csrw sstatus, %[temp]; \t"<br>
>> >                          : [temp] "=r" (temp) : [sstatus] "r" (sstatus):<br>
>> >                          );<br>
>> > +#elif FE3XX<br>
>> > +  __asm__ volatile ("csrci mstatus, 0x8");<br>
>> > +  (void)temp;<br>
>> Consider implementing this proprely as we discussed before. Also,<br>
>> there's no need for the #elif FE3XX; just modify the #else below.<br>
>> Enabling/disabling global interrupts in RISC-V (and this file) is<br>
>> ISA/RISC-V specific, not BSP specific.<br>
>><br>
>> >  #else<br>
>> >    __asm__ __volatile__ ("csrr %[sstatus], mstatus; \t"<br>
>> >                          "andi %[temp], %[sstatus], -2; \t"<br>
>> >                          "csrw mstatus, %[temp]; \t"<br>
>> >                          : [temp] "=r" (temp) : [sstatus] "r" (sstatus):<br>
>> >                          );<br>
>> > -<br>
>> >  #endif<br>
>> >    return sstatus;<br>
>> >  }<br>
>> > @@ -565,15 +566,18 @@ static inline void riscv_interrupt_enable(uint32_<wbr>t<br>
>> > level)<br>
>> >  #ifdef SEL4<br>
>> >    __asm__ __volatile__ ("csrw sstatus, %[level];"<br>
>> >                          :: [level] "r" (level):);<br>
>> > +#elif FE3XX<br>
>> > +  __asm__ volatile ("csrsi mstatus, 0x8");<br>
>> ditto<br>
>> >  #else<br>
>> >    __asm__ __volatile__ ("csrw mstatus, %[level];"<br>
>> >                          :: [level] "r" (level):);<br>
>> > -<br>
>> >  #endif<br>
>> >  }<br>
>> ><br>
>> >  #define _CPU_ISR_Disable( _level ) \<br>
>> > -    _level = riscv_interrupt_disable()<br>
>> > +    do { \<br>
>> > +    _level = riscv_interrupt_disable();\<br>
>> > +    } while(0)<br>
>> ><br>
>> >  /*<br>
>> >   *  Enable interrupts to the previous level (returned by<br>
>> > _CPU_ISR_Disable).<br>
>> > @@ -583,7 +587,9 @@ static inline void riscv_interrupt_enable(uint32_<wbr>t<br>
>> > level)<br>
>> >   */<br>
>> ><br>
>> >  #define _CPU_ISR_Enable( _level )  \<br>
>> > -  riscv_interrupt_enable( _level )<br>
>> > +    do {\<br>
>> > +      riscv_interrupt_enable( _level ); \<br>
>> > +    } while(0)<br>
>> ><br>
>> >  /*<br>
>> >   *  This temporarily restores the interrupt to _level before<br>
>> > immediately<br>
>> > --<br>
>> > 2.1.4<br>
>> ><br>
>> > ______________________________<wbr>_________________<br>
>> > devel mailing list<br>
>> > <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
>> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman<wbr>/listinfo/devel</a><br>
>><br>
>><br>
>><br>
>> --<br>
>> Hesham<br>
><br>
><br>
> As I answered you earlier I tried your solution and it just didn't work.<br>
> I can try it again on the next iteration of the reviewing process, but as I<br>
> said<br>
> the _level value just isn't preserved between interrupts enabling/disabling.<br>
><br>
</div></div>Lack of variable preservation indicates a bug somewhere, probably<br>
either in assembly code or (interrupt) context switching.<br></blockquote><div>I agree, but I still wasn't able to identify it, I'll submit Patch v2 today, </div><div>and start search for the bug. </div></div><div><br></div>-- <br><div class="m_-1215688957641959535gmail_signature" data-smartmail="gmail_signature">Regards, Denis Obrezkov</div>
</div></div>