Rework incorrect use of assert() and panic() in codebase

Assert a valid security state using the macro sec_state_is_valid().
Replace assert() with panic() in those cases that might arise
because of runtime errors and not programming errors.
Replace panic() with assert() in those cases that might arise
because of programming errors.

Fixes ARM-software/tf-issues#96

Change-Id: I51e9ef0439fd5ff5e0edfef49050b69804bf14d5

diff --git a/bl31/bl31_main.c b/bl31/bl31_main.c
index 861b3914c292164d143df9035dd91e1a11ed0246..ff3c53b578c79bae4aeddedd959270fa892b1c3e 100644 (file)
--- a/bl31/bl31_main.c
+++ b/bl31/bl31_main.c
@@ -125,7 +125,7 @@ void bl31_main(void)
  ******************************************************************************/
 void bl31_set_next_image_type(uint32_t security_state)
 {
-       assert(security_state == NON_SECURE || security_state == SECURE);
+       assert(sec_state_is_valid(security_state));
        next_image_type = security_state;
 }
 

diff --git a/bl31/context_mgmt.c b/bl31/context_mgmt.c
index 65f12137e7bbfc1ee5d5820ef406c4597c9568d3..4502e5dcb86f62bc4079de0e2fea85f2fc6fa225 100644 (file)
--- a/bl31/context_mgmt.c
+++ b/bl31/context_mgmt.c
@@ -71,7 +71,7 @@ void cm_init(void)
  ******************************************************************************/
 void *cm_get_context_by_mpidr(uint64_t mpidr, uint32_t security_state)
 {
-       assert(security_state <= NON_SECURE);
+       assert(sec_state_is_valid(security_state));
 
        return get_cpu_data_by_mpidr(mpidr, cpu_context[security_state]);
 }
@@ -82,7 +82,7 @@ void *cm_get_context_by_mpidr(uint64_t mpidr, uint32_t security_state)
  ******************************************************************************/
 void cm_set_context_by_mpidr(uint64_t mpidr, void *context, uint32_t security_state)
 {
-       assert(security_state <= NON_SECURE);
+       assert(sec_state_is_valid(security_state));
 
        set_cpu_data_by_mpidr(mpidr, cpu_context[security_state], context);
 }

diff --git a/bl31/interrupt_mgmt.c b/bl31/interrupt_mgmt.c
index 2b0c7970659aeed9ad7b3ae092a468e3819b4bff..e595634ec6f822ea907b396fed52cb2ff494c787 100644 (file)
--- a/bl31/interrupt_mgmt.c
+++ b/bl31/interrupt_mgmt.c
@@ -107,7 +107,7 @@ uint32_t get_scr_el3_from_routing_model(uint32_t security_state)
 {
        uint32_t scr_el3;
 
-       assert(security_state <= NON_SECURE);
+       assert(sec_state_is_valid(security_state));
        scr_el3 = intr_type_descs[INTR_TYPE_NS].scr_el3[security_state];
        scr_el3 |= intr_type_descs[INTR_TYPE_S_EL1].scr_el3[security_state];
        scr_el3 |= intr_type_descs[INTR_TYPE_EL3].scr_el3[security_state];

diff --git a/common/bl_common.c b/common/bl_common.c
index 60b63f18f53f22693b540f900fd0ba9fb41d3370..d2c60eff8b370c51b7e8ebcffef8de8ee28f450f 100644 (file)
--- a/common/bl_common.c
+++ b/common/bl_common.c
@@ -61,12 +61,11 @@ void change_security_state(unsigned int target_security_state)
 {
        unsigned long scr = read_scr();
 
+       assert(sec_state_is_valid(target_security_state));
        if (target_security_state == SECURE)
                scr &= ~SCR_NS_BIT;
-       else if (target_security_state == NON_SECURE)
-               scr |= SCR_NS_BIT;
        else
-               assert(0);
+               scr |= SCR_NS_BIT;
 
        write_scr(scr);
 }

diff --git a/drivers/arm/gic/arm_gic.c b/drivers/arm/gic/arm_gic.c
index 636348baa26930f7aabbab196a0bda46ede69dba..86aaa9a1ff744ddaa1a0c5256ac86c8aecca9a3d 100644 (file)
--- a/drivers/arm/gic/arm_gic.c
+++ b/drivers/arm/gic/arm_gic.c
@@ -322,7 +322,7 @@ uint32_t arm_gic_interrupt_type_to_line(uint32_t type,
               type == INTR_TYPE_EL3 ||
               type == INTR_TYPE_NS);
 
-       assert(security_state == NON_SECURE || security_state == SECURE);
+       assert(sec_state_is_valid(security_state));
 
        /*
         * We ignore the security state parameter under the assumption that

diff --git a/drivers/arm/tzc400/tzc400.c b/drivers/arm/tzc400/tzc400.c
index c1716db44a7a79bf1e4b5d2902775359c91b2aec..715ea6c0b736da5bd09c1800489fb90cd07fef57 100644 (file)
--- a/drivers/arm/tzc400/tzc400.c
+++ b/drivers/arm/tzc400/tzc400.c
@@ -103,7 +103,7 @@ static uint32_t tzc_get_gate_keeper(uint64_t base, uint8_t filter)
        tmp = (tzc_read_gate_keeper(base) >> GATE_KEEPER_OS_SHIFT) &
                GATE_KEEPER_OS_MASK;
 
-       return tmp >> filter;
+       return (tmp >> filter) & GATE_KEEPER_FILTER_MASK;
 }
 
 /* This function is not MP safe. */
@@ -241,6 +241,13 @@ void tzc_enable_filters(const tzc_instance_t *controller)
        for (filter = 0; filter < controller->num_filters; filter++) {
                state = tzc_get_gate_keeper(controller->base, filter);
                if (state) {
+                       /* The TZC filter is already configured. Changing the
+                        * programmer's view in an active system can cause
+                        * unpredictable behavior therefore panic for now rather
+                        * than try to determine whether this is safe in this
+                        * instance. See:
+                        * http://infocenter.arm.com/help/index.jsp?\
+                        * topic=/com.arm.doc.ddi0504c/CJHHECBF.html */
                        ERROR("TZC : Filter %d Gatekeeper already enabled.\n",
                                filter);
                        panic();

diff --git a/include/common/bl_common.h b/include/common/bl_common.h
index e996fd6aa76274b4a63d111ca2bbb8a836899cca..9945e3a384a17478bad0472f855478d34962593e 100644 (file)
--- a/include/common/bl_common.h
+++ b/include/common/bl_common.h
@@ -33,6 +33,7 @@
 
 #define SECURE         0x0
 #define NON_SECURE     0x1
+#define sec_state_is_valid(s) (((s) == SECURE) || ((s) == NON_SECURE))
 
 #define UP     1
 #define DOWN   0

diff --git a/include/drivers/arm/tzc400.h b/include/drivers/arm/tzc400.h
index b4aa3ba59d8cb642cb1e3dafdfea19ecd89f1be0..03fce546b05662073318a0cac8095ccb3cbf3513 100644 (file)
--- a/include/drivers/arm/tzc400.h
+++ b/include/drivers/arm/tzc400.h
@@ -90,6 +90,7 @@
 #define GATE_KEEPER_OS_MASK    0xf
 #define GATE_KEEPER_OR_SHIFT   0
 #define GATE_KEEPER_OR_MASK    0xf
+#define GATE_KEEPER_FILTER_MASK        0x1
 
 /* Speculation is enabled by default. */
 #define SPECULATION_CTRL_WRITE_DISABLE (1 << 1)

diff --git a/plat/fvp/aarch64/fvp_common.c b/plat/fvp/aarch64/fvp_common.c
index 3fe3a218fa1b4d437680cd97ca64e8849a6dc4da..a10f4e8a7193e032e997ddcaeb9b96e3f01e82f4 100644 (file)
--- a/plat/fvp/aarch64/fvp_common.c
+++ b/plat/fvp/aarch64/fvp_common.c
@@ -237,7 +237,8 @@ uint64_t plat_get_syscnt_freq(void)
        counter_base_frequency = mmio_read_32(SYS_CNTCTL_BASE + CNTFID_OFF);
 
        /* The first entry of the frequency modes table must not be 0 */
-       assert(counter_base_frequency != 0);
+       if (counter_base_frequency == 0)
+               panic();
 
        return counter_base_frequency;
 }

diff --git a/plat/fvp/bl31_fvp_setup.c b/plat/fvp/bl31_fvp_setup.c
index 96f4772a7f0f60048ddb9fb95e4eababc1db7a75..683097ac3da984d3fb2f1ce7365132221fae469a 100644 (file)
--- a/plat/fvp/bl31_fvp_setup.c
+++ b/plat/fvp/bl31_fvp_setup.c
@@ -92,7 +92,7 @@ entry_point_info_t *bl31_plat_get_next_image_ep_info(uint32_t type)
 {
 #if RESET_TO_BL31
 
-       assert(type <= NON_SECURE);
+       assert(sec_state_is_valid(type));
        SET_PARAM_HEAD(&next_image_ep_info,
                                PARAM_EP,
                                VERSION_1,
@@ -116,6 +116,8 @@ entry_point_info_t *bl31_plat_get_next_image_ep_info(uint32_t type)
 #else
        entry_point_info_t *next_image_info;
 
+       assert(sec_state_is_valid(type));
+
        next_image_info = (type == NON_SECURE) ?
                bl2_to_bl31_params->bl33_ep_info :
                bl2_to_bl31_params->bl32_ep_info;

diff --git a/services/spd/tspd/tspd_common.c b/services/spd/tspd/tspd_common.c
index c497670bd271df9675222c7ffa69a0ea809d6c9f..1b9609f4a49530773942bfb196e0cba6a515b943 100644 (file)
--- a/services/spd/tspd/tspd_common.c
+++ b/services/spd/tspd/tspd_common.c
@@ -91,6 +91,7 @@ uint64_t tspd_synchronous_sp_entry(tsp_context_t *tsp_ctx)
 {
        uint64_t rc;
 
+       assert(tsp_ctx != NULL);
        assert(tsp_ctx->c_rt_ctx == 0);
 
        /* Apply the Secure EL1 system register context and switch to it */
@@ -117,6 +118,7 @@ uint64_t tspd_synchronous_sp_entry(tsp_context_t *tsp_ctx)
  ******************************************************************************/
 void tspd_synchronous_sp_exit(tsp_context_t *tsp_ctx, uint64_t ret)
 {
+       assert(tsp_ctx != NULL);
        /* Save the Secure EL1 system register context */
        assert(cm_get_context(SECURE) == &tsp_ctx->cpu_ctx);
        cm_el1_sysregs_context_save(SECURE);