feat(psci): check that CPUs handled a pabandon
Up to now PSCI assumed that if a pabandon happened then the CPU driver
will have handled it. This patch adds a simple protocol to make sure
that this is indeed the case. The chosen method is with a return value
that is highly unlikely on cores that are unaware of pabandon (x0 will
be primed with 1 and if used should be overwritten with the value of
CPUPWRCTLR_EL1 which should have its last bit set to power off and its
top bits RES0; the ACK value is chosen to be the exact opposite). An
alternative method would have been to add a field in cpu_ops, however
that would have required more major refactoring across many cpus and
would have taken up more memory on older platforms, so it was not
chosen.
Change-Id: I5826c0e4802e104d295c4ecbd80b5f676d2cd871
Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
diff --git a/include/lib/cpus/aarch64/cpu_macros.S b/include/lib/cpus/aarch64/cpu_macros.S
index 84107a4..331bfdb 100644
--- a/include/lib/cpus/aarch64/cpu_macros.S
+++ b/include/lib/cpus/aarch64/cpu_macros.S
@@ -653,4 +653,15 @@
#endif
.endm
+/*
+ * Call this just before a return to indicate support for pabandon. Only
+ * necessary on an abandon call, but harmless on a powerdown call.
+ *
+ * PSCI wants us to tell it we handled a pabandon by returning 0. This is the
+ * only way support for it is indicated.
+ */
+.macro signal_pabandon_handled
+ mov_imm x0, PABANDON_ACK
+.endm
+
#endif /* CPU_MACROS_S */
diff --git a/include/lib/cpus/cpu_ops.h b/include/lib/cpus/cpu_ops.h
index 5ba78cf..765cd59 100644
--- a/include/lib/cpus/cpu_ops.h
+++ b/include/lib/cpus/cpu_ops.h
@@ -21,6 +21,12 @@
/* The number of CPU operations allowed */
#define CPU_MAX_PWR_DWN_OPS 2
+/*
+ * value needs to be distinct from CPUPWRCTLR_EL1 likely values: its top bits
+ * are RES0 and its bottom bits will be written to power down. Pick the opposite
+ * with something that looks like "abandon" in the middle.
+ */
+#define PABANDON_ACK 0xffaba4d4aba4d400
/*
* Define the sizes of the fields in the cpu_ops structure. Word size is set per
@@ -104,7 +110,7 @@
void (*e_handler_func)(long es);
#endif /* __aarch64__ */
#if (defined(IMAGE_BL31) || defined(IMAGE_BL32)) && CPU_MAX_PWR_DWN_OPS
- void (*pwr_dwn_ops[CPU_MAX_PWR_DWN_OPS])(void);
+ u_register_t (*pwr_dwn_ops[CPU_MAX_PWR_DWN_OPS])();
#endif /* (defined(IMAGE_BL31) || defined(IMAGE_BL32)) && CPU_MAX_PWR_DWN_OPS */
void *errata_list_start;
void *errata_list_end;
diff --git a/lib/cpus/aarch64/cortex_gelas.S b/lib/cpus/aarch64/cortex_gelas.S
index fc249ae..d322006 100644
--- a/lib/cpus/aarch64/cortex_gelas.S
+++ b/lib/cpus/aarch64/cortex_gelas.S
@@ -52,6 +52,7 @@
sysreg_bit_toggle CORTEX_GELAS_CPUPWRCTLR_EL1, \
CORTEX_GELAS_CPUPWRCTLR_EL1_CORE_PWRDN_BIT
isb
+ signal_pabandon_handled
ret
endfunc cortex_gelas_core_pwr_dwn
diff --git a/lib/cpus/aarch64/travis.S b/lib/cpus/aarch64/travis.S
index 956da5f..a959acb 100644
--- a/lib/cpus/aarch64/travis.S
+++ b/lib/cpus/aarch64/travis.S
@@ -48,6 +48,7 @@
sysreg_bit_toggle TRAVIS_IMP_CPUPWRCTLR_EL1, \
TRAVIS_IMP_CPUPWRCTLR_EL1_CORE_PWRDN_EN_BIT
isb
+ signal_pabandon_handled
ret
endfunc travis_core_pwr_dwn
diff --git a/lib/psci/psci_common.c b/lib/psci/psci_common.c
index 04917ea..08e27ac 100644
--- a/lib/psci/psci_common.c
+++ b/lib/psci/psci_common.c
@@ -1196,7 +1196,7 @@
return (n_valid > 1U) ? 1 : 0;
}
-static void call_cpu_pwr_dwn(unsigned int power_level)
+static u_register_t call_cpu_pwr_dwn(unsigned int power_level)
{
struct cpu_ops *ops = get_cpu_data(cpu_ops_ptr);
@@ -1213,7 +1213,25 @@
static void prepare_cpu_pwr_dwn(unsigned int power_level)
{
- call_cpu_pwr_dwn(power_level);
+ /* ignore the return, all cpus should behave the same */
+ (void)call_cpu_pwr_dwn(power_level);
+}
+
+static void prepare_cpu_pwr_up(unsigned int power_level)
+{
+ /*
+ * Call the pwr_dwn cpu hook again, indicating that an abandon happened.
+ * The cpu driver is expected to clean up. We ask it to return
+ * PABANDON_ACK to indicate that it has handled this. This is a
+ * heuristic: the value has been chosen such that an unported CPU is
+ * extremely unlikely to return this value.
+ */
+ u_register_t ret = call_cpu_pwr_dwn(power_level);
+
+ /* unreachable on AArch32 so cast down to calm the compiler */
+ if (ret != (u_register_t) PABANDON_ACK) {
+ panic();
+ }
}
/*******************************************************************************
@@ -1321,10 +1339,9 @@
/*
* Begin unwinding. Everything can be shared with CPU_ON and co later,
* except the CPU specific bit. Cores that have hardware-assisted
- * coherency don't have much to do so just calling the hook again is
- * the simplest way to achieve this
+ * coherency should be able to handle this.
*/
- prepare_cpu_pwr_dwn(power_level);
+ prepare_cpu_pwr_up(power_level);
}
/*******************************************************************************