From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Dave Martin <Dave.Martin@arm.com>,
linux-rt-users@vger.kernel.org,
Catalin Marinas <catalin.marinas@arm.com>,
Mike Galbraith <efault@gmx.de>, Will Deacon <will.deacon@arm.com>,
linux-kernel@vger.kernel.org, tglx@linutronix.de,
linux-arm-kernel@lists.infradead.org
Subject: [PATCH RT v3] arm64: fpsimd: use preemp_disable in addition to local_bh_disable()
Date: Thu, 26 Jul 2018 17:06:34 +0200 [thread overview]
Message-ID: <20180726150634.cl3wccqur6qhle6p@linutronix.de> (raw)
In-Reply-To: <20180724094623.37430032@gandalf.local.home>
In v4.16-RT I noticed a number of warnings from task_fpsimd_load(). The
code disables BH and expects that it is not preemptible. On -RT the
task remains preemptible but remains the same CPU. This may corrupt the
content of the SIMD registers if the task is preempted during
saving/restoring those registers.
Add preempt_disable()/enable() to enfore the required semantic on -RT.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
This should work. Compiling currently gcc-6 on the box to see what
happens. Since the crypto disables preemption "frequently" and I don't
expect or see anything to worry about.
arch/arm64/kernel/fpsimd.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -157,6 +157,15 @@ static void sve_free(struct task_struct
__sve_free(task);
}
+static void *sve_free_atomic(struct task_struct *task)
+{
+ void *sve_state = task->thread.sve_state;
+
+ WARN_ON(test_tsk_thread_flag(task, TIF_SVE));
+
+ task->thread.sve_state = NULL;
+ return sve_state;
+}
/* Offset of FFR in the SVE register dump */
static size_t sve_ffr_offset(int vl)
@@ -594,6 +603,7 @@ int sve_set_vector_length(struct task_st
* non-SVE thread.
*/
if (task == current) {
+ preempt_disable();
local_bh_disable();
task_fpsimd_save();
@@ -604,8 +614,10 @@ int sve_set_vector_length(struct task_st
if (test_and_clear_tsk_thread_flag(task, TIF_SVE))
sve_to_fpsimd(task);
- if (task == current)
+ if (task == current) {
local_bh_enable();
+ preempt_enable();
+ }
/*
* Force reallocation of task SVE state to the correct size
@@ -837,6 +849,7 @@ asmlinkage void do_sve_acc(unsigned int
sve_alloc(current);
+ preempt_disable();
local_bh_disable();
task_fpsimd_save();
@@ -850,6 +863,7 @@ asmlinkage void do_sve_acc(unsigned int
WARN_ON(1); /* SVE access shouldn't have trapped */
local_bh_enable();
+ preempt_enable();
}
/*
@@ -921,10 +935,12 @@ void fpsimd_thread_switch(struct task_st
void fpsimd_flush_thread(void)
{
int vl, supported_vl;
+ void *mem = NULL;
if (!system_supports_fpsimd())
return;
+ preempt_disable();
local_bh_disable();
memset(¤t->thread.fpsimd_state, 0, sizeof(struct fpsimd_state));
@@ -932,7 +948,7 @@ void fpsimd_flush_thread(void)
if (system_supports_sve()) {
clear_thread_flag(TIF_SVE);
- sve_free(current);
+ mem = sve_free_atomic(current);
/*
* Reset the task vector length as required.
@@ -968,6 +984,8 @@ void fpsimd_flush_thread(void)
set_thread_flag(TIF_FOREIGN_FPSTATE);
local_bh_enable();
+ preempt_enable();
+ kfree(mem);
}
/*
@@ -979,9 +997,11 @@ void fpsimd_preserve_current_state(void)
if (!system_supports_fpsimd())
return;
+ preempt_disable();
local_bh_disable();
task_fpsimd_save();
local_bh_enable();
+ preempt_enable();
}
/*
@@ -1021,6 +1041,7 @@ void fpsimd_restore_current_state(void)
if (!system_supports_fpsimd())
return;
+ preempt_disable();
local_bh_disable();
if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
@@ -1029,6 +1050,7 @@ void fpsimd_restore_current_state(void)
}
local_bh_enable();
+ preempt_enable();
}
/*
@@ -1041,6 +1063,7 @@ void fpsimd_update_current_state(struct
if (!system_supports_fpsimd())
return;
+ preempt_disable();
local_bh_disable();
current->thread.fpsimd_state.user_fpsimd = *state;
@@ -1053,6 +1076,7 @@ void fpsimd_update_current_state(struct
fpsimd_bind_to_cpu();
local_bh_enable();
+ preempt_enable();
}
/*
@@ -1115,6 +1139,7 @@ void kernel_neon_begin(void)
BUG_ON(!may_use_simd());
+ preempt_disable();
local_bh_disable();
__this_cpu_write(kernel_neon_busy, true);
@@ -1131,6 +1156,7 @@ void kernel_neon_begin(void)
preempt_disable();
local_bh_enable();
+ preempt_enable();
}
EXPORT_SYMBOL(kernel_neon_begin);
next prev parent reply other threads:[~2018-07-26 15:06 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-17 12:40 [PATCH RT] arm64: fpsimd: use a local_lock() in addition to local_bh_disable() Sebastian Andrzej Siewior
2018-05-17 18:19 ` Dave Martin
2018-05-18 12:46 ` Dave Martin
2018-05-23 14:34 ` Sebastian Andrzej Siewior
2018-05-23 14:31 ` Sebastian Andrzej Siewior
2018-05-23 14:55 ` Dave Martin
2018-05-22 17:10 ` Steven Rostedt
2018-05-22 17:21 ` Sebastian Andrzej Siewior
2018-05-22 17:24 ` Steven Rostedt
2018-05-22 17:33 ` Sebastian Andrzej Siewior
2018-07-11 13:25 ` Steven Rostedt
2018-07-11 13:31 ` Sebastian Andrzej Siewior
2018-07-11 13:33 ` Steven Rostedt
2018-07-13 17:49 ` [PATCH RT v2] " Sebastian Andrzej Siewior
2018-07-13 17:50 ` [PATCH RT] locallock: add local_lock_bh() Sebastian Andrzej Siewior
2018-07-13 22:03 ` [PATCH RT v2] arm64: fpsimd: use a local_lock() in addition to local_bh_disable() Mike Galbraith
2018-07-15 7:22 ` Mike Galbraith
2018-07-18 10:30 ` Mike Galbraith
2018-07-18 9:27 ` Sebastian Andrzej Siewior
2018-07-18 10:28 ` Mike Galbraith
2018-07-18 10:36 ` Sebastian Andrzej Siewior
2018-07-16 15:17 ` Dave Martin
2018-07-18 9:12 ` Sebastian Andrzej Siewior
2018-07-18 9:24 ` Sebastian Andrzej Siewior
2018-07-24 14:45 ` Dave Martin
2018-07-24 15:15 ` Ard Biesheuvel
2018-07-24 13:46 ` Steven Rostedt
2018-07-24 13:57 ` Sebastian Andrzej Siewior
2018-07-26 15:06 ` Sebastian Andrzej Siewior [this message]
2018-07-27 3:17 ` [PATCH RT v3] arm64: fpsimd: use preemp_disable " Mike Galbraith
2018-07-27 7:56 ` Sebastian Andrzej Siewior
2018-07-27 15:35 ` Dave Martin
2018-07-27 16:26 ` Sebastian Andrzej Siewior
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=20180726150634.cl3wccqur6qhle6p@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=Dave.Martin@arm.com \
--cc=catalin.marinas@arm.com \
--cc=efault@gmx.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=will.deacon@arm.com \
/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).