RISC-V interrupts in HiFive1

[Denis Obrezkov]

2017-08-16 2:06 GMT+02:00 Hesham Almatary <heshamelmatary at gmail.com>:

> /
>
> On Wed, Aug 16, 2017 at 3:03 AM, Denis Obrezkov <denisobrezkov at gmail.com>
> wrote:
> > 2017-08-15 14:57 GMT+02:00 Joel Sherrill <joel at rtems.org>:
> >>
> >>
> >>
> >> On Aug 15, 2017 4:32 AM, "Denis Obrezkov" <denisobrezkov at gmail.com>
> wrote:
> >>
> >> 2017-08-15 5:44 GMT+02:00 Hesham Almatary <heshamelmatary at gmail.com>:
> >>>
> >>> Hi Denis,
> >>>
> >>> You just need to modify riscv_interrupt_disable(). Read the priv-spec
> >>> manual for your RISC-V version, and determine which bit should be
> >>> cleared (it's called MIE in priv-1.10, but you mentioned you work with
> >>> priv-1.9).
> >>
> >>
> >> Is the bit set correctly for the idle thread?
> >>
> >> I assume this is with a real clock driver so is it maintained properly
> >> across an ISR?
> >>
> >> It is either screwed up by the thread context initialization, context
> >> switch, or ISR code. ISR code has two exit paths. It could be the
> preemption
> >> path. That would occur about 5 seconds into the execution of ticker and
> TA1
> >> will preemption idle.
> >>>
> >>>
> >>
> > I understand this but still can find a mistake.
> > This is what I get:
> >
> >
> > *** LOW MEMORY CLOCK TICK TEST ***
> >
> > TA1 - rtems_clock_get_tod - 09:00:00 12/31/1988
> >
> > TA2 - rtems_clock_get_tod - 09:00:00 12/31/1988
> >
> > TA3 - rtems_clock_get_tod - 09:00:00 12/31/1988
> >
> > TA1 - rtems_clock_get_tod - 09:00:04 12/31/1988
> >
> > TA2 - rtems_clock_get_tod - 09:00:09 12/31/1988
> >
> > TA1 - rtems_clock_get_tod - 09:00:09 12/31/1988
> >
> >
> > After that I can examine: p Clock_driver_ticks
> >
> > $8 = 1001
> >
> >
> > mstatus value is 0x0001800 (interrupts disabled)
> >
> > Instruction for enabling global interrupts is:
> > csrsi mstatus, 0x8
> > for disabling:
> > csrci mstatus, 0x8
> > it atomically changes the fourth bit of mstatus (ie bit).
> >
> > I think that something wrong happens in context switches between TA1, TA2
> > and Idle.
> >
> > This is my context switch file:
> > https://github.com/embeddedden/rtems-riscv/blob/
> hifive1/cpukit/score/cpu/riscv32/riscv-context-switch.S
> You added new fields (e.g. mepc, mstatus) to the _CPU_Context_switch
> function without reserving an area for them in the corresponding
> riscv32 Context_Control structure.
>
Yes, I found this mistake already, and decided not to save mstatus, because
there is no point
in saving it at the current stage of development.

>
> > Interrupt disabling:
> > https://github.com/embeddedden/rtems-riscv/blob/
> hifive1/cpukit/score/cpu/riscv32/rtems/score/cpu.h#L542
> That's not right [1]. You should return the previous mstatus content
> (with interrupts enabled i.e. before disabling interrupts). Also, if I
> understand correctly, this [2] is also wrong. You should simply set
> mstatus with the level variable (as my code does). That is supposed to
> get this level value by a previous call to riscv_interrupt_disable(),
> assuming it's implemented correctly. This way you're enabling
> interrupts back.
>
As I said above, there is no need to save mstatus content since we don't
change it in other parts of the program. mstatus is used only for interrupt
enabling/disabling, thus I simple enable and disable interrupts using
appropriate instructions.

>
> [1] https://github.com/embeddedden/rtems-riscv/blob/
> hifive1/cpukit/score/cpu/riscv32/rtems/score/cpu.h#L554.
> [2] https://github.com/embeddedden/rtems-riscv/blob/
> hifive1/cpukit/score/cpu/riscv32/rtems/score/cpu.h#L574
> > Context initialize:
> > https://github.com/embeddedden/rtems-riscv/blob/
> hifive1/cpukit/score/cpu/riscv32/riscv-context-initialize.c
> Now that you use interrupts, it makes sense to add mstatus to
> Context_Control structure for riscv32, and initialize mstatus context
> in this function to enable interrupts (after addressing the above
> comment).
>
I agree that it should be done, but I think I will have no time for this
before evaluation.

>
>
I also think that after GSoC we should refresh our toolchain.
-- 

Regards, Denis Obrezkov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20170816/425da651/attachment-0002.html>