[PATCH] powerpc: special case ibm,suspend-me rtas call

[Nathan Lynch]

Dave C Boutcher wrote:
> Handle the ibm,suspend-me RTAS call specially.  It needs 
> to be wrapped in a set of synchronization hypervisor calls
> (H_Join).  When the H_Join calls are made on all CPUs, the
> intent is that only one will return with H_Continue, meaning
> that he is the "last man standing".  That CPU then issues the 
> ibm,suspend-me call.  What is interesting, of course, is that
> the CPU running when the rtas syscall is made, may NOT be the 
> CPU that ultimately executes the ibm,suspend-me syscall.
>
> This is an alternative to a previous patch I submitted which issues the
> rtas call through a new (and thus evil) /proc file.  I don't especially
> like adding special-case code to the rtas call path, but I don't like
> adding a /proc file either...

Agreed... in that I don't like either option. :)

I think there's coincidentally some discussion on lkml about adding
something like a /sys/hypervisor hierarchy for Xen et al... maybe we
could use that for things like this?  I've been wanting to convert the
dlpar stuff to an interface like that for a while, rather than having
userspace invoke the RTAS calls directly.

Also, I think you probably want a big mutex around the
rtas_ibm_suspend_me stuff... I don't think you'd want two threads
going down that path concurrently.

More comments below.

> +
> +out:
> +#ifdef CONFIG_DETECT_SOFTLOCKUP
> +	/* before we restore interrupts, make sure we don't
> +	 * generate a spurious soft lockup errors
> +	 */
> +	touch_softlockup_watchdog();
> +#endif

ifdef not needed.

> +static int rtas_ibm_suspend_me(struct rtas_args *args)
> +{
> +	int i;
> +
> +	struct rtas_suspend_me_data data;
> +
> +	data.waiting = 1;
> +	data.args = args;
> +
> +	/* Call function on all other CPUs */
> +	if (on_each_cpu(rtas_percpu_suspend_me, &data, 1, 0))
> +		data.waiting = -EINVAL;

I'm feeling soooo nitpicky -- can you please remove the "other" from
this comment (or just ditch the comment altogether)?  It briefly
confused me into thinking rtas_percpu_suspend_me isn't called on this
cpu, until I looked at the definition of on_each_cpu.


> +#else /* CONFIG_PPC_PSERIES */
> +static int rtas_ibm_suspend_me(struct rtas_args *args)
> +{
> +	return -EINVAL;
> +}
> +#endif

-ENOSYS would be better, I think.