From: xobs@kosagi.com (Sean Cross)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: fix broken hibernation
Date: Thu, 02 Apr 2015 12:13:34 +0800 [thread overview]
Message-ID: <551CC1EE.7000205@kosagi.com> (raw)
In-Reply-To: <E1YdKq4-00049v-Bj@rmk-PC.arm.linux.org.uk>
On 01/04/2015 23:45, Russell King wrote:
> Normally, when a CPU wants to clear a cache line to zero in the external
> L2 cache, it would generate bus cycles to write each word as it would do
> with any other data access.
>
> However, a Cortex A9 connected to a L2C-310 has a specific feature where
> the CPU can detect this operation, and signal that it wants to zero an
> entire cache line. This feature, known as Full Line of Zeros (FLZ),
> involves a non-standard AXI signalling mechanism which only the L2C-310
> can properly interpret.
>
> There are separate enable bits in both the L2C-310 and the Cortex A9 -
> the L2C-310 needs to be enabled and have the FLZ enable bit set in the
> auxiliary control register before the Cortex A9 has this feature
> enabled.
>
> Unfortunately, the suspend code was not respecting this - it's not
> obvious from the code:
>
> swsusp_arch_suspend()
> cpu_suspend() /* saves the Cortex A9 auxiliary control register */
> arch_save_image()
> soft_restart() /* turns off FLZ in Cortex A9, and disables L2C */
> cpu_resume() /* restores the Cortex A9 registers, inc auxcr */
>
> At this point, we end up with the L2C disabled, but the Cortex A9 with
> FLZ enabled - which means any memset() or zeroing of a full cache line
> will fail to take effect.
>
> A similar issue exists in the resume path, but it's slightly more
> complex:
>
> swsusp_arch_suspend()
> cpu_suspend() /* saves the Cortex A9 auxiliary control register */
> arch_save_image() /* image with A9 auxcr saved */
> ...
> swsusp_arch_resume()
> call_with_stack()
> arch_restore_image() /* restores image with A9 auxcr saved above */
> soft_restart() /* turns off FLZ in Cortex A9, and disables L2C */
> cpu_resume() /* restores the Cortex A9 registers, inc auxcr */
>
> Again, here we end up with the L2C disabled, but Cortex A9 FLZ enabled.
>
> There's no need to turn off the L2C in either of these two paths; there
> are benefits from not doing so - for example, the page copies will be
> faster with the L2C enabled.
>
> Hence, fix this by providing a variant of soft_restart() which can be
> used without turning the L2 cache controller off, and use it in both
> of these paths to keep the L2C enabled across the respective resume
> transitions.
This patch fixes hibernation on my i.MX6-based Novena, running v3.19.
It has survived thirty hibernate/resume cycles so far. Without the
patch it panics almost instantly after issuing the "hibernate" command.
Tested-by: Sean Cross <xobs@kosagi.com>
> Fixes: 8ef418c7178f ("ARM: l2c: trial at enabling some Cortex-A9 optimisations")
> Reported-by: xobs <-- please give me your preferred email for this!
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> arch/arm/kernel/hibernate.c | 5 +++--
> arch/arm/kernel/process.c | 10 ++++++++--
> arch/arm/kernel/reboot.h | 6 ++++++
> 3 files changed, 17 insertions(+), 4 deletions(-)
> create mode 100644 arch/arm/kernel/reboot.h
>
> diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
> index c4cc50e58c13..cfb354ff2a60 100644
> --- a/arch/arm/kernel/hibernate.c
> +++ b/arch/arm/kernel/hibernate.c
> @@ -22,6 +22,7 @@
> #include <asm/suspend.h>
> #include <asm/memory.h>
> #include <asm/sections.h>
> +#include "reboot.h"
>
> int pfn_is_nosave(unsigned long pfn)
> {
> @@ -61,7 +62,7 @@ static int notrace arch_save_image(unsigned long unused)
>
> ret = swsusp_save();
> if (ret == 0)
> - soft_restart(virt_to_phys(cpu_resume));
> + _soft_restart(virt_to_phys(cpu_resume), false);
> return ret;
> }
>
> @@ -86,7 +87,7 @@ static void notrace arch_restore_image(void *unused)
> for (pbe = restore_pblist; pbe; pbe = pbe->next)
> copy_page(pbe->orig_address, pbe->address);
>
> - soft_restart(virt_to_phys(cpu_resume));
> + _soft_restart(virt_to_phys(cpu_resume), false);
> }
>
> static u64 resume_stack[PAGE_SIZE/2/sizeof(u64)] __nosavedata;
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index c50fe212fd89..d14d38b860f8 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -42,6 +42,7 @@
> #include <asm/mach/time.h>
> #include <asm/tls.h>
> #include <asm/vdso.h>
> +#include "reboot.h"
>
> #ifdef CONFIG_CC_STACKPROTECTOR
> #include <linux/stackprotector.h>
> @@ -96,7 +97,7 @@ static void __soft_restart(void *addr)
> BUG();
> }
>
> -void soft_restart(unsigned long addr)
> +void _soft_restart(unsigned long addr, bool disable_l2)
> {
> u64 *stack = soft_restart_stack + ARRAY_SIZE(soft_restart_stack);
>
> @@ -105,7 +106,7 @@ void soft_restart(unsigned long addr)
> local_fiq_disable();
>
> /* Disable the L2 if we're the last man standing. */
> - if (num_online_cpus() == 1)
> + if (disable_l2)
> outer_disable();
>
> /* Change to the new stack and continue with the reset. */
> @@ -115,6 +116,11 @@ void soft_restart(unsigned long addr)
> BUG();
> }
>
> +void soft_restart(unsigned long addr)
> +{
> + _soft_restart(addr, num_online_cpus() == 1);
> +}
> +
> /*
> * Function pointers to optional machine specific functions
> */
> diff --git a/arch/arm/kernel/reboot.h b/arch/arm/kernel/reboot.h
> new file mode 100644
> index 000000000000..c87f05816d6b
> --- /dev/null
> +++ b/arch/arm/kernel/reboot.h
> @@ -0,0 +1,6 @@
> +#ifndef REBOOT_H
> +#define REBOOT_H
> +
> +extern void _soft_restart(unsigned long addr, bool disable_l2);
> +
> +#endif
prev parent reply other threads:[~2015-04-02 4:13 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-01 15:45 [PATCH] ARM: fix broken hibernation Russell King
2015-04-02 4:13 ` Sean Cross [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=551CC1EE.7000205@kosagi.com \
--to=xobs@kosagi.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).