[PATCH] ARM: Use WFI when possible when halting a core

[PATCH] ARM: Use WFI when possible when halting a core

[Chen-Yu Tsai]

On ARM, the kernel busy loops after cleaning up when a core is to be
halted. This may have the undesired effect of increasing the core
temperature, at a time when no power or thermal management is
available, such as a kernel panic or shutdown to halt.

x86 uses the more efficient HLT (halt) instruction. The equivalent
instruction on ARM is WFI (wait for interrupt).

This patch makes the busy loops in the ARM halt/reboot/panic paths
use WFI when available (as defined in arch/arm/include/asm/barrier.h).

A touch test indicates that this lowers the surface temperature of the
Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully
hot to just warm to the touch after shutdown to halt.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

I asked about this some time ago, and the answer was no one had done
it. So here it is. It is somewhat similar to the patch "arm64:
Improve parking of stopped CPUs", for arm64 obviously.

---
 arch/arm/kernel/reboot.c | 13 +++++++++++--
 arch/arm/kernel/smp.c    |  7 ++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c
index 3b2aa9a9fe26..068aef796de4 100644
--- a/arch/arm/kernel/reboot.c
+++ b/arch/arm/kernel/reboot.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/reboot.h>
 
+#include <asm/barrier.h>
 #include <asm/cacheflush.h>
 #include <asm/idmap.h>
 #include <asm/virt.h>
@@ -107,7 +108,11 @@ void machine_halt(void)
 {
 	local_irq_disable();
 	smp_send_stop();
-	while (1);
+	while (1) {
+#ifdef wfi
+		wfi();
+#endif
+	}
 }
 
 /*
@@ -151,5 +156,9 @@ void machine_restart(char *cmd)
 
 	/* Whoops - the platform was unable to reboot. Tell the user! */
 	printk("Reboot failed -- System halted\n");
-	while (1);
+	while (1) {
+#ifdef wfi
+		wfi();
+#endif
+	}
 }
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index c9a0a5299827..7666fb2ed481 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -32,6 +32,7 @@
 
 #include <linux/atomic.h>
 #include <asm/smp.h>
+#include <asm/barrier.h>
 #include <asm/cacheflush.h>
 #include <asm/cpu.h>
 #include <asm/cputype.h>
@@ -567,8 +568,12 @@ static void ipi_cpu_stop(unsigned int cpu)
 	local_fiq_disable();
 	local_irq_disable();
 
-	while (1)
+	while (1) {
 		cpu_relax();
+#ifdef wfi
+		wfi();
+#endif
+	}
 }
 
 static DEFINE_PER_CPU(struct completion *, cpu_completion);
-- 
2.13.3

Re: [PATCH] ARM: Use WFI when possible when halting a core

[Russell King - ARM Linux]

On Thu, Jul 27, 2017 at 08:08:03PM +0800, Chen-Yu Tsai wrote:
> On ARM, the kernel busy loops after cleaning up when a core is to be
> halted. This may have the undesired effect of increasing the core
> temperature, at a time when no power or thermal management is
> available, such as a kernel panic or shutdown to halt.
> 
> x86 uses the more efficient HLT (halt) instruction. The equivalent
> instruction on ARM is WFI (wait for interrupt).
> 
> This patch makes the busy loops in the ARM halt/reboot/panic paths
> use WFI when available (as defined in arch/arm/include/asm/barrier.h).
> 
> A touch test indicates that this lowers the surface temperature of the
> Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully
> hot to just warm to the touch after shutdown to halt.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> 
> I asked about this some time ago, and the answer was no one had done
> it. So here it is. It is somewhat similar to the patch "arm64:
> Improve parking of stopped CPUs", for arm64 obviously.

There are various errata around the "wfi" instruction on various CPUs
which means that this simple implementation isn't going to work
reliably.

This is where cpu_do_idle() comes in - please use that instead.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] ARM: Use WFI when possible when halting a core

[Chen-Yu Tsai]

On Thu, Jul 27, 2017 at 8:17 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Thu, Jul 27, 2017 at 08:08:03PM +0800, Chen-Yu Tsai wrote:
>> On ARM, the kernel busy loops after cleaning up when a core is to be
>> halted. This may have the undesired effect of increasing the core
>> temperature, at a time when no power or thermal management is
>> available, such as a kernel panic or shutdown to halt.
>>
>> x86 uses the more efficient HLT (halt) instruction. The equivalent
>> instruction on ARM is WFI (wait for interrupt).
>>
>> This patch makes the busy loops in the ARM halt/reboot/panic paths
>> use WFI when available (as defined in arch/arm/include/asm/barrier.h).
>>
>> A touch test indicates that this lowers the surface temperature of the
>> Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully
>> hot to just warm to the touch after shutdown to halt.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>
>> I asked about this some time ago, and the answer was no one had done
>> it. So here it is. It is somewhat similar to the patch "arm64:
>> Improve parking of stopped CPUs", for arm64 obviously.
>
> There are various errata around the "wfi" instruction on various CPUs
> which means that this simple implementation isn't going to work
> reliably.
>
> This is where cpu_do_idle() comes in - please use that instead.

Thanks for the tip. Looks like this one is implemented for every
platform. No need for #ifdefs anymore.

ChenYu

Re: [PATCH] ARM: Use WFI when possible when halting a core

[Chen-Yu Tsai]

On Thu, Jul 27, 2017 at 8:45 PM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Thu, Jul 27, 2017 at 8:17 PM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>> On Thu, Jul 27, 2017 at 08:08:03PM +0800, Chen-Yu Tsai wrote:
>>> On ARM, the kernel busy loops after cleaning up when a core is to be
>>> halted. This may have the undesired effect of increasing the core
>>> temperature, at a time when no power or thermal management is
>>> available, such as a kernel panic or shutdown to halt.
>>>
>>> x86 uses the more efficient HLT (halt) instruction. The equivalent
>>> instruction on ARM is WFI (wait for interrupt).
>>>
>>> This patch makes the busy loops in the ARM halt/reboot/panic paths
>>> use WFI when available (as defined in arch/arm/include/asm/barrier.h).
>>>
>>> A touch test indicates that this lowers the surface temperature of the
>>> Allwinner H3 SoC, with all 4 cores brought up with SMP, from painfully
>>> hot to just warm to the touch after shutdown to halt.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>
>>> I asked about this some time ago, and the answer was no one had done
>>> it. So here it is. It is somewhat similar to the patch "arm64:
>>> Improve parking of stopped CPUs", for arm64 obviously.
>>
>> There are various errata around the "wfi" instruction on various CPUs
>> which means that this simple implementation isn't going to work
>> reliably.
>>
>> This is where cpu_do_idle() comes in - please use that instead.
>
> Thanks for the tip. Looks like this one is implemented for every
> platform. No need for #ifdefs anymore.

So I've done the modifications and re-tested. Unfortunately it no
longer works, either using wfi() directly or cpu_do_idle(). It worked
back in January this year.

I'm not sure what's happening. The chip is still hot. (Hotter than
when it is idle in userspace.) I added some printk calls around the
wfi() / cpu_do_idle() calls and found it continuously looping, instead
of being parked. But of course printk might add to the problem.

AFAIK all interrupts should be disabled by the time the call is
reached. What could possibly bring the CPU out of WFI?

Regards
ChenYu