linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] arm64: suspend: Reconfigure PSTATE after resume from idle
Date: Thu, 20 Oct 2016 12:27:51 +0100	[thread overview]
Message-ID: <20161020112751.GA8731@red-moon> (raw)
In-Reply-To: <1476786468-2173-4-git-send-email-james.morse@arm.com>

On Tue, Oct 18, 2016 at 11:27:48AM +0100, James Morse wrote:
> The suspend/resume path in kernel/sleep.S, as used by cpu-idle, does not
> save/restore PSTATE. As a result of this cpufeatures that were detected
> and have bits in PSTATE get lost when we resume from idle.
> 
> UAO gets set appropriately on the next context switch. PAN will be
> re-enabled next time we return from user-space, but on a preemptible
> kernel we may run work accessing user space before this point.
> 
> Add code to re-enable theses two features in __cpu_suspend_exit().
> We re-use uao_thread_switch() passing current.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> 
> ---
> This patch applies to linux-stable v4.7.8, but with some fuzz...
> but 'git am' rejects it.
> 
> asm/exec.h is my best guess at the appropriate header file. Contradictions
> welcome.

uaccess.h ? It is a shame you have to export uao_thread_switch() (see
below for a possible solution) but I agree that prevents useless code
duplication and that this needs fixing.

>  arch/arm64/include/asm/exec.h |  3 +++
>  arch/arm64/kernel/process.c   |  3 ++-
>  arch/arm64/kernel/suspend.c   | 11 +++++++++++
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/exec.h b/arch/arm64/include/asm/exec.h
> index db0563c23482..f7865dd9d868 100644
> --- a/arch/arm64/include/asm/exec.h
> +++ b/arch/arm64/include/asm/exec.h
> @@ -18,6 +18,9 @@
>  #ifndef __ASM_EXEC_H
>  #define __ASM_EXEC_H
>  
> +#include <linux/sched.h>
> +
>  extern unsigned long arch_align_stack(unsigned long sp);
> +void uao_thread_switch(struct task_struct *next);
>  
>  #endif	/* __ASM_EXEC_H */
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 27b2f1387df4..4f186c56c5eb 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -49,6 +49,7 @@
>  #include <asm/alternative.h>
>  #include <asm/compat.h>
>  #include <asm/cacheflush.h>
> +#include <asm/exec.h>
>  #include <asm/fpsimd.h>
>  #include <asm/mmu_context.h>
>  #include <asm/processor.h>
> @@ -301,7 +302,7 @@ static void tls_thread_switch(struct task_struct *next)
>  }
>  
>  /* Restore the UAO state depending on next's addr_limit */
> -static void uao_thread_switch(struct task_struct *next)
> +void uao_thread_switch(struct task_struct *next)
>  {
>  	if (IS_ENABLED(CONFIG_ARM64_UAO)) {
>  		if (task_thread_info(next)->addr_limit == KERNEL_DS)
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index ad734142070d..bb0cd787a9d3 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -1,8 +1,11 @@
>  #include <linux/ftrace.h>
>  #include <linux/percpu.h>
>  #include <linux/slab.h>
> +#include <asm/alternative.h>
>  #include <asm/cacheflush.h>
> +#include <asm/cpufeature.h>
>  #include <asm/debug-monitors.h>
> +#include <asm/exec.h>
>  #include <asm/pgtable.h>
>  #include <asm/memory.h>
>  #include <asm/mmu_context.h>
> @@ -50,6 +53,14 @@ void notrace __cpu_suspend_exit(void)
>  	set_my_cpu_offset(per_cpu_offset(cpu));
>  
>  	/*
> +	 * PSTATE was not saved over suspend/resume, re-enable any detected
> +	 * features that might not have been set correctly.
> +	 */
> +	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,
> +			CONFIG_ARM64_PAN));
> +	uao_thread_switch(current);

set_fs(get_fs());

would do (?), but that's horrendous to say the least, maybe you can
refactor the code in asm/uaccess.h to achieve the same goal (ie you
factor out the code setting UAO from set_fs() in a separate inline that
you can also reuse in uao_thread_switch() and here).

Other than that:

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> +
> +	/*
>  	 * Restore HW breakpoint registers to sane values
>  	 * before debug exceptions are possibly reenabled
>  	 * through local_dbg_restore.
> -- 
> 2.8.0.rc3
> 

      reply	other threads:[~2016-10-20 11:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-18 10:27 [PATCH 0/3] PAN Fixes James Morse
2016-10-18 10:27 ` [PATCH 1/3] arm64: cpufeature: Schedule enable() calls instead of calling them via IPI James Morse
2016-10-18 10:27 ` [PATCH 2/3] arm64: mm: Set PSTATE.PAN from the cpu_enable_pan() call James Morse
2016-10-19 16:52   ` Will Deacon
2016-10-18 10:27 ` [PATCH 3/3] arm64: suspend: Reconfigure PSTATE after resume from idle James Morse
2016-10-20 11:27   ` Lorenzo Pieralisi [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=20161020112751.GA8731@red-moon \
    --to=lorenzo.pieralisi@arm.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).