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
diff --git a/services/std_svc/psci/psci_afflvl_suspend.c b/services/std_svc/psci/psci_afflvl_suspend.c
index 942e9a1..35f9e4a 100644
--- a/services/std_svc/psci/psci_afflvl_suspend.c
+++ b/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 @@
* 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 @@
*/
psci_do_pwrdown_cache_maintenance(MPIDR_AFFLVL0);
- if (!psci_plat_pm_ops->affinst_suspend)
- return PSCI_E_SUCCESS;
+ assert(psci_plat_pm_ops->affinst_suspend);
/*
* Plat. management: Allow the platform to perform the
@@ -139,12 +127,12 @@
* 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 @@
*/
psci_do_pwrdown_cache_maintenance(MPIDR_AFFLVL1);
- if (!psci_plat_pm_ops->affinst_suspend)
- return PSCI_E_SUCCESS;
+ assert(psci_plat_pm_ops->affinst_suspend);
/*
* Plat. Management. Allow the platform to do its cluster specific
@@ -171,13 +158,13 @@
*/
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 @@
* Plat. Management : Allow the platform to do its bookeeping
* at this affinity level
*/
- if (!psci_plat_pm_ops->affinst_suspend)
- return PSCI_E_SUCCESS;
+ assert(psci_plat_pm_ops->affinst_suspend);
/*
* Sending the psci entrypoint is currently redundant
@@ -212,7 +198,7 @@
*/
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 @@
* 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 @@
if (node == NULL)
continue;
- /*
- * 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;
+ psci_afflvl_suspend_handlers[level](node);
}
-
- return rc;
}
/*******************************************************************************
@@ -271,12 +248,15 @@
* 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 @@
* 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,
- start_afflvl,
- end_afflvl,
- mpidr_nodes);
- if (rc != PSCI_E_SUCCESS)
- return rc;
+ if (psci_get_aff_map_nodes(read_mpidr_el1() & MPIDR_AFFINITY_MASK,
+ start_afflvl, end_afflvl, mpidr_nodes) != PSCI_E_SUCCESS)
+ assert(0);
/*
* This function acquires the lock corresponding to each affinity
@@ -303,6 +281,14 @@
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
* specified.
@@ -326,7 +312,7 @@
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 @@
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 @@
* 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,
+ /* Get the physical state of this cpu */
+ plat_state = get_phys_state(state);
+ 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 @@
/* 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 @@
* then assert as there is no way to recover from this
* situation.
*/
- if (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,
- plat_state);
- assert(rc == PSCI_E_SUCCESS);
- }
+ assert(psci_plat_pm_ops->affinst_suspend_finish);
- return rc;
+ /* Get the physical state of this cpu */
+ plat_state = psci_get_phys_state(cluster_node);
+ psci_plat_pm_ops->affinst_suspend_finish(cluster_node->level,
+ plat_state);
}
-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 @@
* then assert as there is no way to recover from this
* situation.
*/
- if (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,
- plat_state);
- assert(rc == PSCI_E_SUCCESS);
- }
+ assert(psci_plat_pm_ops->affinst_suspend_finish);
- return rc;
+ /* Get the physical state of the system */
+ plat_state = psci_get_phys_state(system_node);
+ psci_plat_pm_ops->affinst_suspend_finish(system_node->level,
+ plat_state);
}
const afflvl_power_on_finisher_t psci_afflvl_suspend_finishers[] = {