<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2017-08-16 2:06 GMT+02:00 Hesham Almatary <span dir="ltr"><<a href="mailto:heshamelmatary@gmail.com" target="_blank">heshamelmatary@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">/<br>
<div><div class="h5"><br>
On Wed, Aug 16, 2017 at 3:03 AM, Denis Obrezkov <<a href="mailto:denisobrezkov@gmail.com">denisobrezkov@gmail.com</a>> wrote:<br>
> 2017-08-15 14:57 GMT+02:00 Joel Sherrill <<a href="mailto:joel@rtems.org">joel@rtems.org</a>>:<br>
>><br>
>><br>
>><br>
>> On Aug 15, 2017 4:32 AM, "Denis Obrezkov" <<a href="mailto:denisobrezkov@gmail.com">denisobrezkov@gmail.com</a>> wrote:<br>
>><br>
>> 2017-08-15 5:44 GMT+02:00 Hesham Almatary <<a href="mailto:heshamelmatary@gmail.com">heshamelmatary@gmail.com</a>>:<br>
>>><br>
>>> Hi Denis,<br>
>>><br>
>>> You just need to modify riscv_interrupt_disable(). Read the priv-spec<br>
>>> manual for your RISC-V version, and determine which bit should be<br>
>>> cleared (it's called MIE in priv-1.10, but you mentioned you work with<br>
>>> priv-1.9).<br>
>><br>
>><br>
>> Is the bit set correctly for the idle thread?<br>
>><br>
>> I assume this is with a real clock driver so is it maintained properly<br>
>> across an ISR?<br>
>><br>
>> It is either screwed up by the thread context initialization, context<br>
>> switch, or ISR code. ISR code has two exit paths. It could be the preemption<br>
>> path. That would occur about 5 seconds into the execution of ticker and TA1<br>
>> will preemption idle.<br>
>>><br>
>>><br>
>><br>
> I understand this but still can find a mistake.<br>
> This is what I get:<br>
><br>
><br>
> *** LOW MEMORY CLOCK TICK TEST ***<br>
><br>
> TA1 - rtems_clock_get_tod - 09:00:00 12/31/1988<br>
><br>
> TA2 - rtems_clock_get_tod - 09:00:00 12/31/1988<br>
><br>
> TA3 - rtems_clock_get_tod - 09:00:00 12/31/1988<br>
><br>
> TA1 - rtems_clock_get_tod - 09:00:04 12/31/1988<br>
><br>
> TA2 - rtems_clock_get_tod - 09:00:09 12/31/1988<br>
><br>
> TA1 - rtems_clock_get_tod - 09:00:09 12/31/1988<br>
><br>
><br>
> After that I can examine: p Clock_driver_ticks<br>
><br>
> $8 = 1001<br>
><br>
><br>
> mstatus value is 0x0001800 (interrupts disabled)<br>
><br>
> Instruction for enabling global interrupts is:<br>
> csrsi mstatus, 0x8<br>
> for disabling:<br>
> csrci mstatus, 0x8<br>
> it atomically changes the fourth bit of mstatus (ie bit).<br>
><br>
> I think that something wrong happens in context switches between TA1, TA2<br>
> and Idle.<br>
><br>
> This is my context switch file:<br>
> <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/cpukit/score/cpu/riscv32/riscv-context-switch.S" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/cpukit/score/cpu/<wbr>riscv32/riscv-context-switch.S</a><br>
</div></div>You added new fields (e.g. mepc, mstatus) to the _CPU_Context_switch<br>
function without reserving an area for them in the corresponding<br>
riscv32 Context_Control structure.<br></blockquote><div>Yes, I found this mistake already, and decided not to save mstatus, because there is no point </div><div>in saving it at the current stage of development. <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Interrupt disabling:<br>
> <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/cpukit/score/cpu/riscv32/rtems/score/cpu.h#L542" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/cpukit/score/cpu/<wbr>riscv32/rtems/score/cpu.h#L542</a><br>
That's not right [1]. You should return the previous mstatus content<br>
(with interrupts enabled i.e. before disabling interrupts). Also, if I<br>
understand correctly, this [2] is also wrong. You should simply set<br>
mstatus with the level variable (as my code does). That is supposed to<br>
get this level value by a previous call to riscv_interrupt_disable(),<br>
assuming it's implemented correctly. This way you're enabling<br>
interrupts back.<br></blockquote><div>As I said above, there is no need to save mstatus content since we don't </div><div>change it in other parts of the program. mstatus is used only for interrupt</div><div>enabling/disabling, thus I simple enable and disable interrupts using </div><div>appropriate instructions.  </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
[1] <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/cpukit/score/cpu/riscv32/rtems/score/cpu.h#L554" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/cpukit/score/cpu/<wbr>riscv32/rtems/score/cpu.h#L554</a><wbr>.<br>
[2] <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/cpukit/score/cpu/riscv32/rtems/score/cpu.h#L574" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/cpukit/score/cpu/<wbr>riscv32/rtems/score/cpu.h#L574</a><br>
> Context initialize:<br>
> <a href="https://github.com/embeddedden/rtems-riscv/blob/hifive1/cpukit/score/cpu/riscv32/riscv-context-initialize.c" rel="noreferrer" target="_blank">https://github.com/<wbr>embeddedden/rtems-riscv/blob/<wbr>hifive1/cpukit/score/cpu/<wbr>riscv32/riscv-context-<wbr>initialize.c</a><br>
Now that you use interrupts, it makes sense to add mstatus to<br>
Context_Control structure for riscv32, and initialize mstatus context<br>
in this function to enable interrupts (after addressing the above<br>
comment).<br></blockquote><div>I agree that it should be done, but I think I will have no time for this before evaluation. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote></div><div class="gmail_extra"><br></div><div class="gmail_extra">I also think that after GSoC we should refresh our toolchain.</div>-- </div><div class="gmail_extra"><br><div class="gmail_signature" data-smartmail="gmail_signature">Regards, Denis Obrezkov</div>
</div></div>