[PATCH 0/5] arm64: Move kernel mode FPSIMD buffer to the stack

[PATCH 0/5] arm64: Move kernel mode FPSIMD buffer to the stack

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Move the buffer for preserving/restoring the kernel mode FPSIMD state on a
context switch out of struct thread_struct, and onto the stack, so that
the memory cost is not imposed needlessly on all tasks in the system.

Patches #1 - #3 contains some prepwork so that patch #4 can tighten the
rules around permitted usage patterns of kernel_neon_begin() and
kernel_neon_end(). This permits #5 to provide a stack buffer to
kernel_neon_begin() transparently, in a manner that ensures that it will
remain available until after the associated call to kernel_neon_end()
returns.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Brown <broonie@kernel.org>

Ard Biesheuvel (5):
  crypto/arm64: aes-ce-ccm - Avoid pointless yield of the NEON unit
  crypto/arm64: sm4-ce-ccm - Avoid pointless yield of the NEON unit
  crypto/arm64: sm4-ce-gcm - Avoid pointless yield of the NEON unit
  arm64/fpsimd: Require kernel NEON begin/end calls from the same scope
  arm64/fpsimd: Allocate kernel mode FP/SIMD buffers on the stack

 arch/arm64/crypto/aes-ce-ccm-glue.c |  5 +--
 arch/arm64/crypto/sm4-ce-ccm-glue.c | 10 ++----
 arch/arm64/crypto/sm4-ce-gcm-glue.c | 10 ++----
 arch/arm64/include/asm/neon.h       |  7 ++--
 arch/arm64/include/asm/processor.h  |  2 +-
 arch/arm64/kernel/fpsimd.c          | 34 +++++++++++++-------
 6 files changed, 34 insertions(+), 34 deletions(-)


base-commit: f83ec76bf285bea5727f478a68b894f5543ca76e
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH 1/5] crypto/arm64: aes-ce-ccm - Avoid pointless yield of the NEON unit

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Kernel mode NEON sections are now preemptible on arm64, and so there is
no need to yield it explicitly in order to prevent scheduling latency
spikes.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/crypto/aes-ce-ccm-glue.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
index 2d791d51891b..2eb4e76cabc3 100644
--- a/arch/arm64/crypto/aes-ce-ccm-glue.c
+++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
@@ -114,11 +114,8 @@ static u32 ce_aes_ccm_auth_data(u8 mac[], u8 const in[], u32 abytes,
 			in += adv;
 			abytes -= adv;
 
-			if (unlikely(rem)) {
-				kernel_neon_end();
-				kernel_neon_begin();
+			if (unlikely(rem))
 				macp = 0;
-			}
 		} else {
 			u32 l = min(AES_BLOCK_SIZE - macp, abytes);
 
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH 2/5] crypto/arm64: sm4-ce-ccm - Avoid pointless yield of the NEON unit

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Kernel mode NEON sections are now preemptible on arm64, and so there is
no need to yield it when calling APIs that may sleep.

Also, move the calls to kernel_neon_end() to the same scope as
kernel_neon_begin(). This is needed for a subsequent change where a
stack buffer is allocated transparently and passed to
kernel_neon_begin().

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/crypto/sm4-ce-ccm-glue.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/crypto/sm4-ce-ccm-glue.c b/arch/arm64/crypto/sm4-ce-ccm-glue.c
index e9cc1c1364ec..f9771ab2a05f 100644
--- a/arch/arm64/crypto/sm4-ce-ccm-glue.c
+++ b/arch/arm64/crypto/sm4-ce-ccm-glue.c
@@ -179,11 +179,7 @@ static int ccm_crypt(struct aead_request *req, struct skcipher_walk *walk,
 				 walk->src.virt.addr, walk->iv,
 				 walk->nbytes - tail, mac);
 
-		kernel_neon_end();
-
 		err = skcipher_walk_done(walk, tail);
-
-		kernel_neon_begin();
 	}
 
 	if (walk->nbytes) {
@@ -193,15 +189,13 @@ static int ccm_crypt(struct aead_request *req, struct skcipher_walk *walk,
 
 		sm4_ce_ccm_final(rkey_enc, ctr0, mac);
 
-		kernel_neon_end();
-
 		err = skcipher_walk_done(walk, 0);
 	} else {
 		sm4_ce_ccm_final(rkey_enc, ctr0, mac);
-
-		kernel_neon_end();
 	}
 
+	kernel_neon_end();
+
 	return err;
 }
 
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH 3/5] crypto/arm64: sm4-ce-gcm - Avoid pointless yield of the NEON unit

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Kernel mode NEON sections are now preemptible on arm64, and so there is
no need to yield it when calling APIs that may sleep.

Also, move the calls to kernel_neon_end() to the same scope as
kernel_neon_begin(). This is needed for a subsequent change where a
stack buffer is allocated transparently and passed to
kernel_neon_begin().

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/crypto/sm4-ce-gcm-glue.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/crypto/sm4-ce-gcm-glue.c b/arch/arm64/crypto/sm4-ce-gcm-glue.c
index c2ea3d5f690b..170cd0151385 100644
--- a/arch/arm64/crypto/sm4-ce-gcm-glue.c
+++ b/arch/arm64/crypto/sm4-ce-gcm-glue.c
@@ -165,26 +165,22 @@ static int gcm_crypt(struct aead_request *req, struct skcipher_walk *walk,
 					       ctx->ghash_table,
 					       (const u8 *)&lengths);
 
-			kernel_neon_end();
-
-			return skcipher_walk_done(walk, 0);
+			err = skcipher_walk_done(walk, 0);
+			goto out;
 		}
 
 		sm4_ce_pmull_gcm_crypt(ctx->key.rkey_enc, dst, src, iv,
 				       walk->nbytes - tail, ghash,
 				       ctx->ghash_table, NULL);
 
-		kernel_neon_end();
-
 		err = skcipher_walk_done(walk, tail);
-
-		kernel_neon_begin();
 	}
 
 	sm4_ce_pmull_gcm_crypt(ctx->key.rkey_enc, NULL, NULL, iv,
 			       walk->nbytes, ghash, ctx->ghash_table,
 			       (const u8 *)&lengths);
 
+out:
 	kernel_neon_end();
 
 	return err;
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH 4/5] arm64/fpsimd: Require kernel NEON begin/end calls from the same scope

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

In preparation for making kernel_neon_begin() allocate a stack based
buffer for stashing the kernel mode FP/SIMD state on a context switch,
redefine kernel_neon_begin() and kernel_neon_end() as macros, in a way
that requires them to be called from the same scope, and therefore,
running from the same stack frame.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/neon.h |  7 +++++--
 arch/arm64/kernel/fpsimd.c    | 12 ++++++------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/neon.h b/arch/arm64/include/asm/neon.h
index d4b1d172a79b..4e24f1058b55 100644
--- a/arch/arm64/include/asm/neon.h
+++ b/arch/arm64/include/asm/neon.h
@@ -13,7 +13,10 @@
 
 #define cpu_has_neon()		system_supports_fpsimd()
 
-void kernel_neon_begin(void);
-void kernel_neon_end(void);
+void __kernel_neon_begin(void);
+void __kernel_neon_end(void);
+
+#define kernel_neon_begin()	do { __kernel_neon_begin()
+#define kernel_neon_end()	__kernel_neon_end(); } while (0)
 
 #endif /* ! __ASM_NEON_H */
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index c37f02d7194e..d7eb073d1366 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1833,7 +1833,7 @@ void fpsimd_save_and_flush_cpu_state(void)
  * The caller may freely use the FPSIMD registers until kernel_neon_end() is
  * called.
  */
-void kernel_neon_begin(void)
+void __kernel_neon_begin(void)
 {
 	if (WARN_ON(!system_supports_fpsimd()))
 		return;
@@ -1875,7 +1875,7 @@ void kernel_neon_begin(void)
 
 	put_cpu_fpsimd_context();
 }
-EXPORT_SYMBOL_GPL(kernel_neon_begin);
+EXPORT_SYMBOL_GPL(__kernel_neon_begin);
 
 /*
  * kernel_neon_end(): give the CPU FPSIMD registers back to the current task
@@ -1886,7 +1886,7 @@ EXPORT_SYMBOL_GPL(kernel_neon_begin);
  * The caller must not use the FPSIMD registers after this function is called,
  * unless kernel_neon_begin() is called again in the meantime.
  */
-void kernel_neon_end(void)
+void __kernel_neon_end(void)
 {
 	if (!system_supports_fpsimd())
 		return;
@@ -1902,7 +1902,7 @@ void kernel_neon_end(void)
 	else
 		clear_thread_flag(TIF_KERNEL_FPSTATE);
 }
-EXPORT_SYMBOL_GPL(kernel_neon_end);
+EXPORT_SYMBOL_GPL(__kernel_neon_end);
 
 #ifdef CONFIG_EFI
 
@@ -1936,7 +1936,7 @@ void __efi_fpsimd_begin(void)
 	WARN_ON(preemptible());
 
 	if (may_use_simd()) {
-		kernel_neon_begin();
+		__kernel_neon_begin();
 	} else {
 		/*
 		 * If !efi_sve_state, SVE can't be in use yet and doesn't need
@@ -1985,7 +1985,7 @@ void __efi_fpsimd_end(void)
 		return;
 
 	if (!efi_fpsimd_state_used) {
-		kernel_neon_end();
+		__kernel_neon_end();
 	} else {
 		if (system_supports_sve() && efi_sve_state_used) {
 			bool ffr = true;
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH 5/5] arm64/fpsimd: Allocate kernel mode FP/SIMD buffers on the stack

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Commit aefbab8e77eb16b5

  ("arm64: fpsimd: Preserve/restore kernel mode NEON at context switch")

added a 'kernel_fpsimd_state' field to struct thread_struct, which is
the arch-specific portion of struct task_struct, and is allocated for
each task in the system. The size of this field is 528 bytes, resulting
in non-trivial bloat of task_struct, and the resulting memory overhead
may impact performance on systems with many processes.

This allocation is only used if the task is scheduled out or interrupted
by a softirq while using the FP/SIMD unit in kernel mode, and given that
calls to kernel_neon_begin() and kernel_neon_end() are now guaranteed to
originate from the same lexical scope, it is possible to transparently
allocate this buffer on the caller's stack instead.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/neon.h      |  4 +--
 arch/arm64/include/asm/processor.h |  2 +-
 arch/arm64/kernel/fpsimd.c         | 26 ++++++++++++++------
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/neon.h b/arch/arm64/include/asm/neon.h
index 4e24f1058b55..acaac98ff449 100644
--- a/arch/arm64/include/asm/neon.h
+++ b/arch/arm64/include/asm/neon.h
@@ -13,10 +13,10 @@
 
 #define cpu_has_neon()		system_supports_fpsimd()
 
-void __kernel_neon_begin(void);
+void __kernel_neon_begin(struct user_fpsimd_state *);
 void __kernel_neon_end(void);
 
-#define kernel_neon_begin()	do { __kernel_neon_begin()
+#define kernel_neon_begin()	do { __kernel_neon_begin(&(struct user_fpsimd_state){})
 #define kernel_neon_end()	__kernel_neon_end(); } while (0)
 
 #endif /* ! __ASM_NEON_H */
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 61d62bfd5a7b..226e635c53d9 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -172,7 +172,7 @@ struct thread_struct {
 	unsigned long		fault_code;	/* ESR_EL1 value */
 	struct debug_info	debug;		/* debugging */
 
-	struct user_fpsimd_state	kernel_fpsimd_state;
+	struct user_fpsimd_state	*kernel_fpsimd_state;
 	unsigned int			kernel_fpsimd_cpu;
 #ifdef CONFIG_ARM64_PTR_AUTH
 	struct ptrauth_keys_user	keys_user;
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index d7eb073d1366..919c53a26484 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1488,21 +1488,23 @@ static void fpsimd_load_kernel_state(struct task_struct *task)
 	 * Elide the load if this CPU holds the most recent kernel mode
 	 * FPSIMD context of the current task.
 	 */
-	if (last->st == &task->thread.kernel_fpsimd_state &&
+	if (last->st == task->thread.kernel_fpsimd_state &&
 	    task->thread.kernel_fpsimd_cpu == smp_processor_id())
 		return;
 
-	fpsimd_load_state(&task->thread.kernel_fpsimd_state);
+	fpsimd_load_state(task->thread.kernel_fpsimd_state);
 }
 
 static void fpsimd_save_kernel_state(struct task_struct *task)
 {
 	struct cpu_fp_state cpu_fp_state = {
-		.st		= &task->thread.kernel_fpsimd_state,
+		.st		= task->thread.kernel_fpsimd_state,
 		.to_save	= FP_STATE_FPSIMD,
 	};
 
-	fpsimd_save_state(&task->thread.kernel_fpsimd_state);
+	BUG_ON(!cpu_fp_state.st);
+
+	fpsimd_save_state(task->thread.kernel_fpsimd_state);
 	fpsimd_bind_state_to_cpu(&cpu_fp_state);
 
 	task->thread.kernel_fpsimd_cpu = smp_processor_id();
@@ -1773,6 +1775,7 @@ void fpsimd_update_current_state(struct user_fpsimd_state const *state)
 void fpsimd_flush_task_state(struct task_struct *t)
 {
 	t->thread.fpsimd_cpu = NR_CPUS;
+	t->thread.kernel_fpsimd_state = NULL;
 	/*
 	 * If we don't support fpsimd, bail out after we have
 	 * reset the fpsimd_cpu for this task and clear the
@@ -1833,7 +1836,7 @@ void fpsimd_save_and_flush_cpu_state(void)
  * The caller may freely use the FPSIMD registers until kernel_neon_end() is
  * called.
  */
-void __kernel_neon_begin(void)
+void __kernel_neon_begin(struct user_fpsimd_state *s)
 {
 	if (WARN_ON(!system_supports_fpsimd()))
 		return;
@@ -1849,6 +1852,13 @@ void __kernel_neon_begin(void)
 	} else {
 		fpsimd_save_user_state();
 
+		/*
+		 * Record the caller provided buffer as the kernel mode FP/SIMD
+		 * buffer for this task, so that the state can be preserved and
+		 * restored on a context switch.
+		 */
+		current->thread.kernel_fpsimd_state = s;
+
 		/*
 		 * Set the thread flag so that the kernel mode FPSIMD state
 		 * will be context switched along with the rest of the task
@@ -1899,8 +1909,8 @@ void __kernel_neon_end(void)
 	if (!IS_ENABLED(CONFIG_PREEMPT_RT) && in_serving_softirq() &&
 	    test_thread_flag(TIF_KERNEL_FPSTATE))
 		fpsimd_load_kernel_state(current);
-	else
-		clear_thread_flag(TIF_KERNEL_FPSTATE);
+	else if (test_and_clear_thread_flag(TIF_KERNEL_FPSTATE))
+		current->thread.kernel_fpsimd_state = NULL;
 }
 EXPORT_SYMBOL_GPL(__kernel_neon_end);
 
@@ -1936,7 +1946,7 @@ void __efi_fpsimd_begin(void)
 	WARN_ON(preemptible());
 
 	if (may_use_simd()) {
-		__kernel_neon_begin();
+		__kernel_neon_begin(&efi_fpsimd_state);
 	} else {
 		/*
 		 * If !efi_sve_state, SVE can't be in use yet and doesn't need
-- 
2.51.0.384.g4c02a37b29-goog

Re: [PATCH 1/5] crypto/arm64: aes-ce-ccm - Avoid pointless yield of the NEON unit

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

On Thu, Sep 18, 2025 at 08:35:41AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Kernel mode NEON sections are now preemptible on arm64, and so there is
> no need to yield it explicitly in order to prevent scheduling latency
> spikes.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/5] crypto/arm64: sm4-ce-ccm - Avoid pointless yield of the NEON unit

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

On Thu, Sep 18, 2025 at 08:35:42AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Kernel mode NEON sections are now preemptible on arm64, and so there is
> no need to yield it when calling APIs that may sleep.
> 
> Also, move the calls to kernel_neon_end() to the same scope as
> kernel_neon_begin(). This is needed for a subsequent change where a
> stack buffer is allocated transparently and passed to
> kernel_neon_begin().

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/5] crypto/arm64: sm4-ce-gcm - Avoid pointless yield of the NEON unit

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

On Thu, Sep 18, 2025 at 08:35:43AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Kernel mode NEON sections are now preemptible on arm64, and so there is
> no need to yield it when calling APIs that may sleep.
> 
> Also, move the calls to kernel_neon_end() to the same scope as
> kernel_neon_begin(). This is needed for a subsequent change where a
> stack buffer is allocated transparently and passed to
> kernel_neon_begin().

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/5] arm64: Move kernel mode FPSIMD buffer to the stack

[Eric Biggers]

On Thu, Sep 18, 2025 at 08:35:40AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Move the buffer for preserving/restoring the kernel mode FPSIMD state on a
> context switch out of struct thread_struct, and onto the stack, so that
> the memory cost is not imposed needlessly on all tasks in the system.
> 
> Patches #1 - #3 contains some prepwork so that patch #4 can tighten the
> rules around permitted usage patterns of kernel_neon_begin() and
> kernel_neon_end(). This permits #5 to provide a stack buffer to
> kernel_neon_begin() transparently, in a manner that ensures that it will
> remain available until after the associated call to kernel_neon_end()
> returns.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> 
> Ard Biesheuvel (5):
>   crypto/arm64: aes-ce-ccm - Avoid pointless yield of the NEON unit
>   crypto/arm64: sm4-ce-ccm - Avoid pointless yield of the NEON unit
>   crypto/arm64: sm4-ce-gcm - Avoid pointless yield of the NEON unit
>   arm64/fpsimd: Require kernel NEON begin/end calls from the same scope
>   arm64/fpsimd: Allocate kernel mode FP/SIMD buffers on the stack
> 
>  arch/arm64/crypto/aes-ce-ccm-glue.c |  5 +--
>  arch/arm64/crypto/sm4-ce-ccm-glue.c | 10 ++----
>  arch/arm64/crypto/sm4-ce-gcm-glue.c | 10 ++----
>  arch/arm64/include/asm/neon.h       |  7 ++--
>  arch/arm64/include/asm/processor.h  |  2 +-
>  arch/arm64/kernel/fpsimd.c          | 34 +++++++++++++-------
>  6 files changed, 34 insertions(+), 34 deletions(-)

This looks like the right decision: saving 528 bytes per task is
significant.  528 bytes is a lot to allocate on the stack too, but
functions that use the NEON registers are either leaf functions or very
close to being leaf functions, so it should be okay.

The implementation is a bit unusual, though:

   #define kernel_neon_begin()	do { __kernel_neon_begin(&(struct user_fpsimd_state){})
   #define kernel_neon_end()	__kernel_neon_end(); } while (0)

It works, but normally macros don't start or end code blocks behind the
scenes like this.  Perhaps it should be more like s390's
kernel_fpu_begin(), where the caller provides the buffer that the
registers are stored in?  

- Eric

Re: [PATCH 0/5] arm64: Move kernel mode FPSIMD buffer to the stack

[Ard Biesheuvel]

On Fri, 19 Sept 2025 at 21:32, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Thu, Sep 18, 2025 at 08:35:40AM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > Move the buffer for preserving/restoring the kernel mode FPSIMD state on a
> > context switch out of struct thread_struct, and onto the stack, so that
> > the memory cost is not imposed needlessly on all tasks in the system.
> >
> > Patches #1 - #3 contains some prepwork so that patch #4 can tighten the
> > rules around permitted usage patterns of kernel_neon_begin() and
> > kernel_neon_end(). This permits #5 to provide a stack buffer to
> > kernel_neon_begin() transparently, in a manner that ensures that it will
> > remain available until after the associated call to kernel_neon_end()
> > returns.
> >
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Brown <broonie@kernel.org>
> >
> > Ard Biesheuvel (5):
> >   crypto/arm64: aes-ce-ccm - Avoid pointless yield of the NEON unit
> >   crypto/arm64: sm4-ce-ccm - Avoid pointless yield of the NEON unit
> >   crypto/arm64: sm4-ce-gcm - Avoid pointless yield of the NEON unit
> >   arm64/fpsimd: Require kernel NEON begin/end calls from the same scope
> >   arm64/fpsimd: Allocate kernel mode FP/SIMD buffers on the stack
> >
> >  arch/arm64/crypto/aes-ce-ccm-glue.c |  5 +--
> >  arch/arm64/crypto/sm4-ce-ccm-glue.c | 10 ++----
> >  arch/arm64/crypto/sm4-ce-gcm-glue.c | 10 ++----
> >  arch/arm64/include/asm/neon.h       |  7 ++--
> >  arch/arm64/include/asm/processor.h  |  2 +-
> >  arch/arm64/kernel/fpsimd.c          | 34 +++++++++++++-------
> >  6 files changed, 34 insertions(+), 34 deletions(-)
>
> This looks like the right decision: saving 528 bytes per task is
> significant.  528 bytes is a lot to allocate on the stack too, but
> functions that use the NEON registers are either leaf functions or very
> close to being leaf functions, so it should be okay.
>

Indeed.

> The implementation is a bit unusual, though:
>
>    #define kernel_neon_begin()  do { __kernel_neon_begin(&(struct user_fpsimd_state){})
>    #define kernel_neon_end()    __kernel_neon_end(); } while (0)
>
> It works, but normally macros don't start or end code blocks behind the
> scenes like this.

That is kind of the point, as it restricts the use of them to an idiom
that guarantees that the stack variable lives long enough.

> Perhaps it should be more like s390's
> kernel_fpu_begin(), where the caller provides the buffer that the
> registers are stored in?
>

If we're happy to change the API on both arm64 and ARM, then we could
make it more explicit. It's a lot more work, though.

Re: [PATCH 0/5] arm64: Move kernel mode FPSIMD buffer to the stack

[Kees Cook]

On Fri, Sep 19, 2025 at 02:32:21PM -0500, Eric Biggers wrote:
> On Thu, Sep 18, 2025 at 08:35:40AM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> > 
> > Move the buffer for preserving/restoring the kernel mode FPSIMD state on a
> > context switch out of struct thread_struct, and onto the stack, so that
> > the memory cost is not imposed needlessly on all tasks in the system.
> > 
> > Patches #1 - #3 contains some prepwork so that patch #4 can tighten the
> > rules around permitted usage patterns of kernel_neon_begin() and
> > kernel_neon_end(). This permits #5 to provide a stack buffer to
> > kernel_neon_begin() transparently, in a manner that ensures that it will
> > remain available until after the associated call to kernel_neon_end()
> > returns.
> > 
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > 
> > Ard Biesheuvel (5):
> >   crypto/arm64: aes-ce-ccm - Avoid pointless yield of the NEON unit
> >   crypto/arm64: sm4-ce-ccm - Avoid pointless yield of the NEON unit
> >   crypto/arm64: sm4-ce-gcm - Avoid pointless yield of the NEON unit
> >   arm64/fpsimd: Require kernel NEON begin/end calls from the same scope
> >   arm64/fpsimd: Allocate kernel mode FP/SIMD buffers on the stack
> > 
> >  arch/arm64/crypto/aes-ce-ccm-glue.c |  5 +--
> >  arch/arm64/crypto/sm4-ce-ccm-glue.c | 10 ++----
> >  arch/arm64/crypto/sm4-ce-gcm-glue.c | 10 ++----
> >  arch/arm64/include/asm/neon.h       |  7 ++--
> >  arch/arm64/include/asm/processor.h  |  2 +-
> >  arch/arm64/kernel/fpsimd.c          | 34 +++++++++++++-------
> >  6 files changed, 34 insertions(+), 34 deletions(-)
> 
> This looks like the right decision: saving 528 bytes per task is
> significant.  528 bytes is a lot to allocate on the stack too, but
> functions that use the NEON registers are either leaf functions or very
> close to being leaf functions, so it should be okay.
> 
> The implementation is a bit unusual, though:
> 
>    #define kernel_neon_begin()	do { __kernel_neon_begin(&(struct user_fpsimd_state){})
>    #define kernel_neon_end()	__kernel_neon_end(); } while (0)
> 
> It works, but normally macros don't start or end code blocks behind the
> scenes like this.  Perhaps it should be more like s390's
> kernel_fpu_begin(), where the caller provides the buffer that the
> registers are stored in?  

We've done stuff like this in the past, but I did wonder if we could use
any of the cleanup.h style logic to do this instead of the split
do/while? I think this would work, totally untested:

DEFINE_LOCK_GUARD_0(neon,
		    __kernel_neon_begin(&(struct user_fpsimd_state){}),
		    __kernel_neon_end())
...
scoped_guard(neon) {
	do neon things
}

It would require a fair bit of refactoring to replace the existing
begin/end code, though...

-- 
Kees Cook

Re: [PATCH 0/5] arm64: Move kernel mode FPSIMD buffer to the stack

[Ard Biesheuvel]

On Sat, 20 Sept 2025 at 08:42, Kees Cook <kees@kernel.org> wrote:
>
> On Fri, Sep 19, 2025 at 02:32:21PM -0500, Eric Biggers wrote:
> > On Thu, Sep 18, 2025 at 08:35:40AM +0200, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Move the buffer for preserving/restoring the kernel mode FPSIMD state on a
> > > context switch out of struct thread_struct, and onto the stack, so that
> > > the memory cost is not imposed needlessly on all tasks in the system.
> > >
> > > Patches #1 - #3 contains some prepwork so that patch #4 can tighten the
> > > rules around permitted usage patterns of kernel_neon_begin() and
> > > kernel_neon_end(). This permits #5 to provide a stack buffer to
> > > kernel_neon_begin() transparently, in a manner that ensures that it will
> > > remain available until after the associated call to kernel_neon_end()
> > > returns.
> > >
> > > Cc: Marc Zyngier <maz@kernel.org>
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Kees Cook <keescook@chromium.org>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Mark Brown <broonie@kernel.org>
> > >
> > > Ard Biesheuvel (5):
> > >   crypto/arm64: aes-ce-ccm - Avoid pointless yield of the NEON unit
> > >   crypto/arm64: sm4-ce-ccm - Avoid pointless yield of the NEON unit
> > >   crypto/arm64: sm4-ce-gcm - Avoid pointless yield of the NEON unit
> > >   arm64/fpsimd: Require kernel NEON begin/end calls from the same scope
> > >   arm64/fpsimd: Allocate kernel mode FP/SIMD buffers on the stack
> > >
> > >  arch/arm64/crypto/aes-ce-ccm-glue.c |  5 +--
> > >  arch/arm64/crypto/sm4-ce-ccm-glue.c | 10 ++----
> > >  arch/arm64/crypto/sm4-ce-gcm-glue.c | 10 ++----
> > >  arch/arm64/include/asm/neon.h       |  7 ++--
> > >  arch/arm64/include/asm/processor.h  |  2 +-
> > >  arch/arm64/kernel/fpsimd.c          | 34 +++++++++++++-------
> > >  6 files changed, 34 insertions(+), 34 deletions(-)
> >
> > This looks like the right decision: saving 528 bytes per task is
> > significant.  528 bytes is a lot to allocate on the stack too, but
> > functions that use the NEON registers are either leaf functions or very
> > close to being leaf functions, so it should be okay.
> >
> > The implementation is a bit unusual, though:
> >
> >    #define kernel_neon_begin()        do { __kernel_neon_begin(&(struct user_fpsimd_state){})
> >    #define kernel_neon_end()  __kernel_neon_end(); } while (0)
> >
> > It works, but normally macros don't start or end code blocks behind the
> > scenes like this.  Perhaps it should be more like s390's
> > kernel_fpu_begin(), where the caller provides the buffer that the
> > registers are stored in?
>
> We've done stuff like this in the past, but I did wonder if we could use
> any of the cleanup.h style logic to do this instead of the split
> do/while? I think this would work, totally untested:
>
> DEFINE_LOCK_GUARD_0(neon,
>                     __kernel_neon_begin(&(struct user_fpsimd_state){}),
>                     __kernel_neon_end())
> ...
> scoped_guard(neon) {
>         do neon things
> }
>
> It would require a fair bit of refactoring to replace the existing
> begin/end code, though...
>

Yeah, but I think it's worth it. And let's call it simd and reuse that
on other architectures as well.

The problem with the above, though, is that the temporary stack
variable goes out of scope when the constructor function returns. So
probably better to do something like

DEFINE_LOCK_GUARD_1(simd,
                    struct user_fpsimd_state,
                    __kernel_neon_begin(_T->lock),
                    __kernel_neon_end(_T->lock));

#define scoped_simd() scoped_guard(simd, &(struct user_fpsimd_state){})

leaving the possibility for callers to use scoped_guard() directly and
pass their own user_fpsimd_state pointer.

We'd need to add some debug checks in __kernel_neon_end() to ensure
that the provided pointer matches the one that was passed to
__kernel_neon_begin().

Re: [PATCH 4/5] arm64/fpsimd: Require kernel NEON begin/end calls from the same scope

[kernel test robot]

Hi Ard,

kernel test robot noticed the following build errors:

[auto build test ERROR on f83ec76bf285bea5727f478a68b894f5543ca76e]

url:    https://github.com/intel-lab-lkp/linux/commits/Ard-Biesheuvel/crypto-arm64-aes-ce-ccm-Avoid-pointless-yield-of-the-NEON-unit/20250918-143859
base:   f83ec76bf285bea5727f478a68b894f5543ca76e
patch link:    https://lore.kernel.org/r/20250918063539.2640512-11-ardb%2Bgit%40google.com
patch subject: [PATCH 4/5] arm64/fpsimd: Require kernel NEON begin/end calls from the same scope
config: arm64-randconfig-r132-20250920 (https://download.01.org/0day-ci/archive/20250922/202509220514.RnYqxP9V-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250922/202509220514.RnYqxP9V-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509220514.RnYqxP9V-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/dc_fpu.c:27:
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/dc_fpu.c: In function 'dc_fpu_begin':
>> drivers/gpu/drm/amd/amdgpu/../display/dc/dc_trace.h:42:9: error: expected 'while' before 'trace_dcn_fpu'
      42 |         trace_dcn_fpu(begin, function, line, ref_count)
         |         ^~~~~~~~~~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/dc_fpu.c:86:9: note: in expansion of macro 'TRACE_DCN_FPU'
      86 |         TRACE_DCN_FPU(true, function_name, line, depth);
         |         ^~~~~~~~~~~~~
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/dc_fpu.c:106:11: error: 'else' without a previous 'if'
     106 |         } else {
         |           ^~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/dc_fpu.c: At top level:
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/dc_fpu.c:99:6: warning: 'dc_fpu_end' defined but not used [-Wunused-function]
      99 | void dc_fpu_end(const char *function_name, const int line)
         |      ^~~~~~~~~~


vim +42 drivers/gpu/drm/amd/amdgpu/../display/dc/dc_trace.h

8b198f6e94d669 Rodrigo Siqueira 2020-09-09  25  
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  26  #define TRACE_DC_PIPE_STATE(pipe_ctx, index, max_pipes) \
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  27  	for (index = 0; index < max_pipes; ++index) { \
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  28  		struct pipe_ctx *pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[index]; \
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  29  		if (pipe_ctx->plane_state) \
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  30  			trace_amdgpu_dm_dc_pipe_state(pipe_ctx->pipe_idx, pipe_ctx->plane_state, \
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  31  						      pipe_ctx->stream, &pipe_ctx->plane_res, \
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  32  						      pipe_ctx->update_flags.raw); \
8b198f6e94d669 Rodrigo Siqueira 2020-09-09  33  	}
13b5ca42ca9c96 Rodrigo Siqueira 2020-09-10  34  
13b5ca42ca9c96 Rodrigo Siqueira 2020-09-10  35  #define TRACE_DCE_CLOCK_STATE(dce_clocks) \
13b5ca42ca9c96 Rodrigo Siqueira 2020-09-10  36  	trace_amdgpu_dm_dce_clocks_state(dce_clocks)
13b5ca42ca9c96 Rodrigo Siqueira 2020-09-10  37  
13b5ca42ca9c96 Rodrigo Siqueira 2020-09-10  38  #define TRACE_DCN_CLOCK_STATE(dcn_clocks) \
13b5ca42ca9c96 Rodrigo Siqueira 2020-09-10  39  	trace_amdgpu_dm_dc_clocks_state(dcn_clocks)
96ee63730fa306 Rodrigo Siqueira 2021-07-26  40  
2d8471dc371f36 Rodrigo Siqueira 2021-07-26  41  #define TRACE_DCN_FPU(begin, function, line, ref_count) \
2d8471dc371f36 Rodrigo Siqueira 2021-07-26 @42  	trace_dcn_fpu(begin, function, line, ref_count)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki