linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Dave.Martin@arm.com (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v11 01/19] arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after invalidating cpu regs
Date: Thu, 24 May 2018 17:56:30 +0100	[thread overview]
Message-ID: <1527181008-13549-2-git-send-email-Dave.Martin@arm.com> (raw)
In-Reply-To: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com>

fpsimd_last_state.st is set to NULL as a way of indicating that
current's FPSIMD registers are no longer loaded in the cpu.  In
particular, this is done when the kernel temporarily uses or
clobbers the FPSIMD registers for its own purposes, as in CPU PM or
kernel-mode NEON, resulting in them being populated with garbage
data not belonging to a task.

Commit 17eed27b02da ("arm64/sve: KVM: Prevent guests from using
SVE") factors this operation out as a new helper
fpsimd_flush_cpu_state() to make it clearer what is being done
here, and on SVE systems this helper is now used, via
kvm_fpsimd_flush_cpu_state(), to invalidate the registers after KVM
has run a vcpu.  The reason for this is that KVM does not yet
understand how to restore the full host SVE registers itself after
loading the guest FPSIMD context into them.

This exposes a particular problem: if fpsimd_last_state.st is set
to NULL without also setting TIF_FOREIGN_FPSTATE, the kernel may
continue to think that current's FPSIMD registers are live even
though they have actually been clobbered.

Prior to the aforementioned commit, the only path where
fpsimd_last_state.st is set to NULL without setting
TIF_FOREIGN_FPSTATE is when kernel_neon_begin() is called by a
kernel thread (where current->mm can be NULL).  This does not
matter, because the only harm is that at context-switch time
fpsimd_thread_switch() may unnecessarily save the FPSIMD registers
back to current's thread_struct (even though kernel threads are not
considered to have any FPSIMD context of their own and the
registers will never be reloaded).

Note that although CPU_PM_ENTER lacks the TIF_FOREIGN_FPSTATE
setting, every CPU passing through that path must subsequently pass
through CPU_PM_EXIT before it can re-enter the kernel proper.
CPU_PM_EXIT sets the flag.

The sve_flush_cpu_state() function added by commit 17eed27b02da
also lacks the proper maintenance of TIF_FOREIGN_FPSTATE.  This may
cause the bits of a host task's SVE registers that do not alias the
FPSIMD register file to spontaneously appear zeroed if a KVM vcpu
runs in the same task in the meantime.  Although this effect is
hidden by the fact that the non-FPSIMD bits of the SVE registers
are zeroed by a syscall anyway, it is doubtless a bad idea to rely
on these different code paths interacting correctly under future
maintenance.

This patch makes TIF_FOREIGN_FPSTATE an unconditional side-effect
of fpsimd_flush_cpu_state(), and removes the set_thread_flag()
calls that become redundant as a result.  This ensures that
TIF_FOREIGN_FPSTATE cannot remain clear if the FPSIMD state in the
FPSIMD registers is invalid.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Reviewed-by: Alex Benn?e <alex.bennee@linaro.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/fpsimd.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 87a3536..12e1c96 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1067,6 +1067,7 @@ void fpsimd_flush_task_state(struct task_struct *t)
 static inline void fpsimd_flush_cpu_state(void)
 {
 	__this_cpu_write(fpsimd_last_state.st, NULL);
+	set_thread_flag(TIF_FOREIGN_FPSTATE);
 }
 
 /*
@@ -1121,10 +1122,8 @@ void kernel_neon_begin(void)
 	__this_cpu_write(kernel_neon_busy, true);
 
 	/* Save unsaved task fpsimd state, if any: */
-	if (current->mm) {
+	if (current->mm)
 		task_fpsimd_save();
-		set_thread_flag(TIF_FOREIGN_FPSTATE);
-	}
 
 	/* Invalidate any task state remaining in the fpsimd regs: */
 	fpsimd_flush_cpu_state();
@@ -1251,8 +1250,6 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self,
 		fpsimd_flush_cpu_state();
 		break;
 	case CPU_PM_EXIT:
-		if (current->mm)
-			set_thread_flag(TIF_FOREIGN_FPSTATE);
 		break;
 	case CPU_PM_ENTER_FAILED:
 	default:
-- 
2.1.4

  reply	other threads:[~2018-05-24 16:56 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-24 16:56 [PATCH v11 00/19] KVM: arm64: Optimise FPSIMD context switching Dave Martin
2018-05-24 16:56 ` Dave Martin [this message]
2018-05-24 16:56 ` [PATCH v11 02/19] thread_info: Add update_thread_flag() helpers Dave Martin
2018-05-24 17:02   ` Peter Zijlstra
2018-05-24 16:56 ` [PATCH v11 03/19] arm64: Use update{,_tsk}_thread_flag() Dave Martin
2018-05-24 16:56 ` [PATCH v11 04/19] KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change Dave Martin
2018-05-24 16:56 ` [PATCH v11 05/19] KVM: arm64: Convert lazy FPSIMD context switch trap to C Dave Martin
2018-05-24 16:56 ` [PATCH v11 06/19] arm64: fpsimd: Generalise context saving for non-task contexts Dave Martin
2018-05-24 16:56 ` [PATCH v11 07/19] arm64: fpsimd: Avoid FPSIMD context leakage for the init task Dave Martin
2018-05-25 10:01   ` Alex Bennée
2018-05-24 16:56 ` [PATCH v11 08/19] arm64: fpsimd: Eliminate task->mm checks Dave Martin
2018-05-25  9:02   ` Christoffer Dall
2018-05-25  9:52     ` Dave Martin
2018-05-25 10:04   ` Alex Bennée
2018-05-25 10:48     ` Dave Martin
2018-05-24 16:56 ` [PATCH v11 09/19] arm64/sve: Refactor user SVE trap maintenance for external use Dave Martin
2018-05-24 16:56 ` [PATCH v11 10/19] KVM: arm64: Repurpose vcpu_arch.debug_flags for general-purpose flags Dave Martin
2018-05-24 16:56 ` [PATCH v11 11/19] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing Dave Martin
2018-05-24 16:56 ` [PATCH v11 12/19] arm64/sve: Move read_zcr_features() out of cpufeature.h Dave Martin
2018-05-24 16:56 ` [PATCH v11 13/19] arm64/sve: Switch sve_pffr() argument from task to thread Dave Martin
2018-05-24 16:56 ` [PATCH v11 14/19] arm64/sve: Move sve_pffr() to fpsimd.h and make inline Dave Martin
2018-05-24 16:56 ` [PATCH v11 15/19] KVM: arm64: Save host SVE context as appropriate Dave Martin
2018-05-24 16:56 ` [PATCH v11 16/19] KVM: arm64: Remove eager host SVE state saving Dave Martin
2018-05-24 16:56 ` [PATCH v11 17/19] KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit() Dave Martin
2018-05-24 16:56 ` [PATCH v11 18/19] KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit() Dave Martin
2018-05-24 16:56 ` [PATCH v11 19/19] KVM: arm64: Invoke FPSIMD context switch trap from C 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=1527181008-13549-2-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;
as well as URLs for NNTP newsgroup(s).