Validate power_state and entrypoint when executing PSCI calls

This patch allows the platform to validate the power_state and
entrypoint information from the normal world early on in PSCI
calls so that we can return the error safely. New optional
pm_ops hooks `validate_power_state` and `validate_ns_entrypoint`
are introduced to do this.

As a result of these changes, all the other pm_ops handlers except
the PSCI_ON handler are expected to be successful. Also, the PSCI
implementation will now assert if a PSCI API is invoked without the
corresponding pm_ops handler being registered by the platform.

NOTE : PLATFORM PORTS WILL BREAK ON MERGE OF THIS COMMIT. The
pm hooks have 2 additional optional callbacks and the return type
of the other hooks have changed.

Fixes ARM-Software/tf-issues#229

Change-Id: I036bc0cff2349187c7b8b687b9ee0620aa7e24dc

docs/porting-guide.md

@@ -1094,13 +1094,14 @@ the passed pointer with a pointer to BL3-1's private `plat_pm_ops` structure.
 
 A description of each member of this structure is given below. Please refer to
 the ARM FVP specific implementation of these handlers in [plat/fvp/fvp_pm.c]
-as an example. A platform port may choose not implement some of the power
+as an example. A platform port is expected to implement these handlers if the
-management operations.
+corresponding PSCI operation is to be supported and these handlers are expected
+to succeed if the return type is `void`.
 
 #### plat_pm_ops.affinst_standby()
 
 Perform the platform-specific setup to enter the standby state indicated by the
-passed argument.
+passed argument. The generic code expects the handler to succeed.
 
 #### plat_pm_ops.affinst_on()
 
@@ -1111,7 +1112,8 @@ by the `MPIDR` (first argument) and `affinity level` (third argument). The
 example, while powering on a CPU, the cluster that contains this CPU might
 already be in the ON state. The platform decides what actions must be taken to
 transition from the current state to the target state (indicated by the power
-management operation).
+management operation). The generic code expects the platform to return
+E_SUCCESS on success or E_INTERN_FAIL for any failure.
 
 #### plat_pm_ops.affinst_off()
 
@@ -1125,7 +1127,7 @@ current state. This gives the platform port an indication of the
 state transition it must make to perform the requested action. For example, if
 the calling CPU is the last powered on CPU in the cluster, after powering down
 affinity level 0 (CPU), the platform port should power down affinity level 1
-(the cluster) as well.
+(the cluster) as well. The generic code expects the handler to succeed.
 
 #### plat_pm_ops.affinst_suspend()
 
@@ -1146,7 +1148,7 @@ is that in the former case, the affinity instance is expected to re-initialize
 its state when its next powered on (see `affinst_on_finish()`). In the latter
 case, the affinity instance is expected to save enough state so that it can
 resume execution by restoring this state when its powered on (see
-`affinst_suspend_finish()`).
+`affinst_suspend_finish()`).The generic code expects the handler to succeed.
 
 #### plat_pm_ops.affinst_on_finish()
 
@@ -1157,7 +1159,8 @@ this CPU to enter the normal world and also provide secure runtime firmware
 services.
 
 The `affinity level` (first argument) and `state` (second argument) have a
-similar meaning as described in the previous operations.
+similar meaning as described in the previous operations. The generic code
+expects the handler to succeed.
 
 #### plat_pm_ops.affinst_on_suspend()
 
@@ -1169,7 +1172,24 @@ restore the saved state for this CPU to resume execution in the normal world
 and also provide secure runtime firmware services.
 
 The `affinity level` (first argument) and `state` (second argument) have a
-similar meaning as described in the previous operations.
+similar meaning as described in the previous operations. The generic code
+expects the platform to succeed.
+
+#### plat_pm_ops.validate_power_state()
+
+This function is called by the PSCI implementation during the `CPU_SUSPEND`
+call to validate the `power_state` parameter of the PSCI API. If the
+`power_state` is known to be invalid, the platform must return
+PSCI_E_INVALID_PARAMS as error, which is propagated back to the normal
+world PSCI client.
+
+#### plat_pm_ops.validate_ns_entrypoint()
+
+This function is called by the PSCI implementation during the `CPU_SUSPEND`
+and `CPU_ON` calls to validate the non-secure `entry_point` parameter passed
+by the normal world. If the `entry_point` is known to be invalid, the platform
+must return PSCI_E_INVALID_PARAMS as error, which is propagated back to the
+normal world PSCI client.
 
 BL3-1 platform initialization code must also detect the system topology and
 the state of each affinity instance in the topology. This information is

include/bl31/services/psci.h

@@ -89,12 +89,12 @@
 #define PSTATE_TYPE_STANDBY	0x0
 #define PSTATE_TYPE_POWERDOWN	0x1
 
-#define psci_get_pstate_id(pstate)	(pstate >> PSTATE_ID_SHIFT) & \
+#define psci_get_pstate_id(pstate)	((pstate >> PSTATE_ID_SHIFT) & \
-					PSTATE_ID_MASK
+					PSTATE_ID_MASK)
-#define psci_get_pstate_type(pstate)	(pstate >> PSTATE_TYPE_SHIFT) & \
+#define psci_get_pstate_type(pstate)	((pstate >> PSTATE_TYPE_SHIFT) & \
-					PSTATE_TYPE_MASK
+					PSTATE_TYPE_MASK)
-#define psci_get_pstate_afflvl(pstate)	(pstate >> PSTATE_AFF_LVL_SHIFT) & \
+#define psci_get_pstate_afflvl(pstate)	((pstate >> PSTATE_AFF_LVL_SHIFT) & \
-					PSTATE_AFF_LVL_MASK
+					PSTATE_AFF_LVL_MASK)
 
 /*******************************************************************************
  * PSCI version
@@ -161,20 +161,22 @@ typedef struct psci_cpu_data {
  * perform common low level pm functions
  ******************************************************************************/
 typedef struct plat_pm_ops {
-	int (*affinst_standby)(unsigned int power_state);
+	void (*affinst_standby)(unsigned int power_state);
 	int (*affinst_on)(unsigned long mpidr,
 			  unsigned long sec_entrypoint,
 			  unsigned int afflvl,
 			  unsigned int state);
-	int (*affinst_off)(unsigned int afflvl, unsigned int state);
+	void (*affinst_off)(unsigned int afflvl, unsigned int state);
-	int (*affinst_suspend)(unsigned long sec_entrypoint,
+	void (*affinst_suspend)(unsigned long sec_entrypoint,
 			       unsigned int afflvl,
 			       unsigned int state);
-	int (*affinst_on_finish)(unsigned int afflvl, unsigned int state);
+	void (*affinst_on_finish)(unsigned int afflvl, unsigned int state);
-	int (*affinst_suspend_finish)(unsigned int afflvl,
+	void (*affinst_suspend_finish)(unsigned int afflvl,
 				      unsigned int state);
 	void (*system_off)(void) __dead2;
 	void (*system_reset)(void) __dead2;
+	int (*validate_power_state)(unsigned int power_state);
+	int (*validate_ns_entrypoint)(unsigned long ns_entrypoint);
 } plat_pm_ops_t;
 
 /*******************************************************************************

plat/fvp/fvp_pm.c

@@ -119,28 +119,14 @@ static int32_t fvp_do_plat_actions(unsigned int afflvl, unsigned int state)
 /*******************************************************************************
  * FVP handler called when an affinity instance is about to enter standby.
  ******************************************************************************/
-int fvp_affinst_standby(unsigned int power_state)
+void fvp_affinst_standby(unsigned int power_state)
 {
-	unsigned int target_afflvl;
-
-	/* Sanity check the requested state */
-	target_afflvl = psci_get_pstate_afflvl(power_state);
-
-	/*
-	 * It's possible to enter standby only on affinity level 0 i.e. a cpu
-	 * on the FVP. Ignore any other affinity level.
-	 */
-	if (target_afflvl != MPIDR_AFFLVL0)
-		return PSCI_E_INVALID_PARAMS;
-
 	/*
 	 * Enter standby state
 	 * dsb is good practice before using wfi to enter low power states
 	 */
 	dsb();
 	wfi();
-
-	return PSCI_E_SUCCESS;
 }
 
 /*******************************************************************************
@@ -190,12 +176,12 @@ int fvp_affinst_on(unsigned long mpidr,
  * global variables across calls. It will be wise to do flush a write to the
  * global to prevent unpredictable results.
  ******************************************************************************/
-int fvp_affinst_off(unsigned int afflvl,
+void fvp_affinst_off(unsigned int afflvl,
 		    unsigned int state)
 {
 	/* Determine if any platform actions need to be executed */
 	if (fvp_do_plat_actions(afflvl, state) == -EAGAIN)
-		return PSCI_E_SUCCESS;
+		return;
 
 	/*
 	 * If execution reaches this stage then this affinity level will be
@@ -207,7 +193,6 @@ int fvp_affinst_off(unsigned int afflvl,
 	if (afflvl != MPIDR_AFFLVL0)
 		fvp_cluster_pwrdwn_common();
 
-	return PSCI_E_SUCCESS;
 }
 
 /*******************************************************************************
@@ -221,7 +206,7 @@ int fvp_affinst_off(unsigned int afflvl,
  * global variables across calls. It will be wise to do flush a write to the
  * global to prevent unpredictable results.
  ******************************************************************************/
-int fvp_affinst_suspend(unsigned long sec_entrypoint,
+void fvp_affinst_suspend(unsigned long sec_entrypoint,
 			unsigned int afflvl,
 			unsigned int state)
 {
@@ -229,7 +214,7 @@ int fvp_affinst_suspend(unsigned long sec_entrypoint,
 
 	/* Determine if any platform actions need to be executed. */
 	if (fvp_do_plat_actions(afflvl, state) == -EAGAIN)
-		return PSCI_E_SUCCESS;
+		return;
 
 	/* Get the mpidr for this cpu */
 	mpidr = read_mpidr_el1();
@@ -246,8 +231,6 @@ int fvp_affinst_suspend(unsigned long sec_entrypoint,
 	/* Perform the common cluster specific operations */
 	if (afflvl != MPIDR_AFFLVL0)
 		fvp_cluster_pwrdwn_common();
-
-	return PSCI_E_SUCCESS;
 }
 
 /*******************************************************************************
@@ -257,15 +240,14 @@ int fvp_affinst_suspend(unsigned long sec_entrypoint,
  * was turned off prior to wakeup and do what's necessary to setup it up
  * correctly.
  ******************************************************************************/
-int fvp_affinst_on_finish(unsigned int afflvl,
+void fvp_affinst_on_finish(unsigned int afflvl,
 			  unsigned int state)
 {
-	int rc = PSCI_E_SUCCESS;
 	unsigned long mpidr;
 
 	/* Determine if any platform actions need to be executed. */
 	if (fvp_do_plat_actions(afflvl, state) == -EAGAIN)
-		return PSCI_E_SUCCESS;
+		return;
 
 	/* Get the mpidr for this cpu */
 	mpidr = read_mpidr_el1();
@@ -301,8 +283,6 @@ int fvp_affinst_on_finish(unsigned int afflvl,
 
 	/* TODO: This setup is needed only after a cold boot */
 	arm_gic_pcpu_distif_setup();
-
-	return rc;
 }
 
 /*******************************************************************************
@@ -312,10 +292,10 @@ int fvp_affinst_on_finish(unsigned int afflvl,
  * TODO: At the moment we reuse the on finisher and reinitialize the secure
  * context. Need to implement a separate suspend finisher.
  ******************************************************************************/
-int fvp_affinst_suspend_finish(unsigned int afflvl,
+void fvp_affinst_suspend_finish(unsigned int afflvl,
 			       unsigned int state)
 {
-	return fvp_affinst_on_finish(afflvl, state);
+	fvp_affinst_on_finish(afflvl, state);
 }
 
 /*******************************************************************************
@@ -341,6 +321,30 @@ static void __dead2 fvp_system_reset(void)
 	panic();
 }
 
+/*******************************************************************************
+ * FVP handler called to check the validity of the power state parameter.
+ ******************************************************************************/
+int fvp_validate_power_state(unsigned int power_state)
+{
+	/* Sanity check the requested state */
+	if (psci_get_pstate_type(power_state) == PSTATE_TYPE_STANDBY) {
+		/*
+		 * It's possible to enter standby only on affinity level 0
+		 * i.e. a cpu on the fvp. Ignore any other affinity level.
+		 */
+		if (psci_get_pstate_afflvl(power_state) != MPIDR_AFFLVL0)
+			return PSCI_E_INVALID_PARAMS;
+	}
+
+	/*
+	 * We expect the 'state id' to be zero.
+	 */
+	if (psci_get_pstate_id(power_state))
+		return PSCI_E_INVALID_PARAMS;
+
+	return PSCI_E_SUCCESS;
+}
+
 /*******************************************************************************
  * Export the platform handlers to enable psci to invoke them
  ******************************************************************************/
@@ -352,7 +356,8 @@ static const plat_pm_ops_t fvp_plat_pm_ops = {
 	.affinst_on_finish = fvp_affinst_on_finish,
 	.affinst_suspend_finish = fvp_affinst_suspend_finish,
 	.system_off = fvp_system_off,
-	.system_reset = fvp_system_reset
+	.system_reset = fvp_system_reset,
+	.validate_power_state = fvp_validate_power_state
 };
 
 /*******************************************************************************

plat/juno/plat_pm.c

@@ -84,6 +84,31 @@ static int32_t juno_do_plat_actions(uint32_t afflvl, uint32_t state)
 	return 0;
 }
 
+/*******************************************************************************
+ * Juno handler called to check the validity of the power state parameter.
+ ******************************************************************************/
+int32_t juno_validate_power_state(unsigned int power_state)
+{
+	/* Sanity check the requested state */
+	if (psci_get_pstate_type(power_state) == PSTATE_TYPE_STANDBY) {
+		/*
+		 * It's possible to enter standby only on affinity level 0 i.e.
+		 * a cpu on the Juno. Ignore any other affinity level.
+		 */
+		if (psci_get_pstate_afflvl(power_state) != MPIDR_AFFLVL0)
+			return PSCI_E_INVALID_PARAMS;
+	}
+
+	/*
+	 * We expect the 'state id' to be zero.
+	 */
+	if (psci_get_pstate_id(power_state))
+		return PSCI_E_INVALID_PARAMS;
+
+	return PSCI_E_SUCCESS;
+}
+
+
 /*******************************************************************************
  * Juno handler called when an affinity instance is about to be turned on. The
  * level and mpidr determine the affinity instance.
@@ -118,13 +143,13 @@ int32_t juno_affinst_on(uint64_t mpidr,
  * was turned off prior to wakeup and do what's necessary to setup it up
  * correctly.
  ******************************************************************************/
-int32_t juno_affinst_on_finish(uint32_t afflvl, uint32_t state)
+void juno_affinst_on_finish(uint32_t afflvl, uint32_t state)
 {
 	unsigned long mpidr;
 
 	/* Determine if any platform actions need to be executed. */
 	if (juno_do_plat_actions(afflvl, state) == -EAGAIN)
-		return PSCI_E_SUCCESS;
+		return;
 
 	/* Get the mpidr for this cpu */
 	mpidr = read_mpidr_el1();
@@ -145,8 +170,6 @@ int32_t juno_affinst_on_finish(uint32_t afflvl, uint32_t state)
 
 	/* Clear the mailbox for this cpu. */
 	juno_program_mailbox(mpidr, 0);
-
-	return PSCI_E_SUCCESS;
 }
 
 /*******************************************************************************
@@ -155,7 +178,7 @@ int32_t juno_affinst_on_finish(uint32_t afflvl, uint32_t state)
  * the highest affinity level which will be powered down. It performs the
  * actions common to the OFF and SUSPEND calls.
  ******************************************************************************/
-static int32_t juno_power_down_common(uint32_t afflvl)
+static void juno_power_down_common(uint32_t afflvl)
 {
 	uint32_t cluster_state = scpi_power_on;
 
@@ -176,8 +199,6 @@ static int32_t juno_power_down_common(uint32_t afflvl)
 				 scpi_power_off,
 				 cluster_state,
 				 scpi_power_on);
-
-	return PSCI_E_SUCCESS;
 }
 
 /*******************************************************************************
@@ -191,13 +212,13 @@ static int32_t juno_power_down_common(uint32_t afflvl)
  * global variables across calls. It will be wise to do flush a write to the
  * global to prevent unpredictable results.
  ******************************************************************************/
-static int32_t juno_affinst_off(uint32_t afflvl, uint32_t state)
+static void juno_affinst_off(uint32_t afflvl, uint32_t state)
 {
 	/* Determine if any platform actions need to be executed */
 	if (juno_do_plat_actions(afflvl, state) == -EAGAIN)
-		return PSCI_E_SUCCESS;
+		return;
 
-	return juno_power_down_common(afflvl);
+	juno_power_down_common(afflvl);
 }
 
 /*******************************************************************************
@@ -212,20 +233,20 @@ static int32_t juno_affinst_off(uint32_t afflvl, uint32_t state)
  * global variables across calls. It will be wise to do flush a write to the
  * global to prevent unpredictable results.
  ******************************************************************************/
-static int32_t juno_affinst_suspend(uint64_t sec_entrypoint,
+static void juno_affinst_suspend(uint64_t sec_entrypoint,
 				    uint32_t afflvl,
 				    uint32_t state)
 {
 	/* Determine if any platform actions need to be executed */
 	if (juno_do_plat_actions(afflvl, state) == -EAGAIN)
-		return PSCI_E_SUCCESS;
+		return;
 
 	/*
 	 * Setup mailbox with address for CPU entrypoint when it next powers up.
 	 */
 	juno_program_mailbox(read_mpidr_el1(), sec_entrypoint);
 
-	return juno_power_down_common(afflvl);
+	juno_power_down_common(afflvl);
 }
 
 /*******************************************************************************
@@ -235,10 +256,10 @@ static int32_t juno_affinst_suspend(uint64_t sec_entrypoint,
  * TODO: At the moment we reuse the on finisher and reinitialize the secure
  * context. Need to implement a separate suspend finisher.
  ******************************************************************************/
-static int32_t juno_affinst_suspend_finish(uint32_t afflvl,
+static void juno_affinst_suspend_finish(uint32_t afflvl,
 					   uint32_t state)
 {
-	return juno_affinst_on_finish(afflvl, state);
+	juno_affinst_on_finish(afflvl, state);
 }
 
 /*******************************************************************************
@@ -279,21 +300,10 @@ static void __dead2 juno_system_reset(void)
 /*******************************************************************************
  * Handler called when an affinity instance is about to enter standby.
  ******************************************************************************/
-int32_t juno_affinst_standby(unsigned int power_state)
+void juno_affinst_standby(unsigned int power_state)
 {
-	unsigned int target_afflvl;
 	unsigned int scr;
 
-	/* Sanity check the requested state */
-	target_afflvl = psci_get_pstate_afflvl(power_state);
-
-	/*
-	 * It's possible to enter standby only on affinity level 0 i.e. a cpu
-	 * on the Juno. Ignore any other affinity level.
-	 */
-	if (target_afflvl != MPIDR_AFFLVL0)
-		return PSCI_E_INVALID_PARAMS;
-
 	scr = read_scr_el3();
 	/* Enable PhysicalIRQ bit for NS world to wake the CPU */
 	write_scr_el3(scr | SCR_IRQ_BIT);
@@ -306,8 +316,6 @@ int32_t juno_affinst_standby(unsigned int power_state)
 	 * done by eret while el3_exit to save some execution cycles.
 	 */
 	write_scr_el3(scr);
-
-	return PSCI_E_SUCCESS;
 }
 
 /*******************************************************************************
@@ -321,7 +329,8 @@ static const plat_pm_ops_t juno_ops = {
 	.affinst_suspend	= juno_affinst_suspend,
 	.affinst_suspend_finish	= juno_affinst_suspend_finish,
 	.system_off		= juno_system_off,
-	.system_reset		= juno_system_reset
+	.system_reset		= juno_system_reset,
+	.validate_power_state	= juno_validate_power_state
 };
 
 /*******************************************************************************

services/std_svc/psci/psci_afflvl_off.c

@@ -31,55 +31,37 @@
 #include <arch.h>
 #include <arch_helpers.h>
 #include <assert.h>
+#include <debug.h>
 #include <string.h>
 #include "psci_private.h"
 
-typedef int (*afflvl_off_handler_t)(aff_map_node_t *node);
+typedef void (*afflvl_off_handler_t)(aff_map_node_t *node);
 
 /*******************************************************************************
  * The next three functions implement a handler for each supported affinity
  * level which is called when that affinity level is turned off.
  ******************************************************************************/
-static int psci_afflvl0_off(aff_map_node_t *cpu_node)
+static void psci_afflvl0_off(aff_map_node_t *cpu_node)
 {
-	int rc;
-
 	assert(cpu_node->level == MPIDR_AFFLVL0);
 
-	/*
-	 * Generic management: Get the index for clearing any lingering re-entry
-	 * information and allow the secure world to switch itself off
-	 */
-
-	/*
-	 * Call the cpu off handler registered by the Secure Payload Dispatcher
-	 * to let it do any bookeeping. Assume that the SPD always reports an
-	 * E_DENIED error if SP refuse to power down
-	 */
-	if (psci_spd_pm && psci_spd_pm->svc_off) {
-		rc = psci_spd_pm->svc_off(0);
-		if (rc)
-			return rc;
-	}
-
 	/*
 	 * Arch. management. Perform the necessary steps to flush all
 	 * cpu caches.
 	 */
 	psci_do_pwrdown_cache_maintenance(MPIDR_AFFLVL0);
 
-	if (!psci_plat_pm_ops->affinst_off)
+	assert(psci_plat_pm_ops->affinst_off);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. management: Perform platform specific actions to turn this
 	 * cpu off e.g. exit cpu coherency, program the power controller etc.
 	 */
-	return psci_plat_pm_ops->affinst_off(cpu_node->level,
+	psci_plat_pm_ops->affinst_off(cpu_node->level,
 				     psci_get_phys_state(cpu_node));
 }
 
-static int psci_afflvl1_off(aff_map_node_t *cluster_node)
+static void psci_afflvl1_off(aff_map_node_t *cluster_node)
 {
 	/* Sanity check the cluster level */
 	assert(cluster_node->level == MPIDR_AFFLVL1);
@@ -90,19 +72,18 @@ static int psci_afflvl1_off(aff_map_node_t *cluster_node)
 	 */
 	psci_do_pwrdown_cache_maintenance(MPIDR_AFFLVL1);
 
-	if (!psci_plat_pm_ops->affinst_off)
+	assert(psci_plat_pm_ops->affinst_off);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. Management. Allow the platform to do its cluster
 	 * specific bookeeping e.g. turn off interconnect coherency,
 	 * program the power controller etc.
 	 */
-	return psci_plat_pm_ops->affinst_off(cluster_node->level,
+	psci_plat_pm_ops->affinst_off(cluster_node->level,
 					     psci_get_phys_state(cluster_node));
 }
 
-static int psci_afflvl2_off(aff_map_node_t *system_node)
+static void psci_afflvl2_off(aff_map_node_t *system_node)
 {
 	/* Cannot go beyond this level */
 	assert(system_node->level == MPIDR_AFFLVL2);
@@ -118,14 +99,13 @@ static int psci_afflvl2_off(aff_map_node_t *system_node)
 	 */
 	psci_do_pwrdown_cache_maintenance(MPIDR_AFFLVL2);
 
-	if (!psci_plat_pm_ops->affinst_off)
+	assert(psci_plat_pm_ops->affinst_off);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. Management : Allow the platform to do its bookeeping
 	 * at this affinity level
 	 */
-	return psci_plat_pm_ops->affinst_off(system_node->level,
+	psci_plat_pm_ops->affinst_off(system_node->level,
 					     psci_get_phys_state(system_node));
 }
 
@@ -140,11 +120,11 @@ static const afflvl_off_handler_t psci_afflvl_off_handlers[] = {
  * topology tree and calls the off handler for the corresponding affinity
  * levels
  ******************************************************************************/
-static int psci_call_off_handlers(aff_map_node_t *mpidr_nodes[],
+static void psci_call_off_handlers(aff_map_node_t *mpidr_nodes[],
 				  int start_afflvl,
 				  int end_afflvl)
 {
-	int rc = PSCI_E_INVALID_PARAMS, level;
+	int level;
 	aff_map_node_t *node;
 
 	for (level = start_afflvl; level <= end_afflvl; level++) {
@@ -152,17 +132,8 @@ static int psci_call_off_handlers(aff_map_node_t *mpidr_nodes[],
 		if (node == NULL)
 			continue;
 
-		/*
+		psci_afflvl_off_handlers[level](node);
-		 * TODO: In case of an error should there be a way
-		 * of restoring what we might have torn down at
-		 * lower affinity levels.
-		 */
-		rc = psci_afflvl_off_handlers[level](node);
-		if (rc != PSCI_E_SUCCESS)
-			break;
 	}
-
-	return rc;
 }
 
 /*******************************************************************************
@@ -187,7 +158,7 @@ static int psci_call_off_handlers(aff_map_node_t *mpidr_nodes[],
 int psci_afflvl_off(int start_afflvl,
 		    int end_afflvl)
 {
-	int rc = PSCI_E_SUCCESS;
+	int rc;
 	mpidr_aff_map_nodes_t mpidr_nodes;
 	unsigned int max_phys_off_afflvl;
 
@@ -195,14 +166,14 @@ int psci_afflvl_off(int start_afflvl,
 	 * Collect the pointers to the nodes in the topology tree for
 	 * each affinity instance in the mpidr. If this function does
 	 * not return successfully then either the mpidr or the affinity
-	 * levels are incorrect. In either case, we cannot return back
+	 * levels are incorrect. Either way, this an internal TF error
-	 * to the caller as it would not know what to do.
+	 * therefore assert.
 	 */
 	rc = psci_get_aff_map_nodes(read_mpidr_el1() & MPIDR_AFFINITY_MASK,
 				    start_afflvl,
 				    end_afflvl,
 				    mpidr_nodes);
-	assert (rc == PSCI_E_SUCCESS);
+	assert(rc == PSCI_E_SUCCESS);
 
 	/*
 	 * This function acquires the lock corresponding to each affinity
@@ -213,6 +184,18 @@ int psci_afflvl_off(int start_afflvl,
 				  end_afflvl,
 				  mpidr_nodes);
 
+
+	/*
+	 * Call the cpu off handler registered by the Secure Payload Dispatcher
+	 * to let it do any bookkeeping. Assume that the SPD always reports an
+	 * E_DENIED error if SP refuse to power down
+	 */
+	if (psci_spd_pm && psci_spd_pm->svc_off) {
+		rc = psci_spd_pm->svc_off(0);
+		if (rc)
+			goto exit;
+	}
+
 	/*
 	 * This function updates the state of each affinity instance
 	 * corresponding to the mpidr in the range of affinity levels
@@ -232,7 +215,7 @@ int psci_afflvl_off(int start_afflvl,
 	psci_set_max_phys_off_afflvl(max_phys_off_afflvl);
 
 	/* Perform generic, architecture and platform specific handling */
-	rc = psci_call_off_handlers(mpidr_nodes,
+	psci_call_off_handlers(mpidr_nodes,
 				    start_afflvl,
 				    end_afflvl);
 
@@ -244,6 +227,7 @@ int psci_afflvl_off(int start_afflvl,
 	 */
 	psci_set_max_phys_off_afflvl(PSCI_INVALID_DATA);
 
+exit:
 	/*
 	 * Release the locks corresponding to each affinity level in the
 	 * reverse order to which they were acquired.

services/std_svc/psci/psci_afflvl_on.c

@@ -33,6 +33,7 @@
 #include <assert.h>
 #include <bl_common.h>
 #include <bl31.h>
+#include <debug.h>
 #include <context_mgmt.h>
 #include <platform.h>
 #include <runtime_svc.h>
@@ -71,19 +72,10 @@ static int psci_afflvl0_on(unsigned long target_cpu,
 	/* Sanity check to safeguard against data corruption */
 	assert(cpu_node->level == MPIDR_AFFLVL0);
 
-	/*
-	 * Call the cpu on handler registered by the Secure Payload Dispatcher
-	 * to let it do any bookeeping. If the handler encounters an error, it's
-	 * expected to assert within
-	 */
-	if (psci_spd_pm && psci_spd_pm->svc_on)
-		psci_spd_pm->svc_on(target_cpu);
-
 	/* Set the secure world (EL3) re-entry point after BL1 */
 	psci_entrypoint = (unsigned long) psci_aff_on_finish_entry;
 
-	if (!psci_plat_pm_ops->affinst_on)
+	assert(psci_plat_pm_ops->affinst_on);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. management: Give the platform the current state
@@ -115,8 +107,7 @@ static int psci_afflvl1_on(unsigned long target_cpu,
 
 	/* State management: Is not required while turning a cluster on */
 
-	if (!psci_plat_pm_ops->affinst_on)
+	assert(psci_plat_pm_ops->affinst_on);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. management: Give the platform the current state
@@ -150,8 +141,7 @@ static int psci_afflvl2_on(unsigned long target_cpu,
 
 	/* State management: Is not required while turning a system on */
 
-	if (!psci_plat_pm_ops->affinst_on)
+	assert(psci_plat_pm_ops->affinst_on);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. management: Give the platform the current state
@@ -225,7 +215,7 @@ int psci_afflvl_on(unsigned long target_cpu,
 		   int start_afflvl,
 		   int end_afflvl)
 {
-	int rc = PSCI_E_SUCCESS;
+	int rc;
 	mpidr_aff_map_nodes_t target_cpu_nodes;
 
 	/*
@@ -238,9 +228,7 @@ int psci_afflvl_on(unsigned long target_cpu,
 				    start_afflvl,
 				    end_afflvl,
 				    target_cpu_nodes);
-	if (rc != PSCI_E_SUCCESS)
+	assert(rc == PSCI_E_SUCCESS);
-		return rc;
-
 
 	/*
 	 * This function acquires the lock corresponding to each affinity
@@ -256,16 +244,26 @@ int psci_afflvl_on(unsigned long target_cpu,
 	 * turned on.
 	 */
 	rc = cpu_on_validate_state(psci_get_state(
-			(aff_map_node_t *)target_cpu_nodes[MPIDR_AFFLVL0]));
+				    target_cpu_nodes[MPIDR_AFFLVL0]));
 	if (rc != PSCI_E_SUCCESS)
 		goto exit;
 
+	/*
+	 * Call the cpu on handler registered by the Secure Payload Dispatcher
+	 * to let it do any bookeeping. If the handler encounters an error, it's
+	 * expected to assert within
+	 */
+	if (psci_spd_pm && psci_spd_pm->svc_on)
+		psci_spd_pm->svc_on(target_cpu);
+
 	/* Perform generic, architecture and platform specific handling. */
 	rc = psci_call_on_handlers(target_cpu_nodes,
 				   start_afflvl,
 				   end_afflvl,
 				   target_cpu);
 
+	assert(rc == PSCI_E_SUCCESS || rc == PSCI_E_INTERN_FAIL);
+
 	/*
 	 * This function updates the state of each affinity instance
 	 * corresponding to the mpidr in the range of affinity levels
@@ -276,6 +274,7 @@ int psci_afflvl_on(unsigned long target_cpu,
 					  end_afflvl,
 					  target_cpu_nodes,
 					  PSCI_STATE_ON_PENDING);
+
 		/*
 		 * Store the re-entry information for the non-secure world.
 		 */
@@ -298,9 +297,9 @@ exit:
  * The following functions finish an earlier affinity power on request. They
  * are called by the common finisher routine in psci_common.c.
  ******************************************************************************/
-static unsigned int psci_afflvl0_on_finish(aff_map_node_t *cpu_node)
+static void psci_afflvl0_on_finish(aff_map_node_t *cpu_node)
 {
-	unsigned int plat_state, state, rc;
+	unsigned int plat_state, state;
 
 	assert(cpu_node->level == MPIDR_AFFLVL0);
 
@@ -314,14 +313,12 @@ static unsigned int psci_afflvl0_on_finish(aff_map_node_t *cpu_node)
 	 * register. The actual state of this cpu has already been
 	 * changed.
 	 */
-	if (psci_plat_pm_ops->affinst_on_finish) {
+	assert(psci_plat_pm_ops->affinst_on_finish);
 
 	/* Get the physical state of this cpu */
 	plat_state = get_phys_state(state);
-		rc = psci_plat_pm_ops->affinst_on_finish(cpu_node->level,
+	psci_plat_pm_ops->affinst_on_finish(cpu_node->level,
 							 plat_state);
-		assert(rc == PSCI_E_SUCCESS);
-	}
 
 	/*
 	 * Arch. management: Enable data cache and manage stack memory
@@ -352,19 +349,15 @@ static unsigned int psci_afflvl0_on_finish(aff_map_node_t *cpu_node)
 
 	/* Clean caches before re-entering normal world */
 	dcsw_op_louis(DCCSW);
-
-	rc = PSCI_E_SUCCESS;
-	return rc;
 }
 
-static unsigned int psci_afflvl1_on_finish(aff_map_node_t *cluster_node)
+static void psci_afflvl1_on_finish(aff_map_node_t *cluster_node)
 {
 	unsigned int plat_state;
 
 	assert(cluster_node->level == MPIDR_AFFLVL1);
 
-	if (!psci_plat_pm_ops->affinst_on_finish)
+	assert(psci_plat_pm_ops->affinst_on_finish);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. management: Perform the platform specific actions
@@ -375,20 +368,19 @@ static unsigned int psci_afflvl1_on_finish(aff_map_node_t *cluster_node)
 	 * situation.
 	 */
 	plat_state = psci_get_phys_state(cluster_node);
-	return psci_plat_pm_ops->affinst_on_finish(cluster_node->level,
+	psci_plat_pm_ops->affinst_on_finish(cluster_node->level,
 						 plat_state);
 }
 
 
-static unsigned int psci_afflvl2_on_finish(aff_map_node_t *system_node)
+static void psci_afflvl2_on_finish(aff_map_node_t *system_node)
 {
 	unsigned int plat_state;
 
 	/* Cannot go beyond this affinity level */
 	assert(system_node->level == MPIDR_AFFLVL2);
 
-	if (!psci_plat_pm_ops->affinst_on_finish)
+	assert(psci_plat_pm_ops->affinst_on_finish);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Currently, there are no architectural actions to perform
@@ -404,7 +396,7 @@ static unsigned int psci_afflvl2_on_finish(aff_map_node_t *system_node)
 	 * situation.
 	 */
 	plat_state = psci_get_phys_state(system_node);
-	return psci_plat_pm_ops->affinst_on_finish(system_node->level,
+	psci_plat_pm_ops->affinst_on_finish(system_node->level,
 						   plat_state);
 }
 

services/std_svc/psci/psci_afflvl_suspend.c

@@ -35,12 +35,13 @@
 #include <context.h>
 #include <context_mgmt.h>
 #include <cpu_data.h>
+#include <debug.h>
 #include <platform.h>
 #include <runtime_svc.h>
 #include <stddef.h>
 #include "psci_private.h"
 
-typedef int (*afflvl_suspend_handler_t)(aff_map_node_t *node);
+typedef void (*afflvl_suspend_handler_t)(aff_map_node_t *node);
 
 /*******************************************************************************
  * This function saves the power state parameter passed in the current PSCI
@@ -102,25 +103,13 @@ int psci_get_suspend_stateid_by_mpidr(unsigned long mpidr)
  * The next three functions implement a handler for each supported affinity
  * level which is called when that affinity level is about to be suspended.
  ******************************************************************************/
-static int psci_afflvl0_suspend(aff_map_node_t *cpu_node)
+static void psci_afflvl0_suspend(aff_map_node_t *cpu_node)
 {
 	unsigned long psci_entrypoint;
 
 	/* Sanity check to safeguard against data corruption */
 	assert(cpu_node->level == MPIDR_AFFLVL0);
 
-	/*
-	 * Generic management: Allow the Secure world to suspend itself
-	 */
-
-	/*
-	 * Call the cpu suspend handler registered by the Secure Payload
-	 * Dispatcher to let it do any bookeeping. If the handler encounters an
-	 * error, it's expected to assert within
-	 */
-	if (psci_spd_pm && psci_spd_pm->svc_suspend)
-		psci_spd_pm->svc_suspend(0);
-
 	/* Set the secure world (EL3) re-entry point after BL1 */
 	psci_entrypoint = (unsigned long) psci_aff_suspend_finish_entry;
 
@@ -130,8 +119,7 @@ static int psci_afflvl0_suspend(aff_map_node_t *cpu_node)
 	 */
 	psci_do_pwrdown_cache_maintenance(MPIDR_AFFLVL0);
 
-	if (!psci_plat_pm_ops->affinst_suspend)
+	assert(psci_plat_pm_ops->affinst_suspend);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. management: Allow the platform to perform the
@@ -139,12 +127,12 @@ static int psci_afflvl0_suspend(aff_map_node_t *cpu_node)
 	 * platform defined mailbox with the psci entrypoint,
 	 * program the power controller etc.
 	 */
-	return psci_plat_pm_ops->affinst_suspend(psci_entrypoint,
+	psci_plat_pm_ops->affinst_suspend(psci_entrypoint,
 						 cpu_node->level,
 						 psci_get_phys_state(cpu_node));
 }
 
-static int psci_afflvl1_suspend(aff_map_node_t *cluster_node)
+static void psci_afflvl1_suspend(aff_map_node_t *cluster_node)
 {
 	unsigned int plat_state;
 	unsigned long psci_entrypoint;
@@ -158,8 +146,7 @@ static int psci_afflvl1_suspend(aff_map_node_t *cluster_node)
 	 */
 	psci_do_pwrdown_cache_maintenance(MPIDR_AFFLVL1);
 
-	if (!psci_plat_pm_ops->affinst_suspend)
+	assert(psci_plat_pm_ops->affinst_suspend);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Plat. Management. Allow the platform to do its cluster specific
@@ -171,13 +158,13 @@ static int psci_afflvl1_suspend(aff_map_node_t *cluster_node)
 	 */
 	plat_state = psci_get_phys_state(cluster_node);
 	psci_entrypoint = (unsigned long) psci_aff_suspend_finish_entry;
-	return psci_plat_pm_ops->affinst_suspend(psci_entrypoint,
+	psci_plat_pm_ops->affinst_suspend(psci_entrypoint,
 						 cluster_node->level,
 						 plat_state);
 }
 
 
-static int psci_afflvl2_suspend(aff_map_node_t *system_node)
+static void psci_afflvl2_suspend(aff_map_node_t *system_node)
 {
 	unsigned int plat_state;
 	unsigned long psci_entrypoint;
@@ -201,8 +188,7 @@ static int psci_afflvl2_suspend(aff_map_node_t *system_node)
 	 * Plat. Management : Allow the platform to do its bookeeping
 	 * at this affinity level
 	 */
-	if (!psci_plat_pm_ops->affinst_suspend)
+	assert(psci_plat_pm_ops->affinst_suspend);
-		return PSCI_E_SUCCESS;
 
 	/*
 	 * Sending the psci entrypoint is currently redundant
@@ -212,7 +198,7 @@ static int psci_afflvl2_suspend(aff_map_node_t *system_node)
 	 */
 	plat_state = psci_get_phys_state(system_node);
 	psci_entrypoint = (unsigned long) psci_aff_suspend_finish_entry;
-	return psci_plat_pm_ops->affinst_suspend(psci_entrypoint,
+	psci_plat_pm_ops->affinst_suspend(psci_entrypoint,
 						 system_node->level,
 						 plat_state);
 }
@@ -228,11 +214,11 @@ static const afflvl_suspend_handler_t psci_afflvl_suspend_handlers[] = {
  * topology tree and calls the suspend handler for the corresponding affinity
  * levels
  ******************************************************************************/
-static int psci_call_suspend_handlers(aff_map_node_t *mpidr_nodes[],
+static void psci_call_suspend_handlers(aff_map_node_t *mpidr_nodes[],
 				      int start_afflvl,
 				      int end_afflvl)
 {
-	int rc = PSCI_E_INVALID_PARAMS, level;
+	int level;
 	aff_map_node_t *node;
 
 	for (level = start_afflvl; level <= end_afflvl; level++) {
@@ -240,17 +226,8 @@ static int psci_call_suspend_handlers(aff_map_node_t *mpidr_nodes[],
 		if (node == NULL)
 			continue;
 
-		/*
+		psci_afflvl_suspend_handlers[level](node);
-		 * TODO: In case of an error should there be a way
-		 * of restoring what we might have torn down at
-		 * lower affinity levels.
-		 */
-		rc = psci_afflvl_suspend_handlers[level](node);
-		if (rc != PSCI_E_SUCCESS)
-			break;
 	}
-
-	return rc;
 }
 
 /*******************************************************************************
@@ -271,12 +248,15 @@ static int psci_call_suspend_handlers(aff_map_node_t *mpidr_nodes[],
  * the lowest to the highest affinity level implemented by the platform because
  * to turn off affinity level X it is neccesary to turn off affinity level X - 1
  * first.
+ *
+ * All the required parameter checks are performed at the beginning and after
+ * the state transition has been done, no further error is expected and it
+ * is not possible to undo any of the actions taken beyond that point.
  ******************************************************************************/
-int psci_afflvl_suspend(entry_point_info_t *ep,
+void psci_afflvl_suspend(entry_point_info_t *ep,
 			int start_afflvl,
 			int end_afflvl)
 {
-	int rc = PSCI_E_SUCCESS;
 	mpidr_aff_map_nodes_t mpidr_nodes;
 	unsigned int max_phys_off_afflvl;
 
@@ -284,14 +264,12 @@ int psci_afflvl_suspend(entry_point_info_t *ep,
 	 * Collect the pointers to the nodes in the topology tree for
 	 * each affinity instance in the mpidr. If this function does
 	 * not return successfully then either the mpidr or the affinity
-	 * levels are incorrect.
+	 * levels are incorrect. Either way, this an internal TF error
+	 * therefore assert.
 	 */
-	rc = psci_get_aff_map_nodes(read_mpidr_el1() & MPIDR_AFFINITY_MASK,
+	if (psci_get_aff_map_nodes(read_mpidr_el1() & MPIDR_AFFINITY_MASK,
-				    start_afflvl,
+		   start_afflvl, end_afflvl, mpidr_nodes) != PSCI_E_SUCCESS)
-				    end_afflvl,
+		assert(0);
-				    mpidr_nodes);
-	if (rc != PSCI_E_SUCCESS)
-		return rc;
 
 	/*
 	 * This function acquires the lock corresponding to each affinity
@@ -302,6 +280,14 @@ int psci_afflvl_suspend(entry_point_info_t *ep,
 				  end_afflvl,
 				  mpidr_nodes);
 
+	/*
+	 * Call the cpu suspend handler registered by the Secure Payload
+	 * Dispatcher to let it do any bookeeping. If the handler encounters an
+	 * error, it's expected to assert within
+	 */
+	if (psci_spd_pm && psci_spd_pm->svc_suspend)
+		psci_spd_pm->svc_suspend(0);
+
 	/*
 	 * This function updates the state of each affinity instance
 	 * corresponding to the mpidr in the range of affinity levels
@@ -326,7 +312,7 @@ int psci_afflvl_suspend(entry_point_info_t *ep,
 	cm_init_context(read_mpidr_el1(), ep);
 
 	/* Perform generic, architecture and platform specific handling */
-	rc = psci_call_suspend_handlers(mpidr_nodes,
+	psci_call_suspend_handlers(mpidr_nodes,
 					start_afflvl,
 					end_afflvl);
 
@@ -344,17 +330,15 @@ int psci_afflvl_suspend(entry_point_info_t *ep,
 	psci_release_afflvl_locks(start_afflvl,
 				  end_afflvl,
 				  mpidr_nodes);
-
-	return rc;
 }
 
 /*******************************************************************************
  * The following functions finish an earlier affinity suspend request. They
  * are called by the common finisher routine in psci_common.c.
  ******************************************************************************/
-static unsigned int psci_afflvl0_suspend_finish(aff_map_node_t *cpu_node)
+static void psci_afflvl0_suspend_finish(aff_map_node_t *cpu_node)
 {
-	unsigned int plat_state, state, rc;
+	unsigned int plat_state, state;
 	int32_t suspend_level;
 	uint64_t counter_freq;
 
@@ -371,16 +355,14 @@ static unsigned int psci_afflvl0_suspend_finish(aff_map_node_t *cpu_node)
 	 * wrong then assert as there is no way to recover from this
 	 * situation.
 	 */
-	if (psci_plat_pm_ops->affinst_suspend_finish) {
+
+	assert(psci_plat_pm_ops->affinst_suspend_finish);
 
 	/* Get the physical state of this cpu */
 	plat_state = get_phys_state(state);
-		rc = psci_plat_pm_ops->affinst_suspend_finish(cpu_node->level,
+	psci_plat_pm_ops->affinst_suspend_finish(cpu_node->level,
 							      plat_state);
-		assert(rc == PSCI_E_SUCCESS);
-	}
 
-	/* Get the index for restoring the re-entry information */
 	/*
 	 * Arch. management: Enable the data cache, manage stack memory and
 	 * restore the stashed EL3 architectural context from the 'cpu_context'
@@ -415,14 +397,11 @@ static unsigned int psci_afflvl0_suspend_finish(aff_map_node_t *cpu_node)
 
 	/* Clean caches before re-entering normal world */
 	dcsw_op_louis(DCCSW);
-
-	rc = PSCI_E_SUCCESS;
-	return rc;
 }
 
-static unsigned int psci_afflvl1_suspend_finish(aff_map_node_t *cluster_node)
+static void psci_afflvl1_suspend_finish(aff_map_node_t *cluster_node)
 {
-	unsigned int plat_state, rc = PSCI_E_SUCCESS;
+	unsigned int plat_state;
 
 	assert(cluster_node->level == MPIDR_AFFLVL1);
 
@@ -434,22 +413,19 @@ static unsigned int psci_afflvl1_suspend_finish(aff_map_node_t *cluster_node)
 	 * then assert as there is no way to recover from this
 	 * situation.
 	 */
-	if (psci_plat_pm_ops->affinst_suspend_finish) {
+
+	assert(psci_plat_pm_ops->affinst_suspend_finish);
 
 	/* Get the physical state of this cpu */
 	plat_state = psci_get_phys_state(cluster_node);
-		rc = psci_plat_pm_ops->affinst_suspend_finish(cluster_node->level,
+	psci_plat_pm_ops->affinst_suspend_finish(cluster_node->level,
 						      plat_state);
-		assert(rc == PSCI_E_SUCCESS);
-	}
-
-	return rc;
 }
 
 
-static unsigned int psci_afflvl2_suspend_finish(aff_map_node_t *system_node)
+static void psci_afflvl2_suspend_finish(aff_map_node_t *system_node)
 {
-	unsigned int plat_state, rc = PSCI_E_SUCCESS;;
+	unsigned int plat_state;
 
 	/* Cannot go beyond this affinity level */
 	assert(system_node->level == MPIDR_AFFLVL2);
@@ -467,16 +443,13 @@ static unsigned int psci_afflvl2_suspend_finish(aff_map_node_t *system_node)
 	 * then assert as there is no way to recover from this
 	 * situation.
 	 */
-	if (psci_plat_pm_ops->affinst_suspend_finish) {
+
+	assert(psci_plat_pm_ops->affinst_suspend_finish);
 
 	/* Get the physical state of the system */
 	plat_state = psci_get_phys_state(system_node);
-		rc = psci_plat_pm_ops->affinst_suspend_finish(system_node->level,
+	psci_plat_pm_ops->affinst_suspend_finish(system_node->level,
 						      plat_state);
-		assert(rc == PSCI_E_SUCCESS);
-	}
-
-	return rc;
 }
 
 const afflvl_power_on_finisher_t psci_afflvl_suspend_finishers[] = {

services/std_svc/psci/psci_common.c

@@ -438,12 +438,12 @@ unsigned short psci_get_phys_state(aff_map_node_t *node)
  * topology tree and calls the physical power on handler for the corresponding
  * affinity levels
  ******************************************************************************/
-static int psci_call_power_on_handlers(aff_map_node_t *mpidr_nodes[],
+static void psci_call_power_on_handlers(aff_map_node_t *mpidr_nodes[],
 				       int start_afflvl,
 				       int end_afflvl,
 				       afflvl_power_on_finisher_t *pon_handlers)
 {
-	int rc = PSCI_E_INVALID_PARAMS, level;
+	int level;
 	aff_map_node_t *node;
 
 	for (level = end_afflvl; level >= start_afflvl; level--) {
@@ -457,12 +457,8 @@ static int psci_call_power_on_handlers(aff_map_node_t *mpidr_nodes[],
 		 * so simply return an error and let the caller take
 		 * care of the situation.
 		 */
-		rc = pon_handlers[level](node);
+		pon_handlers[level](node);
-		if (rc != PSCI_E_SUCCESS)
-			break;
 	}
-
-	return rc;
 }
 
 /*******************************************************************************
@@ -524,12 +520,10 @@ void psci_afflvl_power_on_finish(int start_afflvl,
 	psci_set_max_phys_off_afflvl(max_phys_off_afflvl);
 
 	/* Perform generic, architecture and platform specific handling */
-	rc = psci_call_power_on_handlers(mpidr_nodes,
+	psci_call_power_on_handlers(mpidr_nodes,
 					 start_afflvl,
 					 end_afflvl,
 					 pon_handlers);
-	if (rc != PSCI_E_SUCCESS)
-		panic();
 
 	/*
 	 * This function updates the state of each affinity instance

services/std_svc/psci/psci_main.c

@@ -50,7 +50,16 @@ int psci_cpu_on(unsigned long target_cpu,
 	/* Determine if the cpu exists of not */
 	rc = psci_validate_mpidr(target_cpu, MPIDR_AFFLVL0);
 	if (rc != PSCI_E_SUCCESS) {
-		goto exit;
+		return PSCI_E_INVALID_PARAMS;
+	}
+
+	/* Validate the entrypoint using platform pm_ops */
+	if (psci_plat_pm_ops->validate_ns_entrypoint) {
+		rc = psci_plat_pm_ops->validate_ns_entrypoint(entrypoint);
+		if (rc != PSCI_E_SUCCESS) {
+			assert(rc == PSCI_E_INVALID_PARAMS);
+			return PSCI_E_INVALID_PARAMS;
+		}
 	}
 
 	/*
@@ -74,7 +83,6 @@ int psci_cpu_on(unsigned long target_cpu,
 			    start_afflvl,
 			    end_afflvl);
 
-exit:
 	return rc;
 }
 
@@ -100,6 +108,24 @@ int psci_cpu_suspend(unsigned int power_state,
 	if (target_afflvl > get_max_afflvl())
 		return PSCI_E_INVALID_PARAMS;
 
+	/* Validate the power_state using platform pm_ops */
+	if (psci_plat_pm_ops->validate_power_state) {
+		rc = psci_plat_pm_ops->validate_power_state(power_state);
+		if (rc != PSCI_E_SUCCESS) {
+			assert(rc == PSCI_E_INVALID_PARAMS);
+			return PSCI_E_INVALID_PARAMS;
+		}
+	}
+
+	/* Validate the entrypoint using platform pm_ops */
+	if (psci_plat_pm_ops->validate_ns_entrypoint) {
+		rc = psci_plat_pm_ops->validate_ns_entrypoint(entrypoint);
+		if (rc != PSCI_E_SUCCESS) {
+			assert(rc == PSCI_E_INVALID_PARAMS);
+			return PSCI_E_INVALID_PARAMS;
+		}
+	}
+
 	/* Determine the 'state type' in the 'power_state' parameter */
 	pstate_type = psci_get_pstate_type(power_state);
 
@@ -111,9 +137,8 @@ int psci_cpu_suspend(unsigned int power_state,
 		if  (!psci_plat_pm_ops->affinst_standby)
 			return PSCI_E_INVALID_PARAMS;
 
-		rc = psci_plat_pm_ops->affinst_standby(power_state);
+		psci_plat_pm_ops->affinst_standby(power_state);
-		assert(rc == PSCI_E_INVALID_PARAMS || rc == PSCI_E_SUCCESS);
+		return PSCI_E_SUCCESS;
-		return rc;
 	}
 
 	/*
@@ -130,19 +155,17 @@ int psci_cpu_suspend(unsigned int power_state,
 
 	/*
 	 * Do what is needed to enter the power down state. Upon success,
-	 * enter the final wfi which will power down this cpu else return
+	 * enter the final wfi which will power down this CPU.
-	 * an error.
 	 */
-	rc = psci_afflvl_suspend(&ep,
+	psci_afflvl_suspend(&ep,
 			    MPIDR_AFFLVL0,
 			    target_afflvl);
-	if (rc == PSCI_E_SUCCESS)
+
 	psci_power_down_wfi();
-	assert(rc == PSCI_E_INVALID_PARAMS);
 
 	/* Reset PSCI power state parameter for the core. */
 	psci_set_suspend_power_state(PSCI_INVALID_DATA);
-	return rc;
+	return PSCI_E_SUCCESS;
 }
 
 int psci_cpu_off(void)

services/std_svc/psci/psci_private.h

@@ -75,7 +75,7 @@ typedef struct aff_limits_node {
 } aff_limits_node_t;
 
 typedef aff_map_node_t (*mpidr_aff_map_nodes_t[MPIDR_MAX_AFFLVL + 1]);
-typedef unsigned int (*afflvl_power_on_finisher_t)(aff_map_node_t *);
+typedef void (*afflvl_power_on_finisher_t)(aff_map_node_t *);
 
 /*******************************************************************************
  * Data prototypes
@@ -138,7 +138,7 @@ int psci_afflvl_on(unsigned long target_cpu,
 int psci_afflvl_off(int, int);
 
 /* Private exported functions from psci_affinity_suspend.c */
-int psci_afflvl_suspend(entry_point_info_t *ep,
+void psci_afflvl_suspend(entry_point_info_t *ep,
 			int start_afflvl,
 			int end_afflvl);