[PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Florian Bezdeka]

Hi all,

these are the necessary backports for dovetail 6.1 that would allow us to
enable the in-kernel FPU usage of switchtest. This is currently not
possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
context switching.

I have backported the necessary patches down to 5.10. That would allow
us to unconditionally enable the tests for the xenomai3-next branch. I
will send out the 5.15 and 5.10 series once we agreed that this version
(targeting 6.1) is correct.

Please note that there is a small problem with this series. Patch 1
removes the check/flag that was used by the dovetail enablement code to
detect in-band FPU usage. The new flag is implemented differently in
patch 2. Patch 2 also fixes the usage of that flag. I had no better
idea. Otherwise I would have to delete? the in-kernel fpu preemption
code (temporary).

Previous discussion can be found here:
https://lore.kernel.org/xenomai/cover.1758264904.git.jan.kiszka@siemens.com/T/#t

Best regards,
Florian

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Philippe Gerum <rpm@xenomai.org>

Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
Florian Bezdeka (3):
      arm64: fpsimd: Drop unneeded 'busy' flag
      arm64: fpsimd: Preserve/restore kernel mode NEON at context switch
      arm64/fpsimd: Avoid erroneous elide of user state reload

 arch/arm64/include/asm/processor.h   |   2 +
 arch/arm64/include/asm/simd.h        |  11 +-
 arch/arm64/include/asm/thread_info.h |   3 +-
 arch/arm64/kernel/fpsimd.c           | 193 ++++++++++++++++++++---------------
 4 files changed, 114 insertions(+), 95 deletions(-)
---
base-commit: 501e3ddd3ff59f522d9c7a1f2ee993620d5389f9
change-id: 20251119-flo-6-1-arm64-fpsimd-test-enablement-67294b3d8f0c

Best regards,
-- 
Florian Bezdeka <florian.bezdeka@siemens.com>

[PATCH RFC Dovetail 6.1 1/3] arm64: fpsimd: Drop unneeded 'busy' flag

[Florian Bezdeka]

[ Backport of commit 9b19700e623f ("arm64: fpsimd: Drop unneeded 'busy' flag") ]

Kernel mode NEON will preserve the user mode FPSIMD state by saving it
into the task struct before clobbering the registers. In order to avoid
the need for preserving kernel mode state too, we disallow nested use of
kernel mode NEON, i..e, use in softirq context while the interrupted
task context was using kernel mode NEON too.

Originally, this policy was implemented using a per-CPU flag which was
exposed via may_use_simd(), requiring the users of the kernel mode NEON
to deal with the possibility that it might return false, and having NEON
and non-NEON code paths. This policy was changed by commit
13150149aa6ded1 ("arm64: fpsimd: run kernel mode NEON with softirqs
disabled"), and now, softirq processing is disabled entirely instead,
and so may_use_simd() can never fail when called from task or softirq
context.

This means we can drop the fpsimd_context_busy flag entirely, and
instead, ensure that we disable softirq processing in places where we
formerly relied on the flag for preventing races in the FPSIMD preserve
routines.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Mark Brown <broonie@kernel.org>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20231208113218.3001940-7-ardb@google.com
[will: Folded in fix from CAMj1kXFhzbJRyWHELCivQW1yJaF=p07LLtbuyXYX3G1WtsdyQg@mail.gmail.com]
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm64/include/asm/simd.h | 11 +--------
 arch/arm64/kernel/fpsimd.c    | 53 +++++++++----------------------------------
 2 files changed, 12 insertions(+), 52 deletions(-)

diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
index 6a75d7ecdcaa2..8e86c9e70e483 100644
--- a/arch/arm64/include/asm/simd.h
+++ b/arch/arm64/include/asm/simd.h
@@ -12,8 +12,6 @@
 #include <linux/preempt.h>
 #include <linux/types.h>
 
-DECLARE_PER_CPU(bool, fpsimd_context_busy);
-
 #ifdef CONFIG_KERNEL_MODE_NEON
 
 /*
@@ -28,17 +26,10 @@ static __must_check inline bool may_use_simd(void)
 	/*
 	 * We must make sure that the SVE has been initialized properly
 	 * before using the SIMD in kernel.
-	 * fpsimd_context_busy is only set while preemption is disabled,
-	 * and is clear whenever preemption is enabled. Since
-	 * this_cpu_read() is atomic w.r.t. preemption, fpsimd_context_busy
-	 * cannot change under our feet -- if it's set we cannot be
-	 * migrated, and if it's clear we cannot be migrated to a CPU
-	 * where it is set.
 	 */
 	return !WARN_ON(!system_capabilities_finalized()) &&
 	       system_supports_fpsimd() &&
-	       !in_hardirq() && !irqs_disabled() && !in_nmi() &&
-	       !this_cpu_read(fpsimd_context_busy);
+	       !in_hardirq() && !irqs_disabled() && !in_nmi();
 }
 
 #else /* ! CONFIG_KERNEL_MODE_NEON */
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 0b5c2af07d82a..ab429ff629204 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -85,13 +85,13 @@
  * softirq kicks in. Upon vcpu_put(), KVM will save the vcpu FP state and
  * flag the register state as invalid.
  *
- * In order to allow softirq handlers to use FPSIMD, kernel_neon_begin() may
- * save the task's FPSIMD context back to task_struct from softirq context.
- * To prevent this from racing with the manipulation of the task's FPSIMD state
- * from task context and thereby corrupting the state, it is necessary to
- * protect any manipulation of a task's fpsimd_state or TIF_FOREIGN_FPSTATE
- * flag with {, __}get_cpu_fpsimd_context(). This will still allow softirqs to
- * run but prevent them to use FPSIMD.
+ * In order to allow softirq handlers to use FPSIMD, kernel_neon_begin() may be
+ * called from softirq context, which will save the task's FPSIMD context back
+ * to task_struct. To prevent this from racing with the manipulation of the
+ * task's FPSIMD state from task context and thereby corrupting the state, it
+ * is necessary to protect any manipulation of a task's fpsimd_state or
+ * TIF_FOREIGN_FPSTATE flag with get_cpu_fpsimd_context(), which will suspend
+ * softirq servicing entirely until put_cpu_fpsimd_context() is called.
  *
  * For a certain task, the sequence may look something like this:
  * - the task gets scheduled in; if both the task's fpsimd_cpu field
@@ -219,36 +219,17 @@ static inline void sme_free(struct task_struct *t) { }
 
 #endif
 
-DEFINE_PER_CPU(bool, fpsimd_context_busy);
-EXPORT_PER_CPU_SYMBOL(fpsimd_context_busy);
-
 static void fpsimd_bind_task_to_cpu(void);
 
-static void __get_cpu_fpsimd_context(void)
-{
-	bool busy = __this_cpu_xchg(fpsimd_context_busy, true);
-
-	WARN_ON(busy);
-}
-
-static void __put_cpu_fpsimd_context(void)
-{
-	bool busy = __this_cpu_xchg(fpsimd_context_busy, false);
-
-	WARN_ON(!busy); /* No matching get_cpu_fpsimd_context()? */
-}
-
 #ifdef CONFIG_DOVETAIL
 
 #define get_cpu_fpsimd_context(__flags)			\
 	do {						\
 		(__flags) = hard_preempt_disable();	\
-		__get_cpu_fpsimd_context();		\
 	} while (0)
 
 #define put_cpu_fpsimd_context(__flags)			\
 	do {						\
-		__put_cpu_fpsimd_context();		\
 		hard_preempt_enable(__flags);		\
 	} while (0)
 
@@ -273,9 +254,6 @@ void fpsimd_restore_current_oob(void)
  * The caller may freely manipulate the FPSIMD context metadata until
  * put_cpu_fpsimd_context() is called.
  *
- * The double-underscore version must only be called if you know the task
- * can't be preempted.
- *
  * On RT kernels local_bh_disable() is not sufficient because it only
  * serializes soft interrupt related sections via a local lock, but stays
  * preemptible. Disabling preemption is the right choice here as bottom
@@ -288,7 +266,6 @@ void fpsimd_restore_current_oob(void)
 			local_bh_disable();		\
 		else					\
 			preempt_disable();		\
-		__get_cpu_fpsimd_context();		\
 		(void)(__flags);			\
 	} while (0)
 
@@ -301,7 +278,6 @@ void fpsimd_restore_current_oob(void)
  */
 #define put_cpu_fpsimd_context(__flags)			\
 	do {						\
-		__put_cpu_fpsimd_context();		\
 		if (!IS_ENABLED(CONFIG_PREEMPT_RT))	\
 			local_bh_enable();		\
 		else					\
@@ -311,11 +287,6 @@ void fpsimd_restore_current_oob(void)
 
 #endif	/* !CONFIG_DOVETAIL */
 
-static bool have_cpu_fpsimd_context(void)
-{
-	return !preemptible() && __this_cpu_read(fpsimd_context_busy);
-}
-
 unsigned int task_get_vl(const struct task_struct *task, enum vec_type type)
 {
 	return task->thread.vl[type];
@@ -426,7 +397,8 @@ static void task_fpsimd_load(void)
 	bool restore_ffr;
 
 	WARN_ON(!system_supports_fpsimd());
-	WARN_ON(!hard_irqs_disabled() && !have_cpu_fpsimd_context());
+	WARN_ON(preemptible());
+	WARN_ON(dovetail_debug() && !hard_irqs_disabled());
 
 	/* Check if we should restore SVE first */
 	if (IS_ENABLED(CONFIG_ARM64_SVE) && test_thread_flag(TIF_SVE)) {
@@ -485,7 +457,8 @@ static void __fpsimd_save(void)
 	unsigned int vl;
 
 	WARN_ON(!system_supports_fpsimd());
-	WARN_ON(!hard_irqs_disabled() && !have_cpu_fpsimd_context());
+	WARN_ON(preemptible());
+	WARN_ON(dovetail_debug() && !hard_irqs_disabled());
 
 	if (test_thread_flag(TIF_FOREIGN_FPSTATE))
 		return;
@@ -1629,8 +1602,6 @@ void fpsimd_thread_switch(struct task_struct *next)
 
 	flags = hard_cond_local_irq_save();
 
-	__get_cpu_fpsimd_context();
-
 	/* Save unsaved fpsimd state, if any: */
 	__fpsimd_save();
 
@@ -1646,8 +1617,6 @@ void fpsimd_thread_switch(struct task_struct *next)
 	update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
 			       wrong_task || wrong_cpu);
 
-	__put_cpu_fpsimd_context();
-
 	hard_cond_local_irq_restore(flags);
 }
 

-- 
2.39.5

[PATCH RFC Dovetail 6.1 2/3] arm64: fpsimd: Preserve/restore kernel mode NEON at context switch

[Florian Bezdeka]

[ Backport of commit aefbab8e77eb ("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>
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm64/include/asm/processor.h   |  2 +
 arch/arm64/include/asm/thread_info.h |  3 +-
 arch/arm64/kernel/fpsimd.c           | 98 ++++++++++++++++++++++++++++--------
 3 files changed, 81 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 1b822e618bb4b..7e05173cbf133 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -167,6 +167,8 @@ struct thread_struct {
 	unsigned long		fault_address;	/* fault info */
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
+
+	struct user_fpsimd_state	kernel_fpsimd_state;
 #ifdef CONFIG_ARM64_PTR_AUTH
 	struct ptrauth_keys_user	keys_user;
 #ifdef CONFIG_ARM64_PTR_AUTH_KERNEL
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 589a38b9b5fa3..f9693e04265aa 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -91,7 +91,8 @@ int arch_dup_task_struct(struct task_struct *dst,
 #define TIF_TAGGED_ADDR		26	/* Allow tagged user addresses */
 #define TIF_SME			27	/* SME in use */
 #define TIF_SME_VL_INHERIT	28	/* Inherit SME vl_onexec across exec */
-#define TIF_MAYDAY		29	/* Emergency trap pending */
+#define TIF_KERNEL_FPSTATE	29	/* Task is in a kernel mode FPSIMD section */
+#define TIF_MAYDAY		30	/* Emergency trap pending */
 
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index ab429ff629204..4635c5592a236 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -398,6 +398,7 @@ static void task_fpsimd_load(void)
 
 	WARN_ON(!system_supports_fpsimd());
 	WARN_ON(preemptible());
+	WARN_ON(test_thread_flag(TIF_KERNEL_FPSTATE));
 	WARN_ON(dovetail_debug() && !hard_irqs_disabled());
 
 	/* Check if we should restore SVE first */
@@ -447,7 +448,7 @@ static void task_fpsimd_load(void)
  * than via current, if we are saving KVM state then it will have
  * ensured that the type of registers to save is set in last->to_save.
  */
-static void __fpsimd_save(void)
+static void fpsimd_save_user_state(void)
 {
 	struct fpsimd_last_state_struct const *last =
 		this_cpu_ptr(&fpsimd_last_state);
@@ -513,7 +514,7 @@ void fpsimd_save(void)
 	unsigned long flags;
 
 	flags = hard_cond_local_irq_save();
-	__fpsimd_save();
+	fpsimd_save_user_state();
 	hard_cond_local_irq_restore(flags);
 }
 
@@ -899,7 +900,7 @@ int vec_set_vector_length(struct task_struct *task, enum vec_type type,
 	if (task == current) {
 		get_cpu_fpsimd_context(irqflags);
 
-		__fpsimd_save();
+		fpsimd_save_user_state();
 	}
 
 	fpsimd_flush_task_state(task);
@@ -1592,6 +1593,16 @@ void do_fpsimd_exc(unsigned long esr, struct pt_regs *regs)
 	mark_trap_exit(ARM64_TRAP_FPE, regs);
 }
 
+static void fpsimd_load_kernel_state(struct task_struct *task)
+{
+	fpsimd_load_state(&task->thread.kernel_fpsimd_state);
+}
+
+static void fpsimd_save_kernel_state(struct task_struct *task)
+{
+	fpsimd_save_state(&task->thread.kernel_fpsimd_state);
+}
+
 void fpsimd_thread_switch(struct task_struct *next)
 {
 	bool wrong_task, wrong_cpu;
@@ -1603,19 +1614,28 @@ void fpsimd_thread_switch(struct task_struct *next)
 	flags = hard_cond_local_irq_save();
 
 	/* Save unsaved fpsimd state, if any: */
-	__fpsimd_save();
+	if (test_thread_flag(TIF_KERNEL_FPSTATE))
+		fpsimd_save_kernel_state(current);
+	else
+		fpsimd_save_user_state();
 
-	/*
-	 * Fix up TIF_FOREIGN_FPSTATE to correctly describe next's
-	 * state.  For kernel threads, FPSIMD registers are never loaded
-	 * and wrong_task and wrong_cpu will always be true.
-	 */
-	wrong_task = __this_cpu_read(fpsimd_last_state.st) !=
-					&next->thread.uw.fpsimd_state;
-	wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id();
+	if (test_tsk_thread_flag(next, TIF_KERNEL_FPSTATE)) {
+		fpsimd_load_kernel_state(next);
+		set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
+	} else {
+		/*
+		 * Fix up TIF_FOREIGN_FPSTATE to correctly describe next's
+		 * state.  For kernel threads, FPSIMD registers are never
+		 * loaded with user mode FPSIMD state and so wrong_task and
+		 * wrong_cpu will always be true.
+		 */
+		wrong_task = __this_cpu_read(fpsimd_last_state.st) !=
+			&next->thread.uw.fpsimd_state;
+		wrong_cpu = next->thread.fpsimd_cpu != smp_processor_id();
 
-	update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
-			       wrong_task || wrong_cpu);
+		update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE,
+				       wrong_task || wrong_cpu);
+	}
 
 	hard_cond_local_irq_restore(flags);
 }
@@ -1710,7 +1730,7 @@ void fpsimd_preserve_current_state(void)
 		return;
 
 	get_cpu_fpsimd_context(flags);
-	__fpsimd_save();
+	fpsimd_save_user_state();
 	put_cpu_fpsimd_context(flags);
 }
 
@@ -1923,7 +1943,7 @@ void fpsimd_save_and_flush_cpu_state(void)
 		return;
 	WARN_ON(preemptible());
 	get_cpu_fpsimd_context(flags);
-	__fpsimd_save();
+	fpsimd_save_user_state();
 	fpsimd_flush_cpu_state();
 	put_cpu_fpsimd_context(flags);
 }
@@ -1959,13 +1979,37 @@ void kernel_neon_begin(void)
 	get_cpu_fpsimd_context(flags);
 
 	/* Save unsaved fpsimd state, if any: */
-	__fpsimd_save();
+	if (test_thread_flag(TIF_KERNEL_FPSTATE)) {
+		BUG_ON(IS_ENABLED(CONFIG_PREEMPT_RT) || !in_serving_softirq());
+		fpsimd_save_kernel_state(current);
+	} else {
+		fpsimd_save_user_state();
+
+		/*
+		 * Set the thread flag so that the kernel mode FPSIMD state
+		 * will be context switched along with the rest of the task
+		 * state.
+		 *
+		 * On non-PREEMPT_RT, softirqs may interrupt task level kernel
+		 * mode FPSIMD, but the task will not be preemptible so setting
+		 * TIF_KERNEL_FPSTATE for those would be both wrong (as it
+		 * would mark the task context FPSIMD state as requiring a
+		 * context switch) and unnecessary.
+		 *
+		 * On PREEMPT_RT, softirqs are serviced from a separate thread,
+		 * which is scheduled as usual, and this guarantees that these
+		 * softirqs are not interrupting use of the FPSIMD in kernel
+		 * mode in task context. So in this case, setting the flag here
+		 * is always appropriate.
+		 */
+		if (IS_ENABLED(CONFIG_PREEMPT_RT) || !in_serving_softirq())
+			set_thread_flag(TIF_KERNEL_FPSTATE);
+	}
 
 	/* Invalidate any task state remaining in the fpsimd regs: */
 	fpsimd_flush_cpu_state();
 
-	if (dovetailing())
-		hard_cond_local_irq_restore(flags);
+	put_cpu_fpsimd_context(flags);
 }
 EXPORT_SYMBOL(kernel_neon_begin);
 
@@ -1985,7 +2029,19 @@ void kernel_neon_end(void)
 	if (!system_supports_fpsimd())
 		return;
 
-	put_cpu_fpsimd_context(flags);
+	/*
+	 * If we are returning from a nested use of kernel mode FPSIMD, restore
+	 * the task context kernel mode FPSIMD state. This can only happen when
+	 * running in softirq context on non-PREEMPT_RT.
+	 */
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT) && in_serving_softirq() &&
+	    test_thread_flag(TIF_KERNEL_FPSTATE)) {
+		flags = hard_cond_local_irq_save();
+		fpsimd_load_kernel_state(current);
+		hard_cond_local_irq_restore(flags);
+	} else {
+		clear_thread_flag(TIF_KERNEL_FPSTATE);
+	}
 }
 EXPORT_SYMBOL(kernel_neon_end);
 
@@ -2179,7 +2235,7 @@ void fpsimd_suspend_inband(void)
 	 * inband kernel context currently using the fpu by a thread which resumes
 	 * on the oob stage.
 	 */
-	if (__this_cpu_read(fpsimd_context_busy)) {
+	if (test_thread_flag(TIF_KERNEL_FPSTATE)) {
 		fpsimd_save_state(kfpu);
 		set_thread_flag(TIF_KERNEL_FP_PREEMPTED);
 	}

-- 
2.39.5

[PATCH RFC Dovetail 6.1 3/3] arm64/fpsimd: Avoid erroneous elide of user state reload

[Florian Bezdeka]

[ Backport of commit e92bee9f861b ("arm64/fpsimd: Avoid erroneous elide
of user state reload") ]

TIF_FOREIGN_FPSTATE is a 'convenience' flag that should reflect whether
the current CPU holds the most recent user mode FP/SIMD state of the
current task. It combines two conditions:
- whether the current CPU's FP/SIMD state belongs to the task;
- whether that state is the most recent associated with the task (as a
  task may have executed on other CPUs as well).

When a task is scheduled in and TIF_KERNEL_FPSTATE is set, it means the
task was in a kernel mode NEON section when it was scheduled out, and so
the kernel mode FP/SIMD state is restored. Since this implies that the
current CPU is *not* holding the most recent user mode FP/SIMD state of
the current task, the TIF_FOREIGN_FPSTATE flag is set too, so that the
user mode FP/SIMD state is reloaded from memory when returning to
userland.

However, the task may be scheduled out after completing the kernel mode
NEON section, but before returning to userland. When this happens, the
TIF_FOREIGN_FPSTATE flag will not be preserved, but will be set as usual
the next time the task is scheduled in, and will be based on the above
conditions.

This means that, rather than setting TIF_FOREIGN_FPSTATE when scheduling
in a task with TIF_KERNEL_FPSTATE set, the underlying state should be
updated so that TIF_FOREIGN_FPSTATE will assume the expected value as a
result.

So instead, call fpsimd_flush_cpu_state(), which takes care of this.

Closes: https://lore.kernel.org/all/cb8822182231850108fa43e0446a4c7f@kernel.org
Reported-by: Johannes Nixdorf <mixi@shadowice.org>
Fixes: aefbab8e77eb ("arm64: fpsimd: Preserve/restore kernel mode NEON at context switch")
Cc: Mark Brown <broonie@kernel.org>
Cc: Dave Martin <Dave.Martin@arm.com>
Cc: Janne Grunau <j@jannau.net>
Cc: stable@vger.kernel.org
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Tested-by: Janne Grunau <j@jannau.net>
Tested-by: Johannes Nixdorf <mixi@shadowice.org>
Reviewed-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20240522091335.335346-2-ardb+git@google.com
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
 arch/arm64/kernel/fpsimd.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 4635c5592a236..aabe468e74c46 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1603,6 +1603,27 @@ static void fpsimd_save_kernel_state(struct task_struct *task)
 	fpsimd_save_state(&task->thread.kernel_fpsimd_state);
 }
 
+/*
+ * Invalidate any task's FPSIMD state that is present on this cpu.
+ * The FPSIMD context should be acquired with get_cpu_fpsimd_context()
+ * before calling this function.
+ */
+static void fpsimd_flush_cpu_state(void)
+{
+	WARN_ON(!system_supports_fpsimd());
+	__this_cpu_write(fpsimd_last_state.st, NULL);
+
+	/*
+	 * Leaving streaming mode enabled will cause issues for any kernel
+	 * NEON and leaving streaming mode or ZA enabled may increase power
+	 * consumption.
+	 */
+	if (system_supports_sme())
+		sme_smstop();
+
+	set_thread_flag(TIF_FOREIGN_FPSTATE);
+}
+
 void fpsimd_thread_switch(struct task_struct *next)
 {
 	bool wrong_task, wrong_cpu;
@@ -1621,7 +1642,7 @@ void fpsimd_thread_switch(struct task_struct *next)
 
 	if (test_tsk_thread_flag(next, TIF_KERNEL_FPSTATE)) {
 		fpsimd_load_kernel_state(next);
-		set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE);
+		fpsimd_flush_cpu_state();
 	} else {
 		/*
 		 * Fix up TIF_FOREIGN_FPSTATE to correctly describe next's
@@ -1910,27 +1931,6 @@ void fpsimd_flush_task_state(struct task_struct *t)
 	barrier();
 }
 
-/*
- * Invalidate any task's FPSIMD state that is present on this cpu.
- * The FPSIMD context should be acquired with get_cpu_fpsimd_context()
- * before calling this function.
- */
-static void fpsimd_flush_cpu_state(void)
-{
-	WARN_ON(!system_supports_fpsimd());
-	__this_cpu_write(fpsimd_last_state.st, NULL);
-
-	/*
-	 * Leaving streaming mode enabled will cause issues for any kernel
-	 * NEON and leaving streaming mode or ZA enabled may increase power
-	 * consumption.
-	 */
-	if (system_supports_sme())
-		sme_smstop();
-
-	set_thread_flag(TIF_FOREIGN_FPSTATE);
-}
-
 /*
  * Save the FPSIMD state to memory and invalidate cpu view.
  * This function must be called with preemption disabled.

-- 
2.39.5

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Philippe Gerum]

Florian Bezdeka <florian.bezdeka@siemens.com> writes:

> Hi all,
>
> these are the necessary backports for dovetail 6.1 that would allow us to
> enable the in-kernel FPU usage of switchtest. This is currently not
> possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
> context switching.
>
> I have backported the necessary patches down to 5.10. That would allow
> us to unconditionally enable the tests for the xenomai3-next branch. I
> will send out the 5.15 and 5.10 series once we agreed that this version
> (targeting 6.1) is correct.
>
> Please note that there is a small problem with this series. Patch 1
> removes the check/flag that was used by the dovetail enablement code to
> detect in-band FPU usage. The new flag is implemented differently in
> patch 2. Patch 2 also fixes the usage of that flag. I had no better
> idea. Otherwise I would have to delete? the in-kernel fpu preemption
> code (temporary).

TIF_KERNEL_FPSTATE is not covering a preemption of a softirq handler
using the NEON unit by an oob thread when PREEMPT_RT is disabled, kernel
or user for that matter. i.e. It looks like this aspect was overlooked
during the busy -> TIF_KERNEL_FPSTATE transition for Dovetail, so a
broader fix may be required. We may have to use a marker which covers
any type of preempted context.

-- 
Philippe.

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Florian Bezdeka]

On Mon, 2025-12-01 at 17:29 +0100, Philippe Gerum wrote:
> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
> 
> > Hi all,
> > 
> > these are the necessary backports for dovetail 6.1 that would allow us to
> > enable the in-kernel FPU usage of switchtest. This is currently not
> > possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
> > context switching.
> > 
> > I have backported the necessary patches down to 5.10. That would allow
> > us to unconditionally enable the tests for the xenomai3-next branch. I
> > will send out the 5.15 and 5.10 series once we agreed that this version
> > (targeting 6.1) is correct.
> > 
> > Please note that there is a small problem with this series. Patch 1
> > removes the check/flag that was used by the dovetail enablement code to
> > detect in-band FPU usage. The new flag is implemented differently in
> > patch 2. Patch 2 also fixes the usage of that flag. I had no better
> > idea. Otherwise I would have to delete? the in-kernel fpu preemption
> > code (temporary).
> 
> TIF_KERNEL_FPSTATE is not covering a preemption of a softirq handler
> using the NEON unit by an oob thread when PREEMPT_RT is disabled, kernel
> or user for that matter. i.e. It looks like this aspect was overlooked
> during the busy -> TIF_KERNEL_FPSTATE transition for Dovetail, so a
> broader fix may be required. We may have to use a marker which covers
> any type of preempted context.
> 

I agree, but this is something that we should address in addition / on
top, no?

This issue seems to affect all versions of dovetail. If we do the
backports first we can address the issue on the same feature set. That
should simplify the back-porting again. Did I miss something?

Best regards,
Florian

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Philippe Gerum]

Florian Bezdeka <florian.bezdeka@siemens.com> writes:

> On Mon, 2025-12-01 at 17:29 +0100, Philippe Gerum wrote:
>> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>> 
>> > Hi all,
>> > 
>> > these are the necessary backports for dovetail 6.1 that would allow us to
>> > enable the in-kernel FPU usage of switchtest. This is currently not
>> > possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
>> > context switching.
>> > 
>> > I have backported the necessary patches down to 5.10. That would allow
>> > us to unconditionally enable the tests for the xenomai3-next branch. I
>> > will send out the 5.15 and 5.10 series once we agreed that this version
>> > (targeting 6.1) is correct.
>> > 
>> > Please note that there is a small problem with this series. Patch 1
>> > removes the check/flag that was used by the dovetail enablement code to
>> > detect in-band FPU usage. The new flag is implemented differently in
>> > patch 2. Patch 2 also fixes the usage of that flag. I had no better
>> > idea. Otherwise I would have to delete? the in-kernel fpu preemption
>> > code (temporary).
>> 
>> TIF_KERNEL_FPSTATE is not covering a preemption of a softirq handler
>> using the NEON unit by an oob thread when PREEMPT_RT is disabled, kernel
>> or user for that matter. i.e. It looks like this aspect was overlooked
>> during the busy -> TIF_KERNEL_FPSTATE transition for Dovetail, so a
>> broader fix may be required. We may have to use a marker which covers
>> any type of preempted context.
>> 
>
> I agree, but this is something that we should address in addition / on
> top, no?
>

Yep, would make backporting easier.

> This issue seems to affect all versions of dovetail. If we do the
> backports first we can address the issue on the same feature set. That
> should simplify the back-porting again. Did I miss something?
>

Agreed.

-- 
Philippe.

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Florian Bezdeka]

On Thu, 2025-12-11 at 13:56 +0100, Philippe Gerum wrote:
> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
> 
> > On Mon, 2025-12-01 at 17:29 +0100, Philippe Gerum wrote:
> > > Florian Bezdeka <florian.bezdeka@siemens.com> writes:
> > > 
> > > > Hi all,
> > > > 
> > > > these are the necessary backports for dovetail 6.1 that would allow us to
> > > > enable the in-kernel FPU usage of switchtest. This is currently not
> > > > possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
> > > > context switching.
> > > > 
> > > > I have backported the necessary patches down to 5.10. That would allow
> > > > us to unconditionally enable the tests for the xenomai3-next branch. I
> > > > will send out the 5.15 and 5.10 series once we agreed that this version
> > > > (targeting 6.1) is correct.
> > > > 
> > > > Please note that there is a small problem with this series. Patch 1
> > > > removes the check/flag that was used by the dovetail enablement code to
> > > > detect in-band FPU usage. The new flag is implemented differently in
> > > > patch 2. Patch 2 also fixes the usage of that flag. I had no better
> > > > idea. Otherwise I would have to delete? the in-kernel fpu preemption
> > > > code (temporary).
> > > 
> > > TIF_KERNEL_FPSTATE is not covering a preemption of a softirq handler
> > > using the NEON unit by an oob thread when PREEMPT_RT is disabled, kernel
> > > or user for that matter. i.e. It looks like this aspect was overlooked
> > > during the busy -> TIF_KERNEL_FPSTATE transition for Dovetail, so a
> > > broader fix may be required. We may have to use a marker which covers
> > > any type of preempted context.
> > > 
> > 
> > I agree, but this is something that we should address in addition / on
> > top, no?
> > 
> 
> Yep, would make backporting easier.
> 
> > This issue seems to affect all versions of dovetail. If we do the
> > backports first we can address the issue on the same feature set. That
> > should simplify the back-porting again. Did I miss something?
> > 
> 
> Agreed.

How do we proceed now? Once applied to 6.1 I can send out the versions
for 5.15 and 5.10 so Jan can apply them as well.

Meanwhile I found some time to read the FPU code around
kernel_neon_begin() again. The problem for Dovetail is that there is no
task context for softirqs in the !PREEMPT_RT case. Seems we need some
kind of a per-cpu flag for this scenario to track the FPU usage. Any
thoughts on your end already?

> 
> -- 
> Philippe.

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Jan Kiszka]

On 15.12.25 12:19, Florian Bezdeka wrote:
> On Thu, 2025-12-11 at 13:56 +0100, Philippe Gerum wrote:
>> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>>
>>> On Mon, 2025-12-01 at 17:29 +0100, Philippe Gerum wrote:
>>>> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>>>>
>>>>> Hi all,
>>>>>
>>>>> these are the necessary backports for dovetail 6.1 that would allow us to
>>>>> enable the in-kernel FPU usage of switchtest. This is currently not
>>>>> possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
>>>>> context switching.
>>>>>
>>>>> I have backported the necessary patches down to 5.10. That would allow
>>>>> us to unconditionally enable the tests for the xenomai3-next branch. I
>>>>> will send out the 5.15 and 5.10 series once we agreed that this version
>>>>> (targeting 6.1) is correct.
>>>>>
>>>>> Please note that there is a small problem with this series. Patch 1
>>>>> removes the check/flag that was used by the dovetail enablement code to
>>>>> detect in-band FPU usage. The new flag is implemented differently in
>>>>> patch 2. Patch 2 also fixes the usage of that flag. I had no better
>>>>> idea. Otherwise I would have to delete? the in-kernel fpu preemption
>>>>> code (temporary).
>>>>
>>>> TIF_KERNEL_FPSTATE is not covering a preemption of a softirq handler
>>>> using the NEON unit by an oob thread when PREEMPT_RT is disabled, kernel
>>>> or user for that matter. i.e. It looks like this aspect was overlooked
>>>> during the busy -> TIF_KERNEL_FPSTATE transition for Dovetail, so a
>>>> broader fix may be required. We may have to use a marker which covers
>>>> any type of preempted context.
>>>>
>>>
>>> I agree, but this is something that we should address in addition / on
>>> top, no?
>>>
>>
>> Yep, would make backporting easier.
>>
>>> This issue seems to affect all versions of dovetail. If we do the
>>> backports first we can address the issue on the same feature set. That
>>> should simplify the back-porting again. Did I miss something?
>>>
>>
>> Agreed.
> 
> How do we proceed now? Once applied to 6.1 I can send out the versions
> for 5.15 and 5.10 so Jan can apply them as well.
> 

Philippe, do you plan to update 6.1 soon? Or should I already take over
6.1, given that 6.18 is on the horizon and you would likely stop 6.1 for
evl and dovetail soon?

Jan

> Meanwhile I found some time to read the FPU code around
> kernel_neon_begin() again. The problem for Dovetail is that there is no
> task context for softirqs in the !PREEMPT_RT case. Seems we need some
> kind of a per-cpu flag for this scenario to track the FPU usage. Any
> thoughts on your end already?
> 
>>
>> -- 
>> Philippe.

-- 
Siemens AG, Foundational Technologies
Linux Expert Center

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Philippe Gerum]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 15.12.25 12:19, Florian Bezdeka wrote:
>> On Thu, 2025-12-11 at 13:56 +0100, Philippe Gerum wrote:
>>> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>>>
>>>> On Mon, 2025-12-01 at 17:29 +0100, Philippe Gerum wrote:
>>>>> Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> these are the necessary backports for dovetail 6.1 that would allow us to
>>>>>> enable the in-kernel FPU usage of switchtest. This is currently not
>>>>>> possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
>>>>>> context switching.
>>>>>>
>>>>>> I have backported the necessary patches down to 5.10. That would allow
>>>>>> us to unconditionally enable the tests for the xenomai3-next branch. I
>>>>>> will send out the 5.15 and 5.10 series once we agreed that this version
>>>>>> (targeting 6.1) is correct.
>>>>>>
>>>>>> Please note that there is a small problem with this series. Patch 1
>>>>>> removes the check/flag that was used by the dovetail enablement code to
>>>>>> detect in-band FPU usage. The new flag is implemented differently in
>>>>>> patch 2. Patch 2 also fixes the usage of that flag. I had no better
>>>>>> idea. Otherwise I would have to delete? the in-kernel fpu preemption
>>>>>> code (temporary).
>>>>>
>>>>> TIF_KERNEL_FPSTATE is not covering a preemption of a softirq handler
>>>>> using the NEON unit by an oob thread when PREEMPT_RT is disabled, kernel
>>>>> or user for that matter. i.e. It looks like this aspect was overlooked
>>>>> during the busy -> TIF_KERNEL_FPSTATE transition for Dovetail, so a
>>>>> broader fix may be required. We may have to use a marker which covers
>>>>> any type of preempted context.
>>>>>
>>>>
>>>> I agree, but this is something that we should address in addition / on
>>>> top, no?
>>>>
>>>
>>> Yep, would make backporting easier.
>>>
>>>> This issue seems to affect all versions of dovetail. If we do the
>>>> backports first we can address the issue on the same feature set. That
>>>> should simplify the back-porting again. Did I miss something?
>>>>
>>>
>>> Agreed.
>> 
>> How do we proceed now? Once applied to 6.1 I can send out the versions
>> for 5.15 and 5.10 so Jan can apply them as well.
>> 
>
> Philippe, do you plan to update 6.1 soon? Or should I already take over
> 6.1, given that 6.18 is on the horizon and you would likely stop 6.1 for
> evl and dovetail soon?
>

Please take over.

-- 
Philippe.

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Jan Kiszka]

On 19.11.25 15:25, Florian Bezdeka wrote:
> Hi all,
> 
> these are the necessary backports for dovetail 6.1 that would allow us to
> enable the in-kernel FPU usage of switchtest. This is currently not
> possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
> context switching.
> 
> I have backported the necessary patches down to 5.10. That would allow
> us to unconditionally enable the tests for the xenomai3-next branch. I
> will send out the 5.15 and 5.10 series once we agreed that this version
> (targeting 6.1) is correct.
> 
> Please note that there is a small problem with this series. Patch 1
> removes the check/flag that was used by the dovetail enablement code to
> detect in-band FPU usage. The new flag is implemented differently in
> patch 2. Patch 2 also fixes the usage of that flag. I had no better
> idea. Otherwise I would have to delete? the in-kernel fpu preemption
> code (temporary).
> 
> Previous discussion can be found here:
> https://lore.kernel.org/xenomai/cover.1758264904.git.jan.kiszka@siemens.com/T/#t
> 
> Best regards,
> Florian
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Philippe Gerum <rpm@xenomai.org>
> 
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
> Florian Bezdeka (3):
>       arm64: fpsimd: Drop unneeded 'busy' flag
>       arm64: fpsimd: Preserve/restore kernel mode NEON at context switch
>       arm64/fpsimd: Avoid erroneous elide of user state reload
> 
>  arch/arm64/include/asm/processor.h   |   2 +
>  arch/arm64/include/asm/simd.h        |  11 +-
>  arch/arm64/include/asm/thread_info.h |   3 +-
>  arch/arm64/kernel/fpsimd.c           | 193 ++++++++++++++++++++---------------
>  4 files changed, 114 insertions(+), 95 deletions(-)
> ---
> base-commit: 501e3ddd3ff59f522d9c7a1f2ee993620d5389f9
> change-id: 20251119-flo-6-1-arm64-fpsimd-test-enablement-67294b3d8f0c
> 
> Best regards,

Ok, I've started to integrate those bits over a refresher 6.1 kernel.
Please see v6.1.y-cip-dovetail-rebase, I had to resolve some conflicts
again.

But as we discussed and as Philippe also pointed out, we need some
solution for the softirq oob preemption as well, not only in 6.1.

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center

Re: [PATCH RFC Dovetail 6.1 0/3] arm64: Enable in-kernel FPU / FPSIMD usage

[Florian Bezdeka]

On Wed, 2025-12-17 at 17:51 +0100, Jan Kiszka wrote:
> On 19.11.25 15:25, Florian Bezdeka wrote:
> > Hi all,
> > 
> > these are the necessary backports for dovetail 6.1 that would allow us to
> > enable the in-kernel FPU usage of switchtest. This is currently not
> > possible as dovetail < 6.12 does not allow / implement the necessary fpsimd
> > context switching.
> > 
> > I have backported the necessary patches down to 5.10. That would allow
> > us to unconditionally enable the tests for the xenomai3-next branch. I
> > will send out the 5.15 and 5.10 series once we agreed that this version
> > (targeting 6.1) is correct.
> > 
> > Please note that there is a small problem with this series. Patch 1
> > removes the check/flag that was used by the dovetail enablement code to
> > detect in-band FPU usage. The new flag is implemented differently in
> > patch 2. Patch 2 also fixes the usage of that flag. I had no better
> > idea. Otherwise I would have to delete? the in-kernel fpu preemption
> > code (temporary).
> > 
> > Previous discussion can be found here:
> > https://lore.kernel.org/xenomai/cover.1758264904.git.jan.kiszka@siemens.com/T/#t
> > 
> > Best regards,
> > Florian
> > 
> > Cc: Jan Kiszka <jan.kiszka@siemens.com>
> > Cc: Philippe Gerum <rpm@xenomai.org>
> > 
> > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > ---
> > Florian Bezdeka (3):
> >       arm64: fpsimd: Drop unneeded 'busy' flag
> >       arm64: fpsimd: Preserve/restore kernel mode NEON at context switch
> >       arm64/fpsimd: Avoid erroneous elide of user state reload
> > 
> >  arch/arm64/include/asm/processor.h   |   2 +
> >  arch/arm64/include/asm/simd.h        |  11 +-
> >  arch/arm64/include/asm/thread_info.h |   3 +-
> >  arch/arm64/kernel/fpsimd.c           | 193 ++++++++++++++++++++---------------
> >  4 files changed, 114 insertions(+), 95 deletions(-)
> > ---
> > base-commit: 501e3ddd3ff59f522d9c7a1f2ee993620d5389f9
> > change-id: 20251119-flo-6-1-arm64-fpsimd-test-enablement-67294b3d8f0c
> > 
> > Best regards,
> 
> Ok, I've started to integrate those bits over a refresher 6.1 kernel.
> Please see v6.1.y-cip-dovetail-rebase, I had to resolve some conflicts
> again.
> 
> But as we discussed and as Philippe also pointed out, we need some
> solution for the softirq oob preemption as well, not only in 6.1.
> 

I revisited this problem today. Seems that we have the same problem in
all Dovetail versions and on all architectures. At least I could not
find something that would prevent the same happening on x86.

Did I miss something?

Florian