Skip to content

Commit 8c845e2

Browse files
brooniewilldeacon
authored andcommitted
arm64/sve: Leave SVE enabled on syscall if we don't context switch
The syscall ABI says that the SVE register state not shared with FPSIMD may not be preserved on syscall, and this is the only mechanism we have in the ABI to stop tracking the extra SVE state for a process. Currently we do this unconditionally by means of disabling SVE for the process on syscall, causing userspace to take a trap to EL1 if it uses SVE again. These extra traps result in a noticeable overhead for using SVE instead of FPSIMD in some workloads, especially for simple syscalls where we can return directly to userspace and would not otherwise need to update the floating point registers. Tests with fp-pidbench show an approximately 70% overhead on a range of implementations when SVE is in use - while this is an extreme and entirely artificial benchmark it is clear that there is some useful room for improvement here. Now that we have the ability to track the decision about what to save seprately to TIF_SVE we can improve things by leaving TIF_SVE enabled on syscall but only saving the FPSIMD registers if we are in a syscall. This means that if we need to restore the register state from memory (eg, after a context switch or kernel mode NEON) we will drop TIF_SVE and reenable traps for userspace but if we can just return to userspace then traps will remain disabled. Since our current implementation and hence ABI has the effect of zeroing all the SVE register state not shared with FPSIMD on syscall we replace the disabling of TIF_SVE with a flush of the non-shared register state, this means that there is still some overhead for syscalls when SVE is in use but it is very much reduced. Signed-off-by: Mark Brown <[email protected]> Reviewed-by: Catalin Marinas <[email protected]> Reviewed-by: Marc Zyngier <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Will Deacon <[email protected]>
1 parent bbc6172 commit 8c845e2

File tree

2 files changed

+12
-15
lines changed

2 files changed

+12
-15
lines changed

arch/arm64/kernel/fpsimd.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,13 @@ static void fpsimd_save(void)
481481
if (test_thread_flag(TIF_FOREIGN_FPSTATE))
482482
return;
483483

484-
if ((last->to_save == FP_STATE_CURRENT && test_thread_flag(TIF_SVE)) ||
484+
/*
485+
* If a task is in a syscall the ABI allows us to only
486+
* preserve the state shared with FPSIMD so don't bother
487+
* saving the full SVE state in that case.
488+
*/
489+
if ((last->to_save == FP_STATE_CURRENT && test_thread_flag(TIF_SVE) &&
490+
!in_syscall(current_pt_regs())) ||
485491
last->to_save == FP_STATE_SVE) {
486492
save_sve_regs = true;
487493
save_ffr = true;

arch/arm64/kernel/syscall.c

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -183,21 +183,12 @@ static inline void fp_user_discard(void)
183183
if (!system_supports_sve())
184184
return;
185185

186-
/*
187-
* If SME is not active then disable SVE, the registers will
188-
* be cleared when userspace next attempts to access them and
189-
* we do not need to track the SVE register state until then.
190-
*/
191-
clear_thread_flag(TIF_SVE);
186+
if (test_thread_flag(TIF_SVE)) {
187+
unsigned int sve_vq_minus_one;
192188

193-
/*
194-
* task_fpsimd_load() won't be called to update CPACR_EL1 in
195-
* ret_to_user unless TIF_FOREIGN_FPSTATE is still set, which only
196-
* happens if a context switch or kernel_neon_begin() or context
197-
* modification (sigreturn, ptrace) intervenes.
198-
* So, ensure that CPACR_EL1 is already correct for the fast-path case.
199-
*/
200-
sve_user_disable();
189+
sve_vq_minus_one = sve_vq_from_vl(task_get_sve_vl(current)) - 1;
190+
sve_flush_live(true, sve_vq_minus_one);
191+
}
201192
}
202193

203194
void do_el0_svc(struct pt_regs *regs)

0 commit comments

Comments
 (0)