[PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services

Ard Biesheuvel posted 8 patches 1 week, 6 days ago
There is a newer version of this series
[PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
Posted by Ard Biesheuvel 1 week, 6 days ago
From: Ard Biesheuvel <ardb@kernel.org>

Now that the use of kernel mode FP/SIMD is generally permitted when IRQs
are disabled, the only purpose served by the EFI-specific fallback code
in fpsimd.c is the case where an EFI call occurs from hardirq or NMI
context. No such cases are known to occur in practice, and it is
doubtful whether calling into the EFI firmware for any reason under such
conditions would be a good idea to begin with.

So disallow EFI runtime services in such cases. This means all the
fallback code can be dropped.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/fpsimd.h |   4 -
 arch/arm64/kernel/efi.c         |   8 +-
 arch/arm64/kernel/fpsimd.c      | 121 --------------------
 3 files changed, 6 insertions(+), 127 deletions(-)

diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index b8cf0ea43cc0..139ee1393730 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -458,10 +458,6 @@ static inline size_t sme_state_size(struct task_struct const *task)
 
 #endif /* ! CONFIG_ARM64_SME */
 
-/* For use by EFI runtime services calls only */
-extern void __efi_fpsimd_begin(void);
-extern void __efi_fpsimd_end(void);
-
 #endif
 
 #endif
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 9b03f3d77a25..0d52414415f3 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -14,6 +14,7 @@
 #include <linux/vmalloc.h>
 
 #include <asm/efi.h>
+#include <asm/simd.h>
 #include <asm/stacktrace.h>
 #include <asm/vmap_stack.h>
 
@@ -169,15 +170,18 @@ static DEFINE_RAW_SPINLOCK(efi_rt_lock);
 
 bool arch_efi_call_virt_setup(void)
 {
+	if (!may_use_simd())
+		return false;
+
 	efi_virtmap_load();
 	raw_spin_lock(&efi_rt_lock);
-	__efi_fpsimd_begin();
+	kernel_neon_begin();
 	return true;
 }
 
 void arch_efi_call_virt_teardown(void)
 {
-	__efi_fpsimd_end();
+	kernel_neon_end();
 	raw_spin_unlock(&efi_rt_lock);
 	efi_virtmap_unload();
 }
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 96a226316d1f..e543dd569bd7 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1907,127 +1907,6 @@ void kernel_neon_end(void)
 		clear_thread_flag(TIF_KERNEL_FPSTATE);
 }
 EXPORT_SYMBOL_GPL(kernel_neon_end);
-
-#ifdef CONFIG_EFI
-
-static struct user_fpsimd_state efi_fpsimd_state;
-static bool efi_fpsimd_state_used;
-static bool efi_sve_state_used;
-static bool efi_sm_state;
-
-/*
- * EFI runtime services support functions
- *
- * The ABI for EFI runtime services allows EFI to use FPSIMD during the call.
- * This means that for EFI (and only for EFI), we have to assume that FPSIMD
- * is always used rather than being an optional accelerator.
- *
- * These functions provide the necessary support for ensuring FPSIMD
- * save/restore in the contexts from which EFI is used.
- *
- * Do not use them for any other purpose -- if tempted to do so, you are
- * either doing something wrong or you need to propose some refactoring.
- */
-
-/*
- * __efi_fpsimd_begin(): prepare FPSIMD for making an EFI runtime services call
- */
-void __efi_fpsimd_begin(void)
-{
-	if (!system_supports_fpsimd())
-		return;
-
-	WARN_ON(preemptible());
-
-	if (may_use_simd()) {
-		kernel_neon_begin();
-	} else {
-		/*
-		 * If !efi_sve_state, SVE can't be in use yet and doesn't need
-		 * preserving:
-		 */
-		if (system_supports_sve() && efi_sve_state != NULL) {
-			bool ffr = true;
-			u64 svcr;
-
-			efi_sve_state_used = true;
-
-			if (system_supports_sme()) {
-				svcr = read_sysreg_s(SYS_SVCR);
-
-				efi_sm_state = svcr & SVCR_SM_MASK;
-
-				/*
-				 * Unless we have FA64 FFR does not
-				 * exist in streaming mode.
-				 */
-				if (!system_supports_fa64())
-					ffr = !(svcr & SVCR_SM_MASK);
-			}
-
-			sve_save_state(efi_sve_state + sve_ffr_offset(sve_max_vl()),
-				       &efi_fpsimd_state.fpsr, ffr);
-
-			if (system_supports_sme())
-				sysreg_clear_set_s(SYS_SVCR,
-						   SVCR_SM_MASK, 0);
-
-		} else {
-			fpsimd_save_state(&efi_fpsimd_state);
-		}
-
-		efi_fpsimd_state_used = true;
-	}
-}
-
-/*
- * __efi_fpsimd_end(): clean up FPSIMD after an EFI runtime services call
- */
-void __efi_fpsimd_end(void)
-{
-	if (!system_supports_fpsimd())
-		return;
-
-	if (!efi_fpsimd_state_used) {
-		kernel_neon_end();
-	} else {
-		if (system_supports_sve() && efi_sve_state_used) {
-			bool ffr = true;
-
-			/*
-			 * Restore streaming mode; EFI calls are
-			 * normal function calls so should not return in
-			 * streaming mode.
-			 */
-			if (system_supports_sme()) {
-				if (efi_sm_state) {
-					sysreg_clear_set_s(SYS_SVCR,
-							   0,
-							   SVCR_SM_MASK);
-
-					/*
-					 * Unless we have FA64 FFR does not
-					 * exist in streaming mode.
-					 */
-					if (!system_supports_fa64())
-						ffr = false;
-				}
-			}
-
-			sve_load_state(efi_sve_state + sve_ffr_offset(sve_max_vl()),
-				       &efi_fpsimd_state.fpsr, ffr);
-
-			efi_sve_state_used = false;
-		} else {
-			fpsimd_load_state(&efi_fpsimd_state);
-		}
-
-		efi_fpsimd_state_used = false;
-	}
-}
-
-#endif /* CONFIG_EFI */
-
 #endif /* CONFIG_KERNEL_MODE_NEON */
 
 #ifdef CONFIG_CPU_PM
-- 
2.51.0.384.g4c02a37b29-goog
Re: [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
Posted by Mark Brown 1 week, 6 days ago
On Thu, Sep 18, 2025 at 12:30:16PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>

> Now that the use of kernel mode FP/SIMD is generally permitted when IRQs
> are disabled, the only purpose served by the EFI-specific fallback code
> in fpsimd.c is the case where an EFI call occurs from hardirq or NMI
> context. No such cases are known to occur in practice, and it is
> doubtful whether calling into the EFI firmware for any reason under such
> conditions would be a good idea to begin with.
> 
> So disallow EFI runtime services in such cases. This means all the
> fallback code can be dropped.

This is a really nice simplification, with the fixup rolled in:

Reviewed-by: Mark Brown <broonie@kernel.org>
Re: [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
Posted by Ard Biesheuvel 1 week, 3 days ago
On Thu, 18 Sept 2025 at 15:10, Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Sep 18, 2025 at 12:30:16PM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
>
> > Now that the use of kernel mode FP/SIMD is generally permitted when IRQs
> > are disabled, the only purpose served by the EFI-specific fallback code
> > in fpsimd.c is the case where an EFI call occurs from hardirq or NMI
> > context. No such cases are known to occur in practice, and it is
> > doubtful whether calling into the EFI firmware for any reason under such
> > conditions would be a good idea to begin with.
> >
> > So disallow EFI runtime services in such cases. This means all the
> > fallback code can be dropped.
>
> This is a really nice simplification, with the fixup rolled in:
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Sadly, this is not as simply as I had hoped.

So even if we address the irqs_disabled() case, there are three
remaining code paths where EFI pstore may end up calling the
SetVariable() runtime service in hard IRQ or NMI context: panic(),
oops_exit() and emergency_restart(). So disallowing this is
problematic, as EFI pstore might be the only way to do a post mortem.

As such an IRQ could potentially occur at a time when the FP/SIMD unit
is being used both in task and in softirq context, there still needs
to be some special handling, even though a) this condition is
vanishingly rare, and so having elaborate logic like we do today that
is never exercised is not great
b) much of the logic deals with SVE which is user space only, and we
can just disregard that under the conditions where we may enter in IRQ
context.
Re: [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
Posted by Ard Biesheuvel 1 week, 6 days ago
On Thu, 18 Sept 2025 at 12:30, Ard Biesheuvel <ardb+git@google.com> wrote:
>
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Now that the use of kernel mode FP/SIMD is generally permitted when IRQs
> are disabled, the only purpose served by the EFI-specific fallback code
> in fpsimd.c is the case where an EFI call occurs from hardirq or NMI
> context. No such cases are known to occur in practice, and it is
> doubtful whether calling into the EFI firmware for any reason under such
> conditions would be a good idea to begin with.
>
> So disallow EFI runtime services in such cases. This means all the
> fallback code can be dropped.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/include/asm/fpsimd.h |   4 -
>  arch/arm64/kernel/efi.c         |   8 +-
>  arch/arm64/kernel/fpsimd.c      | 121 --------------------
>  3 files changed, 6 insertions(+), 127 deletions(-)
>

This patch is incomplete, the following needs to be folded in as well:

 arch/arm64/kernel/fpsimd.c | 39 ---------------------------------------
 1 file changed, 39 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index e543dd569bd7..c846161a002b 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -180,13 +180,6 @@
        set_default_vl(ARM64_VEC_SVE, val);
 }

-static u8 *efi_sve_state;
-
-#else /* ! CONFIG_ARM64_SVE */
-
-/* Dummy declaration for code that will be optimised out: */
-extern u8 *efi_sve_state;
-
 #endif /* ! CONFIG_ARM64_SVE */

 #ifdef CONFIG_ARM64_SME
@@ -1086,36 +1079,6 @@
        return 0;
 }

-static void __init sve_efi_setup(void)
-{
-       int max_vl = 0;
-       int i;
-
-       if (!IS_ENABLED(CONFIG_EFI))
-               return;
-
-       for (i = 0; i < ARRAY_SIZE(vl_info); i++)
-               max_vl = max(vl_info[i].max_vl, max_vl);
-
-       /*
-        * alloc_percpu() warns and prints a backtrace if this goes wrong.
-        * This is evidence of a crippled system and we are returning void,
-        * so no attempt is made to handle this situation here.
-        */
-       if (!sve_vl_valid(max_vl))
-               goto fail;
-
-       efi_sve_state = kmalloc(SVE_SIG_REGS_SIZE(sve_vq_from_vl(max_vl)),
-                               GFP_KERNEL);
-       if (!efi_sve_state)
-               goto fail;
-
-       return;
-
-fail:
-       panic("Cannot allocate memory for EFI SVE save/restore");
-}
-
 void cpu_enable_sve(const struct arm64_cpu_capabilities *__always_unused p)
 {
        write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1);
@@ -1176,8 +1139,6 @@
        if (sve_max_virtualisable_vl() < sve_max_vl())
                pr_warn("%s: unvirtualisable vector lengths present\n",
                        info->name);
-
-       sve_efi_setup();
 }

 /*