Why _ThreadProcessSignalsFromIrq() in new exception processing?

[Sergei Organov]

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.

> 
> > If there is one, then why the _ThreadProcessSignalsFromIrq is not in the
> > RTEMS core (like _Thread_Dispatch) and why not all the BSPs call it?
> 
> Because the CTX argument is processor dependent and that the actual handling
> of the software exception may also well contain system/board specific code.

Current code doesn't use CTX argument and I mean that part of code. It is
processor/bsp independent (in fact it is a copy of a few last lines of
_Thread_Dispatch) and so should better reside in the kernel, isn't it? It
could be then called both from _Thread_Dispatch and from
_ThreadProcessSignalsFromIrq.

> > Is there an intention to eventually turn all the BSPs to this new calling
> > sequence?
> 
> I do not care about that but would like to understand your resistance to
> adopt the new exception handling code

Resistance is just helpful. There should be a reason to switch to something
new when old works, not a reason to don't switch, and still I didn't see
anything in the new code that is significant enough *for me* to justify the
switch. Also I prefer to stay with the majority whenever possible, and the new
exception processing is not yet the majority, I think.

> (the EABI broken argument can be fixed by adding a single ASM instruction to
> restore R2 before actually calling the irq handler).

As soon as I've noticed it, that's not a problem anymore. It would be a real
problem if I've started to use the code without noticing that. I know enough
to fix it myself or even to re-implement it entirely if required. I just
wanted to let you (and others) know there possibly is a problem. An
interesting thing though is that there are currently 4(!) almost identical
copies of the 'irq_asm.S' in the PPC BSPs that looks to me as rather bad
style.

> So far you seems to critize something that anyway
> 	1) you do not use
> 	2) have not proposed to enhance/fix yet

Please don't get me wrong. Criticism is not my goal. I just try to understand
particular differences between two implementations, reasons behind them, and
consequences of switching from old to new exception processing.

> 
> I think recent discussion on the mailling list made it clear that :
> 	1) IRQ API should be enhanced

Agreed.

>          and the API proposed on the three BSP you mention is already a big
>          step forward,

Maybe. You insist it is, somebody else posted different feeling. Anyway, API
has nothing to do with the implementation details I was asking about in this
thread, or my understanding of the term "API" is different than yours.

> 	2) Exception handling API is also required on many CPU to handle
> 	   recoverable exception, implement debuggers (e.g remote debugging)
> 	   and was indeed missing in PPC, Ix86.

Yet I didn't say even single word about exception handling. If you are
interested in my opinion though, I agree with you that it's a good idea to
keep exceptions API separate from ISRs API (they can share [parts of]
implementation though).

-- 
Sergei.