Why _ThreadProcessSignalsFromIrq() in new exception processing?

[Sergei Organov]

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

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)?

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

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


-- 
Sergei.