Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote:
> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>> Dmitry Adamushko wrote:
>>> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
Dmitry Adamushko wrote:
> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>
> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this
>> before merging (i.e. make XN_ISR_HANDLED non-zero)?
> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now
> non-zero.
> Actually, at first I wanted to make it just the other way about.
>
Hmm, thinking about this again, I would like to revert my suggestion in
favour of a third approach (trying to keep you busy ;) ).
>>>
>>> Ok, you are wellcome :)
>>>
>>> I didn't get it, at least while looking briefly. To make it a bit
>> easier, at
>>> least for me, let's go another way.
>>>
>>> At the left is the list of allowed values as Philippe pointed out.
>>> At the right is another list which corresponds to the left one but when
>>> NOINT is used instead of HANDLED. Moreover, I have added another case -
>> pure
>>> NOINT. The ISR says it's not mine and, well, it doesn't know whether it
>>> should be propagated or no
>>> (ok, so far CHAINED standed for NOINT).
>>>
>>> 1.) HANDLED -> 0
>>> 2.) HANDLED | ENABLE -> ENABLE
>>> 3.) HANDLED | CHAINED-> CHAINED
>>> 4.) CHAINED -> CHAINED | NOINT
>>> 5.)-> NOINT
>>>
>>> Could you provide the 3-d corresponding table using your new flags?
>>>
>> Still not 3D, but hopefully clarifying: :)
>
>
>
> 1.) XN_ISR_HANDLED |XN_ISR_NO_ENABLE
>> 2.) XN_ISR_HANDLED
>> 3.) XN_ISR_HANDLED | XN_ISR_PROPAGATE
>> (nucleus ignores implicit IRQ enable)
>> 4.) XN_ISR_NOINT | XN_ISR_PROPAGATE
>> 5.) XN_ISR_NOINT
>
>
> Then why do we need XN_ISR_NOINT? is it not just the same as ~HANDLED?
The idea is to have at least a define so that
a) users explicitly have to state what they mean
b) we can map the define to some other value whenever we may need this
in the future
The first property could even be used to apply some debug checks in the
user's return value.
>
> Moreover, I tend to think now that PROPAGATE (CHAINED) should not be
> normally used by ISRs at all. The only exception would be irq sharing
> between RT and non-RT domains, but that's of no concern for most part of rt
> drivers and, as last events revealed, it's very partially supported at the
> moment and possible implementations are quite arguable.
Ack. But you should still export this feature for those who think they
know what they are doing. ;)
>
> So HANDLED means the irq line is .end-ed by xnintr_irq_handler()
> (xnarch_end_irq(irq) is called).
>
> If one wants to do it on his own later, NOENABLE can be returned
> additionally.
Yep. So let's call it XN_ISR_NOENABLE.
> But then, one must use xnintr_end_irq() and not just xnintr_irq_enable().
>
Yep, but a different (add-on) topic.
> If xnintr_end_irq() would be always the same as xnintr_irq_enable() or we
> would decouple ending and enabling operations on the line (don't have
> sources at hand); we could go Linux way and make use of a "nested" counter
> in xnintr_t, so that xnintr_irq_disable() could be called in a nested way.
> This said, calling xnintr_irq_disable() in ISR leads to the interrupt line
> being still disabled upon return from xnintr_irq_handler() and re-enabled
> again by the user later.
>
> ~HANDLED means the ISR was not concerned about this interrupt. i.e. :
>
> - there is ISR in another domain (down the pipeline) which is interested in
> this irq ;
>
> well, the designer of the system should be aware of such cases and
> probably avoid that on hw layer or, at least, to know what she is doing.
>
> - this is a spurious irq.
>
> So upon getting ~HANDLED, xnintr_irq_handler() does :
>
> success = xnintr_irq_chained(irq);
Only if requested via XN_ISR_PROPAGATE (or _CHAINED)! There is still a
real-time ISR registered on that IRQ you are about to forward now, and
you don't know if someone is handling it down the pipeline. Actually,
this is a fatal situation. The system is broken, bail out, and switch
this line off.
>
> if (success)
> {
> /* i.e. another domain is really interested and got this irq as pending*/
>
> return without .end-ing
> }
> else
> {
> /* this is a spurious irq */
>
> make use of some accounting of spurious interrupts (if we need it)), i.e.
> keep the irq line disabled if the % of unhandled requests > some limit.
>
> otherwise
>
> xnintr_end_irq() and return
> }
>
> too long :) ok, it's quite similar indeed to what you have suggested,
> with the exception that
>
> - no need for NOINT (if it's really == ~HANDLED);
> - nucleus provides some additional logic while handling ~HANDLED case.
> - user don't need to return ENABLE explicitly - in most cases, that's what
> he actually needs but sometimes just forgets (I recall a few questions on
> the mailing list abo
Re: [Xenomai-core] [PATCH] provide rtdm_mmap_to_user / rtdm_munmap
Em Quarta 15 Fevereiro 2006 12:53, Rodrigo Rosenfeld Rosas escreveu:
>Em Terça 14 Fevereiro 2006 22:30, Jan Kiszka escreveu:
>>...
>>
You cannot mmap before you know precisely for which user this should
take place.
>>>
>>> Do you mean that if I use the 'current' and current->mm struct of the
>>> driver, when mmaping, the user won't be able to use the returned pointer?
>>
>>To mmap you need to know the target process, more precisely its mm. This
>>is typically derived from the invocation context of the service call
>>("current" is a pointer to the current process). But init_module runs in
>>the context of modprobe. Even worse, the process later opening and
>>mapping some buffer may not even exist at that time!
>
>Right, I've already verified this on practice... I'm mmaping on open handler
>for now just for testing the mmap, but I'll change it to the ioctl mmap
>handler.
>
>It seems to work. I mapped high_memory and could read and modify it from
> user space. The memory values mantained betweens the many open calls. I
> read, printed the values and increment them by one. On next time, the value
> shown was incremented... All seems perfect but I still didn't test with
> real acquire code... When I do so, I'll let you know.
>
>I still need to test the vmaops. I think I'll test them tomorrow. I need to
>begin writing an article that my advisor asked me to. I need to finish it
>until march, 10.
Ok, I tested the vmaops too and it also worked as expected. I think you could
merge rtdm_mmap and related stuff to mainline RTDM. Thank you for your
precious work. Unfortunately you'll need to wait a while until I test them on
the real video driver. I had to stop working on it for writing the article.
When I finish the article I'll test them on real hardware but I see no
reasons for not working...
Best Regards,
Rodrigo.
___
Yahoo! Acesso Grátis - Internet rápida e grátis. Instale o discador agora!
http://br.acesso.yahoo.com
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
Dmitry Adamushko wrote:> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:>> Dmitry Adamushko wrote:>>> On 16/02/06, Jan Kiszka <
[EMAIL PROTECTED]> wrote:>> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this before merging (i.e. make XN_ISR_HANDLED non-zero)?>>>
>>> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now>>> non-zero.>>> Actually, at first I wanted to make it just the other way about.> Hmm, thinking about this again, I would like to revert my suggestion in
>> favour of a third approach (trying to keep you busy ;) ).>>> Ok, you are wellcome :)>> I didn't get it, at least while looking briefly. To make it a bit easier, at> least for me, let's go another way.
>> At the left is the list of allowed values as Philippe pointed out.> At the right is another list which corresponds to the left one but when> NOINT is used instead of HANDLED. Moreover, I have added another case - pure
> NOINT. The ISR says it's not mine and, well, it doesn't know whether it> should be propagated or no> (ok, so far CHAINED standed for NOINT).>>
1.)
HANDLED
-> 0> 2.) HANDLED | ENABLE -> ENABLE> 3.) HANDLED | CHAINED-> CHAINED>
4.)
CHAINED -> CHAINED
| NOINT>
5.)->
NOINT>> Could you provide the 3-d corresponding table using your new flags?>Still not 3D, but hopefully clarifying: :)
1.) XN_ISR_HANDLED |XN_ISR_NO_ENABLE
2.) XN_ISR_HANDLED3.) XN_ISR_HANDLED | XN_ISR_PROPAGATE(nucleus ignores implicit IRQ enable)4.) XN_ISR_NOINT | XN_ISR_PROPAGATE5.) XN_ISR_NOINT
Then why do we need XN_ISR_NOINT? is it not just the same as ~HANDLED?
Moreover, I tend to think now that PROPAGATE (CHAINED) should not be
normally used by ISRs at all. The only exception would be irq sharing
between RT and non-RT domains, but that's of no concern for most part
of rt drivers and, as last events revealed, it's very partially
supported at the moment and possible implementations are quite arguable.
So HANDLED means the irq line is .end-ed by xnintr_irq_handler() (xnarch_end_irq(irq) is called).
If one wants to do it on his own later, NOENABLE can be returned additionally.
But then, one must use xnintr_end_irq() and not just xnintr_irq_enable().
If xnintr_end_irq() would be always the same as xnintr_irq_enable() or
we would decouple ending and enabling operations on the line (don't
have sources at hand); we could go Linux way and make use of a "nested"
counter in xnintr_t, so that xnintr_irq_disable() could be called in a
nested way.
This said, calling xnintr_irq_disable() in ISR leads to the interrupt
line being still disabled upon return from xnintr_irq_handler() and
re-enabled again by the user later.
~HANDLED means the ISR was not concerned about this interrupt. i.e. :
- there is ISR in another domain (down the pipeline) which is interested in this irq ;
well, the designer of the system should be
aware of such cases and probably avoid that on hw layer or, at least,
to know what she is doing.
- this is a spurious irq.
So upon getting ~HANDLED, xnintr_irq_handler() does :
success = xnintr_irq_chained(irq);
if (success)
{
/* i.e. another domain is really interested and got this irq as pending*/
return without .end-ing
}
else
{
/* this is a spurious irq */
make use of some accounting of spurious interrupts (if we need it)),
i.e. keep the irq line disabled if the % of unhandled requests >
some limit.
otherwise
xnintr_end_irq() and return
}
too long :) ok, it's quite similar indeed to what you have suggested,
with the exception that
- no need for NOINT (if it's really == ~HANDLED);
- nucleus provides some additional logic while handling ~HANDLED case.
- user don't need to return ENABLE explicitly - in most cases, that's
what he actually needs but sometimes just forgets (I recall a few
questions on the mailing list about "only one interrupt".
Jan-- Best regards,Dmitry Adamushko
Re: [Xenomai-core] More on Shared interrupts
Anders Blomdell wrote:
For the last few days, I have tried to figure out a good way to share
interrupts between RT and non-RT domains. This has included looking
through Dmitry's patch, correcting bugs and testing what is possible in
my specific case. I'll therefore try to summarize at least a few of my
thoughts.
1. When looking through Dmitry's patch I get the impression that the
iack handler has very little to do with each interrupt (the test
'prev->iack != intr->iack' is a dead giveaway), but is more of a
domain-specific function (or perhaps even just a placeholder for the
hijacked Linux ack-function).
2. Somewhat inspired by the figure in "Life with Adeos", I have
identified the following cases:
irq K | --- | ---o| // Linux only
...
irq L | ---o| | // RT-only
...
irq M | ---o--- | ---o| // Shared between domains
...
irq N | ---o---o--- | | // Shared inside single domain
...
irq O | ---o---o--- | ---o| // Shared between and inside single
domain
Xenomai currently handles the K & L cases, Dmitrys patch addresses the N
case, with edge triggered interrupts the M (and O after Dmitry's patch)
case(s) might be handled by returning RT_INTR_CHAINED | RT_INTR_ENABLE
from the interrupt handler, for level triggered interrupt the M and O
cases can't be handled.
If one looks more closely at the K case (Linux only interrupt), it works
by when an interrupt occurs, the call to irq_end is postponed until the
Linux interrupt handler has run, i.e. further interrupts are disabled.
This can be seen as a lazy version of Philippe's idea of disabling all
non-RT interrupts until the RT-domain is idle, i.e. the interrupt is
disabled only if it indeed occurs.
If this idea should be generalized to the M (and O) case(s), one can't
rely on postponing the irq_end call (since the interrupt is still needed
in the RT-domain), but has to rely on some function that disables all
non-RT hardware that generates interrupts on that irq-line; such a
function naturally has to have intimate knowledge of all hardware that
can generate interrupts in order to be able to disable those interrupt
sources that are non-RT.
If we then take Jan's observation about the many (Linux-only) interrupts
present in an ordinary PC and add it to Philippe's idea of disabling all
non-RT interrupts while executing in the RT-domain, I think that the
following is a workable (and fairly efficient) way of handling this:
Add hardware dependent enable/disable functions, where the enable is
called just before normal execution in a domain starts (i.e. when
playing back interrupts, the disable is still in effect), and disable is
called when normal domain execution end. This does effectively handle
the K case above, with the added benefit that NO non-RT interrupts will
occur during RT execution.
In the 8259 case, the disable function could look something like:
domain_irq_disable(uint irqmask) {
if (irqmask & 0xff00 != 0xff00) {
irqmask &= ~0x0004; // Cascaded interrupt is still needed
outb(irqmask >> 8, PIC_SLAVE_IMR);
}
outb(irqmask, PIC_MASTER_IMR);
}
If we should extend this to handle the M (and O) case(s), the disable
function could look like:
domain_irq_disable(uint irqmask, shared_irq_t *shared[]) {
int i;
for (i = 0 ; i < MAX_IRQ ; i++) {
if (shared[i]) {
shared_irq_t *next = shared[i];
irqmask &= ~(1next;
}
}
}
if (irqmask & 0xff00 != 0xff00) {
irqmask &= ~0x0004; // Cascaded interrupt is still needed
outb(irqmask >> 8, PIC_SLAVE_IMR);
}
outb(irqmask, PIC_MASTER_IMR);
}
An obvious optimization of the above scheme, is to never call the
disable (or enable) function for the RT-domain, since there all
interrupt processing is protected by the hardware.
Comments, anyone?
OK, I have finally got around to do some interrupt timing tests on a PrPMC800
(450 MHz PowerPC/G4) with the following interrupt sources:
3: 10 Khz watchdog interrupt (Linux)
10: 100 Mbit/s ethernet (Linux)
16: mailbox interrupt (RT) + UART (Linux)
I have measured interrupt latency, task latency (time from interrupt until a
task signalled from interrupt handler has started) and the semaphore latency
(time from task semaphore is signalled until task has started).
I have tested 4 different ways of handling shared Linux/RT interrupts:
1. When UART interrupt occurs, disable further UART interrupts, signal low
priority UART reenable task, return XN_ISR_ENABLE | XN_ISR_CHAINED.
In low priority UART reenable task, reenable UART when Linux has handled
the interrupt.
2. Disable UART interrupts, and poll them at 1kHz from low priority RT task,
and rthal_irq_host_pend them as they occur.
3. Modified Xenomai, where non-RT interrupts are disabled when entering
the RT domain, and enabled when entering th
Re: [Xenomai-core] [PATCH] separate queue debugging switch
Jan Kiszka wrote:
Philippe Gerum wrote:
Jan Kiszka wrote:
Philippe Gerum wrote:
Jan Kiszka wrote:
Philippe Gerum wrote:
Jan Kiszka wrote:
Hi,
while XENO_OPT_DEBUG is generally a useful switch for tracing
potential
issues in the core and the skins, it also introduces high
latencies via
the queue debugging feature (due to checks iterating over whole
queues).
This patch introduces separate control over queue debugging so
that you
can have debug checks without too dramatic slowdowns.
Maybe it's time to introduce debug levels, so that we could reuse them
in order to
add more (selectable) debug instrumentation; queue debugging could
then
be given a
certain level (likely something like CONFIG_XENO_DEBUG_LEVEL=8712 for
this one...), instead of going for a specific conditional each time we
introduce new checks?
Hmm, this means someone have to define what should be printed at which
level - tend to be hard decisions... Often it is at least as much
useful
to have debug groups so that specific parts can be excluded from
debugging. I'm pro such groups (one would be those queues e.g.) but
contra too many levels (2, at most 3).
Ack, selection by increasingly verbose/high-overhead groups is what I
have in mind.
At this chance, I would also suggest to introduce some ASSERT macro
(per
group, per level). That could be used to instrument the core with
runtime checks. But it could also be quickly removed at compilation
time, reducing the code size (e.g. checks at the nucleus layer against
buggy skins or at RTDM layer against rough drivers).
I'm not opposed to that, if we keep the noise / signal ratio of those
assertions at the reasonable low-level throughout the code, and don't
use this to enforce silly parametrical checks.
Then let's discuss how to implement and control this. Say we have some
macros for marking code as "depends on debug group X":
#if XENO_DEBUG_GROUP(group)
code;
#endif /* XENO_DEBUG_GROUP(group) */
XENO_IF_DEBUG_GROUP(group, code);
(or do you prefere XNPOD_xxx?)
This debug code may span feature/component boundaries, so XENO_ is better.
Additionally, we could introduce that assertion macro:
XENO_ASSERT(group, expression, failure_code);
But how to control the groups now? Via Kconfig bool options?
Yes, I think so. From some specialized Debug menu in the generic
portion. We would need this to keep the (unused) debug code out of
production systems.
And what
groups to define? Per subsytem? Or per disturbance level (latency
regression)? If we control the group selection via Kconfig, we could
define pseudo bool options like "All debug groups" or "Low-intrusive
debug groups" that select the fitting concrete groups.
We won't be able to anticipate on each and every debug spots we might
need in the future, and in any case, debug triggers may well span
multiple sub-systems. I'd go for defining levels depending on the
throroughness/complexity of their checks.
To keep it simple:
XNDBG_LIGHT /* simple checks with low constant overhead */
XNDBG_HEAVY /* complex checks with high or unknown overhead */
Those two could become #defines and would have to be provide as first
argument to our debug macros.
Or we directly merge the attribute into the macro name:
XENO_DEBUG_LIGHT, XENO_IF_DEBUG_LIGHT(), XENO_ASSERT_LIGHT()
XENO_DEBUG_HEAVY, XENO_IF_DEBUG_HEAVY(), XENO_ASSERT_HEAVY()
There's no need to limit the number of debug levels, since defining what goes
into
each of them is a matter of developer's appreciation, wrt induced overhead, and/or
debug thoroughness. IOW, let's not impose any policy here. To keep things really
simple, we'd also need to decouple the assertion mechanism from the debug
thoroughness, E.g.
#if XENO_DEBUG_LEVEL > 0
#define XENO_ASSERT(cond,action,fmt,args...) do { \
if (unlikely((cond) != 0)) \
(action)(fmt , ##args); \
} while(0)
#else /* DEBUG OFF */
#define XENO_ASSERT(cond,action,fmt,args...) do { } while(0)
#endif /* DEBUG ON */
#define XENO_BUGON(cond) \
XENO_ASSERT(cond,xnpod_fatal,"assertion failed at %s:%d",__FILE__,__LINE__)
This way, we could reserve level #1 (and above by extension) to activate all
assertions; if people need to even control the thoroughness of assertions, they
could just resort to implementing the assertion code in a separate debug function
testing for particular debug levels, then calling this routine as XENO_BUGON's
"cond" argument. Keeping level #0 free from assertions would be nice for
production systems. E.g.
#if XENO_DEBUG_LEVEL > 2
check_all_nucleus_queues();
#endif
...
XENO_BUGON(I_am_so_sorry);
and so on.
Setting a value for XENO_DEBUG_LEVEL would be trivial using Kconfig. Dynamic
setting of the debug level through /proc could be obtained by forcing
XENO_DEBUG_LEVEL to MAX_INT (i.e. when dynamic debug levels are selected from
Kconfig), and testing some addition global variable within the debug code to
filter out unwanted checks.
Alternatively, we could
RE: [Xenomai-core] Handling PCI MSI interrupts
> The latest patch was incomplete; you might be luckier with > this one. I've merged Jeroen's last observations on this issue and mine. I tried this patch and it doesn't solve the issue I'm facing. With and without this patch, my symptoms are the same. I'm running a Dell 2850, dual CPU machine. When I build a kernel without Adeos then things are fine. When I build with Adeos and MSI enabled the following occurs: 1) If BIOS has USB disabled then the system will hang without even a num-lock respose (i.e. tapping the num-lock key doesn't toggle the light). The hang occurs just about the time the E1000 driver would load and enable an MSI interrupt. 2) If BIOS has USB enabled then the system will run much longer but may hang during heavy interrupt load on the E1000 driver. My assumption based on past experience is that no num-lock response means an infinite interrupt loop. When I build a kernel with Adeos but disable MSI then the system works fine for the most part. There is one scenario where the system will still hang doing disk and network accesses under a moderate load of I/O. Both of these tests are just to get a stable kernel before I really start using Adeos. So Adeos is in its default configuration and I haven't loaded Xenomai modules when these hangs occur. I'm currently running the 2.6.14.4 kernel with the 2.6.14-1.0-12 patch of adeos and then I included your msi.c patch from the previous e-mail. If you have any further hints or suggestions I'll try them. Meanwhile I'm trying different versions of various drivers (e1000 and scsi) as well as updating the patch level of the kernel itself. Russ
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote: > On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote: >> Dmitry Adamushko wrote: >>> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote: >>> >>> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this before merging (i.e. make XN_ISR_HANDLED non-zero)? >>> >>> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now >>> non-zero. >>> Actually, at first I wanted to make it just the other way about. >>> >> Hmm, thinking about this again, I would like to revert my suggestion in >> favour of a third approach (trying to keep you busy ;) ). > > > Ok, you are wellcome :) > > I didn't get it, at least while looking briefly. To make it a bit easier, at > least for me, let's go another way. > > At the left is the list of allowed values as Philippe pointed out. > At the right is another list which corresponds to the left one but when > NOINT is used instead of HANDLED. Moreover, I have added another case - pure > NOINT. The ISR says it's not mine and, well, it doesn't know whether it > should be propagated or no > (ok, so far CHAINED standed for NOINT). > > 1.) HANDLED -> 0 > 2.) HANDLED | ENABLE -> ENABLE > 3.) HANDLED | CHAINED-> CHAINED > 4.) CHAINED -> CHAINED | NOINT > 5.)-> NOINT > > Could you provide the 3-d corresponding table using your new flags? > Still not 3D, but hopefully clarifying: :) 1.) XN_ISR_HANDLED |XN_ISR_NO_ENABLE 2.) XN_ISR_HANDLED 3.) XN_ISR_HANDLED | XN_ISR_PROPAGATE (nucleus ignores implicit IRQ enable) 4.) XN_ISR_NOINT | XN_ISR_PROPAGATE 5.) XN_ISR_NOINT 2.) and 5.) are most common, the others for special scenarios. Especially, as long as we have no API for ending IRQs outside the handler, 1.) is of limited usage I think. Jan signature.asc Description: OpenPGP digital signature
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote: Dmitry Adamushko wrote:> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:>> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this>> before merging ( i.e. make XN_ISR_HANDLED non-zero)?>>> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now> non-zero.> Actually, at first I wanted to make it just the other way about. >Hmm, thinking about this again, I would like to revert my suggestion infavour of a third approach (trying to keep you busy ;) ). Ok, you are wellcome :) I didn't get it, at least while looking briefly. To make it a bit easier, at least for me, let's go another way. At the left is the list of allowed values as Philippe pointed out. At the right is another list which corresponds to the left one but when NOINT is used instead of HANDLED. Moreover, I have added another case - pure NOINT. The ISR says it's not mine and, well, it doesn't know whether it should be propagated or no (ok, so far CHAINED standed for NOINT). HANDLED -> 0 HANDLED | ENABLE -> ENABLE HANDLED | CHAINED -> CHAINED CHAINED -> CHAINED | NOINT -> NOINT Could you provide the 3-d corresponding table using your new flags? > xnintr_t contains just the link "char *name" as you suggested but RT_INTR > contains the name[XNOBJECT_NAME_LEN] member (as it was before actually).>> If it's ok, then I'll send um... yet another final patch that addresses> both issues :)I'm fine with this as well if you prefer it, no problem. Yep, let's go this way. Jan -- Best regards,Dmitry Adamushko
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote:
> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>
> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this
>> before merging (i.e. make XN_ISR_HANDLED non-zero)?
>
>
> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now
> non-zero.
> Actually, at first I wanted to make it just the other way about.
>
Hmm, thinking about this again, I would like to revert my suggestion in
favour of a third approach (trying to keep you busy ;) ).
Recommended default values:
XN_ISR_HANDLED => XN_ISR_HANDLED | XN_ISR_ENABLE
XN_ISR_NOINT => XN_ISR_NO_ENABLE
Additional bits:
XN_ISR_NO_ENABLE
=> indicate that XN_ISR_ENABLE is not set
(use case: handle but delay re-enable)
XN_ISR_PROPAGATE
=> XN_ISR_CHAINED (the new name is closer to the related function
calls - at least for me).
Not expressible combination:
XN_ISR_ENABLE | XN_ISR_NOINT
=> Doesn't make sense, does it?
Still expressible invalid combination:
XN_ISR_HANDLED | XN_ISR_PROPAGATE
=> The implicit enable request should be easy to ignore at nucleus
level.
Rational:
Most users will either indicate that an IRQ was handled and should be
re-enabled as soon as possible or that it was unhandled and should
better remain masked if no one else cares.
XN_ISR_PROPAGATE (or related skin defines) are only useful for very very
special corner-case drivers and applications. Normal generic drivers
should not touch this (including my own xeno_16550A... bah!). BTW, if
there is some unintentional IRQ sharing so that spurious RT interrupts
occur, I think we should detect this at nucleus level and bail out.
Otherwise, systems quickly lock up (I just faced this once again two
days ago on an old RTAI box).
This will break some stuff (RTDM e.g., but we have some version flags
for such cases), but it takes us closer to the standard Linux API
without loosing control over the details when required.
Comments?
>
> Moreover, I attached an add-on patch to overcome the name buffer in
>> xnintr_t. Note that this patch is only compile-tested, I have no native
>> interrupt test-case at hand.
>
>
>
> Heh.. someone once suggested the -p key with "diff" to me? :o)
Mmpf. Must have been too late, and I was too impressed how interdiff
worked out this patch. :)
>
> I bet your patch would even work but it's, well, slightly broken indeed ;-)
>
> Look at the native/syscall.c : __rt_intr_delete() :
>
> RT_INTR *intr = (RT_INTR *)rt_registry_fetch(ph.opaque);
> ...
> xnfree(intr);
>
> "intr" actually points to intr_buf->intr and it works as expected only
> because "RT_INTR intr" is the first member of the struct, so that it's
> address == the address of the whole object of this struct.
But this is not an unusual trick, and I see no practical problems.
>
> To do it properly you would need to make your "unnamed" struct visible
> for both __rt_intr_create() and __rt_intr_delete(). Something like :
>
> struct RT_INTR_ENV {
>
> #define link2intr_env(laddr) \
> ((RT_INTR_ENV *)(((char* )laddr) - (int)(&((RT_INTR_ENV *)0)->intr)))
>
> RT_INTR intr;
> char name[XNOBJECT_NAME_LEN];
> };
>
> and then make use of link2intr_env() in __rt_intr_delet().
>
>
> Ok, with you approach we would avoid allocating memory
> for name[XNOBJECT_NAME_LEN] when an object is created in kernel-mode
> and do it via RT_INTR_ENV only when it's created in user-mode.
>
> We could even use that approach for all objects in native skin, but we can't
> because of "anonymous" objects supported in kernel-mode too and, well, I
> personally would not like those RT_OBJECT_ENV structures all over the map in
> syscall.c :)
Me too.
>
> So I don't want to break the whole picture only for the RT_INTR object (it
> doesn't support "anonymous" objects now).
>
> Nevertheless, if you still want to save a hundred bytes of data or so (how
> many interrupt objects would a normal system have? and I guess the lenght of
> name is about 10 symbols on average :)
RT_INTR is now disabled by default in the kernel config. Thus most users
should not see its code in their kernels, only the additional data and
code related to xnintr_t.name.
>
> for the users that don't use native skin, then I would suggest the following
> way :
>
> xnintr_t contains just the link "char *name" as you suggested but RT_INTR
> contains the name[XNOBJECT_NAME_LEN] member (as it was before actually).
>
> If it's ok, then I'll send um... yet another final patch that addresses
> both issues :)
I'm fine with this as well if you prefer it, no problem.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote:
> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>> Dmitry Adamushko wrote:
>>> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
Dmitry Adamushko wrote:
> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>
> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this
>> before merging (i.e. make XN_ISR_HANDLED non-zero)?
> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now
> non-zero.
> Actually, at first I wanted to make it just the other way about.
>
Hmm, thinking about this again, I would like to revert my suggestion in
favour of a third approach (trying to keep you busy ;) ).
>>>
>>> Ok, you are wellcome :)
>>>
>>> I didn't get it, at least while looking briefly. To make it a bit
>> easier, at
>>> least for me, let's go another way.
>>>
>>> At the left is the list of allowed values as Philippe pointed out.
>>> At the right is another list which corresponds to the left one but when
>>> NOINT is used instead of HANDLED. Moreover, I have added another case -
>> pure
>>> NOINT. The ISR says it's not mine and, well, it doesn't know whether it
>>> should be propagated or no
>>> (ok, so far CHAINED standed for NOINT).
>>>
>>> 1.) HANDLED -> 0
>>> 2.) HANDLED | ENABLE -> ENABLE
>>> 3.) HANDLED | CHAINED-> CHAINED
>>> 4.) CHAINED -> CHAINED | NOINT
>>> 5.)-> NOINT
>>>
>>> Could you provide the 3-d corresponding table using your new flags?
>>>
>> Still not 3D, but hopefully clarifying: :)
>
>
>
> 1.) XN_ISR_HANDLED |XN_ISR_NO_ENABLE
>> 2.) XN_ISR_HANDLED
>> 3.) XN_ISR_HANDLED | XN_ISR_PROPAGATE
>> (nucleus ignores implicit IRQ enable)
>> 4.) XN_ISR_NOINT | XN_ISR_PROPAGATE
>> 5.) XN_ISR_NOINT
>
>
> Then why do we need XN_ISR_NOINT? is it not just the same as ~HANDLED?
The idea is to have at least a define so that
a) users explicitly have to state what they mean
b) we can map the define to some other value whenever we may need this
in the future
The first property could even be used to apply some debug checks in the
user's return value.
>
> Moreover, I tend to think now that PROPAGATE (CHAINED) should not be
> normally used by ISRs at all. The only exception would be irq sharing
> between RT and non-RT domains, but that's of no concern for most part of rt
> drivers and, as last events revealed, it's very partially supported at the
> moment and possible implementations are quite arguable.
Ack. But you should still export this feature for those who think they
know what they are doing. ;)
>
> So HANDLED means the irq line is .end-ed by xnintr_irq_handler()
> (xnarch_end_irq(irq) is called).
>
> If one wants to do it on his own later, NOENABLE can be returned
> additionally.
Yep. So let's call it XN_ISR_NOENABLE.
> But then, one must use xnintr_end_irq() and not just xnintr_irq_enable().
>
Yep, but a different (add-on) topic.
> If xnintr_end_irq() would be always the same as xnintr_irq_enable() or we
> would decouple ending and enabling operations on the line (don't have
> sources at hand); we could go Linux way and make use of a "nested" counter
> in xnintr_t, so that xnintr_irq_disable() could be called in a nested way.
> This said, calling xnintr_irq_disable() in ISR leads to the interrupt line
> being still disabled upon return from xnintr_irq_handler() and re-enabled
> again by the user later.
>
> ~HANDLED means the ISR was not concerned about this interrupt. i.e. :
>
> - there is ISR in another domain (down the pipeline) which is interested in
> this irq ;
>
> well, the designer of the system should be aware of such cases and
> probably avoid that on hw layer or, at least, to know what she is doing.
>
> - this is a spurious irq.
>
> So upon getting ~HANDLED, xnintr_irq_handler() does :
>
> success = xnintr_irq_chained(irq);
Only if requested via XN_ISR_PROPAGATE (or _CHAINED)! There is still a
real-time ISR registered on that IRQ you are about to forward now, and
you don't know if someone is handling it down the pipeline. Actually,
this is a fatal situation. The system is broken, bail out, and switch
this line off.
>
> if (success)
> {
> /* i.e. another domain is really interested and got this irq as pending*/
>
> return without .end-ing
> }
> else
> {
> /* this is a spurious irq */
>
> make use of some accounting of spurious interrupts (if we need it)), i.e.
> keep the irq line disabled if the % of unhandled requests > some limit.
>
> otherwise
>
> xnintr_end_irq() and return
> }
>
> too long :) ok, it's quite similar indeed to what you have suggested,
> with the exception that
>
> - no need for NOINT (if it's really == ~HANDLED);
> - nucleus provides some additional logic while handling ~HANDLED case.
> - user don't need to return ENABLE explicitly - in most cases, that's what
> he actually needs but sometimes just forgets (I recall a few questions on
> the mailing list abo
Re: [Xenomai-core] [PATCH] provide rtdm_mmap_to_user / rtdm_munmap
Em Quarta 15 Fevereiro 2006 12:53, Rodrigo Rosenfeld Rosas escreveu:
>Em Terça 14 Fevereiro 2006 22:30, Jan Kiszka escreveu:
>>...
>>
You cannot mmap before you know precisely for which user this should
take place.
>>>
>>> Do you mean that if I use the 'current' and current->mm struct of the
>>> driver, when mmaping, the user won't be able to use the returned pointer?
>>
>>To mmap you need to know the target process, more precisely its mm. This
>>is typically derived from the invocation context of the service call
>>("current" is a pointer to the current process). But init_module runs in
>>the context of modprobe. Even worse, the process later opening and
>>mapping some buffer may not even exist at that time!
>
>Right, I've already verified this on practice... I'm mmaping on open handler
>for now just for testing the mmap, but I'll change it to the ioctl mmap
>handler.
>
>It seems to work. I mapped high_memory and could read and modify it from
> user space. The memory values mantained betweens the many open calls. I
> read, printed the values and increment them by one. On next time, the value
> shown was incremented... All seems perfect but I still didn't test with
> real acquire code... When I do so, I'll let you know.
>
>I still need to test the vmaops. I think I'll test them tomorrow. I need to
>begin writing an article that my advisor asked me to. I need to finish it
>until march, 10.
Ok, I tested the vmaops too and it also worked as expected. I think you could
merge rtdm_mmap and related stuff to mainline RTDM. Thank you for your
precious work. Unfortunately you'll need to wait a while until I test them on
the real video driver. I had to stop working on it for writing the article.
When I finish the article I'll test them on real hardware but I see no
reasons for not working...
Best Regards,
Rodrigo.
___
Yahoo! Acesso Grátis - Internet rápida e grátis. Instale o discador agora!
http://br.acesso.yahoo.com
___
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 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
Dmitry Adamushko wrote:> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:>> Dmitry Adamushko wrote:>>> On 16/02/06, Jan Kiszka <
[EMAIL PROTECTED]> wrote:>> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this before merging (i.e. make XN_ISR_HANDLED non-zero)?>>>
>>> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now>>> non-zero.>>> Actually, at first I wanted to make it just the other way about.> Hmm, thinking about this again, I would like to revert my suggestion in
>> favour of a third approach (trying to keep you busy ;) ).>>> Ok, you are wellcome :)>> I didn't get it, at least while looking briefly. To make it a bit easier, at> least for me, let's go another way.
>> At the left is the list of allowed values as Philippe pointed out.> At the right is another list which corresponds to the left one but when> NOINT is used instead of HANDLED. Moreover, I have added another case - pure
> NOINT. The ISR says it's not mine and, well, it doesn't know whether it> should be propagated or no> (ok, so far CHAINED standed for NOINT).>>
1.)
HANDLED
-> 0> 2.) HANDLED | ENABLE -> ENABLE> 3.) HANDLED | CHAINED-> CHAINED>
4.)
CHAINED -> CHAINED
| NOINT>
5.)->
NOINT>> Could you provide the 3-d corresponding table using your new flags?>Still not 3D, but hopefully clarifying: :)
1.) XN_ISR_HANDLED |XN_ISR_NO_ENABLE
2.) XN_ISR_HANDLED3.) XN_ISR_HANDLED | XN_ISR_PROPAGATE(nucleus ignores implicit IRQ enable)4.) XN_ISR_NOINT | XN_ISR_PROPAGATE5.) XN_ISR_NOINT
Then why do we need XN_ISR_NOINT? is it not just the same as ~HANDLED?
Moreover, I tend to think now that PROPAGATE (CHAINED) should not be
normally used by ISRs at all. The only exception would be irq sharing
between RT and non-RT domains, but that's of no concern for most part
of rt drivers and, as last events revealed, it's very partially
supported at the moment and possible implementations are quite arguable.
So HANDLED means the irq line is .end-ed by xnintr_irq_handler() (xnarch_end_irq(irq) is called).
If one wants to do it on his own later, NOENABLE can be returned additionally.
But then, one must use xnintr_end_irq() and not just xnintr_irq_enable().
If xnintr_end_irq() would be always the same as xnintr_irq_enable() or
we would decouple ending and enabling operations on the line (don't
have sources at hand); we could go Linux way and make use of a "nested"
counter in xnintr_t, so that xnintr_irq_disable() could be called in a
nested way.
This said, calling xnintr_irq_disable() in ISR leads to the interrupt
line being still disabled upon return from xnintr_irq_handler() and
re-enabled again by the user later.
~HANDLED means the ISR was not concerned about this interrupt. i.e. :
- there is ISR in another domain (down the pipeline) which is interested in this irq ;
well, the designer of the system should be
aware of such cases and probably avoid that on hw layer or, at least,
to know what she is doing.
- this is a spurious irq.
So upon getting ~HANDLED, xnintr_irq_handler() does :
success = xnintr_irq_chained(irq);
if (success)
{
/* i.e. another domain is really interested and got this irq as pending*/
return without .end-ing
}
else
{
/* this is a spurious irq */
make use of some accounting of spurious interrupts (if we need it)),
i.e. keep the irq line disabled if the % of unhandled requests >
some limit.
otherwise
xnintr_end_irq() and return
}
too long :) ok, it's quite similar indeed to what you have suggested,
with the exception that
- no need for NOINT (if it's really == ~HANDLED);
- nucleus provides some additional logic while handling ~HANDLED case.
- user don't need to return ENABLE explicitly - in most cases, that's
what he actually needs but sometimes just forgets (I recall a few
questions on the mailing list about "only one interrupt".
Jan-- Best regards,Dmitry Adamushko
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]
> wrote:
Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve thisbefore merging (
i.e. make XN_ISR_HANDLED non-zero)?
Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now non-zero.
Actually, at first I wanted to make it just the other way about.
Moreover, I attached an add-on patch to overcome the name buffer inxnintr_t. Note that this patch is only compile-tested, I have no native
interrupt test-case at hand.
Heh.. someone once suggested the -p key with "diff" to me? :o)
I bet your patch would even work but it's, well, slightly broken indeed ;-)
Look at the native/syscall.c : __rt_intr_delete() :
RT_INTR *intr = (RT_INTR *)rt_registry_fetch(ph.opaque);
...
xnfree(intr);
"intr" actually points to intr_buf->intr and it works as expected only
because "RT_INTR intr" is the first member of the struct, so that it's
address == the address of the whole object of this struct.
To do it properly you would need to make your "unnamed" struct visible
for both __rt_intr_create() and __rt_intr_delete(). Something like :
struct RT_INTR_ENV {
#define link2intr_env(laddr) \
((RT_INTR_ENV *)(((char* )laddr) - (int)(&((RT_INTR_ENV *)0)->intr)))
RT_INTR intr;
char name[XNOBJECT_NAME_LEN];
};
and then make use of link2intr_env() in __rt_intr_delet().
Ok, with you approach we would avoid allocating memory
for name[XNOBJECT_NAME_LEN] when an object is created in kernel-mode
and do it via RT_INTR_ENV only when it's created in user-mode.
We could even use that approach for all objects in native skin, but we can't
because of "anonymous" objects supported in kernel-mode too and, well,
I personally would not like those RT_OBJECT_ENV structures all over the
map in syscall.c :)
So I don't want to break the whole picture only for the RT_INTR object (it doesn't support "anonymous" objects now).
Nevertheless, if you still want to save a hundred bytes of data or so
(how many interrupt objects would a normal system have? and I guess the
lenght of name is about 10 symbols on average :)
for the users that don't use native skin, then I would suggest the following way :
xnintr_t contains just the link "char *name" as you suggested but
RT_INTR contains the name[XNOBJECT_NAME_LEN] member (as it was before
actually).
If it's ok, then I'll send um... yet another final patch that addresses both issues :)
Jan-- Best regards,Dmitry Adamushko
Re: [Xenomai-core] Handling PCI MSI interrupts
Russell Johnson wrote:
It's definitely an Adeos issue and msi.c needs fixing. What
about this patch, do
things improve with it (against 2.6.15-ipipe-1.2-00)?
I'm currently patching my setup which started with ipipe-2.6.14-i386-1.0-12.
I've been having no luck with any MSI devices in the system even if they
have supposedly had MSI disabled. I'll post my testing results in the next
day or so.
The latest patch was incomplete; you might be luckier with this one. I've merged
Jeroen's last observations on this issue and mine.
--- 2.6.15/drivers/pci/msi.c2006-01-03 04:21:10.0 +0100
+++ 2.6.15-ipipe/drivers/pci/msi.c 2006-02-16 10:30:27.0 +0100
@@ -149,6 +149,21 @@
msi_set_mask_bit(vector, 0);
}
+#ifdef CONFIG_IPIPE
+static void ack_MSI_irq_w_maskbits(unsigned int vector)
+{
+mask_MSI_irq(vector);
+__ack_APIC_irq();
+}
+static void ack_MSI_irq_wo_maskbits(unsigned int vector)
+{
+__ack_APIC_irq();
+}
+#else /* !CONFIG_IPIPE */
+#define ack_MSI_irq_wo_maskbits do_nothing
+#define ack_MSI_irq_w_maskbits mask_MSI_irq
+#endif /* CONFIG_IPIPE */
+
static unsigned int startup_msi_irq_wo_maskbit(unsigned int vector)
{
struct msi_desc *entry;
@@ -212,7 +227,7 @@
.shutdown = shutdown_msi_irq,
.enable = unmask_MSI_irq,
.disable= mask_MSI_irq,
- .ack= mask_MSI_irq,
+ .ack= ack_MSI_irq_w_maskbits,
.end= end_msi_irq_w_maskbit,
.set_affinity = set_msi_irq_affinity
};
@@ -228,7 +243,7 @@
.shutdown = shutdown_msi_irq,
.enable = unmask_MSI_irq,
.disable= mask_MSI_irq,
- .ack= mask_MSI_irq,
+ .ack= ack_MSI_irq_w_maskbits,
.end= end_msi_irq_w_maskbit,
.set_affinity = set_msi_irq_affinity
};
@@ -244,7 +259,7 @@
.shutdown = shutdown_msi_irq,
.enable = do_nothing,
.disable= do_nothing,
- .ack= do_nothing,
+ .ack= ack_MSI_irq_wo_maskbits,
.end= end_msi_irq_wo_maskbit,
.set_affinity = set_msi_irq_affinity
};
--
Philippe.
Re: [Xenomai-core] More on Shared interrupts
Anders Blomdell wrote:
For the last few days, I have tried to figure out a good way to share
interrupts between RT and non-RT domains. This has included looking
through Dmitry's patch, correcting bugs and testing what is possible in
my specific case. I'll therefore try to summarize at least a few of my
thoughts.
1. When looking through Dmitry's patch I get the impression that the
iack handler has very little to do with each interrupt (the test
'prev->iack != intr->iack' is a dead giveaway), but is more of a
domain-specific function (or perhaps even just a placeholder for the
hijacked Linux ack-function).
2. Somewhat inspired by the figure in "Life with Adeos", I have
identified the following cases:
irq K | --- | ---o| // Linux only
...
irq L | ---o| | // RT-only
...
irq M | ---o--- | ---o| // Shared between domains
...
irq N | ---o---o--- | | // Shared inside single domain
...
irq O | ---o---o--- | ---o| // Shared between and inside single
domain
Xenomai currently handles the K & L cases, Dmitrys patch addresses the N
case, with edge triggered interrupts the M (and O after Dmitry's patch)
case(s) might be handled by returning RT_INTR_CHAINED | RT_INTR_ENABLE
from the interrupt handler, for level triggered interrupt the M and O
cases can't be handled.
If one looks more closely at the K case (Linux only interrupt), it works
by when an interrupt occurs, the call to irq_end is postponed until the
Linux interrupt handler has run, i.e. further interrupts are disabled.
This can be seen as a lazy version of Philippe's idea of disabling all
non-RT interrupts until the RT-domain is idle, i.e. the interrupt is
disabled only if it indeed occurs.
If this idea should be generalized to the M (and O) case(s), one can't
rely on postponing the irq_end call (since the interrupt is still needed
in the RT-domain), but has to rely on some function that disables all
non-RT hardware that generates interrupts on that irq-line; such a
function naturally has to have intimate knowledge of all hardware that
can generate interrupts in order to be able to disable those interrupt
sources that are non-RT.
If we then take Jan's observation about the many (Linux-only) interrupts
present in an ordinary PC and add it to Philippe's idea of disabling all
non-RT interrupts while executing in the RT-domain, I think that the
following is a workable (and fairly efficient) way of handling this:
Add hardware dependent enable/disable functions, where the enable is
called just before normal execution in a domain starts (i.e. when
playing back interrupts, the disable is still in effect), and disable is
called when normal domain execution end. This does effectively handle
the K case above, with the added benefit that NO non-RT interrupts will
occur during RT execution.
In the 8259 case, the disable function could look something like:
domain_irq_disable(uint irqmask) {
if (irqmask & 0xff00 != 0xff00) {
irqmask &= ~0x0004; // Cascaded interrupt is still needed
outb(irqmask >> 8, PIC_SLAVE_IMR);
}
outb(irqmask, PIC_MASTER_IMR);
}
If we should extend this to handle the M (and O) case(s), the disable
function could look like:
domain_irq_disable(uint irqmask, shared_irq_t *shared[]) {
int i;
for (i = 0 ; i < MAX_IRQ ; i++) {
if (shared[i]) {
shared_irq_t *next = shared[i];
irqmask &= ~(1next;
}
}
}
if (irqmask & 0xff00 != 0xff00) {
irqmask &= ~0x0004; // Cascaded interrupt is still needed
outb(irqmask >> 8, PIC_SLAVE_IMR);
}
outb(irqmask, PIC_MASTER_IMR);
}
An obvious optimization of the above scheme, is to never call the
disable (or enable) function for the RT-domain, since there all
interrupt processing is protected by the hardware.
Comments, anyone?
OK, I have finally got around to do some interrupt timing tests on a PrPMC800
(450 MHz PowerPC/G4) with the following interrupt sources:
3: 10 Khz watchdog interrupt (Linux)
10: 100 Mbit/s ethernet (Linux)
16: mailbox interrupt (RT) + UART (Linux)
I have measured interrupt latency, task latency (time from interrupt until a
task signalled from interrupt handler has started) and the semaphore latency
(time from task semaphore is signalled until task has started).
I have tested 4 different ways of handling shared Linux/RT interrupts:
1. When UART interrupt occurs, disable further UART interrupts, signal low
priority UART reenable task, return XN_ISR_ENABLE | XN_ISR_CHAINED.
In low priority UART reenable task, reenable UART when Linux has handled
the interrupt.
2. Disable UART interrupts, and poll them at 1kHz from low priority RT task,
and rthal_irq_host_pend them as they occur.
3. Modified Xenomai, where non-RT interrupts are disabled when entering
the RT domain, and enabled when entering th
Re: [Xenomai-core] [PATCH] separate queue debugging switch
Jan Kiszka wrote:
Philippe Gerum wrote:
Jan Kiszka wrote:
Philippe Gerum wrote:
Jan Kiszka wrote:
Philippe Gerum wrote:
Jan Kiszka wrote:
Hi,
while XENO_OPT_DEBUG is generally a useful switch for tracing
potential
issues in the core and the skins, it also introduces high
latencies via
the queue debugging feature (due to checks iterating over whole
queues).
This patch introduces separate control over queue debugging so
that you
can have debug checks without too dramatic slowdowns.
Maybe it's time to introduce debug levels, so that we could reuse them
in order to
add more (selectable) debug instrumentation; queue debugging could
then
be given a
certain level (likely something like CONFIG_XENO_DEBUG_LEVEL=8712 for
this one...), instead of going for a specific conditional each time we
introduce new checks?
Hmm, this means someone have to define what should be printed at which
level - tend to be hard decisions... Often it is at least as much
useful
to have debug groups so that specific parts can be excluded from
debugging. I'm pro such groups (one would be those queues e.g.) but
contra too many levels (2, at most 3).
Ack, selection by increasingly verbose/high-overhead groups is what I
have in mind.
At this chance, I would also suggest to introduce some ASSERT macro
(per
group, per level). That could be used to instrument the core with
runtime checks. But it could also be quickly removed at compilation
time, reducing the code size (e.g. checks at the nucleus layer against
buggy skins or at RTDM layer against rough drivers).
I'm not opposed to that, if we keep the noise / signal ratio of those
assertions at the reasonable low-level throughout the code, and don't
use this to enforce silly parametrical checks.
Then let's discuss how to implement and control this. Say we have some
macros for marking code as "depends on debug group X":
#if XENO_DEBUG_GROUP(group)
code;
#endif /* XENO_DEBUG_GROUP(group) */
XENO_IF_DEBUG_GROUP(group, code);
(or do you prefere XNPOD_xxx?)
This debug code may span feature/component boundaries, so XENO_ is better.
Additionally, we could introduce that assertion macro:
XENO_ASSERT(group, expression, failure_code);
But how to control the groups now? Via Kconfig bool options?
Yes, I think so. From some specialized Debug menu in the generic
portion. We would need this to keep the (unused) debug code out of
production systems.
And what
groups to define? Per subsytem? Or per disturbance level (latency
regression)? If we control the group selection via Kconfig, we could
define pseudo bool options like "All debug groups" or "Low-intrusive
debug groups" that select the fitting concrete groups.
We won't be able to anticipate on each and every debug spots we might
need in the future, and in any case, debug triggers may well span
multiple sub-systems. I'd go for defining levels depending on the
throroughness/complexity of their checks.
To keep it simple:
XNDBG_LIGHT /* simple checks with low constant overhead */
XNDBG_HEAVY /* complex checks with high or unknown overhead */
Those two could become #defines and would have to be provide as first
argument to our debug macros.
Or we directly merge the attribute into the macro name:
XENO_DEBUG_LIGHT, XENO_IF_DEBUG_LIGHT(), XENO_ASSERT_LIGHT()
XENO_DEBUG_HEAVY, XENO_IF_DEBUG_HEAVY(), XENO_ASSERT_HEAVY()
There's no need to limit the number of debug levels, since defining what goes
into
each of them is a matter of developer's appreciation, wrt induced overhead, and/or
debug thoroughness. IOW, let's not impose any policy here. To keep things really
simple, we'd also need to decouple the assertion mechanism from the debug
thoroughness, E.g.
#if XENO_DEBUG_LEVEL > 0
#define XENO_ASSERT(cond,action,fmt,args...) do { \
if (unlikely((cond) != 0)) \
(action)(fmt , ##args); \
} while(0)
#else /* DEBUG OFF */
#define XENO_ASSERT(cond,action,fmt,args...) do { } while(0)
#endif /* DEBUG ON */
#define XENO_BUGON(cond) \
XENO_ASSERT(cond,xnpod_fatal,"assertion failed at %s:%d",__FILE__,__LINE__)
This way, we could reserve level #1 (and above by extension) to activate all
assertions; if people need to even control the thoroughness of assertions, they
could just resort to implementing the assertion code in a separate debug function
testing for particular debug levels, then calling this routine as XENO_BUGON's
"cond" argument. Keeping level #0 free from assertions would be nice for
production systems. E.g.
#if XENO_DEBUG_LEVEL > 2
check_all_nucleus_queues();
#endif
...
XENO_BUGON(I_am_so_sorry);
and so on.
Setting a value for XENO_DEBUG_LEVEL would be trivial using Kconfig. Dynamic
setting of the debug level through /proc could be obtained by forcing
XENO_DEBUG_LEVEL to MAX_INT (i.e. when dynamic debug levels are selected from
Kconfig), and testing some addition global variable within the debug code to
filter out unwanted checks.
Alternatively, we could
RE: [Xenomai-core] Handling PCI MSI interrupts
> The latest patch was incomplete; you might be luckier with > this one. I've merged Jeroen's last observations on this issue and mine. I tried this patch and it doesn't solve the issue I'm facing. With and without this patch, my symptoms are the same. I'm running a Dell 2850, dual CPU machine. When I build a kernel without Adeos then things are fine. When I build with Adeos and MSI enabled the following occurs: 1) If BIOS has USB disabled then the system will hang without even a num-lock respose (i.e. tapping the num-lock key doesn't toggle the light). The hang occurs just about the time the E1000 driver would load and enable an MSI interrupt. 2) If BIOS has USB enabled then the system will run much longer but may hang during heavy interrupt load on the E1000 driver. My assumption based on past experience is that no num-lock response means an infinite interrupt loop. When I build a kernel with Adeos but disable MSI then the system works fine for the most part. There is one scenario where the system will still hang doing disk and network accesses under a moderate load of I/O. Both of these tests are just to get a stable kernel before I really start using Adeos. So Adeos is in its default configuration and I haven't loaded Xenomai modules when these hangs occur. I'm currently running the 2.6.14.4 kernel with the 2.6.14-1.0-12 patch of adeos and then I included your msi.c patch from the previous e-mail. If you have any further hints or suggestions I'll try them. Meanwhile I'm trying different versions of various drivers (e1000 and scsi) as well as updating the patch level of the kernel itself. Russ ___ 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: > On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote: >> Dmitry Adamushko wrote: >>> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote: >>> >>> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this before merging (i.e. make XN_ISR_HANDLED non-zero)? >>> >>> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now >>> non-zero. >>> Actually, at first I wanted to make it just the other way about. >>> >> Hmm, thinking about this again, I would like to revert my suggestion in >> favour of a third approach (trying to keep you busy ;) ). > > > Ok, you are wellcome :) > > I didn't get it, at least while looking briefly. To make it a bit easier, at > least for me, let's go another way. > > At the left is the list of allowed values as Philippe pointed out. > At the right is another list which corresponds to the left one but when > NOINT is used instead of HANDLED. Moreover, I have added another case - pure > NOINT. The ISR says it's not mine and, well, it doesn't know whether it > should be propagated or no > (ok, so far CHAINED standed for NOINT). > > 1.) HANDLED -> 0 > 2.) HANDLED | ENABLE -> ENABLE > 3.) HANDLED | CHAINED-> CHAINED > 4.) CHAINED -> CHAINED | NOINT > 5.)-> NOINT > > Could you provide the 3-d corresponding table using your new flags? > Still not 3D, but hopefully clarifying: :) 1.) XN_ISR_HANDLED |XN_ISR_NO_ENABLE 2.) XN_ISR_HANDLED 3.) XN_ISR_HANDLED | XN_ISR_PROPAGATE (nucleus ignores implicit IRQ enable) 4.) XN_ISR_NOINT | XN_ISR_PROPAGATE 5.) XN_ISR_NOINT 2.) and 5.) are most common, the others for special scenarios. Especially, as long as we have no API for ending IRQs outside the handler, 1.) is of limited usage I think. Jan 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 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote: Dmitry Adamushko wrote:> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:>> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this>> before merging ( i.e. make XN_ISR_HANDLED non-zero)?>>> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now> non-zero.> Actually, at first I wanted to make it just the other way about. >Hmm, thinking about this again, I would like to revert my suggestion infavour of a third approach (trying to keep you busy ;) ). Ok, you are wellcome :) I didn't get it, at least while looking briefly. To make it a bit easier, at least for me, let's go another way. At the left is the list of allowed values as Philippe pointed out. At the right is another list which corresponds to the left one but when NOINT is used instead of HANDLED. Moreover, I have added another case - pure NOINT. The ISR says it's not mine and, well, it doesn't know whether it should be propagated or no (ok, so far CHAINED standed for NOINT). HANDLED -> 0 HANDLED | ENABLE -> ENABLE HANDLED | CHAINED -> CHAINED CHAINED -> CHAINED | NOINT -> NOINT Could you provide the 3-d corresponding table using your new flags? > xnintr_t contains just the link "char *name" as you suggested but RT_INTR > contains the name[XNOBJECT_NAME_LEN] member (as it was before actually).>> If it's ok, then I'll send um... yet another final patch that addresses> both issues :)I'm fine with this as well if you prefer it, no problem. Yep, let's go this way. 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)
Dmitry Adamushko wrote:
> On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>
> Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this
>> before merging (i.e. make XN_ISR_HANDLED non-zero)?
>
>
> Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now
> non-zero.
> Actually, at first I wanted to make it just the other way about.
>
Hmm, thinking about this again, I would like to revert my suggestion in
favour of a third approach (trying to keep you busy ;) ).
Recommended default values:
XN_ISR_HANDLED => XN_ISR_HANDLED | XN_ISR_ENABLE
XN_ISR_NOINT => XN_ISR_NO_ENABLE
Additional bits:
XN_ISR_NO_ENABLE
=> indicate that XN_ISR_ENABLE is not set
(use case: handle but delay re-enable)
XN_ISR_PROPAGATE
=> XN_ISR_CHAINED (the new name is closer to the related function
calls - at least for me).
Not expressible combination:
XN_ISR_ENABLE | XN_ISR_NOINT
=> Doesn't make sense, does it?
Still expressible invalid combination:
XN_ISR_HANDLED | XN_ISR_PROPAGATE
=> The implicit enable request should be easy to ignore at nucleus
level.
Rational:
Most users will either indicate that an IRQ was handled and should be
re-enabled as soon as possible or that it was unhandled and should
better remain masked if no one else cares.
XN_ISR_PROPAGATE (or related skin defines) are only useful for very very
special corner-case drivers and applications. Normal generic drivers
should not touch this (including my own xeno_16550A... bah!). BTW, if
there is some unintentional IRQ sharing so that spurious RT interrupts
occur, I think we should detect this at nucleus level and bail out.
Otherwise, systems quickly lock up (I just faced this once again two
days ago on an old RTAI box).
This will break some stuff (RTDM e.g., but we have some version flags
for such cases), but it takes us closer to the standard Linux API
without loosing control over the details when required.
Comments?
>
> Moreover, I attached an add-on patch to overcome the name buffer in
>> xnintr_t. Note that this patch is only compile-tested, I have no native
>> interrupt test-case at hand.
>
>
>
> Heh.. someone once suggested the -p key with "diff" to me? :o)
Mmpf. Must have been too late, and I was too impressed how interdiff
worked out this patch. :)
>
> I bet your patch would even work but it's, well, slightly broken indeed ;-)
>
> Look at the native/syscall.c : __rt_intr_delete() :
>
> RT_INTR *intr = (RT_INTR *)rt_registry_fetch(ph.opaque);
> ...
> xnfree(intr);
>
> "intr" actually points to intr_buf->intr and it works as expected only
> because "RT_INTR intr" is the first member of the struct, so that it's
> address == the address of the whole object of this struct.
But this is not an unusual trick, and I see no practical problems.
>
> To do it properly you would need to make your "unnamed" struct visible
> for both __rt_intr_create() and __rt_intr_delete(). Something like :
>
> struct RT_INTR_ENV {
>
> #define link2intr_env(laddr) \
> ((RT_INTR_ENV *)(((char* )laddr) - (int)(&((RT_INTR_ENV *)0)->intr)))
>
> RT_INTR intr;
> char name[XNOBJECT_NAME_LEN];
> };
>
> and then make use of link2intr_env() in __rt_intr_delet().
>
>
> Ok, with you approach we would avoid allocating memory
> for name[XNOBJECT_NAME_LEN] when an object is created in kernel-mode
> and do it via RT_INTR_ENV only when it's created in user-mode.
>
> We could even use that approach for all objects in native skin, but we can't
> because of "anonymous" objects supported in kernel-mode too and, well, I
> personally would not like those RT_OBJECT_ENV structures all over the map in
> syscall.c :)
Me too.
>
> So I don't want to break the whole picture only for the RT_INTR object (it
> doesn't support "anonymous" objects now).
>
> Nevertheless, if you still want to save a hundred bytes of data or so (how
> many interrupt objects would a normal system have? and I guess the lenght of
> name is about 10 symbols on average :)
RT_INTR is now disabled by default in the kernel config. Thus most users
should not see its code in their kernels, only the additional data and
code related to xnintr_t.name.
>
> for the users that don't use native skin, then I would suggest the following
> way :
>
> xnintr_t contains just the link "char *name" as you suggested but RT_INTR
> contains the name[XNOBJECT_NAME_LEN] member (as it was before actually).
>
> If it's ok, then I'll send um... yet another final patch that addresses
> both issues :)
I'm fine with this as well if you prefer it, no problem.
Jan
signature.asc
Description: OpenPGP digital signature
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
On 16/02/06, Jan Kiszka <[EMAIL PROTECTED]
> wrote:
Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve thisbefore merging (
i.e. make XN_ISR_HANDLED non-zero)?
Ok, XN_ISR_NOINT is removed in favor of XN_ISR_HANDLED which is now non-zero.
Actually, at first I wanted to make it just the other way about.
Moreover, I attached an add-on patch to overcome the name buffer inxnintr_t. Note that this patch is only compile-tested, I have no native
interrupt test-case at hand.
Heh.. someone once suggested the -p key with "diff" to me? :o)
I bet your patch would even work but it's, well, slightly broken indeed ;-)
Look at the native/syscall.c : __rt_intr_delete() :
RT_INTR *intr = (RT_INTR *)rt_registry_fetch(ph.opaque);
...
xnfree(intr);
"intr" actually points to intr_buf->intr and it works as expected only
because "RT_INTR intr" is the first member of the struct, so that it's
address == the address of the whole object of this struct.
To do it properly you would need to make your "unnamed" struct visible
for both __rt_intr_create() and __rt_intr_delete(). Something like :
struct RT_INTR_ENV {
#define link2intr_env(laddr) \
((RT_INTR_ENV *)(((char* )laddr) - (int)(&((RT_INTR_ENV *)0)->intr)))
RT_INTR intr;
char name[XNOBJECT_NAME_LEN];
};
and then make use of link2intr_env() in __rt_intr_delet().
Ok, with you approach we would avoid allocating memory
for name[XNOBJECT_NAME_LEN] when an object is created in kernel-mode
and do it via RT_INTR_ENV only when it's created in user-mode.
We could even use that approach for all objects in native skin, but we can't
because of "anonymous" objects supported in kernel-mode too and, well,
I personally would not like those RT_OBJECT_ENV structures all over the
map in syscall.c :)
So I don't want to break the whole picture only for the RT_INTR object (it doesn't support "anonymous" objects now).
Nevertheless, if you still want to save a hundred bytes of data or so
(how many interrupt objects would a normal system have? and I guess the
lenght of name is about 10 symbols on average :)
for the users that don't use native skin, then I would suggest the following way :
xnintr_t contains just the link "char *name" as you suggested but
RT_INTR contains the name[XNOBJECT_NAME_LEN] member (as it was before
actually).
If it's ok, then I'll send um... yet another final patch that addresses both issues :)
Jan-- Best regards,Dmitry Adamushko
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Handling PCI MSI interrupts
Russell Johnson wrote:
It's definitely an Adeos issue and msi.c needs fixing. What
about this patch, do
things improve with it (against 2.6.15-ipipe-1.2-00)?
I'm currently patching my setup which started with ipipe-2.6.14-i386-1.0-12.
I've been having no luck with any MSI devices in the system even if they
have supposedly had MSI disabled. I'll post my testing results in the next
day or so.
The latest patch was incomplete; you might be luckier with this one. I've merged
Jeroen's last observations on this issue and mine.
--- 2.6.15/drivers/pci/msi.c2006-01-03 04:21:10.0 +0100
+++ 2.6.15-ipipe/drivers/pci/msi.c 2006-02-16 10:30:27.0 +0100
@@ -149,6 +149,21 @@
msi_set_mask_bit(vector, 0);
}
+#ifdef CONFIG_IPIPE
+static void ack_MSI_irq_w_maskbits(unsigned int vector)
+{
+mask_MSI_irq(vector);
+__ack_APIC_irq();
+}
+static void ack_MSI_irq_wo_maskbits(unsigned int vector)
+{
+__ack_APIC_irq();
+}
+#else /* !CONFIG_IPIPE */
+#define ack_MSI_irq_wo_maskbits do_nothing
+#define ack_MSI_irq_w_maskbits mask_MSI_irq
+#endif /* CONFIG_IPIPE */
+
static unsigned int startup_msi_irq_wo_maskbit(unsigned int vector)
{
struct msi_desc *entry;
@@ -212,7 +227,7 @@
.shutdown = shutdown_msi_irq,
.enable = unmask_MSI_irq,
.disable= mask_MSI_irq,
- .ack= mask_MSI_irq,
+ .ack= ack_MSI_irq_w_maskbits,
.end= end_msi_irq_w_maskbit,
.set_affinity = set_msi_irq_affinity
};
@@ -228,7 +243,7 @@
.shutdown = shutdown_msi_irq,
.enable = unmask_MSI_irq,
.disable= mask_MSI_irq,
- .ack= mask_MSI_irq,
+ .ack= ack_MSI_irq_w_maskbits,
.end= end_msi_irq_w_maskbit,
.set_affinity = set_msi_irq_affinity
};
@@ -244,7 +259,7 @@
.shutdown = shutdown_msi_irq,
.enable = do_nothing,
.disable= do_nothing,
- .ack= do_nothing,
+ .ack= ack_MSI_irq_wo_maskbits,
.end= end_msi_irq_wo_maskbit,
.set_affinity = set_msi_irq_affinity
};
--
Philippe.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] separate queue debugging switch
Philippe Gerum wrote: > Jan Kiszka wrote: >> Philippe Gerum wrote: >> >>> Jan Kiszka wrote: >>> Philippe Gerum wrote: > Jan Kiszka wrote: > > >> Hi, >> >> while XENO_OPT_DEBUG is generally a useful switch for tracing >> potential >> issues in the core and the skins, it also introduces high >> latencies via >> the queue debugging feature (due to checks iterating over whole >> queues). >> >> This patch introduces separate control over queue debugging so >> that you >> can have debug checks without too dramatic slowdowns. >> > > Maybe it's time to introduce debug levels, so that we could reuse them > in order to > add more (selectable) debug instrumentation; queue debugging could > then > be given a > certain level (likely something like CONFIG_XENO_DEBUG_LEVEL=8712 for > this one...), instead of going for a specific conditional each time we > introduce new checks? > Hmm, this means someone have to define what should be printed at which level - tend to be hard decisions... Often it is at least as much useful to have debug groups so that specific parts can be excluded from debugging. I'm pro such groups (one would be those queues e.g.) but contra too many levels (2, at most 3). >>> >>> Ack, selection by increasingly verbose/high-overhead groups is what I >>> have in mind. >>> >>> At this chance, I would also suggest to introduce some ASSERT macro (per group, per level). That could be used to instrument the core with runtime checks. But it could also be quickly removed at compilation time, reducing the code size (e.g. checks at the nucleus layer against buggy skins or at RTDM layer against rough drivers). >>> >>> I'm not opposed to that, if we keep the noise / signal ratio of those >>> assertions at the reasonable low-level throughout the code, and don't >>> use this to enforce silly parametrical checks. >>> >> >> >> Then let's discuss how to implement and control this. Say we have some >> macros for marking code as "depends on debug group X": >> >> #if XENO_DEBUG_GROUP(group) >> code; >> #endif /* XENO_DEBUG_GROUP(group) */ >> >> XENO_IF_DEBUG_GROUP(group, code); >> >> (or do you prefere XNPOD_xxx?) >> > > This debug code may span feature/component boundaries, so XENO_ is better. > >> Additionally, we could introduce that assertion macro: >> >> XENO_ASSERT(group, expression, failure_code); >> >> But how to control the groups now? Via Kconfig bool options? > > Yes, I think so. From some specialized Debug menu in the generic > portion. We would need this to keep the (unused) debug code out of > production systems. > > And what >> groups to define? Per subsytem? Or per disturbance level (latency >> regression)? If we control the group selection via Kconfig, we could >> define pseudo bool options like "All debug groups" or "Low-intrusive >> debug groups" that select the fitting concrete groups. >> > > We won't be able to anticipate on each and every debug spots we might > need in the future, and in any case, debug triggers may well span > multiple sub-systems. I'd go for defining levels depending on the > throroughness/complexity of their checks. > To keep it simple: XNDBG_LIGHT /* simple checks with low constant overhead */ XNDBG_HEAVY /* complex checks with high or unknown overhead */ Those two could become #defines and would have to be provide as first argument to our debug macros. Or we directly merge the attribute into the macro name: XENO_DEBUG_LIGHT, XENO_IF_DEBUG_LIGHT(), XENO_ASSERT_LIGHT() XENO_DEBUG_HEAVY, XENO_IF_DEBUG_HEAVY(), XENO_ASSERT_HEAVY() >> Alternatively, we could make the group selection a runtime switch, >> controlled via a global bitmask that can be modified through /proc e.g. >> Only switching of CONFIG_XENO_OPT_DEBUG would then remove all debugging >> code, otherwise the execution of the checks would depend on the current >> bitmask content. > > We could cumulate this with the static selection. > Then this is also a perfect add-on - for later work. :) Jan signature.asc Description: OpenPGP digital signature
[Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge)
Dmitry Adamushko wrote:
> Hello everybody,
>
> being inspired by successful results of tests conducted recently by Jan &
> team,
> I'm presenting the final (yep, yet another final :) combo-patch.
>
> The shirq support now is optional, so that
>
> CONFIG_XENO_OPT_SHIRQ_LEVEL -enables shirq for level-triggered
> interrupts;
>
> CONFIG_XENO_OPT_SHIRQ_EDGE --//- for edge-triggered ones.
>
> I addressed all the remarks and now, IMHO, it's (hopefully) ready for merge.
>
Hmm, I still find XN_ISR_NOINT in the patch. Shouldn't we solve this
before merging (i.e. make XN_ISR_HANDLED non-zero)?
Moreover, I attached an add-on patch to overcome the name buffer in
xnintr_t. Note that this patch is only compile-tested, I have no native
interrupt test-case at hand.
Jan
diff -u include/nucleus/intr.h include/nucleus/intr.h
--- include/nucleus/intr.h (Arbeitskopie)
+++ include/nucleus/intr.h (Arbeitskopie)
@@ -54,7 +54,7 @@
xniack_t iack; /* !< Interrupt acknowledge routine. */
-char name[XNOBJECT_NAME_LEN]; /* !< Symbolic name. */
+const char *name; /* !< Symbolic name. */
} xnintr_t;
diff -u ksrc/skins/native/syscall.c ksrc/skins/native/syscall.c
--- ksrc/skins/native/syscall.c (Arbeitskopie)
+++ ksrc/skins/native/syscall.c (Arbeitskopie)
@@ -2883,23 +2883,15 @@
char name[XNOBJECT_NAME_LEN];
RT_INTR_PLACEHOLDER ph;
int err, mode;
-RT_INTR *intr;
+struct {
+ RT_INTR intr;
+ char name[XNOBJECT_NAME_LEN];
+} *intr_buf;
unsigned irq;
if (!__xn_access_ok(curr,VERIFY_WRITE,__xn_reg_arg1(regs),sizeof(ph)))
return -EFAULT;
-if (__xn_reg_arg2(regs))
- {
- if (!__xn_access_ok(curr,VERIFY_READ,__xn_reg_arg2(regs),sizeof(name)))
- return -EFAULT;
-
- __xn_strncpy_from_user(curr,name,(const char __user *)__xn_reg_arg2(regs),sizeof(name) - 1);
- name[sizeof(name) - 1] = '\0';
- }
-else
- *name = '\0';
-
/* Interrupt line number. */
irq = (unsigned)__xn_reg_arg3(regs);
@@ -2909,23 +2901,41 @@
if (mode & ~(I_AUTOENA|I_PROPAGATE))
return -EINVAL;
-intr = (RT_INTR *)xnmalloc(sizeof(*intr));
+intr_buf = (typeof(intr_buf))xnmalloc(sizeof(*intr_buf));
-if (!intr)
+if (!intr_buf)
return -ENOMEM;
-err = rt_intr_create(intr,name,irq,&rt_intr_handler,NULL,0);
+if (__xn_reg_arg2(regs))
+ {
+ if (!__xn_access_ok(curr,VERIFY_READ,__xn_reg_arg2(regs),sizeof(name)))
+ {
+ xnfree(intr_buf);
+ return -EFAULT;
+ }
+
+ __xn_strncpy_from_user(curr,
+ intr_buf->name,
+ (const char __user *)__xn_reg_arg2(regs),
+ sizeof(intr_buf->name) - 1);
+ intr_buf->name[sizeof(intr_buf->name) - 1] = '\0';
+ }
+else
+ intr_buf->name[0] = '\0';
+
+
+err = rt_intr_create(&intr_buf->intr,intr_buf->name,irq,&rt_intr_handler,NULL,0);
if (err == 0)
{
- intr->mode = mode;
- intr->cpid = curr->pid;
+ intr_buf->intr.mode = mode;
+ intr_buf->intr.cpid = curr->pid;
/* Copy back the registry handle to the ph struct. */
- ph.opaque = intr->handle;
+ ph.opaque = intr_buf->intr.handle;
__xn_copy_to_user(curr,(void __user *)__xn_reg_arg1(regs),&ph,sizeof(ph));
}
else
- xnfree(intr);
+ xnfree(intr_buf);
return err;
}
diff -u ksrc/nucleus/intr.c ksrc/nucleus/intr.c
--- ksrc/nucleus/intr.c (Arbeitskopie)
+++ ksrc/nucleus/intr.c (Arbeitskopie)
@@ -148,7 +148,7 @@
intr->cookie = NULL;
intr->hits = 0;
intr->flags = flags;
-xnobject_copy_name(intr->name,name);
+intr->name = name;
#if defined(CONFIG_XENO_OPT_SHIRQ_LEVEL) || defined(CONFIG_XENO_OPT_SHIRQ_EDGE)
intr->next = NULL;
#endif /* CONFIG_XENO_OPT_SHIRQ_LEVEL || CONFIG_XENO_OPT_SHIRQ_EDGE */
signature.asc
Description: OpenPGP digital signature
RE: [Xenomai-core] Handling PCI MSI interrupts
> It's definitely an Adeos issue and msi.c needs fixing. What > about this patch, do > things improve with it (against 2.6.15-ipipe-1.2-00)? I'm currently patching my setup which started with ipipe-2.6.14-i386-1.0-12. I've been having no luck with any MSI devices in the system even if they have supposedly had MSI disabled. I'll post my testing results in the next day or so. Russ
