[PATCH] MIPS: Provide fallback reboot/poweroff/halt implementations

[PATCH] MIPS: Provide fallback reboot/poweroff/halt implementations

[Andrew Bresticker]

If a machine-specific hook is not implemented for restart, poweroff,
or halt, fall back to halting secondary CPUs, disabling interrupts,
and spinning.  In the case of restart, attempt to restart the system
via do_kernel_restart() (which will call any registered restart
handlers) before halting.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 arch/mips/kernel/reset.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c
index 07fc524..87b1f08 100644
--- a/arch/mips/kernel/reset.c
+++ b/arch/mips/kernel/reset.c
@@ -29,16 +29,36 @@ void machine_restart(char *command)
 {
 	if (_machine_restart)
 		_machine_restart(command);
+
+#ifdef CONFIG_SMP
+	smp_send_stop();
+#endif
+	do_kernel_restart(command);
+	pr_emerg("Reboot failed -- System halted\n");
+	local_irq_disable();
+	while (1);
 }
 
 void machine_halt(void)
 {
 	if (_machine_halt)
 		_machine_halt();
+
+#ifdef CONFIG_SMP
+	smp_send_stop();
+#endif
+	local_irq_disable();
+	while (1);
 }
 
 void machine_power_off(void)
 {
 	if (pm_power_off)
 		pm_power_off();
+
+#ifdef CONFIG_SMP
+	smp_send_stop();
+#endif
+	local_irq_disable();
+	while (1);
 }
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH] MIPS: Provide fallback reboot/poweroff/halt implementations

[Maciej W. Rozycki]

On Wed, 4 Mar 2015, Andrew Bresticker wrote:

> If a machine-specific hook is not implemented for restart, poweroff,
> or halt, fall back to halting secondary CPUs, disabling interrupts,
> and spinning.  In the case of restart, attempt to restart the system
> via do_kernel_restart() (which will call any registered restart
> handlers) before halting.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> ---
>  arch/mips/kernel/reset.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c
> index 07fc524..87b1f08 100644
> --- a/arch/mips/kernel/reset.c
> +++ b/arch/mips/kernel/reset.c
> @@ -29,16 +29,36 @@ void machine_restart(char *command)
>  {
>  	if (_machine_restart)
>  		_machine_restart(command);
> +
> +#ifdef CONFIG_SMP
> +	smp_send_stop();
> +#endif
> +	do_kernel_restart(command);
> +	pr_emerg("Reboot failed -- System halted\n");
> +	local_irq_disable();
> +	while (1);

 I think it would make sense to put WAIT or suchlike in the final loop 
so that such a halted system does not suck power unnecessarily.  I think 
bits in arch/mips/kernel/idle.c should do, after some massaging.  As the 
next step perhaps, don't treat it as a NACK for this change.

  Maciej

Re: [PATCH] MIPS: Provide fallback reboot/poweroff/halt implementations

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]

Hi Andrew,

On 04/03/15 23:59, Andrew Bresticker wrote:
> If a machine-specific hook is not implemented for restart, poweroff,
> or halt, fall back to halting secondary CPUs, disabling interrupts,
> and spinning.  In the case of restart, attempt to restart the system
> via do_kernel_restart() (which will call any registered restart
> handlers) before halting.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> ---
>  arch/mips/kernel/reset.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c
> index 07fc524..87b1f08 100644
> --- a/arch/mips/kernel/reset.c
> +++ b/arch/mips/kernel/reset.c
> @@ -29,16 +29,36 @@ void machine_restart(char *command)
>  {
>  	if (_machine_restart)
>  		_machine_restart(command);
> +
> +#ifdef CONFIG_SMP
> +	smp_send_stop();

Maybe local_irq_disable should be before smp_send_stop() to avoid
deadlocks (same below)?

See for example commit 44424c34049f41123a3a8b4853822f47f4ff03a2 ("ARM:
7803/1: Fix deadlock scenario with smp_send_stop()").

> +#endif
> +	do_kernel_restart(command);
> +	pr_emerg("Reboot failed -- System halted\n");

Perhaps we could have a grace period like ARM does here:

> /* Give a grace period for failure to restart of 1s */
> mdelay(1000);

Otherwise with this patch a reboot on Malta usually shows some/all of
this pr_emerg message prior to actually restarting. (Arguably thats a
failing of Malta's _machine_restart not to have a delay... thoughts?).

Cheers
James

> +	local_irq_disable();
> +	while (1);
>  }
>  
>  void machine_halt(void)
>  {
>  	if (_machine_halt)
>  		_machine_halt();
> +
> +#ifdef CONFIG_SMP
> +	smp_send_stop();
> +#endif
> +	local_irq_disable();
> +	while (1);
>  }
>  
>  void machine_power_off(void)
>  {
>  	if (pm_power_off)
>  		pm_power_off();
> +
> +#ifdef CONFIG_SMP
> +	smp_send_stop();
> +#endif
> +	local_irq_disable();
> +	while (1);
>  }
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] MIPS: Provide fallback reboot/poweroff/halt implementations

[Andrew Bresticker]

Hi James,

On Tue, Mar 24, 2015 at 7:22 AM, James Hogan <james.hogan@imgtec.com> wrote:
> Hi Andrew,
>
> On 04/03/15 23:59, Andrew Bresticker wrote:
>> If a machine-specific hook is not implemented for restart, poweroff,
>> or halt, fall back to halting secondary CPUs, disabling interrupts,
>> and spinning.  In the case of restart, attempt to restart the system
>> via do_kernel_restart() (which will call any registered restart
>> handlers) before halting.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> ---
>>  arch/mips/kernel/reset.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c
>> index 07fc524..87b1f08 100644
>> --- a/arch/mips/kernel/reset.c
>> +++ b/arch/mips/kernel/reset.c
>> @@ -29,16 +29,36 @@ void machine_restart(char *command)
>>  {
>>       if (_machine_restart)
>>               _machine_restart(command);
>> +
>> +#ifdef CONFIG_SMP
>> +     smp_send_stop();
>
> Maybe local_irq_disable should be before smp_send_stop() to avoid
> deadlocks (same below)?
>
> See for example commit 44424c34049f41123a3a8b4853822f47f4ff03a2 ("ARM:
> 7803/1: Fix deadlock scenario with smp_send_stop()").

Ah right, good catch.

>> +#endif
>> +     do_kernel_restart(command);
>> +     pr_emerg("Reboot failed -- System halted\n");
>
> Perhaps we could have a grace period like ARM does here:
>
>> /* Give a grace period for failure to restart of 1s */
>> mdelay(1000);
>
> Otherwise with this patch a reboot on Malta usually shows some/all of
> this pr_emerg message prior to actually restarting. (Arguably thats a
> failing of Malta's _machine_restart not to have a delay... thoughts?).

Yes, I think it's reasonable to put a delay here.

Thanks,
Andrew

Re: [PATCH] MIPS: Provide fallback reboot/poweroff/halt implementations

[Andrew Bresticker]

On Tue, Mar 24, 2015 at 7:22 AM, James Hogan <james.hogan@imgtec.com> wrote:
> Hi Andrew,
>
> On 04/03/15 23:59, Andrew Bresticker wrote:
>> If a machine-specific hook is not implemented for restart, poweroff,
>> or halt, fall back to halting secondary CPUs, disabling interrupts,
>> and spinning.  In the case of restart, attempt to restart the system
>> via do_kernel_restart() (which will call any registered restart
>> handlers) before halting.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> ---
>>  arch/mips/kernel/reset.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c
>> index 07fc524..87b1f08 100644
>> --- a/arch/mips/kernel/reset.c
>> +++ b/arch/mips/kernel/reset.c
>> @@ -29,16 +29,36 @@ void machine_restart(char *command)
>>  {
>>       if (_machine_restart)
>>               _machine_restart(command);
>> +
>> +#ifdef CONFIG_SMP
>> +     smp_send_stop();
>
> Maybe local_irq_disable should be before smp_send_stop() to avoid
> deadlocks (same below)?
>
> See for example commit 44424c34049f41123a3a8b4853822f47f4ff03a2 ("ARM:
> 7803/1: Fix deadlock scenario with smp_send_stop()").

Actually, disabling IRQs before calling smp_send_stop() triggers the
WARN in kernel/smp.c:smp_call_function_many(), but I think we can
simply disable preemption to prevent the deadlock.