Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Anders Blomdell wrote:
Jan Kiszka wrote:
Hi Dmitry,
Dmitry Adamushko wrote:
Hi Jan,
let's make yet another revision of the bits :
new XN_ISR_HANDLED == old XN_ISR_HANDLED + old XN_ISR_NO_ENABLE
ok.
new XN_ISR_NOENABLE == ~ old XN_ISR_ENABLE
ok.
new XN_ISR_PROPAGATE == XN_ISR_CHAINED
ok.
Just to make sure that you understand my weird ideas: each of the three
new XN_ISR_xxx above should be encoded with an individual bit
new XN_ISR_NOINT == ?
does it suppose the interrupt line to be .end-ed (enabled) and irq
not to be
propagated? Should be so, I guess, if it's different from 5). Then
nucleus
ignores implicit IRQ enable for 5) as well as for 3).
Do we really need that NOINT then, as it seems to be the same as
~HANDLED?
or NOINT == 0 and then it's a scalar value, not a bit.
So one may consider HANDLED == 1 and NOINT == 0 as really scalar values
and
NOENABLE and PROPAGATE as additional bits (used only if needed).
My idea is to urge the user specifying one of the base return types
(HANDLED or NOINT) + any of the two additional bits (NOENABLE and
PROPAGATE).
For correct drivers NOINT could be 0 indeed, but to check that the user
picked a new constant we may want to set NOINT != 0. With the old API
return 0 expressed HANDLED + ~ENABLE for the old API. With the new one
the user signals no interest and the nucleus may raise a warning that a
spurious IRQ occurred. So I would add a debug bit for NOINT here to
optionally (on OPT_XENO_DEBUG) detect old-style usage (return 0).
Moreover, we gain freedom to move bits in the future when every state is
encoded via constants. Or am I too paranoid here?
After reading the above discussion (of which I understand very little),
and looking at (what I believe to be) the relevant code:
+intr = shirq-handlers;
+
+while (intr)
+{
+s |= intr-isr(intr);
+++intr-hits;
+intr = intr-next;
+}
+xnintr_shirq_unlock(shirq);
+
+--sched-inesting;
+
+if (s XN_ISR_ENABLE)
+ xnarch_end_irq(irq);
+else if (s XN_ISR_CHAINED)
+ xnarch_chain_irq(irq);
A number of questions arise:
1. What happens if one of the shared handlers leaves the interrupt
asserted, returns NOENABLE|HANDLED and another return only HANDLED?
2. What happens if one returns PROPAGATE and another returns HANDLED?
As far as I can tell, after all RT handlers havve run, the following
scenarios are possible:
1. The interrupt is deasserted (i.e. it was a RT interrupt)
2. The interrupt is still asserted, it will be deasserted later
by some RT task (i.e. it was a RT interrupt)
3. The interrupt is still asserted and will be deasserted
by the Linux IRQ handler.
IMHO that leads to the conclusion that the IRQ handlers should return a
scalar
#define UNHANDLED 0
#define HANDLED_ENABLE 1
#define HANDLED_NOENABLE 2
#define PROPAGATE 3
and the loop should be
s = UNHANDLED
while (intr) {
tmp = intr-isr(intr);
if (tmp s) { s = tmp; }
intr = intr-next;
}
if (s == PROPAGATE) {
xnarch_chain_irq(irq);
} else if (s == HANDLED_ENABLE) {
xnarch_end_irq(irq);
}
This is a very useful suggestion, specifically this escalation of the
return value in the shared case. I would just let UNHANLDED start with 1
for the reasons I explained here:
https://mail.gna.org/public/xenomai-core/2006-02/msg00186.html
Ok, I would say let's got for this and finalise the patch!
To be really honest, I think that PROPAGATE should be excluded from the
RT IRQ-handlers, since with the current scheme all RT-handlers has to
test if the IRQ was a Linux interrupt (otherwise the system will only
work when the handler that returns PROPAGATE is installed)
Indeed, but I'm with Philippe here: do not exclude the strange corner
case scenarios users may craft with the appropriate care. I could, e.g.,
imagine a scenario where an RT handler takes the arrival time stamp of a
non-RT IRQ and then propagates it.
Jsn
signature.asc
Description: OpenPGP digital signature
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 21/02/06, Jan Kiszka [EMAIL PROTECTED] wrote: Dmitry Adamushko wrote: N.B. Amongst other things, some thoughts about CHAINED with shared interrupts. On 20/02/06, Anders Blomdell [EMAIL PROTECTED] wrote: A number of questions arise: 1. What happens if one of the shared handlers leaves the interrupt asserted, returns NOENABLE|HANDLED and another return only HANDLED? 2. What happens if one returns PROPAGATE and another returns HANDLED? Yep, each ISR may return a different value and all of them are accumulated in the s variable ( s |= intr-isr(intr); ). So the loop may end up with s which contains all of the possible bits: (e.g. isr1 - HANDLED | ENABLE isr2 - HANDLED (don't want the irq to be enabled) isr3 - CHAINED ) s = HANDLED | ENABLE | CHAINED; Then CHAINED will be ignored because of the following code : +if (s XN_ISR_ENABLE) + xnarch_end_irq(irq); +else if (s XN_ISR_CHAINED)(*) + xnarch_chain_irq(irq);Which may actually be fatal: consider one ISR intentionally not handling an IRQ but propagating it while another ISR thinks it has to re-enableit - bang! Anders' as well as my original suggestion were to ignore theENABLE in this case. the current code in the CVS doen not contain else in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination. This said, we suppose that one knows what he is doing. In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE). So my feeling is that CHAINED should not be used by drivers which registered their ISRs as SHARED.That's true for standard drivers, but we should not prevent specialcorner-case designs which can be tuned to make even shared RT-IRQs + propagation possible. Sharing does not necessarily mean that differentRT drivers are involved... Yep. But in this case the designer of the system should _care_ about all corners of the system, i.e. to make sure that all ISRs return values which are not contradictory to each other. e.g. 1) isr1 : HANDLED (keep disabled) isr2 : CHAINED or 2) isr1 : HANDLED | NOENABLE (will be enabled by rt_task1) isr2 : HANDLED | NOENABLE (-//- by rt_task2) 1) and 2) are wrong! Both lead to the IRQ line being .end-ed (xnarch_end_irq()) twice! And on some archs, as we have seen before, that may lead to lost interrupts. Of course, when designing the real-time system, one should take charge of all corners of the system, so that analyzing the possible return values of all other ISRs that may share the same IRQ line is not anything extravagant, but a requirement. This said, I see no reason then why we should prevent users from some cases (like CHAINED | ENABLE) and let him do the ones like 2). 2) is much more common scenario for irq sharing and looks sane from the point of view of a separate ISR, when the global picture (and nucleus innards) is not taken into account. p.s. my 2) is equal to 2 ISRs returning HANDLED_NOENABLE in terms of Anders. This case is one of the most common with shared interrupts which I would imagine. And it nevertheless leads to problems when the whole picture is not taken into account by the designer of a given ISR. Moreover, I actually see the only scenario of CHAINED (I provided it before) : all ISRs in the primary domain have reported UNHANDLED = nucleus propagates the interrupt down the pipeline with xnacrh_chain_irq(). This call actually returns 1 upon successful propagation (some domain down the pipeline was interested in this irq) and 0 otherwise.But this only means that some handler is registered down there, it cannot predict if that handler will actually care about the event. Linux (not only vanilla, but ipipe-aware too) always .end-s (calls desc-handler-end(irq)) on return from __do_IRQ(), no matter this interrupt was handled by some ISR or not. It also re-enables the IRQ line if it was not disabled by some ISR explicitly. We don't care about anything else. Jan -- Best regards,Dmitry Adamushko ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 21/02/06, Anders Blomdell [EMAIL PROTECTED] wrote: Dmitry Adamushko wrote: N.B. Amongst other things, some thoughts about CHAINED with shared interrupts. On 20/02/06, *Anders Blomdell* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: A number of questions arise: 1. What happens if one of the shared handlers leaves the interrupt asserted, returns NOENABLE|HANDLED and another return only HANDLED? 2. What happens if one returns PROPAGATE and another returns HANDLED? Yep, each ISR may return a different value and all of them are accumulated in the s variable ( s |= intr-isr(intr); ). So the loop may end up with s which contains all of the possible bits: (e.g. isr1 - HANDLED | ENABLE isr2 - HANDLED (don't want the irq to be enabled) isr3 - CHAINED ) s = HANDLED | ENABLE | CHAINED; Then CHAINED will be ignored because of the following code : +if (s XN_ISR_ENABLE) + xnarch_end_irq(irq); +else if (s XN_ISR_CHAINED)(*) + xnarch_chain_irq(irq); Which is the worst way possible of prioritizing them, if a Linux interrupt isactive when we get there with ENABLE|CHAINED, interrupts will be enabled withthe Linux interrupt still asserted - the IRQ-handlers will be called once more, probably returning ENABLE|CHAINED again - infinite loop... the current code in the CVS doen not contain else in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination. This said, we suppose that one knows what he is doing. In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE).Which is somewhat contrary to the concept of shared interrupts, if we have to take care of the global picture, why make them shared in the first place?(I like the concept of shared interrupts, but it is important that the frameworkgives a separation of concerns) Unfortunately, it looks to me that the current picture (even with your scalar values) requires from the user who develops a given IRQ to take into account the possible return values of other ISRs. As I pointed out, the situation when 2 ISRs return HANDLED_NOENABLE may lead to problems on some archs. --- ... I'll take a look at the rest of the message a bit later. -- Best regards,Dmitry Adamushko ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Timer and calibration
ROSSIER Daniel wrote: Hello, We are currently porting Adeos/Xenomai with Linux 2.6.14 on a ARM9-based Freescale i.mx21 (litekit) development board. We started from the available patch for the ARM-based Integrator board. We are now facing some interesting problems regarding clock/timer frequencies with this board, but they are about to be solved J However, we have a question of understanding; as far as we know, ipipe starts with an aperiodic (one-shot) timer at the initialization time, and that before the calibrate function has been called. So, we get one interrupt only since the xenomai scheduler has not been registered (we understand that the xenomai scheduler should give the next timer shot, but since it is not registered yet, no timer reprogramming is achieved). So, how can the calibrate function can be invoked safely if no timer IRQ is received since this kind of calibration comes before the xenomai registration (the calibrate function needs IRQ timers to calibrate the number of busy loops between two jiffies) ? Unlike Linux's calibrate_delay loop, Xenomai's timer calibration code does not measure the CPU performance level, but only the average cost of programming the underlying timer hw in oneshot mode (time-wise), so that the nucleus can take this unavoidable delay into account when programming the next shot. Nowadays, this is basically an x86+8254 PIT only issue, since on this platform, one has to go through the ISA bus to (re)program the PIT for the next timer shot, and this is quite expensive (1.5 - 2.5 us for each outb, and you need two of them for programming the shot). Btw, this is the reason why using the APIC when available on x86 is always a good idea, since it only costs ~100 ns to program the timer there through a memory mapped register. IOW, the code does not wait for any timer IRQ, but only measures the average time for setting up the proper hw registers in order to program a timer shot. How is it realized with a x86 architecture (another timer source?) Look at ksrc/arch/*/hal.c, where all archs implement rthal_timer_calibrate() their own way. Is there any documentation – or discussion threads - which gives some information about the use of timers/RTC/TSC with Xenomai? Maybe. Try browsing the archive. Thanks so much for your help Kind regards Daniel Rossier ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core -- Philippe. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote:
N.B. Amongst other things, some thoughts about CHAINED with shared
interrupts.
On 20/02/06, *Anders Blomdell* [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
A number of questions arise:
1. What happens if one of the shared handlers leaves the interrupt
asserted,
returns NOENABLE|HANDLED and another return only HANDLED?
2. What happens if one returns PROPAGATE and another returns HANDLED?
Yep, each ISR may return a different value and all of them are
accumulated in the s variable ( s |= intr-isr(intr); ).
So the loop may end up with s which contains all of the possible bits:
(e.g.
isr1 - HANDLED | ENABLE
isr2 - HANDLED (don't want the irq to be enabled)
isr3 - CHAINED
)
s = HANDLED | ENABLE | CHAINED;
Then CHAINED will be ignored because of the following code :
+if (s XN_ISR_ENABLE)
+ xnarch_end_irq(irq);
+else if (s XN_ISR_CHAINED)(*)
+ xnarch_chain_irq(irq);
Which is the worst way possible of prioritizing them, if a Linux interrupt is
active when we get there with ENABLE|CHAINED, interrupts will be enabled with
the Linux interrupt still asserted - the IRQ-handlers will be called once more,
probably returning ENABLE|CHAINED again - infinite loop...
the current code in the CVS doen not contain else in (*), so that
ENABLE | CHAINED is possible, though it's a wrong combination.
This said, we suppose that one knows what he is doing.
In the case of a single ISR per line, it's not that difficult to
achieve. But if there are a few ISRs, then one should analize and take
into account all possible return values of all the ISRs, as each of them
may affect others (e.g. if one returns CHAINED when another - HANDLED |
ENABLE).
Which is somewhat contrary to the concept of shared interrupts, if we have to
take care of the global picture, why make them shared in the first place?
(I like the concept of shared interrupts, but it is important that the framework
gives a separation of concerns)
So my feeling is that CHAINED should not be used by drivers which
registered their ISRs as SHARED.
Well, CHAINED should not be used by drivers which return ENABLE (and are of
course hence incompatible with true realtime IRQ's)
Moreover, I actually see the only scenario of CHAINED (I provided it
before) :
all ISRs in the primary domain have reported UNHANDLED = nucleus
propagates the interrupt down the pipeline with xnacrh_chain_irq().
This call actually returns 1 upon successful propagation (some domain
down the pipeline was interested in this irq) and 0 otherwise.
Upon 0, this is a spurious irq (none of domains was interested in its
handling).
ok, let's suppose now :
we have 2 ISRs on the same shared line :
isr1 : HANDLED (will be enabled by rt task. Note, rt task must call
xnarch_end_irq() and not just xnarch_enable_irq()! )
isr2 : CHAINED
So HANDLED | CHAINED is ok for the single ISR on the line, but it may
lead to HANDLED | CHAINED | ENABLE in a case of the shared line.
rt task that works jointly with isr1 just calls xnarch_end_irq() at some
moment of time and some ISR in the linux domain does the same later =
the line is .end-ed 2 times.
ISR should never return CHAINED as to indicate _only_ that it is not
interested in this irq, but ~HANDLED or NOINT (if we'll support it) instead.
If the ISR nevertheless wants to propagate the IRQ to the Linux domain
_explicitly_, it _must not_ register itself as SHARED, i.e. it _must_ be
the only ISR on this line, otherwise that may lead to the IRQ line being
.end-ed twice (lost interrupts in some cases).
#define UNHANDLED 0
#define HANDLED_ENABLE 1
#define HANDLED_NOENABLE 2
#define PROPAGATE 3
Yep, I'd agree with you. Moreover, PROPAGATE should not be used for
shared interrupts.
My feeling is that it should be considered an error to attach a RT IRQ handler
to a line that has a Linux IRQ handler (this should be possible to check, since
/proc/interrupts contains the relevant information), unless a Linux IRQ-mask
function is installed. This IRQ-mask function should the be called:
1. each time domains are switched
2. each time an interrupt is generated
The IRQ-mask function should look something like:
unsigned int rt_irq_mask(struct ipipe_domain *ipd, unsigned int irq)
{
int result = 0;
static int enabled = true;
int enable = enabled;
if (irq = 0) {
// Interrupt has occured, we are about to run IRQ handlers
if (disable_early) {
enable = false;
}
if (for_linux(irq)) {
result = XN_ISR_CHAINED;
}
} else if (ipd == ipipe_root_domain) {
// Entering Linux
enable = true;
} else {
// Other doamin, block linux interrupts
enable = false;
}
if (enable != enabled) {
enabled = enable
if (enable) {
// Enable Linux interrupts by unmasking appropriate
// device registers (and possibly entire IRQ's)
} else {
// Disable Linux interrupts by
Re: [Xenomai-core] Timer and calibration
ROSSIER Daniel wrote: Hi Philippe, I perfectly got the point that the pipeline is already up and running when calibrate_delay() is called. On our ARM board, the pipeline seems to work perfectly. I however misunderstands something. The ipipe configures the timer in one-shot mode when ipipe_init is called, right? (also according to what it is mentioned in the Adeos-enhanced configuration file of Linux). Tracing the kernel during the boot process showed us one timer IRQ only; this is coherent. Is it correct that right after ipipe_init(), there is no realtime scheduler yet? The pipeline doesn't care about the schedule (it's the stuff of the Xenomai pod), and therefore, the pipline doesn't reprogram the timer by itself, right? - and it would be necessary to do that if the timer is in oneshot mode. So, if calibrate_delay() -not used by Adeos/Xenomai, I agree - is doing a busy loop to evaluate the CPU performance, waiting for the next incoming timer IRQ, the system doesn't exit the loop and freezes. It actually works as long as the timer is in periodic mode. It's actually the behaviour we observed on the ARM board with the ARM patch. I probably missed something, please correct me. On platforms that do work natively in one shot mode instead of using a PIT, the I-pipe reprograms itself the underlying hw timer, _except_ if it has been told differently by some domain, in which case it delegates this to the said domain. Have a look at linux/arch/arm/mach-integrator/core.c in the Integrator/CP patch: integrator_timer_interrupt(). IOW, the I-pipe reprograms the timer until Xenomai grabs the hw timer. Your patch likely lacks the corresponding code to handle that part in the platform-dependent section applicable to your board. Thanks again for your input. Daniel -Original Message- From: Philippe Gerum [mailto:[EMAIL PROTECTED] Sent: Tue 2/21/2006 12:57 PM To: ROSSIER Daniel Subject: Re: [Xenomai-core] Timer and calibration ROSSIER Daniel wrote: Hi Philippe, Great! Thx a lot for these precisions, it will help us. But regarding calibrate_delay(), it's still called in the normal boot process of Linux even with the Adeos/Xenomai patch. Sure it is. The interrupt pipeline is up and running at this point (ipipe_init), so there is no issue getting IRQs there. I've also seen that the boot process on a x86 architecture actually works since the timer value is read directly from the timer (calibrate_delay_direct() in init/calibrate.c) and this doesn't need to get a timer IRQ. But without that, and if calibrate_delay() is called instead of calibrate_delay_direct(), it fails. It actually works in both cases. You do have the IRQ sub-system already virtualized at that point. ipipe_init has engaged the pipeline, local_irq_enable() right before the calibration delay unstalls the root domain, so the timer ticks can flow through the pipeline, and they actually do (well, otherwise, you would not have a single Adeos-enabled x86 box working since 2002 :o) You can convince yourself by commenting out the local_irq_enable() statement before the calibration is started: then your x86 box would completely stall. (Sorry for my previous mail; when I talked about timer calibration, it was delay calibration) Daniel -Message d'origine- De : Philippe Gerum [mailto:[EMAIL PROTECTED] Envoyé : mardi, 21. février 2006 12:41 À : ROSSIER Daniel Cc : xenomai-core@gna.org Objet : Re: [Xenomai-core] Timer and calibration ROSSIER Daniel wrote: Hello, We are currently porting Adeos/Xenomai with Linux 2.6.14 on a ARM9-based Freescale i.mx21 (litekit) development board. We started from the available patch for the ARM-based Integrator board. We are now facing some interesting problems regarding clock/timer frequencies with this board, but they are about to be solved J However, we have a question of understanding; as far as we know, ipipe starts with an aperiodic (one-shot) timer at the initialization time, and that before the calibrate function has been called. So, we get one interrupt only since the xenomai scheduler has not been registered (we understand that the xenomai scheduler should give the next timer shot, but since it is not registered yet, no timer reprogramming is achieved). So, how can the calibrate function can be invoked safely if no timer IRQ is received since this kind of calibration comes before the xenomai registration (the calibrate function needs IRQ timers to calibrate the number of busy loops between two jiffies) ? Unlike Linux's calibrate_delay loop, Xenomai's timer calibration code does not measure the CPU performance level, but only the average cost of programming the underlying timer hw in oneshot mode (time-wise), so that the nucleus can take this unavoidable delay into account when programming the next shot. Nowadays, this is basically an x86+8254 PIT only issue, since on this platform, one has to go through the ISA bus
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). HANDLED_NOEBNABLE could be supported too. There would be no need in reenventing a wheel, just do it the way Linux does it. But it's about some additional re-designing of the current codebase (e.g. nested calling for irq_enable/disable()) I'm not sure we do need it for something else rather than irq sharing code but it affects the rest of the code. And we have a kind of wrong concept : XN_ISR_ENABLE (or NOENABLE) corresponds to xnarch_end_irq(). But the later one is not only about enabling the line, but on some archs - about .end-ing it too (sending EOI). And to support HANDLED_NOENABLE properly, those 2 have to be decoupled, i.e. EOI should always be sent from xnintr_shirq_handler(). Yes, should. And this should is best be handled by a) Documenting the potential conflict in the same place when describing the return values b) Placing some debug warning in the nucleus' IRQ trampoline function to bail out (once per line) when running into such situation But I'm against any further runtime restrictions, especially as most drivers will never return anything else than NOT_HANDLED or HANDLED. Actually, this was the reason why I tried to separate the NO_ENABLE and PROPAGATE features as *additional* bits from HANDLED and NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit combination present as constants can be more helpful for the user. We just have to draw some line between the standard values and the additional gurus return codes (documentation: don't use NO_ENABLE or PROPAGATE unless you understand their side-effects and pitfalls precisely). I agree with you on PROPAGATE case, but NO_ENABLE that, as pointed out above, should (IMHO and at least, in theory) only mean keep the IRQ line disabled (and have nothing to do with .end-ing the IRQ line) would be better supported. But this is, again as was pointed out above, about some redesigning of the current code = some overhead that likely affects non-shared aware code too. So on one hand, I'm ready to re-work code with : HANDLED and UNHANDLED (or NOINT) + 2 additional bits : NOENABLE and PROPAGATE. and document it like you suggested don't use NO_ENABLE or PROPAGATE with shared interrupts unless you understand their side-effects and pitfalls precisely; on the other hand, I'd say that I'm almost ready to vote against merging the irq sharing code at all as it looks to be a rather partial solution. -- Best regards,Dmitry Adamushko ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote: Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). HANDLED_NOEBNABLE could be supported too. Yes, but it breaks decoupling between shared handlers; interrupts will be deferred for all [shared] handlers until it is properly ended. There would be no need in reenventing a wheel, just do it the way Linux does it. But it's about some additional re-designing of the current codebase (e.g. nested calling for irq_enable/disable()) I'm not sure we do need it for something else rather than irq sharing code but it affects the rest of the code. And we have a kind of wrong concept : XN_ISR_ENABLE (or NOENABLE) corresponds to xnarch_end_irq(). Agree But the later one is not only about enabling the line, but on some archs - about .end-ing it too (sending EOI). And to support HANDLED_NOENABLE properly, those 2 have to be decoupled, i.e. EOI should always be sent from xnintr_shirq_handler(). But the one returning HANDLED_NOENABLE is likely to leave the interrupt asserted, hence we can't EOI at this point (unless NO_ENABLE means DISABLE). Yes, should. And this should is best be handled by a) Documenting the potential conflict in the same place when describing the return values b) Placing some debug warning in the nucleus' IRQ trampoline function to bail out (once per line) when running into such situation But I'm against any further runtime restrictions, especially as most drivers will never return anything else than NOT_HANDLED or HANDLED. Actually, this was the reason why I tried to separate the NO_ENABLE and PROPAGATE features as *additional* bits from HANDLED and NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit combination present as constants can be more helpful for the user. We just have to draw some line between the standard values and the additional gurus return codes (documentation: don't use NO_ENABLE or PROPAGATE unless you understand their side-effects and pitfalls precisely). I agree with you on PROPAGATE case, but NO_ENABLE that, as pointed out above, should (IMHO and at least, in theory) only mean keep the IRQ line disabled (and have nothing to do with .end-ing the IRQ line) would be better supported. But this is, again as was pointed out above, about some redesigning of the current code = some overhead that likely affects non-shared aware code too. So on one hand, I'm ready to re-work code with : HANDLED and UNHANDLED (or NOINT) + 2 additional bits : NOENABLE and PROPAGATE. and document it like you suggested don't use NO_ENABLE or PROPAGATE with shared interrupts unless you understand their side-effects and pitfalls precisely; on the other hand, I'd say that I'm almost ready to vote against merging the irq sharing code at all as it looks to be a rather partial solution. I vote for (even though I'm the one who complains the most), BUT I think it is important to keep the rules for using it simple (that's why I worry about the plethora of return-flags). -- Regards Anders ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Jan Kiszka wrote: Anders Blomdell wrote: Dmitry Adamushko wrote: Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). HANDLED_NOEBNABLE could be supported too. Yes, but it breaks decoupling between shared handlers; interrupts will be deferred for all [shared] handlers until it is properly ended. There would be no need in reenventing a wheel, just do it the way Linux does it. But it's about some additional re-designing of the current codebase (e.g. nested calling for irq_enable/disable()) I'm not sure we do need it for something else rather than irq sharing code but it affects the rest of the code. And we have a kind of wrong concept : XN_ISR_ENABLE (or NOENABLE) corresponds to xnarch_end_irq(). Agree But the later one is not only about enabling the line, but on some archs - about .end-ing it too (sending EOI). And to support HANDLED_NOENABLE properly, those 2 have to be decoupled, i.e. EOI should always be sent from xnintr_shirq_handler(). But the one returning HANDLED_NOENABLE is likely to leave the interrupt asserted, hence we can't EOI at this point (unless NO_ENABLE means DISABLE). I guess this is what Dmitry meant: explicitly call disable() if one or more ISRs returned NOENABLE - at least on archs where end != enable. Will this work? We could then keep on using the existing IRQ-enable API from bottom-half IRQ tasks. But what about NOENABLE+PROPAGATE? Will this special case still mean NOT to end the ISR (as Linux will do)? Bah, we are running in circles, I'm afraid. I think it's better to call NOENABLE NOEOI, which will indeed mean to not end this line (as it is the current situation anyway, isn't it?), and leave the user with what (s)he can do with such a feature. We found out that there are trillions of ways to shoot oneself into the foot with NOENABLE and PROPAGATE, and we cannot prevent most of them. So let's stop trying, at least for this patch! Yes, should. And this should is best be handled by a) Documenting the potential conflict in the same place when describing the return values b) Placing some debug warning in the nucleus' IRQ trampoline function to bail out (once per line) when running into such situation But I'm against any further runtime restrictions, especially as most drivers will never return anything else than NOT_HANDLED or HANDLED. Actually, this was the reason why I tried to separate the NO_ENABLE and PROPAGATE features as *additional* bits from HANDLED and NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit combination present as constants can be more helpful for the user. We just have to draw some line between the standard values and the additional gurus return codes (documentation: don't use NO_ENABLE or PROPAGATE unless you understand their side-effects and pitfalls precisely). I agree with you on PROPAGATE case, but NO_ENABLE that, as pointed out above, should (IMHO and at least, in theory) only mean keep the IRQ line disabled (and have nothing to do with .end-ing the IRQ line) would be better supported. But this is, again as was pointed out above, about some redesigning of the current code = some overhead that likely affects non-shared aware code too. So on one hand, I'm ready to re-work code with : HANDLED and UNHANDLED (or NOINT) + 2 additional bits : NOENABLE and PROPAGATE. and document it like you suggested don't use NO_ENABLE or PROPAGATE with shared interrupts unless you understand their side-effects and pitfalls precisely; on the other hand, I'd say that I'm almost ready to vote against merging the irq sharing code at all as it looks to be a rather partial solution. I vote for (even though I'm the one who complains the most), BUT I think it is important to keep the rules for using it simple (that's why I worry about the plethora of return-flags). And I'm with you here: My original proposal (2 base-states + 2 bits) created 8 expressible states while your version only knows 4 states - those which make sense most (and 2 of them are still the ones recommand for the masses). For RTDM I'm now almost determined to rework the API in way that only HANDLED/UNHANDLED (or what ever their names will be) get exported, any additional guru features will remain excluded as long as we have no clean usage policy for them. You have my vote for this. -- Anders ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Anders Blomdell wrote:
Jan Kiszka wrote:
Hi Dmitry,
Dmitry Adamushko wrote:
Hi Jan,
let's make yet another revision of the bits :
new XN_ISR_HANDLED == old XN_ISR_HANDLED + old XN_ISR_NO_ENABLE
ok.
new XN_ISR_NOENABLE == ~ old XN_ISR_ENABLE
ok.
new XN_ISR_PROPAGATE == XN_ISR_CHAINED
ok.
Just to make sure that you understand my weird ideas: each of the three
new XN_ISR_xxx above should be encoded with an individual bit
new XN_ISR_NOINT == ?
does it suppose the interrupt line to be .end-ed (enabled) and irq
not to be
propagated? Should be so, I guess, if it's different from 5). Then
nucleus
ignores implicit IRQ enable for 5) as well as for 3).
Do we really need that NOINT then, as it seems to be the same as
~HANDLED?
or NOINT == 0 and then it's a scalar value, not a bit.
So one may consider HANDLED == 1 and NOINT == 0 as really scalar values
and
NOENABLE and PROPAGATE as additional bits (used only if needed).
My idea is to urge the user specifying one of the base return types
(HANDLED or NOINT) + any of the two additional bits (NOENABLE and
PROPAGATE).
For correct drivers NOINT could be 0 indeed, but to check that the user
picked a new constant we may want to set NOINT != 0. With the old API
return 0 expressed HANDLED + ~ENABLE for the old API. With the new one
the user signals no interest and the nucleus may raise a warning that a
spurious IRQ occurred. So I would add a debug bit for NOINT here to
optionally (on OPT_XENO_DEBUG) detect old-style usage (return 0).
Moreover, we gain freedom to move bits in the future when every state is
encoded via constants. Or am I too paranoid here?
After reading the above discussion (of which I understand very little),
and looking at (what I believe to be) the relevant code:
+intr = shirq-handlers;
+
+while (intr)
+{
+s |= intr-isr(intr);
+++intr-hits;
+intr = intr-next;
+}
+xnintr_shirq_unlock(shirq);
+
+--sched-inesting;
+
+if (s XN_ISR_ENABLE)
+ xnarch_end_irq(irq);
+else if (s XN_ISR_CHAINED)
+ xnarch_chain_irq(irq);
A number of questions arise:
1. What happens if one of the shared handlers leaves the interrupt
asserted, returns NOENABLE|HANDLED and another return only HANDLED?
2. What happens if one returns PROPAGATE and another returns HANDLED?
As far as I can tell, after all RT handlers havve run, the following
scenarios are possible:
1. The interrupt is deasserted (i.e. it was a RT interrupt)
2. The interrupt is still asserted, it will be deasserted later
by some RT task (i.e. it was a RT interrupt)
3. The interrupt is still asserted and will be deasserted
by the Linux IRQ handler.
IMHO that leads to the conclusion that the IRQ handlers should return a
scalar
#define UNHANDLED 0
#define HANDLED_ENABLE 1
#define HANDLED_NOENABLE 2
#define PROPAGATE 3
and the loop should be
s = UNHANDLED
while (intr) {
tmp = intr-isr(intr);
if (tmp s) { s = tmp; }
intr = intr-next;
}
if (s == PROPAGATE) {
xnarch_chain_irq(irq);
} else if (s == HANDLED_ENABLE) {
xnarch_end_irq(irq);
}
This is a very useful suggestion, specifically this escalation of the
return value in the shared case. I would just let UNHANLDED start with 1
for the reasons I explained here:
https://mail.gna.org/public/xenomai-core/2006-02/msg00186.html
Ok, I would say let's got for this and finalise the patch!
To be really honest, I think that PROPAGATE should be excluded from the
RT IRQ-handlers, since with the current scheme all RT-handlers has to
test if the IRQ was a Linux interrupt (otherwise the system will only
work when the handler that returns PROPAGATE is installed)
Indeed, but I'm with Philippe here: do not exclude the strange corner
case scenarios users may craft with the appropriate care. I could, e.g.,
imagine a scenario where an RT handler takes the arrival time stamp of a
non-RT IRQ and then propagates it.
Jsn
signature.asc
Description: OpenPGP digital signature
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote: N.B. Amongst other things, some thoughts about CHAINED with shared interrupts. On 20/02/06, Anders Blomdell [EMAIL PROTECTED] wrote: A number of questions arise: 1. What happens if one of the shared handlers leaves the interrupt asserted, returns NOENABLE|HANDLED and another return only HANDLED? 2. What happens if one returns PROPAGATE and another returns HANDLED? Yep, each ISR may return a different value and all of them are accumulated in the s variable ( s |= intr-isr(intr); ). So the loop may end up with s which contains all of the possible bits: (e.g. isr1 - HANDLED | ENABLE isr2 - HANDLED (don't want the irq to be enabled) isr3 - CHAINED ) s = HANDLED | ENABLE | CHAINED; Then CHAINED will be ignored because of the following code : +if (s XN_ISR_ENABLE) + xnarch_end_irq(irq); +else if (s XN_ISR_CHAINED)(*) + xnarch_chain_irq(irq); Which may actually be fatal: consider one ISR intentionally not handling an IRQ but propagating it while another ISR thinks it has to re-enable it - bang! Anders' as well as my original suggestion were to ignore the ENABLE in this case. the current code in the CVS doen not contain else in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination. This said, we suppose that one knows what he is doing. In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE). So my feeling is that CHAINED should not be used by drivers which registered their ISRs as SHARED. That's true for standard drivers, but we should not prevent special corner-case designs which can be tuned to make even shared RT-IRQs + propagation possible. Sharing does not necessarily mean that different RT drivers are involved... Moreover, I actually see the only scenario of CHAINED (I provided it before) : all ISRs in the primary domain have reported UNHANDLED = nucleus propagates the interrupt down the pipeline with xnacrh_chain_irq(). This call actually returns 1 upon successful propagation (some domain down the pipeline was interested in this irq) and 0 otherwise. But this only means that some handler is registered down there, it cannot predict if that handler will actually care about the event. Upon 0, this is a spurious irq (none of domains was interested in its handling). ok, let's suppose now : we have 2 ISRs on the same shared line : isr1 : HANDLED (will be enabled by rt task. Note, rt task must call xnarch_end_irq() and not just xnarch_enable_irq()! ) isr2 : CHAINED So HANDLED | CHAINED is ok for the single ISR on the line, but it may lead to HANDLED | CHAINED | ENABLE in a case of the shared line. rt task that works jointly with isr1 just calls xnarch_end_irq() at some moment of time and some ISR in the linux domain does the same later = the line is .end-ed 2 times. ISR should never return CHAINED as to indicate _only_ that it is not interested in this irq, but ~HANDLED or NOINT (if we'll support it) instead. If the ISR nevertheless wants to propagate the IRQ to the Linux domain _explicitly_, it _must not_ register itself as SHARED, i.e. it _must_ be the only ISR on this line, otherwise that may lead to the IRQ line being .end-ed twice (lost interrupts in some cases). #define UNHANDLED 0 #define HANDLED_ENABLE 1 #define HANDLED_NOENABLE 2 #define PROPAGATE 3 Yep, I'd agree with you. Moreover, PROPAGATE should not be used for shared interrupts. -- Best regards, Dmitry Adamushko Jan signature.asc Description: OpenPGP digital signature
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 21/02/06, Jan Kiszka [EMAIL PROTECTED] wrote: Dmitry Adamushko wrote: N.B. Amongst other things, some thoughts about CHAINED with shared interrupts. On 20/02/06, Anders Blomdell [EMAIL PROTECTED] wrote: A number of questions arise: 1. What happens if one of the shared handlers leaves the interrupt asserted, returns NOENABLE|HANDLED and another return only HANDLED? 2. What happens if one returns PROPAGATE and another returns HANDLED? Yep, each ISR may return a different value and all of them are accumulated in the s variable ( s |= intr-isr(intr); ). So the loop may end up with s which contains all of the possible bits: (e.g. isr1 - HANDLED | ENABLE isr2 - HANDLED (don't want the irq to be enabled) isr3 - CHAINED ) s = HANDLED | ENABLE | CHAINED; Then CHAINED will be ignored because of the following code : +if (s XN_ISR_ENABLE) + xnarch_end_irq(irq); +else if (s XN_ISR_CHAINED)(*) + xnarch_chain_irq(irq);Which may actually be fatal: consider one ISR intentionally not handling an IRQ but propagating it while another ISR thinks it has to re-enableit - bang! Anders' as well as my original suggestion were to ignore theENABLE in this case. the current code in the CVS doen not contain else in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination. This said, we suppose that one knows what he is doing. In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE). So my feeling is that CHAINED should not be used by drivers which registered their ISRs as SHARED.That's true for standard drivers, but we should not prevent specialcorner-case designs which can be tuned to make even shared RT-IRQs + propagation possible. Sharing does not necessarily mean that differentRT drivers are involved... Yep. But in this case the designer of the system should _care_ about all corners of the system, i.e. to make sure that all ISRs return values which are not contradictory to each other. e.g. 1) isr1 : HANDLED (keep disabled) isr2 : CHAINED or 2) isr1 : HANDLED | NOENABLE (will be enabled by rt_task1) isr2 : HANDLED | NOENABLE (-//- by rt_task2) 1) and 2) are wrong! Both lead to the IRQ line being .end-ed (xnarch_end_irq()) twice! And on some archs, as we have seen before, that may lead to lost interrupts. Of course, when designing the real-time system, one should take charge of all corners of the system, so that analyzing the possible return values of all other ISRs that may share the same IRQ line is not anything extravagant, but a requirement. This said, I see no reason then why we should prevent users from some cases (like CHAINED | ENABLE) and let him do the ones like 2). 2) is much more common scenario for irq sharing and looks sane from the point of view of a separate ISR, when the global picture (and nucleus innards) is not taken into account. p.s. my 2) is equal to 2 ISRs returning HANDLED_NOENABLE in terms of Anders. This case is one of the most common with shared interrupts which I would imagine. And it nevertheless leads to problems when the whole picture is not taken into account by the designer of a given ISR. Moreover, I actually see the only scenario of CHAINED (I provided it before) : all ISRs in the primary domain have reported UNHANDLED = nucleus propagates the interrupt down the pipeline with xnacrh_chain_irq(). This call actually returns 1 upon successful propagation (some domain down the pipeline was interested in this irq) and 0 otherwise.But this only means that some handler is registered down there, it cannot predict if that handler will actually care about the event. Linux (not only vanilla, but ipipe-aware too) always .end-s (calls desc-handler-end(irq)) on return from __do_IRQ(), no matter this interrupt was handled by some ISR or not. It also re-enables the IRQ line if it was not disabled by some ISR explicitly. We don't care about anything else. Jan -- Best regards,Dmitry Adamushko
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 21/02/06, Anders Blomdell [EMAIL PROTECTED] wrote: Dmitry Adamushko wrote: N.B. Amongst other things, some thoughts about CHAINED with shared interrupts. On 20/02/06, *Anders Blomdell* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: A number of questions arise: 1. What happens if one of the shared handlers leaves the interrupt asserted, returns NOENABLE|HANDLED and another return only HANDLED? 2. What happens if one returns PROPAGATE and another returns HANDLED? Yep, each ISR may return a different value and all of them are accumulated in the s variable ( s |= intr-isr(intr); ). So the loop may end up with s which contains all of the possible bits: (e.g. isr1 - HANDLED | ENABLE isr2 - HANDLED (don't want the irq to be enabled) isr3 - CHAINED ) s = HANDLED | ENABLE | CHAINED; Then CHAINED will be ignored because of the following code : +if (s XN_ISR_ENABLE) + xnarch_end_irq(irq); +else if (s XN_ISR_CHAINED)(*) + xnarch_chain_irq(irq); Which is the worst way possible of prioritizing them, if a Linux interrupt isactive when we get there with ENABLE|CHAINED, interrupts will be enabled withthe Linux interrupt still asserted - the IRQ-handlers will be called once more, probably returning ENABLE|CHAINED again - infinite loop... the current code in the CVS doen not contain else in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination. This said, we suppose that one knows what he is doing. In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE).Which is somewhat contrary to the concept of shared interrupts, if we have to take care of the global picture, why make them shared in the first place?(I like the concept of shared interrupts, but it is important that the frameworkgives a separation of concerns) Unfortunately, it looks to me that the current picture (even with your scalar values) requires from the user who develops a given IRQ to take into account the possible return values of other ISRs. As I pointed out, the situation when 2 ISRs return HANDLED_NOENABLE may lead to problems on some archs. --- ... I'll take a look at the rest of the message a bit later. -- Best regards,Dmitry Adamushko
[Xenomai-core] Timer and calibration
Hello, We are currently porting Adeos/Xenomai with Linux 2.6.14 on a ARM9-based Freescale i.mx21 (litekit) development board. We started from the available patch for the ARM-based Integrator board. We are now facing some interesting problems regarding clock/timer frequencies with this board, but they are about to be solved J However, we have a question of understanding; as far as we know, ipipe starts with an aperiodic (one-shot) timer at the initialization time, and that before the calibrate function has been called. So, we get one interrupt only since the xenomai scheduler has not been registered (we understand that the xenomai scheduler should give the next timer shot, but since it is not registered yet, no timer reprogramming is achieved). So, how can the calibrate function can be invoked safely if no timer IRQ is received since this kind of calibration comes before the xenomai registration (the calibrate function needs IRQ timers to calibrate the number of busy loops between two jiffies) ? How is it realized with a x86 architecture (another timer source?) Is there any documentation or discussion threads - which gives some information about the use of timers/RTC/TSC with Xenomai? Thanks so much for your help Kind regards Daniel Rossier
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote: On 21/02/06, *Anders Blomdell* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: Dmitry Adamushko wrote: N.B. Amongst other things, some thoughts about CHAINED with shared interrupts. On 20/02/06, *Anders Blomdell* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] mailto:[EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: A number of questions arise: 1. What happens if one of the shared handlers leaves the interrupt asserted, returns NOENABLE|HANDLED and another return only HANDLED? 2. What happens if one returns PROPAGATE and another returns HANDLED? Yep, each ISR may return a different value and all of them are accumulated in the s variable ( s |= intr-isr(intr); ). So the loop may end up with s which contains all of the possible bits: (e.g. isr1 - HANDLED | ENABLE isr2 - HANDLED (don't want the irq to be enabled) isr3 - CHAINED ) s = HANDLED | ENABLE | CHAINED; Then CHAINED will be ignored because of the following code : +if (s XN_ISR_ENABLE) + xnarch_end_irq(irq); +else if (s XN_ISR_CHAINED)(*) + xnarch_chain_irq(irq); Which is the worst way possible of prioritizing them, if a Linux interrupt is active when we get there with ENABLE|CHAINED, interrupts will be enabled with the Linux interrupt still asserted - the IRQ-handlers will be called once more, probably returning ENABLE|CHAINED again - infinite loop... the current code in the CVS doen not contain else in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination. This said, we suppose that one knows what he is doing. In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE). Which is somewhat contrary to the concept of shared interrupts, if we have to take care of the global picture, why make them shared in the first place? (I like the concept of shared interrupts, but it is important that the framework gives a separation of concerns) Unfortunately, it looks to me that the current picture (even with your scalar values) requires from the user who develops a given IRQ to take into account the possible return values of other ISRs. As I pointed out, the situation when 2 ISRs return HANDLED_NOENABLE may lead to problems on some archs. Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). -- Anders
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote:
N.B. Amongst other things, some thoughts about CHAINED with shared
interrupts.
On 20/02/06, *Anders Blomdell* [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
A number of questions arise:
1. What happens if one of the shared handlers leaves the interrupt
asserted,
returns NOENABLE|HANDLED and another return only HANDLED?
2. What happens if one returns PROPAGATE and another returns HANDLED?
Yep, each ISR may return a different value and all of them are
accumulated in the s variable ( s |= intr-isr(intr); ).
So the loop may end up with s which contains all of the possible bits:
(e.g.
isr1 - HANDLED | ENABLE
isr2 - HANDLED (don't want the irq to be enabled)
isr3 - CHAINED
)
s = HANDLED | ENABLE | CHAINED;
Then CHAINED will be ignored because of the following code :
+if (s XN_ISR_ENABLE)
+ xnarch_end_irq(irq);
+else if (s XN_ISR_CHAINED)(*)
+ xnarch_chain_irq(irq);
Which is the worst way possible of prioritizing them, if a Linux interrupt is
active when we get there with ENABLE|CHAINED, interrupts will be enabled with
the Linux interrupt still asserted - the IRQ-handlers will be called once more,
probably returning ENABLE|CHAINED again - infinite loop...
the current code in the CVS doen not contain else in (*), so that
ENABLE | CHAINED is possible, though it's a wrong combination.
This said, we suppose that one knows what he is doing.
In the case of a single ISR per line, it's not that difficult to
achieve. But if there are a few ISRs, then one should analize and take
into account all possible return values of all the ISRs, as each of them
may affect others (e.g. if one returns CHAINED when another - HANDLED |
ENABLE).
Which is somewhat contrary to the concept of shared interrupts, if we have to
take care of the global picture, why make them shared in the first place?
(I like the concept of shared interrupts, but it is important that the framework
gives a separation of concerns)
So my feeling is that CHAINED should not be used by drivers which
registered their ISRs as SHARED.
Well, CHAINED should not be used by drivers which return ENABLE (and are of
course hence incompatible with true realtime IRQ's)
Moreover, I actually see the only scenario of CHAINED (I provided it
before) :
all ISRs in the primary domain have reported UNHANDLED = nucleus
propagates the interrupt down the pipeline with xnacrh_chain_irq().
This call actually returns 1 upon successful propagation (some domain
down the pipeline was interested in this irq) and 0 otherwise.
Upon 0, this is a spurious irq (none of domains was interested in its
handling).
ok, let's suppose now :
we have 2 ISRs on the same shared line :
isr1 : HANDLED (will be enabled by rt task. Note, rt task must call
xnarch_end_irq() and not just xnarch_enable_irq()! )
isr2 : CHAINED
So HANDLED | CHAINED is ok for the single ISR on the line, but it may
lead to HANDLED | CHAINED | ENABLE in a case of the shared line.
rt task that works jointly with isr1 just calls xnarch_end_irq() at some
moment of time and some ISR in the linux domain does the same later =
the line is .end-ed 2 times.
ISR should never return CHAINED as to indicate _only_ that it is not
interested in this irq, but ~HANDLED or NOINT (if we'll support it) instead.
If the ISR nevertheless wants to propagate the IRQ to the Linux domain
_explicitly_, it _must not_ register itself as SHARED, i.e. it _must_ be
the only ISR on this line, otherwise that may lead to the IRQ line being
.end-ed twice (lost interrupts in some cases).
#define UNHANDLED 0
#define HANDLED_ENABLE 1
#define HANDLED_NOENABLE 2
#define PROPAGATE 3
Yep, I'd agree with you. Moreover, PROPAGATE should not be used for
shared interrupts.
My feeling is that it should be considered an error to attach a RT IRQ handler
to a line that has a Linux IRQ handler (this should be possible to check, since
/proc/interrupts contains the relevant information), unless a Linux IRQ-mask
function is installed. This IRQ-mask function should the be called:
1. each time domains are switched
2. each time an interrupt is generated
The IRQ-mask function should look something like:
unsigned int rt_irq_mask(struct ipipe_domain *ipd, unsigned int irq)
{
int result = 0;
static int enabled = true;
int enable = enabled;
if (irq = 0) {
// Interrupt has occured, we are about to run IRQ handlers
if (disable_early) {
enable = false;
}
if (for_linux(irq)) {
result = XN_ISR_CHAINED;
}
} else if (ipd == ipipe_root_domain) {
// Entering Linux
enable = true;
} else {
// Other doamin, block linux interrupts
enable = false;
}
if (enable != enabled) {
enabled = enable
if (enable) {
// Enable Linux interrupts by unmasking appropriate
// device registers (and possibly entire IRQ's)
} else {
// Disable Linux interrupts by
Re: [Xenomai-core] Timer and calibration
ROSSIER Daniel wrote: Hello, We are currently porting Adeos/Xenomai with Linux 2.6.14 on a ARM9-based Freescale i.mx21 (litekit) development board. We started from the available patch for the ARM-based Integrator board. We are now facing some interesting problems regarding clock/timer frequencies with this board, but they are about to be solved J However, we have a question of understanding; as far as we know, ipipe starts with an aperiodic (one-shot) timer at the initialization time, and that before the calibrate function has been called. So, we get one interrupt only since the xenomai scheduler has not been registered (we understand that the xenomai scheduler should give the next timer shot, but since it is not registered yet, no timer reprogramming is achieved). So, how can the calibrate function can be invoked safely if no timer IRQ is received since this kind of calibration comes before the xenomai registration (the calibrate function needs IRQ timers to calibrate the number of busy loops between two jiffies) ? Unlike Linux's calibrate_delay loop, Xenomai's timer calibration code does not measure the CPU performance level, but only the average cost of programming the underlying timer hw in oneshot mode (time-wise), so that the nucleus can take this unavoidable delay into account when programming the next shot. Nowadays, this is basically an x86+8254 PIT only issue, since on this platform, one has to go through the ISA bus to (re)program the PIT for the next timer shot, and this is quite expensive (1.5 - 2.5 us for each outb, and you need two of them for programming the shot). Btw, this is the reason why using the APIC when available on x86 is always a good idea, since it only costs ~100 ns to program the timer there through a memory mapped register. IOW, the code does not wait for any timer IRQ, but only measures the average time for setting up the proper hw registers in order to program a timer shot. How is it realized with a x86 architecture (another timer source?) Look at ksrc/arch/*/hal.c, where all archs implement rthal_timer_calibrate() their own way. Is there any documentation – or discussion threads - which gives some information about the use of timers/RTC/TSC with Xenomai? Maybe. Try browsing the archive. Thanks so much for your help Kind regards Daniel Rossier ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core -- Philippe.
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Anders Blomdell wrote: Dmitry Adamushko wrote: On 21/02/06, *Anders Blomdell* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: Dmitry Adamushko wrote: N.B. Amongst other things, some thoughts about CHAINED with shared interrupts. On 20/02/06, *Anders Blomdell* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] mailto:[EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: A number of questions arise: 1. What happens if one of the shared handlers leaves the interrupt asserted, returns NOENABLE|HANDLED and another return only HANDLED? 2. What happens if one returns PROPAGATE and another returns HANDLED? Yep, each ISR may return a different value and all of them are accumulated in the s variable ( s |= intr-isr(intr); ). So the loop may end up with s which contains all of the possible bits: (e.g. isr1 - HANDLED | ENABLE isr2 - HANDLED (don't want the irq to be enabled) isr3 - CHAINED ) s = HANDLED | ENABLE | CHAINED; Then CHAINED will be ignored because of the following code : +if (s XN_ISR_ENABLE) + xnarch_end_irq(irq); +else if (s XN_ISR_CHAINED)(*) + xnarch_chain_irq(irq); Which is the worst way possible of prioritizing them, if a Linux interrupt is active when we get there with ENABLE|CHAINED, interrupts will be enabled with the Linux interrupt still asserted - the IRQ-handlers will be called once more, probably returning ENABLE|CHAINED again - infinite loop... the current code in the CVS doen not contain else in (*), so that ENABLE | CHAINED is possible, though it's a wrong combination. This said, we suppose that one knows what he is doing. In the case of a single ISR per line, it's not that difficult to achieve. But if there are a few ISRs, then one should analize and take into account all possible return values of all the ISRs, as each of them may affect others (e.g. if one returns CHAINED when another - HANDLED | ENABLE). Which is somewhat contrary to the concept of shared interrupts, if we have to take care of the global picture, why make them shared in the first place? (I like the concept of shared interrupts, but it is important that the framework gives a separation of concerns) Unfortunately, it looks to me that the current picture (even with your scalar values) requires from the user who develops a given IRQ to take into account the possible return values of other ISRs. As I pointed out, the situation when 2 ISRs return HANDLED_NOENABLE may lead to problems on some archs. Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). Yes, should. And this should is best be handled by a) Documenting the potential conflict in the same place when describing the return values b) Placing some debug warning in the nucleus' IRQ trampoline function to bail out (once per line) when running into such situation But I'm against any further runtime restrictions, especially as most drivers will never return anything else than NOT_HANDLED or HANDLED. Actually, this was the reason why I tried to separate the NO_ENABLE and PROPAGATE features as *additional* bits from HANDLED and NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit combination present as constants can be more helpful for the user. We just have to draw some line between the standard values and the additional gurus return codes (documentation: don't use NO_ENABLE or PROPAGATE unless you understand their side-effects and pitfalls precisely). Jan signature.asc Description: OpenPGP digital signature
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). HANDLED_NOEBNABLE could be supported too. There would be no need in reenventing a wheel, just do it the way Linux does it. But it's about some additional re-designing of the current codebase (e.g. nested calling for irq_enable/disable()) I'm not sure we do need it for something else rather than irq sharing code but it affects the rest of the code. And we have a kind of wrong concept : XN_ISR_ENABLE (or NOENABLE) corresponds to xnarch_end_irq(). But the later one is not only about enabling the line, but on some archs - about .end-ing it too (sending EOI). And to support HANDLED_NOENABLE properly, those 2 have to be decoupled, i.e. EOI should always be sent from xnintr_shirq_handler(). Yes, should. And this should is best be handled by a) Documenting the potential conflict in the same place when describing the return values b) Placing some debug warning in the nucleus' IRQ trampoline function to bail out (once per line) when running into such situation But I'm against any further runtime restrictions, especially as most drivers will never return anything else than NOT_HANDLED or HANDLED. Actually, this was the reason why I tried to separate the NO_ENABLE and PROPAGATE features as *additional* bits from HANDLED and NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit combination present as constants can be more helpful for the user. We just have to draw some line between the standard values and the additional gurus return codes (documentation: don't use NO_ENABLE or PROPAGATE unless you understand their side-effects and pitfalls precisely). I agree with you on PROPAGATE case, but NO_ENABLE that, as pointed out above, should (IMHO and at least, in theory) only mean keep the IRQ line disabled (and have nothing to do with .end-ing the IRQ line) would be better supported. But this is, again as was pointed out above, about some redesigning of the current code = some overhead that likely affects non-shared aware code too. So on one hand, I'm ready to re-work code with : HANDLED and UNHANDLED (or NOINT) + 2 additional bits : NOENABLE and PROPAGATE. and document it like you suggested don't use NO_ENABLE or PROPAGATE with shared interrupts unless you understand their side-effects and pitfalls precisely; on the other hand, I'd say that I'm almost ready to vote against merging the irq sharing code at all as it looks to be a rather partial solution. -- Best regards,Dmitry Adamushko
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote: Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). HANDLED_NOEBNABLE could be supported too. Yes, but it breaks decoupling between shared handlers; interrupts will be deferred for all [shared] handlers until it is properly ended. There would be no need in reenventing a wheel, just do it the way Linux does it. But it's about some additional re-designing of the current codebase (e.g. nested calling for irq_enable/disable()) I'm not sure we do need it for something else rather than irq sharing code but it affects the rest of the code. And we have a kind of wrong concept : XN_ISR_ENABLE (or NOENABLE) corresponds to xnarch_end_irq(). Agree But the later one is not only about enabling the line, but on some archs - about .end-ing it too (sending EOI). And to support HANDLED_NOENABLE properly, those 2 have to be decoupled, i.e. EOI should always be sent from xnintr_shirq_handler(). But the one returning HANDLED_NOENABLE is likely to leave the interrupt asserted, hence we can't EOI at this point (unless NO_ENABLE means DISABLE). Yes, should. And this should is best be handled by a) Documenting the potential conflict in the same place when describing the return values b) Placing some debug warning in the nucleus' IRQ trampoline function to bail out (once per line) when running into such situation But I'm against any further runtime restrictions, especially as most drivers will never return anything else than NOT_HANDLED or HANDLED. Actually, this was the reason why I tried to separate the NO_ENABLE and PROPAGATE features as *additional* bits from HANDLED and NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit combination present as constants can be more helpful for the user. We just have to draw some line between the standard values and the additional gurus return codes (documentation: don't use NO_ENABLE or PROPAGATE unless you understand their side-effects and pitfalls precisely). I agree with you on PROPAGATE case, but NO_ENABLE that, as pointed out above, should (IMHO and at least, in theory) only mean keep the IRQ line disabled (and have nothing to do with .end-ing the IRQ line) would be better supported. But this is, again as was pointed out above, about some redesigning of the current code = some overhead that likely affects non-shared aware code too. So on one hand, I'm ready to re-work code with : HANDLED and UNHANDLED (or NOINT) + 2 additional bits : NOENABLE and PROPAGATE. and document it like you suggested don't use NO_ENABLE or PROPAGATE with shared interrupts unless you understand their side-effects and pitfalls precisely; on the other hand, I'd say that I'm almost ready to vote against merging the irq sharing code at all as it looks to be a rather partial solution. I vote for (even though I'm the one who complains the most), BUT I think it is important to keep the rules for using it simple (that's why I worry about the plethora of return-flags). -- Regards Anders
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Anders Blomdell wrote: Dmitry Adamushko wrote: Good point, leaves us with 2 possible return values for shared handlers: HANDLED NOT_HANDLED i.e. shared handlers should never defer the end'ing of the interrupt (which makes sense, since this would affect the other [shared] handlers). HANDLED_NOEBNABLE could be supported too. Yes, but it breaks decoupling between shared handlers; interrupts will be deferred for all [shared] handlers until it is properly ended. There would be no need in reenventing a wheel, just do it the way Linux does it. But it's about some additional re-designing of the current codebase (e.g. nested calling for irq_enable/disable()) I'm not sure we do need it for something else rather than irq sharing code but it affects the rest of the code. And we have a kind of wrong concept : XN_ISR_ENABLE (or NOENABLE) corresponds to xnarch_end_irq(). Agree But the later one is not only about enabling the line, but on some archs - about .end-ing it too (sending EOI). And to support HANDLED_NOENABLE properly, those 2 have to be decoupled, i.e. EOI should always be sent from xnintr_shirq_handler(). But the one returning HANDLED_NOENABLE is likely to leave the interrupt asserted, hence we can't EOI at this point (unless NO_ENABLE means DISABLE). I guess this is what Dmitry meant: explicitly call disable() if one or more ISRs returned NOENABLE - at least on archs where end != enable. Will this work? We could then keep on using the existing IRQ-enable API from bottom-half IRQ tasks. But what about NOENABLE+PROPAGATE? Will this special case still mean NOT to end the ISR (as Linux will do)? Bah, we are running in circles, I'm afraid. I think it's better to call NOENABLE NOEOI, which will indeed mean to not end this line (as it is the current situation anyway, isn't it?), and leave the user with what (s)he can do with such a feature. We found out that there are trillions of ways to shoot oneself into the foot with NOENABLE and PROPAGATE, and we cannot prevent most of them. So let's stop trying, at least for this patch! Yes, should. And this should is best be handled by a) Documenting the potential conflict in the same place when describing the return values b) Placing some debug warning in the nucleus' IRQ trampoline function to bail out (once per line) when running into such situation But I'm against any further runtime restrictions, especially as most drivers will never return anything else than NOT_HANDLED or HANDLED. Actually, this was the reason why I tried to separate the NO_ENABLE and PROPAGATE features as *additional* bits from HANDLED and NOT_HANDLED/UNHANDLED/NOINT. But I acknowledge that having all valid bit combination present as constants can be more helpful for the user. We just have to draw some line between the standard values and the additional gurus return codes (documentation: don't use NO_ENABLE or PROPAGATE unless you understand their side-effects and pitfalls precisely). I agree with you on PROPAGATE case, but NO_ENABLE that, as pointed out above, should (IMHO and at least, in theory) only mean keep the IRQ line disabled (and have nothing to do with .end-ing the IRQ line) would be better supported. But this is, again as was pointed out above, about some redesigning of the current code = some overhead that likely affects non-shared aware code too. So on one hand, I'm ready to re-work code with : HANDLED and UNHANDLED (or NOINT) + 2 additional bits : NOENABLE and PROPAGATE. and document it like you suggested don't use NO_ENABLE or PROPAGATE with shared interrupts unless you understand their side-effects and pitfalls precisely; on the other hand, I'd say that I'm almost ready to vote against merging the irq sharing code at all as it looks to be a rather partial solution. I vote for (even though I'm the one who complains the most), BUT I think it is important to keep the rules for using it simple (that's why I worry about the plethora of return-flags). And I'm with you here: My original proposal (2 base-states + 2 bits) created 8 expressible states while your version only knows 4 states - those which make sense most (and 2 of them are still the ones recommand for the masses). For RTDM I'm now almost determined to rework the API in way that only HANDLED/UNHANDLED (or what ever their names will be) get exported, any additional guru features will remain excluded as long as we have no clean usage policy for them. Jan signature.asc Description: OpenPGP digital signature
