Why _ThreadProcessSignalsFromIrq() in new exception processing?

[Sergei Organov]

Valette Eric <eric.valette at free.fr> writes:
> > 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 did not want to imply the old code was wrong. What I wanted to say is that
> *not* calling ThreadProcessSignalsFromIrq in new exception code is broken
> because as Joel and myself have pointed out, it is required for posix signal
> handling. If you really do not use signal handling at all you can simpyfy the
> code...

Now I understand. But I've never have any intention to remove it from new
exception processing -- just understand why there is no such code in the old
one. Now after this and other answers everything is pretty clear to me.

[...]

> > Resistance is just helpful.
> 
> I would say constructive criticsm and proposal for enhancement is.

Sure it is.

> Resistance is futile :-)

No.

> 
> 
> > 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.
> 
> For you maybe. For many others wanted to use either remote debugging,
> exception handling it is.

That's one of "freedoms" the free software is about. Those who want -- switch,
those who don't -- don't switch. Nothing wrong about it, I think.

> Besides, old code lead to other duplication of code *inside each BSP* to
> effectively enable interrupt at PIC level (understand SIU+CPM or OpenPic+
> 8259) at IRQ connexion and also to acknowledge, mask current IRQ for each
> ISR handler.
> 
> > Also I prefer to stay with the majority whenever possible, and the new
> > exception processing is not yet the majority, I think.
> 
> Most *current, active* PPC BSP use the new exception scheme I think (most
> motorola board, many mbx8XX, ... All Ix86 BSP use it. It is unfortunate that
> the 8260 code do not use it.

Though I didn't write it, but it seems it uses new exception processing. At
least it does have it's own copy of irq_asm.S ;-)

> 
>  > I just wanted to let you (and others) know there possibly is a problem.
> 
> There is a problem *when you use non standard compilation switches*. I bet if
> we play with other gcc switches we may find others...

Non standard? Hmm... I'd call PPC EABI the natural standard for embedded
applications... If it's not standard for PPC port of RTEMS, it's failure of
the port, I believe. Besides, old exception processing code did handle it fine.

> Beside and you agreed, it could be curred by a simple sed command...

That's true.

> > 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.
> 
> I think having diffrent ISR asm code for 8XX, and 6XX is quite normal as 8XX
> have integrated interrupt controller (SIU + CPM, ...) and 6XX usually have
> an OpenPIC + 8259.

What does it have to do with code in irq_asm.S? I thought you supposed to have
BSP-dependent code in the C_dispatch_irq_handler(), isn't it? It seems that
currently it is done this way.

> Having the same code duplicated for one familly is just a matter of someone
> making autoconf/automake makefile part clever as I have done on ix86 related
> BSP... Do you candidate?

I believe the problem is not in autoconf/automake files. The problem is that
the code in shared/irq_asm.S is not parametrized by CPU features as the old
exception code was. Because of this BSP writers seem to take simple way of
making a copy of the file and making required small changes in the copy.

> 
> NB : I maintain IRQ handling should stay BSP dependend has more and more
> processor have external PIC to multiplex irq on top of a single IRQ line. If
> you change the PIC, It can happen that code is totally different. That said,
> avoiding code duplication is indeed a good goal.
> 
> > 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 wording old/new exception processing is misleading. Among other
> things  that the new IRQ handling schemes brings are :
> 
> 	1) provide the On/Off/IsON handler that simplify the BSP code because
> you have not to call the connect part and after that duplicate code to enable
> the interrupt on the device, manage PIC, ...

Sorry, I consider this to be more a drawback than an advantage. The extra
level of abstraction in disabling/enabling/query interrupt status doesn't come
without cost. For example, in my case it would replace about 3 asm
instructions required to read/modify/store a hardware register with indirect
function call through a pointer in the interrupt structure that even can't be
inlined due to its indirect nature :-(

Also, simplification of the BSP code is questionable. These on/off/isOn
handlers are forced to either calculate what they should do depending on the
vector number stored in the rtems_irq_connect_data that is inefficient, or,
alternatively, separate function could be installed for every vector, but
then it would increase code size.

Also, connection of interrupt handler and enabling the interrupt are rather
different activities, so I don't see where code is duplicated in the first
place.

> 	2) It manage all generic part of the irq handling code (not just
> calling the handler) and release you of the need to acknowledge the IRQ,
> masks other IRQ at PIC level, reenable IRQ once handler has been called. You
> just need to handle the *real* device level,

Sorry, but this also seems to be a wrong approach for me. I doubt there exists
generic algorithm that handles all the cases the way I need it. Just for
example, what if I don't need to re-enable the IRQ once handler has been
called as I have ISR handler that only posts a semaphore and then manage the
hardware from inside a thread waiting on this semaphore. In this case
re-enabling IRQ will result in forever loop as the device wasn't yet managed
and thus didn't yet remove its interrupt request.

Also very frequently I don't need to enable an IRQ automagically after I've
installed ISR handler to it. All this automatic handling just brings more
troubles, IMHO.

Overall, I'd like to see some justification for the statements you make.
Preferably a part of real code that in your opinion indeed benefits from this
on/off/isOn machinery.

> 
> 	3) Separate IRQ handling from exception handling,

OK.

-- 
Sergei.