Skip to content

Commit f130ac0

Browse files
mrutland-armwilldeacon
authored andcommitted
arm64: syscall: unmask DAIF earlier for SVCs
For a number of historical reasons, when handling SVCs we don't unmask DAIF in el0_svc() or el0_svc_compat(), and instead do so later in el0_svc_common(). This is unfortunate and makes it harder to make changes to the DAIF management in entry-common.c as we'd like to do as cleanup and preparation for FEAT_NMI support. We can move the DAIF unmasking to entry-common.c as long as we also hoist the fp_user_discard() logic, as reasoned below. We converted the syscall trace logic from assembly to C in commit: f37099b ("arm64: convert syscall trace logic to C") ... which was intended to have no functional change, and mirrored the existing assembly logic to avoid the risk of any functional regression. With the logic in C, it's clear that there is currently no reason to unmask DAIF so late within el0_svc_common(): * The thread flags are read prior to unmasking DAIF, but are not consumed until after DAIF is unmasked, and we don't perform a read-modify-write sequence of the thread flags for which we might need to serialize against an IPI modifying the flags. Similarly, for any thread flags set by other threads, whether DAIF is masked or not has no impact. The read_thread_flags() helpers performs a single-copy-atomic read of the flags, and so this can safely be moved after unmasking DAIF. * The pt_regs::orig_x0 and pt_regs::syscallno fields are neither consumed nor modified by the handler for any DAIF exception (e.g. these do not exist in the `perf_event_arm_regs` enum and are not sampled by perf in its IRQ handler). Thus, the manipulation of pt_regs::orig_x0 and pt_regs::syscallno can safely be moved after unmasking DAIF. Given the above, we can safely hoist unmasking of DAIF out of el0_svc_common(), and into its immediate callers: do_el0_svc() and do_el0_svc_compat(). Further: * In do_el0_svc(), we sample the syscall number from pt_regs::regs[8]. This is not modified by the handler for any DAIF exception, and thus can safely be moved after unmasking DAIF. As fp_user_discard() operates on the live FP/SVE/SME register state, this needs to occur before we clear DAIF.IF, as interrupts could result in preemption which would cause this state to become foreign. As fp_user_discard() is the first function called within do_el0_svc(), it has no dependency on other parts of do_el0_svc() and can be moved earlier so long as it is called prior to unmasking DAIF.IF. * In do_el0_svc_compat(), we sample the syscall number from pt_regs::regs[7]. This is not modified by the handler for any DAIF exception, and thus can safely be moved after unmasking DAIF. Compat threads cannot use SVE or SME, so there's no need for el0_svc_compat() to call fp_user_discard(). Given the above, we can safely hoist the unmasking of DAIF out of do_el0_svc() and do_el0_svc_compat(), and into their immediate callers: el0_svc() and el0_svc_compat(), so long a we also hoist fp_user_discard() into el0_svc(). Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Marc Zyngier <maz@kernel.org> Cc: Mark Brown <broonie@kernel.org> Cc: Will Deacon <will@kernel.org> Reviewed-by: Mark Brown <broonie@kernel.org> Link: https://lore.kernel.org/r/20230808101148.1064172-1-mark.rutland@arm.com Signed-off-by: Will Deacon <will@kernel.org>
1 parent 6eaae19 commit f130ac0

File tree

2 files changed

+32
-33
lines changed

2 files changed

+32
-33
lines changed

arch/arm64/kernel/entry-common.c

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,35 @@ static bool cortex_a76_erratum_1463225_debug_handler(struct pt_regs *regs)
355355
}
356356
#endif /* CONFIG_ARM64_ERRATUM_1463225 */
357357

358+
/*
359+
* As per the ABI exit SME streaming mode and clear the SVE state not
360+
* shared with FPSIMD on syscall entry.
361+
*/
362+
static inline void fp_user_discard(void)
363+
{
364+
/*
365+
* If SME is active then exit streaming mode. If ZA is active
366+
* then flush the SVE registers but leave userspace access to
367+
* both SVE and SME enabled, otherwise disable SME for the
368+
* task and fall through to disabling SVE too. This means
369+
* that after a syscall we never have any streaming mode
370+
* register state to track, if this changes the KVM code will
371+
* need updating.
372+
*/
373+
if (system_supports_sme())
374+
sme_smstop_sm();
375+
376+
if (!system_supports_sve())
377+
return;
378+
379+
if (test_thread_flag(TIF_SVE)) {
380+
unsigned int sve_vq_minus_one;
381+
382+
sve_vq_minus_one = sve_vq_from_vl(task_get_sve_vl(current)) - 1;
383+
sve_flush_live(true, sve_vq_minus_one);
384+
}
385+
}
386+
358387
UNHANDLED(el1t, 64, sync)
359388
UNHANDLED(el1t, 64, irq)
360389
UNHANDLED(el1t, 64, fiq)
@@ -644,6 +673,8 @@ static void noinstr el0_svc(struct pt_regs *regs)
644673
{
645674
enter_from_user_mode(regs);
646675
cortex_a76_erratum_1463225_svc_handler();
676+
fp_user_discard();
677+
local_daif_restore(DAIF_PROCCTX);
647678
do_el0_svc(regs);
648679
exit_to_user_mode(regs);
649680
}
@@ -783,6 +814,7 @@ static void noinstr el0_svc_compat(struct pt_regs *regs)
783814
{
784815
enter_from_user_mode(regs);
785816
cortex_a76_erratum_1463225_svc_handler();
817+
local_daif_restore(DAIF_PROCCTX);
786818
do_el0_svc_compat(regs);
787819
exit_to_user_mode(regs);
788820
}

arch/arm64/kernel/syscall.c

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include <linux/randomize_kstack.h>
99
#include <linux/syscalls.h>
1010

11-
#include <asm/daifflags.h>
1211
#include <asm/debug-monitors.h>
1312
#include <asm/exception.h>
1413
#include <asm/fpsimd.h>
@@ -101,8 +100,6 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
101100
* (Similarly for HVC and SMC elsewhere.)
102101
*/
103102

104-
local_daif_restore(DAIF_PROCCTX);
105-
106103
if (flags & _TIF_MTE_ASYNC_FAULT) {
107104
/*
108105
* Process the asynchronous tag check fault before the actual
@@ -153,38 +150,8 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
153150
syscall_trace_exit(regs);
154151
}
155152

156-
/*
157-
* As per the ABI exit SME streaming mode and clear the SVE state not
158-
* shared with FPSIMD on syscall entry.
159-
*/
160-
static inline void fp_user_discard(void)
161-
{
162-
/*
163-
* If SME is active then exit streaming mode. If ZA is active
164-
* then flush the SVE registers but leave userspace access to
165-
* both SVE and SME enabled, otherwise disable SME for the
166-
* task and fall through to disabling SVE too. This means
167-
* that after a syscall we never have any streaming mode
168-
* register state to track, if this changes the KVM code will
169-
* need updating.
170-
*/
171-
if (system_supports_sme())
172-
sme_smstop_sm();
173-
174-
if (!system_supports_sve())
175-
return;
176-
177-
if (test_thread_flag(TIF_SVE)) {
178-
unsigned int sve_vq_minus_one;
179-
180-
sve_vq_minus_one = sve_vq_from_vl(task_get_sve_vl(current)) - 1;
181-
sve_flush_live(true, sve_vq_minus_one);
182-
}
183-
}
184-
185153
void do_el0_svc(struct pt_regs *regs)
186154
{
187-
fp_user_discard();
188155
el0_svc_common(regs, regs->regs[8], __NR_syscalls, sys_call_table);
189156
}
190157

0 commit comments

Comments
 (0)