Commit 5717aae1 authored by Achin Gupta's avatar Achin Gupta
Browse files

Fix handling of spurious interrupts in BL3_1

There are couple of issues with how the interrupt routing framework in BL3_1
handles spurious interrupts.

1. In the macro 'handle_interrupt_exception', if a spurious interrupt is
   detected by plat_ic_get_pending_interrupt_type(), then execution jumps to
   'interrupt_exit_\label'. This macro uses the el3_exit() function to return to
   the original exception level. el3_exit() attempts to restore the SPSR_EL3 and
   ELR_EL3 registers with values from the current CPU context. Since these
   registers were not saved in this code path, it programs stale values into
   these registers. This leads to unpredictable behaviour after the execution of
   the ERET instruction.

2. When an interrupt is routed to EL3, it could be de-asserted before the
   GICC_HPPIR is read in plat_ic_get_pending_interrupt_type(). There could be
   another interrupt pending at the same time e.g. a non-secure interrupt. Its
   type will be returned instead of the original interrupt. This would result in
   a call to get_interrupt_type_handler(). The firmware will panic if the
   handler for this type of interrupt has not been registered.

This patch fixes the first problem by saving SPSR_EL3 and ELR_EL3 early in the
'handle_interrupt_exception' macro, instead of only doing so once the validity
of the interrupt has been determined.

The second problem is fixed by returning execution back to the lower exception
level through the 'interrupt_exit_\label' label instead of treating it as an
error condition. The 'interrupt_error_\label' label has been removed since it is
no longer used.

Fixes ARM-software/tf-issues#305

Change-Id: I81c729a206d461084db501bb81b44dff435021e8
parent aaa48a86
...@@ -79,6 +79,14 @@ ...@@ -79,6 +79,14 @@
str x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR] str x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
bl save_gp_registers bl save_gp_registers
/*
* Save the EL3 system registers needed to return from
* this exception.
*/
mrs x0, spsr_el3
mrs x1, elr_el3
stp x0, x1, [sp, #CTX_EL3STATE_OFFSET + CTX_SPSR_EL3]
/* Switch to the runtime stack i.e. SP_EL0 */ /* Switch to the runtime stack i.e. SP_EL0 */
ldr x2, [sp, #CTX_EL3STATE_OFFSET + CTX_RUNTIME_SP] ldr x2, [sp, #CTX_EL3STATE_OFFSET + CTX_RUNTIME_SP]
mov x20, sp mov x20, sp
...@@ -96,13 +104,29 @@ ...@@ -96,13 +104,29 @@
/* /*
* Get the registered handler for this interrupt type. A * Get the registered handler for this interrupt type. A
* NULL return value implies that an interrupt was generated * NULL return value could be 'cause of the following
* for which there is no handler registered or the interrupt * conditions:
* was routed incorrectly. This is a problem of the framework *
* so report it as an error. * a. An interrupt of a type was routed correctly but a
* handler for its type was not registered.
*
* b. An interrupt of a type was not routed correctly so
* a handler for its type was not registered.
*
* c. An interrupt of a type was routed correctly to EL3,
* but was deasserted before its pending state could
* be read. Another interrupt of a different type pended
* at the same time and its type was reported as pending
* instead. However, a handler for this type was not
* registered.
*
* a. and b. can only happen due to a programming error.
* The occurrence of c. could be beyond the control of
* Trusted Firmware. It makes sense to return from this
* exception instead of reporting an error.
*/ */
bl get_interrupt_type_handler bl get_interrupt_type_handler
cbz x0, interrupt_error_\label cbz x0, interrupt_exit_\label
mov x21, x0 mov x21, x0
mov x0, #INTR_ID_UNAVAILABLE mov x0, #INTR_ID_UNAVAILABLE
...@@ -117,14 +141,6 @@ ...@@ -117,14 +141,6 @@
b.eq interrupt_exit_\label b.eq interrupt_exit_\label
#endif #endif
/*
* Save the EL3 system registers needed to return from
* this exception.
*/
mrs x3, spsr_el3
mrs x4, elr_el3
stp x3, x4, [x20, #CTX_EL3STATE_OFFSET + CTX_SPSR_EL3]
/* Set the current security state in the 'flags' parameter */ /* Set the current security state in the 'flags' parameter */
mrs x2, scr_el3 mrs x2, scr_el3
ubfx x1, x2, #0, #1 ubfx x1, x2, #0, #1
...@@ -142,13 +158,6 @@ interrupt_exit_\label: ...@@ -142,13 +158,6 @@ interrupt_exit_\label:
/* Return from exception, possibly in a different security state */ /* Return from exception, possibly in a different security state */
b el3_exit b el3_exit
/*
* This label signifies a problem with the interrupt management
* framework where it is not safe to go back to the instruction
* where the interrupt was generated.
*/
interrupt_error_\label:
bl report_unhandled_interrupt
.endm .endm
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment