You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Brennan Ashton <ba...@brennanashton.com> on 2020/06/21 18:38:27 UTC

Interrupt restore behavior on task activate

I'm bringing up support for a new RISC-V core but I'm running into some
issues around the sys tick.

My tick timer interrupt is disabled at activating a task, clearly this is
not correct.  This is code used across all architectures so I'm sure I just
do not understand something

void nxtask_activate(FAR struct tcb_s *tcb)
{
  irqstate_t flags = enter_critical_section();
  up_unblock_task(tcb);
  leave_critical_section(flags);
}


Won't we perform a context switch here and not leave the critical section?

--Brennan

Re: Interrupt restore behavior on task activate

Posted by Brennan Ashton <ba...@brennanashton.com>.
On Sun, Jun 21, 2020 at 11:51 PM Ishikawa, Masayuki (SHES)
<Ma...@sony.com> wrote:
>
> Hi, Brennan,
>
> Did you check up_get_newintctx() ?
>
> The function is called from up_initial_state() to create a new task
> and needs to return (MSTATUS_MPPM | MSTATUS_MPIE).

Yeah I had that already setup and I could not figure out what was
going on which is why I stepped back to make sure I was not missing
something more fundamental.

After much digging which I guess was not for nothing since I
understand the RISC-V opcodes much better, I figured out what was
going on.  When the syscall is made via ecall the handler does the
work of restoring the context, including setting the MPIE bit in the
mstatus register (if required).  Then at the end mret is called which
will do the proper jump and move the value in MPIE bit to MIE bit.

Unfortunately this core takes a shortcut here in the RTL to save space
and does not support reading or writing the MPIE bit in mstatus, so
interrupts would always be disabled out of a context switch.  I have
not quite figured out how to work around this unfortunately, need to
dig into the RTL a bit more for the core and see what I might be able
to do here.

Thanks for the pointer though.

--Brennan

Re: Interrupt restore behavior on task activate

Posted by "Ishikawa, Masayuki (SHES)" <Ma...@sony.com>.
Hi, Brennan,

Did you check up_get_newintctx() ?

The function is called from up_initial_state() to create a new task
and needs to return (MSTATUS_MPPM | MSTATUS_MPIE).

Masayuki

________________________________
From: Brennan Ashton <ba...@brennanashton.com>
Sent: Monday, June 22, 2020 3:38
To: dev@nuttx.apache.org <de...@nuttx.apache.org>
Subject: Interrupt restore behavior on task activate

I'm bringing up support for a new RISC-V core but I'm running into some
issues around the sys tick.

My tick timer interrupt is disabled at activating a task, clearly this is
not correct.  This is code used across all architectures so I'm sure I just
do not understand something

void nxtask_activate(FAR struct tcb_s *tcb)
{
  irqstate_t flags = enter_critical_section();
  up_unblock_task(tcb);
  leave_critical_section(flags);
}


Won't we perform a context switch here and not leave the critical section?

--Brennan

Re: Interrupt restore behavior on task activate

Posted by Brennan Ashton <ba...@brennanashton.com>.
> The interrupts interrupts should be restore when the new context is
> instantiated in up_unblock_task().  There is no call to irq_restore().
> I don't use RISC-V, but I can show you for ARMv7-M:
>
>   arch/arm/src/armv7-m/arm_unblocktask.c
>
> 103           /* Then switch contexts */
> 104
> 105           arm_restorestate(rtcb->xcp.regs);
>
> The state of the interrupts is just part of the state of the task that
> is saved and restore on context switches.

Thanks Greg.  This helps point me in the right place, this core does
not quite follow the full RISC-V spec which I think is where I am
hitting some walls.
It is interesting to me because of how small it is,  it is fairly
trivial to implement a design with several hundred cores.

--Brennan

Re: Interrupt restore behavior on task activate

Posted by Gregory Nutt <sp...@gmail.com>.
On 6/21/2020 12:54 PM, Brennan Ashton wrote:
> On Sun, Jun 21, 2020, 11:46 AM Gregory Nutt <sp...@gmail.com> wrote:
>
>>> I'm bringing up support for a new RISC-V core but I'm running into some
>>> issues around the sys tick.
>>>
>>> My tick timer interrupt is disabled at activating a task, clearly this is
>>> not correct.  This is code used across all architectures so I'm sure I
>> just
>>> do not understand something
>>>
>>> void nxtask_activate(FAR struct tcb_s *tcb)
>>> {
>>>     irqstate_t flags = enter_critical_section();
>>>     up_unblock_task(tcb);
>>>     leave_critical_section(flags);
>>> }
>>>
>>>
>>> Won't we perform a context switch here and not leave the critical
>> section?
>>
>> No, the critical section is left automatically when the context switch
>> occurs.  The critical section is not a global attribute; it is a per
>> task attribute.  If Task A enters the critical section then suspends (as
>> above) the state of critical section is saved and the new state of the
>> critical section for the newly started Task B is instantiated.  For a
>> new task like this, the initial state of the critical section will be
>> "not-in-a-critical section".
>>
>> This is described in a Wiki pages somewhere, but I don't recall which.
>> In know that is mentioned in the Wiki page on the critical section
>> monitor but I don't think that is the authoritative reference.
>>
>> So, don't worry.  This has all been carefully thought through and has
>> worked well for 13.3 years.
>>
> I totally agree that the os is working as expected and that I am porting
> some functionally wrong, but I am trying to understand where the interrupts
> get re-enabled in this flow.  The irq save/restore look like this, but when
> I instrument them I see a save called on the entry to the init task, but
> never a restore. At that point the timer isr is never triggered to indicate
> an OS tick. Is there another place I should be hooking to renenable the
> interrupt on the context switch?

The interrupts interrupts should be restore when the new context is 
instantiated in up_unblock_task().  There is no call to irq_restore().  
I don't use RISC-V, but I can show you for ARMv7-M:

  arch/arm/src/armv7-m/arm_unblocktask.c

103           /* Then switch contexts */
104
105           arm_restorestate(rtcb->xcp.regs);

The state of the interrupts is just part of the state of the task that 
is saved and restore on context switches.

Greg



Re: Interrupt restore behavior on task activate

Posted by Brennan Ashton <ba...@brennanashton.com>.
On Sun, Jun 21, 2020, 11:46 AM Gregory Nutt <sp...@gmail.com> wrote:

>
> > I'm bringing up support for a new RISC-V core but I'm running into some
> > issues around the sys tick.
> >
> > My tick timer interrupt is disabled at activating a task, clearly this is
> > not correct.  This is code used across all architectures so I'm sure I
> just
> > do not understand something
> >
> > void nxtask_activate(FAR struct tcb_s *tcb)
> > {
> >    irqstate_t flags = enter_critical_section();
> >    up_unblock_task(tcb);
> >    leave_critical_section(flags);
> > }
> >
> >
> > Won't we perform a context switch here and not leave the critical
> section?
>
> No, the critical section is left automatically when the context switch
> occurs.  The critical section is not a global attribute; it is a per
> task attribute.  If Task A enters the critical section then suspends (as
> above) the state of critical section is saved and the new state of the
> critical section for the newly started Task B is instantiated.  For a
> new task like this, the initial state of the critical section will be
> "not-in-a-critical section".
>
> This is described in a Wiki pages somewhere, but I don't recall which.
> In know that is mentioned in the Wiki page on the critical section
> monitor but I don't think that is the authoritative reference.
>
> So, don't worry.  This has all been carefully thought through and has
> worked well for 13.3 years.
>

I totally agree that the os is working as expected and that I am porting
some functionally wrong, but I am trying to understand where the interrupts
get re-enabled in this flow.  The irq save/restore look like this, but when
I instrument them I see a save called on the entry to the init task, but
never a restore. At that point the timer isr is never triggered to indicate
an OS tick. Is there another place I should be hooking to renenable the
interrupt on the context switch?

irqstate_t up_irq_save(void)
{
  uint64_t oldstat;

  /* Read mstatus & clear machine interrupt enable (MIE) in mstatus */

  asm volatile ("csrrc %0, mstatus, %1": "=r" (oldstat) : "r"(MSTATUS_MIE));
  return oldstat;
}


void up_irq_restore(irqstate_t flags)
{
  /* Write flags to mstatus */

  asm volatile("csrw mstatus, %0" : /* no output */ : "r" (flags));
}

>

Re: Interrupt restore behavior on task activate

Posted by Gregory Nutt <sp...@gmail.com>.
> I'm bringing up support for a new RISC-V core but I'm running into some
> issues around the sys tick.
>
> My tick timer interrupt is disabled at activating a task, clearly this is
> not correct.  This is code used across all architectures so I'm sure I just
> do not understand something
>
> void nxtask_activate(FAR struct tcb_s *tcb)
> {
>    irqstate_t flags = enter_critical_section();
>    up_unblock_task(tcb);
>    leave_critical_section(flags);
> }
>
>
> Won't we perform a context switch here and not leave the critical section?

No, the critical section is left automatically when the context switch 
occurs.  The critical section is not a global attribute; it is a per 
task attribute.  If Task A enters the critical section then suspends (as 
above) the state of critical section is saved and the new state of the 
critical section for the newly started Task B is instantiated.  For a 
new task like this, the initial state of the critical section will be 
"not-in-a-critical section".

This is described in a Wiki pages somewhere, but I don't recall which.  
In know that is mentioned in the Wiki page on the critical section 
monitor but I don't think that is the authoritative reference.

So, don't worry.  This has all been carefully thought through and has 
worked well for 13.3 years.