Why _ThreadProcessSignalsFromIrq() in new exception processing?

[Joel Sherrill]

Sergei Organov wrote:
> 
> Joel Sherrill <joel.sherrill at OARcorp.com> writes:
> > Sergei Organov wrote:
> > >
> > > Eric Valette <eric.valette at free.fr> writes:
> > > > Sergei Organov wrote:
> > > >
> > > > > Most of BSPs do basically the same thing. If
> > > > > _ISR_Signals_to_thread_executing is non-zero, they set it to zero and
> > > > > either call _Thread_Dispatch or return to _ISR_Thread_dispatch that in
> > > > > turn calls _Thread_Dispatch. This is the documented way to do things.
> > > > > Fine so far. However, there are 3 BSPs that do this differently in an
> > > > > undocumented way. Instead of calling _Thread_Dispatch directly or
> > > > > indirectly, they call their own _ThreadProcessSignalsFromIrq. Maybe
> > > > > it's OK to do it this way, but the question is: why? I guess there
> > > > > should be some reason for that. What is the reason?
> > > >
> > > > The reason is that having a context to implement software exception
> > > > (which is what this code does even if not said that way and used in a
> > > > particular way for posix signal) is almost mandatory (e.g to correctly
> > > > implement signals on alternate stack, kernel debuggers, ...). So, when
> > > > implementing exception on those three BSP, I added the push of a context
> > > > similar to the one pushed on real hardware exceptions as this path is
> > > > very unfrequent and anyway performs heavy operation like functions
> > > > calls. The normal path is optimized as usual (or maybe a little bit more
> > > > even).
> > >
> > > This is what I've asked about. Your words "almost mandatory" imply that
> > > the BSPs that don't do that are "almost broken", isn't it? And Joel says
> > > they are fine.
> >
> > I was only referring to the processing of signals by ISR disptaching
> > code NOT to ANY exception handling processing.  The only requirement I
> > stated was that the exit patch of the ISR processing code MUST honor
> > that if an ISR sent a Classic API or POSIX API signal to the currently
> > executing thread, then that signal must be processed befure the thread
> > returns to its normal execution if that signal is enabled.
> >
> > The comment about exceptions which might have confused you is that
> > some software assumes that it will get a SIGFPU for a floating point
> > exception.  Thus some exception handlers will generate POSIX signals
> > to provide a more UNIX-like environment.  The GNU Ada run-time library
> > actually uses POSIX signals as the underlying mechanism for Ada
> > language level interrupt processing.
> 
> Does it mean that only those mentioned 3 BSPs are OK w.r.t. requirements of
> POSIX signals/Ada run-time? I don't think so. So my phrase above "Joel says
> they are fine" remains correct w.r.t. other BSPs, right?
> 
> Overall, I think the following conclusion would be correct:
> 
> 1. All the BSPs are OK with the issue. Fine.
> 
> 2. There are three BSPs that handle the case differently than all other BSPs.
>    They handle it differently because Eric likes it to be done that way. Fine.
> 
> The only problem with this conclusion I see is Eric's explanation why it
> *should* be done his way instead of the documented way. The fact that I don't
> quite understand him could well be my own failure, but IMHO his explanation
> implies that all other BSPs are somewhat broken :-( that in turn contradicts
> with statement (1)
> 
> It seems it's time to look at the actual relevant code.
> 
> Here is the documented way to exit from ISR:
> 
>   if ( _Context_Switch_necessary || _ISR_Signals_to_thread_executing ) {
>     _ISR_Signals_to_thread_executing = FALSE;
>     _Thread_Dispatch();
>   }
>   // prepare to get out of interrupt
>   // return from interrupt

That's the logical but there is some stack/interrupt state management
that goes with it.  Normally, you would the "test" part of that in
ISR state and decide to do some magic to get the body executed in 
"thread state".  You have to be VERY careful to make sure the CPU 
looks like it is in "thread state" when the signal handler itself
is executed.

> Here is how it is done in 3 BSPs in question (PPC variant converted to
> pseudo-code by me):
> 
>   if ( _Context_Switch_necessary ) {
>     _Thread_Dispatch();
>     // prepare to get out of interrupt
>     // return from interrupt
>   }
>   if ( _ISR_Signals_to_thread_executing ) {
>     _ISR_Signals_to_thread_executing = FALSE;
>     // push a complete exception like frame
>     _ThreadProcessSignalsFromIrq(context*);
>     // restore exception like frame
>     // prepare to get out of interrupt
>     // return from interrupt
>   }
> 
> Where _ThreadProcessSignalsFromIrq currently does basically the same thing as
> the _Thread_Dispatch would do if no context switch is necessary, so from
> functional point of view it could be considered as an optimized version of
> _Thread_Dispatch.
> 
> >From the point of view of observable behavior, there are two differences
> between these two methods (differences between _Thread_Dispatch and
> _ThreadProcessSignalsFromIrq aside):
> 
> D1. The latter method always stores the whole (exception) context before
>     calling _ThreadProcessSignalsFromIrq.
> 
> D2. The latter method fails to reset _ISR_Signals_to_thread_executing to FALSE
>     if _Context_Switch_necessary is non-zero.
> 
> These differences lead to the following questions:
> 
> 1. Is the whole (exception) context is *strictly required* at the call to
>    _Thread_Dispatch when _ISR_Signals_to_thread_executing is non-zero from the
>    point of view of correct behavior of the rest of RTEMS (including POSIX
>    signals and Ada runtime)?

I would think so since an exception handler might be used to implement
missing instructions.  In this case, the handler would have to tinker
with
the values of any register touched by the unimplemented instruction.

Similarly, in some cases, FPU exceptions have to clean up the results
and fix registers before returning.  I recall Greg Menke saying the
MIPS FPU exception code from NetBSD was pretty ugly.  The MIPS stack
frame from RTEMS in compatable with that one.  

> 2. Should FPU context be stored along with the whole (exception) context?

No.  The integer context is really fragile and must be preserved.  If
some
piece of code wants to touch the FPU, it can do it directly OR save it
explicitly in a local data structure.  How does gdb do it?

> 3. Is the difference (D2) a bug? If yes, the bug in the former code or in
>    the later?

I don't think the FP context is ever saved on any interrupt or exception
path by RTEMS.  It is assumed that if the handler needs the FP context,
it will deal with it itself.  This could literally adds 100s of CPU
cycles on some architectures.  

> --
> Sergei.