Merge pull request #177 from jcastillo-arm/jc/tf-issues/096

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

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;
 }
 

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);
 }

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];

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);
 }

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

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();

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

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)

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;
 }

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;

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);