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 861b391..ff3c53b 100644
--- a/bl31/bl31_main.c
+++ b/bl31/bl31_main.c
@@ -125,7 +125,7 @@
******************************************************************************/
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 65f1213..4502e5d 100644
--- a/bl31/context_mgmt.c
+++ b/bl31/context_mgmt.c
@@ -71,7 +71,7 @@
******************************************************************************/
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_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 2b0c797..e595634 100644
--- a/bl31/interrupt_mgmt.c
+++ b/bl31/interrupt_mgmt.c
@@ -107,7 +107,7 @@
{
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 60b63f1..d2c60ef 100644
--- a/common/bl_common.c
+++ b/common/bl_common.c
@@ -61,12 +61,11 @@
{
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 636348b..86aaa9a 100644
--- a/drivers/arm/gic/arm_gic.c
+++ b/drivers/arm/gic/arm_gic.c
@@ -322,7 +322,7 @@
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 c1716db..715ea6c 100644
--- a/drivers/arm/tzc400/tzc400.c
+++ b/drivers/arm/tzc400/tzc400.c
@@ -103,7 +103,7 @@
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 @@
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 e996fd6..9945e3a 100644
--- 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 b4aa3ba..03fce54 100644
--- 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 3fe3a21..a10f4e8 100644
--- a/plat/fvp/aarch64/fvp_common.c
+++ b/plat/fvp/aarch64/fvp_common.c
@@ -237,7 +237,8 @@
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 96f4772..683097a 100644
--- a/plat/fvp/bl31_fvp_setup.c
+++ b/plat/fvp/bl31_fvp_setup.c
@@ -92,7 +92,7 @@
{
#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 @@
#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 c497670..1b9609f 100644
--- a/services/spd/tspd/tspd_common.c
+++ b/services/spd/tspd/tspd_common.c
@@ -91,6 +91,7 @@
{
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 @@
******************************************************************************/
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);