There are several issues with the rtas_ibm_suspend_me code, which
enables platform-assisted suspension of an LPAR for migration or
hibernation as covered in PAPR 2.2.
1.) rtas_ibm_suspend_me uses on_each_cpu() to invoke
rtas_percpu_suspend_me on all cpus via IPI:
if (on_each_cpu(rtas_percpu_suspend_me, data, 1, 0))
...
'data' is on the calling task's stack, but rtas_ibm_suspend_me takes
no measures to ensure that all instances of rtas_percpu_suspend_me are
finished accessing 'data' before returning. This can result in the
IPI'd cpus accessing random stack data and getting stuck in H_JOIN.
This is addressed by using an atomic count of workers and a completion
on the stack.
2.) rtas_percpu_suspend_me is needlessly calling H_JOIN in a loop.
The only event that can cause a cpu to return from H_JOIN is an H_PROD
from another cpu or a NMI/system reset. Each cpu need call H_JOIN
only once per suspend operation.
Remove the loop and the now unnecessary 'waiting' state variable.
3.) H_JOIN must be called with MSR[EE] off, but lazy interrupt
disabling may cause the caller of rtas_ibm_suspend_me to call H_JOIN
with it on; the local_irq_disable() in on_each_cpu() is not
sufficient.
Fix this by explicitly saving the MSR and clearing the EE bit before
calling H_JOIN.
4.) H_PROD is being called with the Linux logical cpu number as the
parameter, not the platform interrupt server value. (It's also being
called for all possible cpus, which is harmless, but unnecessary.)
This is fixed by calling H_PROD for each online cpu using
get_hard_smp_processor_id(cpu) for the argument.
Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
---
Paul Mackerras wrote:
On a uniprocessor configuration, with this patch I get:
CC arch/powerpc/kernel/rtas.o
/home/paulus/kernel/powerpc/arch/powerpc/kernel/rtas.c: In function
‘rtas_percpu_suspend_me’:
/home/paulus/kernel/powerpc/arch/powerpc/kernel/rtas.c:702: error: implicit
declaration of function ‘get_hard_smp_processor_id
make[2]: *** [arch/powerpc/kernel/rtas.o] Error 1
I think you need to #include asm/smp.h in rtas.c.
Rather sloppy of me, sorry.
Changes since v2:
- Add appropriate #includes for APIs used, fixing SMP=n build
Changes since v1:
- the completion is adequate for ensuring that all IPIs have
completed, so there's no need for a mutex and rtas_suspend_me_data
can be on the stack as before.
- add fix for hard-disabling interrupts before H_JOIN
- remove unnecessary H_JOIN loop and state machinery
arch/powerpc/kernel/rtas.c | 99 ++--
1 files changed, 58 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 2147807..52e95c2 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -19,6 +19,9 @@
#include linux/init.h
#include linux/capability.h
#include linux/delay.h
+#include linux/smp.h
+#include linux/completion.h
+#include linux/cpumask.h
#include asm/prom.h
#include asm/rtas.h
@@ -34,6 +37,8 @@
#include asm/lmb.h
#include asm/udbg.h
#include asm/syscalls.h
+#include asm/smp.h
+#include asm/atomic.h
struct rtas_t rtas = {
.lock = SPIN_LOCK_UNLOCKED
@@ -41,8 +46,10 @@ struct rtas_t rtas = {
EXPORT_SYMBOL(rtas);
struct rtas_suspend_me_data {
- long waiting;
- struct rtas_args *args;
+ atomic_t working; /* number of cpus accessing this struct */
+ int token; /* ibm,suspend-me */
+ int error;
+ struct completion *complete; /* wait on this until working == 0 */
};
DEFINE_SPINLOCK(rtas_data_buf_lock);
@@ -657,50 +664,62 @@ static int ibm_suspend_me_token = RTAS_UNKNOWN_SERVICE;
#ifdef CONFIG_PPC_PSERIES
static void rtas_percpu_suspend_me(void *info)
{
- int i;
long rc;
- long flags;
+ unsigned long msr_save;
+ int cpu;
struct rtas_suspend_me_data *data =
(struct rtas_suspend_me_data *)info;
- /*
-* We use waiting to indicate our state. As long
-* as it is 0, we are still trying to all join up.
-* If it goes to 0, we have successfully joined up and
-* one thread got H_CONTINUE. If any error happens,
-* we set it to 0.
-*/
- local_irq_save(flags);
- do {
- rc = plpar_hcall_norets(H_JOIN);
- smp_rmb();
- } while (rc == H_SUCCESS data-waiting 0);
- if (rc == H_SUCCESS)
- goto out;
+ atomic_inc(data-working);
+
+ /* really need to ensure MSR.EE is off for H_JOIN */
+ msr_save = mfmsr();
+ mtmsr(msr_save ~(MSR_EE));
+
+ rc = plpar_hcall_norets(H_JOIN);
+
+ mtmsr(msr_save);
- if (rc == H_CONTINUE) {
- data-waiting = 0;
- data-args-args[data-args-nargs] =
- rtas_call(ibm_suspend_me_token, 0, 1, NULL);
- for_each_possible_cpu(i)
- plpar_hcall_norets(H_PROD,i);
+ if (rc ==
