[PATCH 0/2] efi/arm64: Simplify SVE/SME state preserve/restore

[PATCH 0/2] efi/arm64: Simplify SVE/SME state preserve/restore

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

The EFI specific SVE/SME state preserve/restore logic is unneeded, and
can be removed. Patch #1 adds some lkdtm test cases to emulate the
condition under which this code might be called.

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

Ard Biesheuvel (2):
  lkdtm/bugs: Add cases for BUG and PANIC occurring in hardirq context
  arm64/efi: Remove unneeded SVE/SME fallback preserve/store handling

 arch/arm64/kernel/fpsimd.c | 130 +++-----------------
 drivers/misc/lkdtm/bugs.c  |  53 ++++++++
 2 files changed, 73 insertions(+), 110 deletions(-)

-- 
2.52.0.rc1.455.g30608eb744-goog

[PATCH 1/2] lkdtm/bugs: Add cases for BUG and PANIC occurring in hardirq context

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Add lkdtm cases to trigger a BUG() or panic() from hardirq context. This
is useful for testing pstore behavior being invoked from such contexts.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/misc/lkdtm/bugs.c | 53 ++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 376047beea3d..fa05d77acb55 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -8,6 +8,7 @@
 #include "lkdtm.h"
 #include <linux/cpu.h>
 #include <linux/list.h>
+#include <linux/hrtimer.h>
 #include <linux/sched.h>
 #include <linux/sched/signal.h>
 #include <linux/sched/task_stack.h>
@@ -100,11 +101,61 @@ static void lkdtm_PANIC_STOP_IRQOFF(void)
 	stop_machine(panic_stop_irqoff_fn, &v, cpu_online_mask);
 }
 
+static bool wait_for_panic;
+
+static enum hrtimer_restart panic_in_hardirq(struct hrtimer *timer)
+{
+	panic("from hard IRQ context");
+
+	wait_for_panic = false;
+	return HRTIMER_NORESTART;
+}
+
+static void lkdtm_PANIC_IN_HARDIRQ(void)
+{
+	struct hrtimer timer;
+
+	wait_for_panic = true;
+	hrtimer_setup_on_stack(&timer, panic_in_hardirq,
+			       CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
+	hrtimer_start(&timer, us_to_ktime(100), HRTIMER_MODE_REL_HARD);
+
+	while (wait_for_panic)
+		;
+
+	hrtimer_cancel(&timer);
+}
+
 static void lkdtm_BUG(void)
 {
 	BUG();
 }
 
+static bool wait_for_bug;
+
+static enum hrtimer_restart bug_in_hardirq(struct hrtimer *timer)
+{
+	BUG();
+
+	wait_for_bug = false;
+	return HRTIMER_NORESTART;
+}
+
+static void lkdtm_BUG_IN_HARDIRQ(void)
+{
+	struct hrtimer timer;
+
+	wait_for_bug = true;
+	hrtimer_setup_on_stack(&timer, bug_in_hardirq,
+			       CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
+	hrtimer_start(&timer, us_to_ktime(100), HRTIMER_MODE_REL_HARD);
+
+	while (wait_for_bug)
+		;
+
+	hrtimer_cancel(&timer);
+}
+
 static int warn_counter;
 
 static void lkdtm_WARNING(void)
@@ -696,7 +747,9 @@ static noinline void lkdtm_CORRUPT_PAC(void)
 static struct crashtype crashtypes[] = {
 	CRASHTYPE(PANIC),
 	CRASHTYPE(PANIC_STOP_IRQOFF),
+	CRASHTYPE(PANIC_IN_HARDIRQ),
 	CRASHTYPE(BUG),
+	CRASHTYPE(BUG_IN_HARDIRQ),
 	CRASHTYPE(WARNING),
 	CRASHTYPE(WARNING_MESSAGE),
 	CRASHTYPE(EXCEPTION),
-- 
2.52.0.rc1.455.g30608eb744-goog

[PATCH 2/2] arm64/efi: Remove unneeded SVE/SME fallback preserve/store handling

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Since commit 7137a203b251 ("arm64/fpsimd: Permit kernel mode NEON with
IRQs off"), the only condition under which the fallback path is taken
for FP/SIMD preserve/restore across a EFI runtime call is when it is
called from hardirq or NMI context.

In practice, this only happens when the EFI pstore driver is called to
dump the kernel log buffer into a EFI variable under a panic, oops or
emergency_restart() condition, and none of these can be expected to
result in a return to user space for the task in question.

This means that the existing EFI-specific logic for preserving and
restoring SVE/SME state is pointless, and can be removed.

Instead, kill the task, so that an exceedingly unlikely inadvertent
return to user space does not proceed with a corrupted FP/SIMD state.
Also, retain the preserve and restore of the base FP/SIMD state, as that
might belong to kernel mode use of FP/SIMD. (Note that EFI runtime calls
are never invoked reentrantly, even in this case, and so any interrupted
kernel mode FP/SIMD usage will be unrelated to EFI)

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/fpsimd.c | 130 +++-----------------
 1 file changed, 20 insertions(+), 110 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index c154f72634e0..9de1d8a604cb 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -180,13 +180,6 @@ static inline void set_sve_default_vl(int val)
 	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
@@ -1095,36 +1088,6 @@ int vec_verify_vq_map(enum vec_type type)
 	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);
@@ -1185,8 +1148,6 @@ void __init sve_setup(void)
 	if (sve_max_virtualisable_vl() < sve_max_vl())
 		pr_warn("%s: unvirtualisable vector lengths present\n",
 			info->name);
-
-	sve_efi_setup();
 }
 
 /*
@@ -1947,9 +1908,6 @@ 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
@@ -1976,43 +1934,26 @@ void __efi_fpsimd_begin(void)
 	if (may_use_simd()) {
 		kernel_neon_begin(&efi_fpsimd_state);
 	} else {
-		WARN_ON(preemptible());
-
 		/*
-		 * If !efi_sve_state, SVE can't be in use yet and doesn't need
-		 * preserving:
+		 * We are running in hardirq or NMI context, and the only
+		 * legitimate case where this might happen is when EFI pstore
+		 * is attempting to record the system's dying gasps into EFI
+		 * variables. This could be due to an oops, a panic or a call
+		 * to emergency_restart(), and in none of those cases, we can
+		 * expect the current task to ever return to user space again,
+		 * or for the kernel to resume any normal execution, for that
+		 * matter (an oops in hardirq context triggers a panic too).
+		 *
+		 * Therefore, there is no point in attempting to preserve any
+		 * SVE/SME state here. On the off chance that we might have
+		 * ended up here for a different reason inadvertently, kill the
+		 * task and preserve/restore the base FP/SIMD state, which
+		 * might belong to kernel mode FP/SIMD.
 		 */
-		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;
+		pr_warn_ratelimited("Calling EFI runtime from %s context\n",
+				    in_nmi() ? "NMI" : "hardirq");
+		force_signal_inject(SIGKILL, SI_KERNEL, 0, 0);
+		fpsimd_save_state(&efi_fpsimd_state);
 	}
 }
 
@@ -2024,41 +1965,10 @@ void __efi_fpsimd_end(void)
 	if (!system_supports_fpsimd())
 		return;
 
-	if (!efi_fpsimd_state_used) {
+	if (may_use_simd()) {
 		kernel_neon_end(&efi_fpsimd_state);
 	} 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;
+		fpsimd_load_state(&efi_fpsimd_state);
 	}
 }
 
-- 
2.52.0.rc1.455.g30608eb744-goog

Re: [PATCH 1/2] lkdtm/bugs: Add cases for BUG and PANIC occurring in hardirq context

[Catalin Marinas]

On Wed, Nov 12, 2025 at 06:53:18PM +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Add lkdtm cases to trigger a BUG() or panic() from hardirq context. This
> is useful for testing pstore behavior being invoked from such contexts.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/misc/lkdtm/bugs.c | 53 ++++++++++++++++++++
>  1 file changed, 53 insertions(+)

Kees, are you ok with this patch? Happy to take it through the arm64
tree together with the second one.

Thanks.

-- 
Catalin

Re: [PATCH 1/2] lkdtm/bugs: Add cases for BUG and PANIC occurring in hardirq context

[Kees Cook]

On Wed, Nov 12, 2025 at 06:53:18PM +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Add lkdtm cases to trigger a BUG() or panic() from hardirq context. This
> is useful for testing pstore behavior being invoked from such contexts.

Sorry I lost this! Thanks for the offline ping. :)

> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/misc/lkdtm/bugs.c | 53 ++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
> index 376047beea3d..fa05d77acb55 100644
> --- a/drivers/misc/lkdtm/bugs.c
> +++ b/drivers/misc/lkdtm/bugs.c
> @@ -8,6 +8,7 @@
>  #include "lkdtm.h"
>  #include <linux/cpu.h>
>  #include <linux/list.h>
> +#include <linux/hrtimer.h>
>  #include <linux/sched.h>
>  #include <linux/sched/signal.h>
>  #include <linux/sched/task_stack.h>
> @@ -100,11 +101,61 @@ static void lkdtm_PANIC_STOP_IRQOFF(void)
>  	stop_machine(panic_stop_irqoff_fn, &v, cpu_online_mask);
>  }
>  
> +static bool wait_for_panic;
> +
> +static enum hrtimer_restart panic_in_hardirq(struct hrtimer *timer)
> +{
> +	panic("from hard IRQ context");
> +
> +	wait_for_panic = false;
> +	return HRTIMER_NORESTART;
> +}
> +
> +static void lkdtm_PANIC_IN_HARDIRQ(void)
> +{
> +	struct hrtimer timer;
> +
> +	wait_for_panic = true;
> +	hrtimer_setup_on_stack(&timer, panic_in_hardirq,
> +			       CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
> +	hrtimer_start(&timer, us_to_ktime(100), HRTIMER_MODE_REL_HARD);
> +
> +	while (wait_for_panic)
> +		;
> +
> +	hrtimer_cancel(&timer);
> +}
> +
>  static void lkdtm_BUG(void)
>  {
>  	BUG();
>  }
>  
> +static bool wait_for_bug;
> +
> +static enum hrtimer_restart bug_in_hardirq(struct hrtimer *timer)
> +{
> +	BUG();
> +
> +	wait_for_bug = false;
> +	return HRTIMER_NORESTART;
> +}
> +
> +static void lkdtm_BUG_IN_HARDIRQ(void)
> +{
> +	struct hrtimer timer;
> +
> +	wait_for_bug = true;
> +	hrtimer_setup_on_stack(&timer, bug_in_hardirq,
> +			       CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
> +	hrtimer_start(&timer, us_to_ktime(100), HRTIMER_MODE_REL_HARD);
> +
> +	while (wait_for_bug)
> +		;
> +
> +	hrtimer_cancel(&timer);
> +}
> +
>  static int warn_counter;
>  
>  static void lkdtm_WARNING(void)
> @@ -696,7 +747,9 @@ static noinline void lkdtm_CORRUPT_PAC(void)
>  static struct crashtype crashtypes[] = {
>  	CRASHTYPE(PANIC),
>  	CRASHTYPE(PANIC_STOP_IRQOFF),
> +	CRASHTYPE(PANIC_IN_HARDIRQ),
>  	CRASHTYPE(BUG),
> +	CRASHTYPE(BUG_IN_HARDIRQ),
>  	CRASHTYPE(WARNING),
>  	CRASHTYPE(WARNING_MESSAGE),
>  	CRASHTYPE(EXCEPTION),

This all looks good to me. I always ask that new tests also get added to
the lkdtm selftest runner list in tools/testing/selftests/lkdtm/tests.txt

IIUC, bug in hardirq will result in a panic, so both these tests will
take out the entire system, so they should be skipped for the selftest.
Perhaps:


diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
index cff124c1eddd..67cd53715d93 100644
--- a/tools/testing/selftests/lkdtm/tests.txt
+++ b/tools/testing/selftests/lkdtm/tests.txt
@@ -1,6 +1,8 @@
 #PANIC
 #PANIC_STOP_IRQOFF Crashes entire system
+#PANIC_IN_HARDIRQ Crashes entire system
 BUG kernel BUG at
+#BUG_IN_HARDIRQ Crashes entire system
 WARNING WARNING:
 WARNING_MESSAGE message trigger
 EXCEPTION




-- 
Kees Cook

Re: [PATCH 1/2] lkdtm/bugs: Add cases for BUG and PANIC occurring in hardirq context

[Kees Cook]

On Wed, Nov 19, 2025 at 05:54:14PM +0000, Catalin Marinas wrote:
> On Wed, Nov 12, 2025 at 06:53:18PM +0100, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> > 
> > Add lkdtm cases to trigger a BUG() or panic() from hardirq context. This
> > is useful for testing pstore behavior being invoked from such contexts.
> > 
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  drivers/misc/lkdtm/bugs.c | 53 ++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> 
> Kees, are you ok with this patch? Happy to take it through the arm64
> tree together with the second one.

With the small addition I just sent, yeah, this is fine by me. :)

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook