From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 02/33] arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after invalidating cpu regs
Date: Fri, 1 Jun 2018 16:27:16 +0100 [thread overview]
Message-ID: <20180601152747.23613-3-marc.zyngier@arm.com> (raw)
In-Reply-To: <20180601152747.23613-1-marc.zyngier@arm.com>
From: Dave Martin <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>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
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 87a35364e750..12e1c967c7b5 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.17.1
next prev parent reply other threads:[~2018-06-01 15:27 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-01 15:27 [GIT PULL] KVM/ARM updates for 4.18 Marc Zyngier
2018-06-01 15:27 ` [PATCH 01/33] arm64: KVM: Use lm_alias() for kvm_ksym_ref() Marc Zyngier
2018-06-01 15:27 ` Marc Zyngier [this message]
2018-06-01 15:27 ` [PATCH 03/33] thread_info: Add update_thread_flag() helpers Marc Zyngier
2018-06-01 15:27 ` [PATCH 04/33] arm64: Use update{,_tsk}_thread_flag() Marc Zyngier
2018-06-01 15:27 ` [PATCH 05/33] KVM: arm/arm64: Introduce kvm_arch_vcpu_run_pid_change Marc Zyngier
2018-06-01 15:27 ` [PATCH 06/33] KVM: arm64: Convert lazy FPSIMD context switch trap to C Marc Zyngier
2018-06-01 15:27 ` [PATCH 07/33] arm64: fpsimd: Generalise context saving for non-task contexts Marc Zyngier
2018-06-01 15:27 ` [PATCH 08/33] arm64: fpsimd: Avoid FPSIMD context leakage for the init task Marc Zyngier
2018-06-01 15:27 ` [PATCH 09/33] arm64: fpsimd: Eliminate task->mm checks Marc Zyngier
2018-06-01 15:27 ` [PATCH 10/33] arm64/sve: Refactor user SVE trap maintenance for external use Marc Zyngier
2018-06-01 15:27 ` [PATCH 11/33] KVM: arm64: Repurpose vcpu_arch.debug_flags for general-purpose flags Marc Zyngier
2018-06-01 15:27 ` [PATCH 12/33] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing Marc Zyngier
2018-06-01 15:27 ` [PATCH 13/33] arm64/sve: Move read_zcr_features() out of cpufeature.h Marc Zyngier
2018-06-01 15:27 ` [PATCH 14/33] arm64/sve: Switch sve_pffr() argument from task to thread Marc Zyngier
2018-06-01 15:27 ` [PATCH 15/33] arm64/sve: Move sve_pffr() to fpsimd.h and make inline Marc Zyngier
2018-06-01 15:27 ` [PATCH 16/33] KVM: arm64: Save host SVE context as appropriate Marc Zyngier
2018-06-01 15:27 ` [PATCH 17/33] KVM: arm64: Remove eager host SVE state saving Marc Zyngier
2018-06-01 15:27 ` [PATCH 18/33] KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit() Marc Zyngier
2018-06-01 15:27 ` [PATCH 19/33] KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit() Marc Zyngier
2018-06-01 15:27 ` [PATCH 20/33] KVM: arm64: Invoke FPSIMD context switch trap from C Marc Zyngier
2018-06-01 15:27 ` [PATCH 21/33] KVM: arm/arm64: Set dist->spis to NULL after kfree Marc Zyngier
2018-06-01 15:27 ` [PATCH 22/33] KVM: arm/arm64: Document KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION Marc Zyngier
2018-06-01 15:27 ` [PATCH 23/33] KVM: arm/arm64: Replace the single rdist region by a list Marc Zyngier
2018-06-01 15:27 ` [PATCH 24/33] KVM: arm/arm64: Helper to locate free rdist index Marc Zyngier
2018-06-01 15:27 ` [PATCH 25/33] KVM: arm/arm64: Revisit Redistributor TYPER last bit computation Marc Zyngier
2018-06-01 15:27 ` [PATCH 26/33] KVM: arm/arm64: Adapt vgic_v3_check_base to multiple rdist regions Marc Zyngier
2018-06-01 15:27 ` [PATCH 27/33] KVM: arm/arm64: Helper to register a new redistributor region Marc Zyngier
2018-06-01 15:27 ` [PATCH 28/33] KVM: arm/arm64: Remove kvm_vgic_vcpu_early_init Marc Zyngier
2018-06-01 15:27 ` [PATCH 29/33] KVM: arm/arm64: Check vcpu redist base before registering an iodev Marc Zyngier
2018-06-01 15:27 ` [PATCH 30/33] KVM: arm/arm64: Check all vcpu redistributors are set on map_resources Marc Zyngier
2018-06-01 15:27 ` [PATCH 31/33] KVM: arm/arm64: Add KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION Marc Zyngier
2018-06-01 15:27 ` [PATCH 32/33] KVM: arm/arm64: Implement KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION Marc Zyngier
2018-06-01 15:27 ` [PATCH 33/33] KVM: arm/arm64: Bump VGIC_V3_MAX_CPUS to 512 Marc Zyngier
2018-06-01 17:18 ` [GIT PULL] KVM/ARM updates for 4.18 Paolo Bonzini
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=20180601152747.23613-3-marc.zyngier@arm.com \
--to=marc.zyngier@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