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/plat/fvp/fvp_pm.c b/plat/fvp/fvp_pm.c
index fdf3209..9044e69 100644
--- a/plat/fvp/fvp_pm.c
+++ b/plat/fvp/fvp_pm.c
@@ -119,28 +119,14 @@
/*******************************************************************************
* 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 @@
* 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 @@
if (afflvl != MPIDR_AFFLVL0)
fvp_cluster_pwrdwn_common();
- return PSCI_E_SUCCESS;
}
/*******************************************************************************
@@ -221,7 +206,7 @@
* 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 @@
/* 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 @@
/* Perform the common cluster specific operations */
if (afflvl != MPIDR_AFFLVL0)
fvp_cluster_pwrdwn_common();
-
- return PSCI_E_SUCCESS;
}
/*******************************************************************************
@@ -257,15 +240,14 @@
* 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 @@
/* TODO: This setup is needed only after a cold boot */
arm_gic_pcpu_distif_setup();
-
- return rc;
}
/*******************************************************************************
@@ -312,10 +292,10 @@
* 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);
}
/*******************************************************************************
@@ -342,6 +322,30 @@
}
/*******************************************************************************
+ * 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
******************************************************************************/
static const plat_pm_ops_t fvp_plat_pm_ops = {
@@ -352,7 +356,8 @@
.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
};
/*******************************************************************************
diff --git a/plat/juno/plat_pm.c b/plat/juno/plat_pm.c
index a390706..47338cf 100644
--- a/plat/juno/plat_pm.c
+++ b/plat/juno/plat_pm.c
@@ -85,6 +85,31 @@
}
/*******************************************************************************
+ * 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 @@
* 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 @@
/* Clear the mailbox for this cpu. */
juno_program_mailbox(mpidr, 0);
-
- return PSCI_E_SUCCESS;
}
/*******************************************************************************
@@ -155,7 +178,7 @@
* 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 @@
scpi_power_off,
cluster_state,
scpi_power_on);
-
- return PSCI_E_SUCCESS;
}
/*******************************************************************************
@@ -191,13 +212,13 @@
* 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 @@
* 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 @@
* 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 @@
/*******************************************************************************
* 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 @@
* done by eret while el3_exit to save some execution cycles.
*/
write_scr_el3(scr);
-
- return PSCI_E_SUCCESS;
}
/*******************************************************************************
@@ -321,7 +329,8 @@
.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
};
/*******************************************************************************