public inbox for ecos-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug 1001160] New: FIQ can trash stack when interrupting IRQ
@ 2011-02-22 16:15 bugzilla-daemon
  2011-02-22 16:16 ` [Bug 1001160] " bugzilla-daemon
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-22 16:15 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

           Summary: FIQ can trash stack when interrupting IRQ
           Product: eCos
           Version: 3.0
          Platform: All
        OS/Version: ARM
            Status: UNCONFIRMED
          Severity: critical
          Priority: low
         Component: HAL
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: ml@tctechnologies.tc
                CC: ecos-bugs@ecos.sourceware.org
             Class: Advice Request


There is a small unprotected critical section in vectors.s. The opening is
exactly one instruction in size so it is hard to reproduce unless there is a
high frequency of FIQ;s and IRQ's.
We have seen various reports about this dating as far back as 2004 but we have
seen no solutions. A solution is suggested in this bug report.

Please note that this bug is only relevant for implementations using both FIQ
and IRQ interrupt sources.

Scenario:
IRQ event arrives.
Handler will execute:
IRQ:
        // Note: I use this exception stack while saving the context because
        // the current SP does not seem to be always valid in this CPU mode.
        ldr     sp,.__exception_stack   // get good stack
        stmfd   sp!,{r0-r5}             // save some supervisor regs
        sub     r0,lr,#4                // PC at time of interrupt
        mrs     r1,spsr
        mov     r2,#CYGNUM_HAL_VECTOR_IRQ
        mov     r3,sp

At this point r0-r5 of the interrupted code is stored in __exception_stack.
If any FIQ happens the FIQ handler will detect that the mode is CPSR_IRQ_MODE
and disable itself. That protects this region so far.

The IRQ handler continues:
        mrs     r4,cpsr                 // switch to Supervisor Mode
        bic     r4,r4,#CPSR_MODE_BITS
        // When handling an IRQ we must disable FIQ unless the current 
        // mode in CPSR is IRQ. If we were to get a FIQ while in another 
        // mode, the FIQ handling code would transform the FIQ into an 
        // IRQ and call the non-reentrant IRQ handler again. As a result, 
        // for example, the stack pointer would be set to the beginning 
        // of the exception_stack clobbering the registers we have just 
        // saved.
        orr     r4,r4,#CPSR_SUPERVISOR_MODE|CPSR_FIQ_DISABLE
        msr     cpsr,r4

The problem is in the last instruction above. The ARM7 architecture
documentation states:
"If an interrupt is received by the core during execution of an instruction
that disables interrupts, the ARM7 family will still take the interrupt. This
occurs for both IRQ and FIQ interrupts."

So assume the FIQ is happening while executing msr     cpsr,r4. The instruction
will complete and then the FIQ will run. Unfortunately the processor is not in
CPSR_IRQ_MODE any more so the FIQ will fall through into the IRQ, trashing the
stored r0-r5.

After the FIQ is done its work, the original IRQ will continue and eventually
return to the interrupted code with r0-r5 corrupted.

This can obviously result in all sorts of crashes and usually the code will
eventually end up in an exception handler or run wild.

We would like to propose a fix for this problem as well. This fix has been
tested and it solves the problem. The fix is to first disable FIQ and then
change mode. The following code snippet does that:

IRQ:
        // Note: I use this exception stack while saving the context because
        // the current SP does not seem to be always valid in this CPU mode.
        ldr     sp,.__exception_stack   // get good stack
        stmfd   sp!,{r0-r5}             // save some supervisor regs
        sub     r0,lr,#4                // PC at time of interrupt
        mrs     r1,spsr
        mov     r2,#CYGNUM_HAL_VECTOR_IRQ
        mov     r3,sp

        mrs     r4,cpsr                 // switch to Supervisor Mode

        // ML/BK18-01-2011, small chance of contention with FIQ
        //   we should disable FIQ while still in IRQ mode and then
        //   change to SVC mode
        orr     r4,r4,#CPSR_FIQ_DISABLE        
        msr     cpsr,r4

        bic     r4,r4,#CPSR_MODE_BITS
        // When handling an IRQ we must disable FIQ unless the current 
        // mode in CPSR is IRQ. If we were to get a FIQ while in another 
        // mode, the FIQ handling code would transform the FIQ into an 
        // IRQ and call the non-reentrant IRQ handler again. As a result, 
        // for example, the stack pointer would be set to the beginning 
        // of the exception_stack clobbering the registers we have just 
        // saved.
        orr     r4,r4,#CPSR_SUPERVISOR_MODE|CPSR_FIQ_DISABLE
        msr     cpsr,r4


Brian Karr and Morten Lave
TC applied Technologies.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
@ 2011-02-22 16:16 ` bugzilla-daemon
  2011-02-23 11:02 ` bugzilla-daemon
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-22 16:16 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Morten Lave <ml@tctechnologies.tc> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|low                         |high
                 CC|                            |ml@tctechnologies.tc

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
  2011-02-22 16:16 ` [Bug 1001160] " bugzilla-daemon
@ 2011-02-23 11:02 ` bugzilla-daemon
  2011-02-23 11:20 ` bugzilla-daemon
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-23 11:02 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

John Dallaway <john@dallaway.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |john@dallaway.org.uk

--- Comment #1 from John Dallaway <john@dallaway.org.uk> 2011-02-23 11:02:28 GMT ---
Morten and Brian, thank you for the bug report. Your explanation of the problem
makes good sense to me.

The proposed fix adds 2 instructions to the default IRQ VSR which should only
be necessary when using both IRQ and FIQ interrupt sources. I this an optimal
fix? Is it worth including these extra instructions conditionally based on a
CDL interface which could be implemented by any eCos hardware driver package?

Comments requested from the ARM experts please.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
  2011-02-22 16:16 ` [Bug 1001160] " bugzilla-daemon
  2011-02-23 11:02 ` bugzilla-daemon
@ 2011-02-23 11:20 ` bugzilla-daemon
  2011-02-23 15:07 ` bugzilla-daemon
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-23 11:20 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jifl@ecoscentric.com

--- Comment #2 from Jonathan Larmour <jifl@ecoscentric.com> 2011-02-23 11:20:01 GMT ---
I'm not sure myself, but in that case it would effectively mean that FIQ
support as a whole should be controlled by a CDL option, not just these two
instructions. Maybe that would be no bad thing as it isn't used very often; and
when it _is_ used, really people should be using a VSR, rather than letting
heavyweight code kludge it into something like an ISR (the effect of which is
to make FIQs slower than IRQs which can hardly be the point).

Personally I think that would probably be a useful change.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
                   ` (2 preceding siblings ...)
  2011-02-23 11:20 ` bugzilla-daemon
@ 2011-02-23 15:07 ` bugzilla-daemon
  2011-03-01  0:06 ` bugzilla-daemon
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-23 15:07 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

--- Comment #3 from Morten Lave <ml@tctechnologies.tc> 2011-02-23 15:07:10 GMT ---
Absolutely valid comments. I do not believe it makes sense to make the fix a
CDL option as it would not make sense to use FIQ without the fix. It is a
disaster waiting to happen. It does make sense to make the whole FIQ a CDL
option though.
I see two objectives (apart form not using FIQ at all):
1) The FIQ happens to be hardwired to some function in an ARM chip and it is
not possible to avoid using it. In that case you would want the FIQ code with
the proposed fix.

2) You need the FIQ to do something really high priority, low latency. In that
case the user might want to disable eCos FIQ handling all together, modify the
HAL interrupt functions to not touch the F bit and likewise modify Vectors.s to
not touch the F bit.


option 2) has been discussed on the forums and we have at some point done
something like this ad-hoc for audio processing but have not made it generic
enough to be part of a release. We would love to do that with some help.

It could be nice with a CDL option selecting:
- No eCos FIQ handling.
- Standard eCos FIQ handling (FIQ morphs into IRQ)
- FIQ independent of eCos (no kernel calls allowed)

Maybe the simple first step is to make an option to include the FIQ->isr/dsr or
not. If not included it will just be a VSR (as suggested by Jonathan Larmour).
For super low latency requirements one can still manually modify vectors.s and
the HAL interrupt functions to prevent the F bit from being set.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
                   ` (3 preceding siblings ...)
  2011-02-23 15:07 ` bugzilla-daemon
@ 2011-03-01  0:06 ` bugzilla-daemon
  2011-03-09 16:32 ` bugzilla-daemon
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-03-01  0:06 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

--- Comment #4 from Jonathan Larmour <jifl@ecoscentric.com> 2011-03-01 00:06:25 GMT ---
(In reply to comment #3)
> Absolutely valid comments. I do not believe it makes sense to make the fix a
> CDL option as it would not make sense to use FIQ without the fix. It is a
> disaster waiting to happen. It does make sense to make the whole FIQ a CDL
> option though.

Glad we agree :-).

> I see two objectives (apart form not using FIQ at all):
> 1) The FIQ happens to be hardwired to some function in an ARM chip and it is
> not possible to avoid using it. In that case you would want the FIQ code with
> the proposed fix.

The processor HAL could and should provide the FIQ handler in the form of a
VSR, not a C level interrupt handler. Usually a FIQ is a FIQ (not an IRQ) for a
good reason, and should be handled quickly, and I wouldn't expect that to be
different here.

> 2) You need the FIQ to do something really high priority, low latency. In that
> case the user might want to disable eCos FIQ handling all together, modify the
> HAL interrupt functions to not touch the F bit and likewise modify Vectors.s to
> not touch the F bit.

I see that point yes. The problem there is what happens when the user _does_
want the FIQ handler to interact with the rest of the system in some way, which
will probably lead to wanting to disable FIQs as well as IRQs *some*times.
Having a single HAL_DISABLE_INTERRUPT() interface is not enough.

On the other hand, this sort of problem may be too hard to solve at any generic
level, so leaving it to the user to decide at CDL level maybe is the best
course.

> option 2) has been discussed on the forums and we have at some point done
> something like this ad-hoc for audio processing but have not made it generic
> enough to be part of a release. We would love to do that with some help.
> 
> It could be nice with a CDL option selecting:
> - No eCos FIQ handling.
> - Standard eCos FIQ handling (FIQ morphs into IRQ)
> - FIQ independent of eCos (no kernel calls allowed)

Given that description, the first and third seem the same, given it will always
be possible to set a FIQ VSR.

Just to clarify what I believe you are getting at:
- Standard eCos FIQ handling (FIQ morphs into IRQ)
- No eCos FIQ handling. Enable/disable interrupts also disables FIQs.
- No eCos FIQ handling. Enable/disable interrupts does not mask FIQs (does not
change F bit), which instead must be dealt with (masked/unmasked) by user
manually.

> Maybe the simple first step is to make an option to include the FIQ->isr/dsr or
> not. If not included it will just be a VSR (as suggested by Jonathan Larmour).
> For super low latency requirements one can still manually modify vectors.s and
> the HAL interrupt functions to prevent the F bit from being set.

There are only a few places the F-bit is modified, so hopefully it wouldn't be
hard to address this all at once. Basically nearly every mention of
CPSR_FIQ_DISABLE in the ARM architecture HAL.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
                   ` (4 preceding siblings ...)
  2011-03-01  0:06 ` bugzilla-daemon
@ 2011-03-09 16:32 ` bugzilla-daemon
  2012-04-23 19:38 ` bugzilla-daemon
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-03-09 16:32 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

--- Comment #5 from Morten Lave <ml@tctechnologies.tc> 2011-03-09 16:32:30 GMT ---
I agree that two options would be enough to cover most cases.
1) As is with the fix, FIQ morphs into IRQ.
2) FIQ is a VSR.

If someone needs to have FIQ not be disabled by the kernel they can do the
following:
- Redefine HAL_xxxxx_INTERRUP functions to not touch the F bit.
- Make a modified vectors.s which does not touch the F bit

My third suggestion was 2) combined with the FIQ disable fix but it might not
be generic enough to add as a CDL option.

I am not sure what the procedure is from here. Can you provide me with a link
describing how a bug report eventually results in a fix?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
                   ` (5 preceding siblings ...)
  2011-03-09 16:32 ` bugzilla-daemon
@ 2012-04-23 19:38 ` bugzilla-daemon
  2012-05-11 10:36 ` bugzilla-daemon
  2012-06-04  2:14 ` bugzilla-daemon
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2012-04-23 19:38 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |1001572

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
                   ` (6 preceding siblings ...)
  2012-04-23 19:38 ` bugzilla-daemon
@ 2012-05-11 10:36 ` bugzilla-daemon
  2012-06-04  2:14 ` bugzilla-daemon
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2012-05-11 10:36 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Bug 1001160 depends on bug 1001572, which changed state.

Bug 1001572 Summary: Separate FIQ and IRQ management
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001572

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |CURRENTRELEASE

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
                   ` (7 preceding siblings ...)
  2012-05-11 10:36 ` bugzilla-daemon
@ 2012-06-04  2:14 ` bugzilla-daemon
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2012-06-04  2:14 UTC (permalink / raw)
  To: unassigned

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |CURRENTRELEASE

--- Comment #6 from Jonathan Larmour <jifl@ecoscentric.com> 2012-06-04 03:14:34 BST ---
I think given the resolution of bug 1001572 and associated patch, this should
be marked resolved now. The original problem reported in this bug should now be
fixed.

The only difference is as mentioned in 
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001572#c2 but given the
subsequent reply from Nick, I don't think this is going to be pursued, or at
least, not by us.

So I'm marking this one resolved now.

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
                   ` (7 preceding siblings ...)
  2012-05-11 10:36 ` bugzilla-daemon
@ 2012-06-04  2:15 ` bugzilla-daemon
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2012-06-04  2:15 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |CURRENTRELEASE

--- Comment #6 from Jonathan Larmour <jifl@ecoscentric.com> 2012-06-04 03:14:34 BST ---
I think given the resolution of bug 1001572 and associated patch, this should
be marked resolved now. The original problem reported in this bug should now be
fixed.

The only difference is as mentioned in 
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001572#c2 but given the
subsequent reply from Nick, I don't think this is going to be pursued, or at
least, not by us.

So I'm marking this one resolved now.

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
                   ` (6 preceding siblings ...)
  2012-04-23 19:39 ` bugzilla-daemon
@ 2012-05-11 10:36 ` bugzilla-daemon
  2012-06-04  2:15 ` bugzilla-daemon
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2012-05-11 10:36 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Bug 1001160 depends on bug 1001572, which changed state.

Bug 1001572 Summary: Separate FIQ and IRQ management
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001572

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |CURRENTRELEASE

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
                   ` (5 preceding siblings ...)
  2011-03-09 16:32 ` bugzilla-daemon
@ 2012-04-23 19:39 ` bugzilla-daemon
  2012-05-11 10:36 ` bugzilla-daemon
  2012-06-04  2:15 ` bugzilla-daemon
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2012-04-23 19:39 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |1001572

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
                   ` (4 preceding siblings ...)
  2011-03-01  0:06 ` bugzilla-daemon
@ 2011-03-09 16:32 ` bugzilla-daemon
  2012-04-23 19:39 ` bugzilla-daemon
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-03-09 16:32 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

--- Comment #5 from Morten Lave <ml@tctechnologies.tc> 2011-03-09 16:32:30 GMT ---
I agree that two options would be enough to cover most cases.
1) As is with the fix, FIQ morphs into IRQ.
2) FIQ is a VSR.

If someone needs to have FIQ not be disabled by the kernel they can do the
following:
- Redefine HAL_xxxxx_INTERRUP functions to not touch the F bit.
- Make a modified vectors.s which does not touch the F bit

My third suggestion was 2) combined with the FIQ disable fix but it might not
be generic enough to add as a CDL option.

I am not sure what the procedure is from here. Can you provide me with a link
describing how a bug report eventually results in a fix?

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
                   ` (3 preceding siblings ...)
  2011-02-23 15:07 ` bugzilla-daemon
@ 2011-03-01  0:06 ` bugzilla-daemon
  2011-03-09 16:32 ` bugzilla-daemon
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-03-01  0:06 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

--- Comment #4 from Jonathan Larmour <jifl@ecoscentric.com> 2011-03-01 00:06:25 GMT ---
(In reply to comment #3)
> Absolutely valid comments. I do not believe it makes sense to make the fix a
> CDL option as it would not make sense to use FIQ without the fix. It is a
> disaster waiting to happen. It does make sense to make the whole FIQ a CDL
> option though.

Glad we agree :-).

> I see two objectives (apart form not using FIQ at all):
> 1) The FIQ happens to be hardwired to some function in an ARM chip and it is
> not possible to avoid using it. In that case you would want the FIQ code with
> the proposed fix.

The processor HAL could and should provide the FIQ handler in the form of a
VSR, not a C level interrupt handler. Usually a FIQ is a FIQ (not an IRQ) for a
good reason, and should be handled quickly, and I wouldn't expect that to be
different here.

> 2) You need the FIQ to do something really high priority, low latency. In that
> case the user might want to disable eCos FIQ handling all together, modify the
> HAL interrupt functions to not touch the F bit and likewise modify Vectors.s to
> not touch the F bit.

I see that point yes. The problem there is what happens when the user _does_
want the FIQ handler to interact with the rest of the system in some way, which
will probably lead to wanting to disable FIQs as well as IRQs *some*times.
Having a single HAL_DISABLE_INTERRUPT() interface is not enough.

On the other hand, this sort of problem may be too hard to solve at any generic
level, so leaving it to the user to decide at CDL level maybe is the best
course.

> option 2) has been discussed on the forums and we have at some point done
> something like this ad-hoc for audio processing but have not made it generic
> enough to be part of a release. We would love to do that with some help.
> 
> It could be nice with a CDL option selecting:
> - No eCos FIQ handling.
> - Standard eCos FIQ handling (FIQ morphs into IRQ)
> - FIQ independent of eCos (no kernel calls allowed)

Given that description, the first and third seem the same, given it will always
be possible to set a FIQ VSR.

Just to clarify what I believe you are getting at:
- Standard eCos FIQ handling (FIQ morphs into IRQ)
- No eCos FIQ handling. Enable/disable interrupts also disables FIQs.
- No eCos FIQ handling. Enable/disable interrupts does not mask FIQs (does not
change F bit), which instead must be dealt with (masked/unmasked) by user
manually.

> Maybe the simple first step is to make an option to include the FIQ->isr/dsr or
> not. If not included it will just be a VSR (as suggested by Jonathan Larmour).
> For super low latency requirements one can still manually modify vectors.s and
> the HAL interrupt functions to prevent the F bit from being set.

There are only a few places the F-bit is modified, so hopefully it wouldn't be
hard to address this all at once. Basically nearly every mention of
CPSR_FIQ_DISABLE in the ARM architecture HAL.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
                   ` (2 preceding siblings ...)
  2011-02-23 11:20 ` bugzilla-daemon
@ 2011-02-23 15:07 ` bugzilla-daemon
  2011-03-01  0:06 ` bugzilla-daemon
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-23 15:07 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

--- Comment #3 from Morten Lave <ml@tctechnologies.tc> 2011-02-23 15:07:10 GMT ---
Absolutely valid comments. I do not believe it makes sense to make the fix a
CDL option as it would not make sense to use FIQ without the fix. It is a
disaster waiting to happen. It does make sense to make the whole FIQ a CDL
option though.
I see two objectives (apart form not using FIQ at all):
1) The FIQ happens to be hardwired to some function in an ARM chip and it is
not possible to avoid using it. In that case you would want the FIQ code with
the proposed fix.

2) You need the FIQ to do something really high priority, low latency. In that
case the user might want to disable eCos FIQ handling all together, modify the
HAL interrupt functions to not touch the F bit and likewise modify Vectors.s to
not touch the F bit.


option 2) has been discussed on the forums and we have at some point done
something like this ad-hoc for audio processing but have not made it generic
enough to be part of a release. We would love to do that with some help.

It could be nice with a CDL option selecting:
- No eCos FIQ handling.
- Standard eCos FIQ handling (FIQ morphs into IRQ)
- FIQ independent of eCos (no kernel calls allowed)

Maybe the simple first step is to make an option to include the FIQ->isr/dsr or
not. If not included it will just be a VSR (as suggested by Jonathan Larmour).
For super low latency requirements one can still manually modify vectors.s and
the HAL interrupt functions to prevent the F bit from being set.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
  2011-02-22 16:16 ` [Bug 1001160] " bugzilla-daemon
  2011-02-23 11:02 ` bugzilla-daemon
@ 2011-02-23 11:20 ` bugzilla-daemon
  2011-02-23 15:07 ` bugzilla-daemon
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-23 11:20 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jifl@ecoscentric.com

--- Comment #2 from Jonathan Larmour <jifl@ecoscentric.com> 2011-02-23 11:20:01 GMT ---
I'm not sure myself, but in that case it would effectively mean that FIQ
support as a whole should be controlled by a CDL option, not just these two
instructions. Maybe that would be no bad thing as it isn't used very often; and
when it _is_ used, really people should be using a VSR, rather than letting
heavyweight code kludge it into something like an ISR (the effect of which is
to make FIQs slower than IRQs which can hardly be the point).

Personally I think that would probably be a useful change.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
  2011-02-22 16:16 ` [Bug 1001160] " bugzilla-daemon
@ 2011-02-23 11:02 ` bugzilla-daemon
  2011-02-23 11:20 ` bugzilla-daemon
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-23 11:02 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

John Dallaway <john@dallaway.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |john@dallaway.org.uk

--- Comment #1 from John Dallaway <john@dallaway.org.uk> 2011-02-23 11:02:28 GMT ---
Morten and Brian, thank you for the bug report. Your explanation of the problem
makes good sense to me.

The proposed fix adds 2 instructions to the default IRQ VSR which should only
be necessary when using both IRQ and FIQ interrupt sources. I this an optimal
fix? Is it worth including these extra instructions conditionally based on a
CDL interface which could be implemented by any eCos hardware driver package?

Comments requested from the ARM experts please.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Bug 1001160] FIQ can trash stack when interrupting IRQ
  2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
@ 2011-02-22 16:16 ` bugzilla-daemon
  2011-02-23 11:02 ` bugzilla-daemon
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: bugzilla-daemon @ 2011-02-22 16:16 UTC (permalink / raw)
  To: ecos-bugs

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

Morten Lave <ml@tctechnologies.tc> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|low                         |high
                 CC|                            |ml@tctechnologies.tc

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2012-06-04  2:15 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-22 16:15 [Bug 1001160] New: FIQ can trash stack when interrupting IRQ bugzilla-daemon
2011-02-22 16:16 ` [Bug 1001160] " bugzilla-daemon
2011-02-23 11:02 ` bugzilla-daemon
2011-02-23 11:20 ` bugzilla-daemon
2011-02-23 15:07 ` bugzilla-daemon
2011-03-01  0:06 ` bugzilla-daemon
2011-03-09 16:32 ` bugzilla-daemon
2012-04-23 19:38 ` bugzilla-daemon
2012-05-11 10:36 ` bugzilla-daemon
2012-06-04  2:14 ` bugzilla-daemon
2011-02-22 16:15 [Bug 1001160] New: " bugzilla-daemon
2011-02-22 16:16 ` [Bug 1001160] " bugzilla-daemon
2011-02-23 11:02 ` bugzilla-daemon
2011-02-23 11:20 ` bugzilla-daemon
2011-02-23 15:07 ` bugzilla-daemon
2011-03-01  0:06 ` bugzilla-daemon
2011-03-09 16:32 ` bugzilla-daemon
2012-04-23 19:39 ` bugzilla-daemon
2012-05-11 10:36 ` bugzilla-daemon
2012-06-04  2:15 ` bugzilla-daemon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).