Merge pull request #177 from jcastillo-arm/jc/tf-issues/096
Rework incorrect use of assert() and panic() in codebase
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 3926239..d22fd55 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 ca72aa9..21fca70 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);