public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Dave.Martin@arm.com (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: fpsimd: Fix failure to restore FPSIMD state after signals
Date: Thu, 30 Nov 2017 11:56:37 +0000	[thread overview]
Message-ID: <1512042997-25945-1-git-send-email-Dave.Martin@arm.com> (raw)

The fpsimd_update_current_state() function is responsible for
loading the FPSIMD state from the user signal frame into the
current task during sigreturn.  When implementing support for SVE,
conditional code was added to this function in order to handle the
case where SVE state need to be loaded for the task and merged with
the FPSIMD data from the signal frame; however, the FPSIMD-only
case was unintentionally dropped.

As a result of this, sigreturn does not currently restore the
FPSIMD state of the task, except in the case where the system
supports SVE and the signal frame contains SVE state in addition to
FPSIMD state.

This patch fixes this bug by making the copy-in of the FPSIMD data
from the signal frame to thread_struct unconditional.

This remains a performance regression from v4.14, since the FPSIMD
state is now copied into thread_struct and then loaded back,
instead of _only_ being loaded into the CPU FPSIMD registers.
However, it is essential to call task_fpsimd_load() here anyway in
order to ensure that the SVE enable bit in CPACR_EL1 is set
correctly before returning to userspace.  This could use some
refactoring, but since sigreturn is not a fast path I have kept
this patch as a pure fix and left the refactoring for later.

Fixes: 8cd969d28fd2 ("arm64/sve: Signal handling support")
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reported-by: Alex Benn?e <alex.bennee@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
---

Initial testing of this patch looks OK, but I will continue to bash it.

While debugging this issue, I also hit another possible register
corruption issue that I don't have an explanation for, but I wanted to
get this patch out first since this issue at least is fairly
straightforward and fixing it is required anyway.

I will continue to investigate.

 arch/arm64/kernel/fpsimd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 143b3e7..5084e69 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1026,10 +1026,10 @@ void fpsimd_update_current_state(struct fpsimd_state *state)
 
 	local_bh_disable();
 
-	if (system_supports_sve() && test_thread_flag(TIF_SVE)) {
-		current->thread.fpsimd_state = *state;
+	current->thread.fpsimd_state = *state;
+	if (system_supports_sve() && test_thread_flag(TIF_SVE))
 		fpsimd_to_sve(current);
-	}
+
 	task_fpsimd_load();
 
 	if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
-- 
2.1.4

             reply	other threads:[~2017-11-30 11:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-30 11:56 Dave Martin [this message]
2017-11-30 12:08 ` [PATCH] arm64: fpsimd: Fix failure to restore FPSIMD state after signals Will Deacon
2017-11-30 12:32   ` Dave Martin
2017-11-30 16:27 ` Alex Bennée
2017-11-30 16:30   ` Dave Martin
2017-11-30 16:29 ` Dave Martin

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=1512042997-25945-1-git-send-email-Dave.Martin@arm.com \
    --to=dave.martin@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