Skip to content

Commit aefbab8

Browse files
ardbiesheuvelwilldeacon
authored andcommitted
arm64: fpsimd: Preserve/restore kernel mode NEON at context switch
Currently, the FPSIMD register file is not preserved and restored along with the general registers on exception entry/exit or context switch. For this reason, we disable preemption when enabling FPSIMD for kernel mode use in task context, and suspend the processing of softirqs so that there are no concurrent uses in the kernel. (Kernel mode FPSIMD may not be used at all in other contexts). Disabling preemption while doing CPU intensive work on inputs of potentially unbounded size is bad for real-time performance, which is why we try and ensure that SIMD crypto code does not operate on more than ~4k at a time, which is an arbitrary limit and requires assembler code to implement efficiently. We can avoid the need for disabling preemption if we can ensure that any in-kernel users of the NEON will not lose the FPSIMD register state across a context switch. And given that disabling softirqs implicitly disables preemption as well, we will also have to ensure that a softirq that runs code using FPSIMD can safely interrupt an in-kernel user. So introduce a thread_info flag TIF_KERNEL_FPSTATE, and modify the context switch hook for FPSIMD to preserve and restore the kernel mode FPSIMD to/from struct thread_struct when it is set. This avoids any scheduling blackouts due to prolonged use of FPSIMD in kernel mode, without the need for manual yielding. In order to support softirq processing while FPSIMD is being used in kernel task context, use the same flag to decide whether the kernel mode FPSIMD state needs to be preserved and restored before allowing FPSIMD to be used in softirq context. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Reviewed-by: Mark Brown <broonie@kernel.org> Reviewed-by: Mark Rutland <mark.rutland@arm.com> Link: https://lore.kernel.org/r/20231208113218.3001940-8-ardb@google.com Signed-off-by: Will Deacon <will@kernel.org>
1 parent 9b19700 commit aefbab8

3 files changed

Lines changed: 77 additions & 18 deletions

File tree

arch/arm64/include/asm/processor.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ struct thread_struct {
167167
unsigned long fault_address; /* fault info */
168168
unsigned long fault_code; /* ESR_EL1 value */
169169
struct debug_info debug; /* debugging */
170+
171+
struct user_fpsimd_state kernel_fpsimd_state;
170172
#ifdef CONFIG_ARM64_PTR_AUTH
171173
struct ptrauth_keys_user keys_user;
172174
#ifdef CONFIG_ARM64_PTR_AUTH_KERNEL

arch/arm64/include/asm/thread_info.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ void arch_setup_new_exec(void);
8080
#define TIF_TAGGED_ADDR 26 /* Allow tagged user addresses */
8181
#define TIF_SME 27 /* SME in use */
8282
#define TIF_SME_VL_INHERIT 28 /* Inherit SME vl_onexec across exec */
83+
#define TIF_KERNEL_FPSTATE 29 /* Task is in a kernel mode FPSIMD section */
8384

8485
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
8586
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)

arch/arm64/kernel/fpsimd.c

Lines changed: 74 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ static void task_fpsimd_load(void)
357357

358358
WARN_ON(!system_supports_fpsimd());
359359
WARN_ON(preemptible());
360+
WARN_ON(test_thread_flag(TIF_KERNEL_FPSTATE));
360361

361362
if (system_supports_sve() || system_supports_sme()) {
362363
switch (current->thread.fp_type) {
@@ -379,7 +380,7 @@ static void task_fpsimd_load(void)
379380
default:
380381
/*
381382
* This indicates either a bug in
382-
* fpsimd_save() or memory corruption, we
383+
* fpsimd_save_user_state() or memory corruption, we
383384
* should always record an explicit format
384385
* when we save. We always at least have the
385386
* memory allocated for FPSMID registers so
@@ -430,7 +431,7 @@ static void task_fpsimd_load(void)
430431
* than via current, if we are saving KVM state then it will have
431432
* ensured that the type of registers to save is set in last->to_save.
432433
*/
433-
static void fpsimd_save(void)
434+
static void fpsimd_save_user_state(void)
434435
{
435436
struct cpu_fp_state const *last =
436437
this_cpu_ptr(&fpsimd_last_state);
@@ -861,7 +862,7 @@ int vec_set_vector_length(struct task_struct *task, enum vec_type type,
861862
if (task == current) {
862863
get_cpu_fpsimd_context();
863864

864-
fpsimd_save();
865+
fpsimd_save_user_state();
865866
}
866867

867868
fpsimd_flush_task_state(task);
@@ -1473,6 +1474,16 @@ void do_fpsimd_exc(unsigned long esr, struct pt_regs *regs)
14731474
current);
14741475
}
14751476

1477+
static void fpsimd_load_kernel_state(struct task_struct *task)
1478+
{
1479+
fpsimd_load_state(&task->thread.kernel_fpsimd_state);
1480+
}
1481+
1482+
static void fpsimd_save_kernel_state(struct task_struct *task)
1483+
{
1484+
fpsimd_save_state(&task->thread.kernel_fpsimd_state);
1485+
}
1486+
14761487
void fpsimd_thread_switch(struct task_struct *next)
14771488
{
14781489
bool wrong_task, wrong_cpu;
@@ -1483,19 +1494,28 @@ void fpsimd_thread_switch(struct task_struct *next)
14831494
WARN_ON_ONCE(!irqs_disabled());
14841495

14851496
/* Save unsaved fpsimd state, if any: */
1486-
fpsimd_save();
1497+
if (test_thread_flag(TIF_KERNEL_FPSTATE))
1498+
fpsimd_save_kernel_state(current);
1499+
else
1500+
fpsimd_save_user_state();
14871501

1488-
/*
1489-
* Fix up TIF_FOREIGN_FPSTATE to correctly describe next's
1490-
* state. For kernel threads, FPSIMD registers are never loaded
1491-
* and wrong_task and wrong_cpu will always be true.
1492-
*/
1493-
wrong_task = __this_cpu_read(fpsimd_last_state.st) !=
1494-
&next->thread.uw.fpsimd_state;
1495-
wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id();
1502+
if (test_tsk_thread_flag(next, TIF_KERNEL_FPSTATE)) {
1503+
fpsimd_load_kernel_state(next);
1504+
set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
1505+
} else {
1506+
/*
1507+
* Fix up TIF_FOREIGN_FPSTATE to correctly describe next's
1508+
* state. For kernel threads, FPSIMD registers are never
1509+
* loaded with user mode FPSIMD state and so wrong_task and
1510+
* wrong_cpu will always be true.
1511+
*/
1512+
wrong_task = __this_cpu_read(fpsimd_last_state.st) !=
1513+
&next->thread.uw.fpsimd_state;
1514+
wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id();
14961515

1497-
update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
1498-
wrong_task || wrong_cpu);
1516+
update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
1517+
wrong_task || wrong_cpu);
1518+
}
14991519
}
15001520

15011521
static void fpsimd_flush_thread_vl(enum vec_type type)
@@ -1585,7 +1605,7 @@ void fpsimd_preserve_current_state(void)
15851605
return;
15861606

15871607
get_cpu_fpsimd_context();
1588-
fpsimd_save();
1608+
fpsimd_save_user_state();
15891609
put_cpu_fpsimd_context();
15901610
}
15911611

@@ -1803,7 +1823,7 @@ void fpsimd_save_and_flush_cpu_state(void)
18031823
return;
18041824
WARN_ON(preemptible());
18051825
local_irq_save(flags);
1806-
fpsimd_save();
1826+
fpsimd_save_user_state();
18071827
fpsimd_flush_cpu_state();
18081828
local_irq_restore(flags);
18091829
}
@@ -1837,10 +1857,37 @@ void kernel_neon_begin(void)
18371857
get_cpu_fpsimd_context();
18381858

18391859
/* Save unsaved fpsimd state, if any: */
1840-
fpsimd_save();
1860+
if (test_thread_flag(TIF_KERNEL_FPSTATE)) {
1861+
BUG_ON(IS_ENABLED(CONFIG_PREEMPT_RT) || !in_serving_softirq());
1862+
fpsimd_save_kernel_state(current);
1863+
} else {
1864+
fpsimd_save_user_state();
1865+
1866+
/*
1867+
* Set the thread flag so that the kernel mode FPSIMD state
1868+
* will be context switched along with the rest of the task
1869+
* state.
1870+
*
1871+
* On non-PREEMPT_RT, softirqs may interrupt task level kernel
1872+
* mode FPSIMD, but the task will not be preemptible so setting
1873+
* TIF_KERNEL_FPSTATE for those would be both wrong (as it
1874+
* would mark the task context FPSIMD state as requiring a
1875+
* context switch) and unnecessary.
1876+
*
1877+
* On PREEMPT_RT, softirqs are serviced from a separate thread,
1878+
* which is scheduled as usual, and this guarantees that these
1879+
* softirqs are not interrupting use of the FPSIMD in kernel
1880+
* mode in task context. So in this case, setting the flag here
1881+
* is always appropriate.
1882+
*/
1883+
if (IS_ENABLED(CONFIG_PREEMPT_RT) || !in_serving_softirq())
1884+
set_thread_flag(TIF_KERNEL_FPSTATE);
1885+
}
18411886

18421887
/* Invalidate any task state remaining in the fpsimd regs: */
18431888
fpsimd_flush_cpu_state();
1889+
1890+
put_cpu_fpsimd_context();
18441891
}
18451892
EXPORT_SYMBOL_GPL(kernel_neon_begin);
18461893

@@ -1858,7 +1905,16 @@ void kernel_neon_end(void)
18581905
if (!system_supports_fpsimd())
18591906
return;
18601907

1861-
put_cpu_fpsimd_context();
1908+
/*
1909+
* If we are returning from a nested use of kernel mode FPSIMD, restore
1910+
* the task context kernel mode FPSIMD state. This can only happen when
1911+
* running in softirq context on non-PREEMPT_RT.
1912+
*/
1913+
if (!IS_ENABLED(CONFIG_PREEMPT_RT) && in_serving_softirq() &&
1914+
test_thread_flag(TIF_KERNEL_FPSTATE))
1915+
fpsimd_load_kernel_state(current);
1916+
else
1917+
clear_thread_flag(TIF_KERNEL_FPSTATE);
18621918
}
18631919
EXPORT_SYMBOL_GPL(kernel_neon_end);
18641920

0 commit comments

Comments
 (0)