From 5471841870074043dc53f2bd1a52bc48970038e2 Mon Sep 17 00:00:00 2001 From: Soby Mathew Date: Tue, 27 Oct 2015 10:01:06 +0000 Subject: [PATCH] Remove the IMF_READ_INTERRUPT_ID build option The IMF_READ_INTERRUPT_ID build option enables a feature where the interrupt ID of the highest priority pending interrupt is passed as a parameter to the interrupt handler registered for that type of interrupt. This additional read of highest pending interrupt id from GIC is problematic as it is possible that the original interrupt may get deasserted and another interrupt of different type maybe become the highest pending interrupt. Hence it is safer to prevent such behaviour by removing the IMF_READ_INTERRUPT_ID build option. The `id` parameter of the interrupt handler `interrupt_type_handler_t` is now made a reserved parameter with this patch. It will always contain INTR_ID_UNAVAILABLE. Fixes ARM-software/tf-issues#307 Change-Id: I2173aae1dd37edad7ba6bdfb1a99868635fa34de --- bl31/aarch64/runtime_exceptions.S | 10 ---------- bl31/bl31.mk | 7 ------- docs/interrupt-framework-design.md | 17 +++++++---------- docs/porting-guide.md | 5 ++--- docs/user-guide.md | 5 ----- services/spd/opteed/opteed_main.c | 5 ----- services/spd/tspd/tspd_main.c | 9 --------- 7 files changed, 9 insertions(+), 49 deletions(-) diff --git a/bl31/aarch64/runtime_exceptions.S b/bl31/aarch64/runtime_exceptions.S index bbc7f1b46..28353202d 100644 --- a/bl31/aarch64/runtime_exceptions.S +++ b/bl31/aarch64/runtime_exceptions.S @@ -130,16 +130,6 @@ mov x21, x0 mov x0, #INTR_ID_UNAVAILABLE -#if IMF_READ_INTERRUPT_ID - /* - * Read the id of the highest priority pending interrupt. If - * no interrupt is asserted then return to where we came from. - */ - mov x19, #INTR_ID_UNAVAILABLE - bl plat_ic_get_pending_interrupt_id - cmp x19, x0 - b.eq interrupt_exit_\label -#endif /* Set the current security state in the 'flags' parameter */ mrs x2, scr_el3 diff --git a/bl31/bl31.mk b/bl31/bl31.mk index 04e154200..a31c1f475 100644 --- a/bl31/bl31.mk +++ b/bl31/bl31.mk @@ -60,13 +60,6 @@ endif BL31_LINKERFILE := bl31/bl31.ld.S -# Flag used by the generic interrupt management framework to determine if -# upon the assertion of an interrupt, it should pass the interrupt id or not -IMF_READ_INTERRUPT_ID := 0 - -$(eval $(call assert_boolean,IMF_READ_INTERRUPT_ID)) -$(eval $(call add_define,IMF_READ_INTERRUPT_ID)) - # Flag used to inidicate if Crash reporting via console should be included # in BL3-1. This defaults to being present in DEBUG builds only ifndef CRASH_REPORTING diff --git a/docs/interrupt-framework-design.md b/docs/interrupt-framework-design.md index cee29a318..53707ae9d 100644 --- a/docs/interrupt-framework-design.md +++ b/docs/interrupt-framework-design.md @@ -293,11 +293,9 @@ This component declares the following prototype for a handler of an interrupt ty void *handle, void *cookie); -The value of the `id` parameter depends upon the definition of the -`IMF_READ_INTERRUPT_ID` build time flag. When the flag is defined, `id` contains -the number of the highest priority pending interrupt of the type that this -handler was registered for. When the flag is not defined `id` contains -`INTR_ID_UNAVAILABLE`. +The `id` is parameter is reserved and could be used in the future for passing +the interrupt id of the highest pending interrupt only if there is a foolproof +way of determining the id. Currently it contains `INTR_ID_UNAVAILABLE`. The `flags` parameter contains miscellaneous information as follows. @@ -583,11 +581,10 @@ responsible for: irrecoverable error condition. 6. Calling the registered handler function for the interrupt type generated. - The firmware also determines the interrupt id if the IMF_READ_INTERRUPT_ID - build time flag is set. The id is set to `INTR_ID_UNAVAILABLE` if the flag - is not set. The id along with the current security state and a reference to - the `cpu_context_t` structure for the current security state are passed to - the handler function as its arguments. + The `id` parameter is set to `INTR_ID_UNAVAILABLE` currently. The id along + with the current security state and a reference to the `cpu_context_t` + structure for the current security state are passed to the handler function + as its arguments. The handler function returns a reference to the per-cpu `cpu_context_t` structure for the target security state. diff --git a/docs/porting-guide.md b/docs/porting-guide.md index 6e71025cc..8c0d7b7cc 100644 --- a/docs/porting-guide.md +++ b/docs/porting-guide.md @@ -1473,9 +1473,8 @@ of interrupt depends upon the id value as follows. Return : uint32_t This API returns the id of the highest priority pending interrupt at the -platform IC. The IMF passes the id returned by this API to the registered -handler for the pending interrupt if the `IMF_READ_INTERRUPT_ID` build time flag -is set. INTR_ID_UNAVAILABLE is returned when there is no interrupt pending. +platform IC. INTR_ID_UNAVAILABLE is returned when there is no interrupt +pending. ARM standard platforms read the _Highest Priority Pending Interrupt Register_ (`GICC_HPPIR`) to determine the id of the pending interrupt. The id diff --git a/docs/user-guide.md b/docs/user-guide.md index a7a0c11f3..b7b152e4f 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -250,11 +250,6 @@ performed. is used to determine the number of valid slave interfaces available in the ARM CCI driver. Default is 400 (that is, CCI-400). -* `IMF_READ_INTERRUPT_ID`: Boolean flag used by the interrupt management - framework to enable passing of the interrupt id to its handler. The id is - read using a platform GIC API. `INTR_ID_UNAVAILABLE` is passed instead if - this option set to 0. Default is 0. - * `RESET_TO_BL31`: Enable BL3-1 entrypoint as the CPU reset vector instead of the BL1 entrypoint. It can take the value 0 (CPU reset to BL1 entrypoint) or 1 (CPU reset to BL3-1 entrypoint). diff --git a/services/spd/opteed/opteed_main.c b/services/spd/opteed/opteed_main.c index fefc8a75e..7796fc4a5 100644 --- a/services/spd/opteed/opteed_main.c +++ b/services/spd/opteed/opteed_main.c @@ -85,11 +85,6 @@ static uint64_t opteed_sel1_interrupt_handler(uint32_t id, /* Check the security state when the exception was generated */ assert(get_interrupt_src_ss(flags) == NON_SECURE); -#if IMF_READ_INTERRUPT_ID - /* Check the security status of the interrupt */ - assert(plat_ic_get_interrupt_type(id) == INTR_TYPE_S_EL1); -#endif - /* Sanity check the pointer to this cpu's context */ assert(handle == cm_get_context(NON_SECURE)); diff --git a/services/spd/tspd/tspd_main.c b/services/spd/tspd/tspd_main.c index b8b67fadc..622316014 100644 --- a/services/spd/tspd/tspd_main.c +++ b/services/spd/tspd/tspd_main.c @@ -106,11 +106,6 @@ static uint64_t tspd_sel1_interrupt_handler(uint32_t id, /* Check the security state when the exception was generated */ assert(get_interrupt_src_ss(flags) == NON_SECURE); -#if IMF_READ_INTERRUPT_ID - /* Check the security status of the interrupt */ - assert(plat_ic_get_interrupt_type(id) == INTR_TYPE_S_EL1); -#endif - /* Sanity check the pointer to this cpu's context */ assert(handle == cm_get_context(NON_SECURE)); @@ -173,10 +168,6 @@ static uint64_t tspd_ns_interrupt_handler(uint32_t id, /* Check the security state when the exception was generated */ assert(get_interrupt_src_ss(flags) == SECURE); -#if IMF_READ_INTERRUPT_ID - /* Check the security status of the interrupt */ - assert(plat_ic_get_interrupt_type(id) == INTR_TYPE_NS); -#endif /* * Disable the routing of NS interrupts from secure world to EL3 while * interrupted on this core.