public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] arm64: Make EFI calls preemptible
@ 2025-09-18 10:30 Ard Biesheuvel
  2025-09-18 10:30 ` [PATCH v3 1/8] efi: Add missing static initializer for efi_mm::cpus_allowed_lock Ard Biesheuvel
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

From: Ard Biesheuvel <ardb@kernel.org>

The arm64 port permits the use of the baseline FP/SIMD register file in
kernel mode, and no longer requires preemption to be disabled. Now that
the EFI spec is being clarified to state that EFI runtime services may
only use baseline FP/SIMD, the fact that EFI may code may use FP/SIMD
registers (while executing at the same privilege level as the kernel) is
no longer a reason to disable preemption when invoking them.

This means that the only remaining reason for disabling preemption is
the fact that the active mm is swapped out and replaced with efi_mm in a
way that is hidden from the scheduler, and so scheduling is not
supported currently. However, given that virtually all (*) EFI runtime
calls are made from the efi_rts_wq workqueue, the efi_mm can simply be
loaded into the workqueue worker kthread while the call is in progress,
and this does not require preemption to be disabled.

Note that this is only a partial solution in terms of RT guarantees,
given that the runtime services execute at the same privilege level as
the kernel, and can therefore disable interrupts (and therefore
preemption) directly. But it should prevent scheduling latency spikes
for EFI calls that simply take a long time to run to completion.

Changes since v2:
- Permit ordinary kernel mode FP/SIMD with IRQs disabled, so that the
  special EFI case only deals with invocations in hardirq or NMI context
- Disallow EFI runtime calls in hardirq or NMI context, so that the
  special FP/SIMD handling for EFI can be dropped entirely
- Use a mutex rather than a semaphore for the arm64 EFI runtime lock,
  now that it is never trylock()ed in IRQ or NMI context.

Changes since v1/RFC:
- Disable uaccess for SWPAN before updating the preserved TTBR0 value
- Document why disabling migration is needed
- Rebase onto v6.17-rc1

(*) only efi_reset_system() and EFI pstore invoke EFI runtime services
    without going through the workqueue, and the latter only when saving
    a kernel oops log to the EFI varstore

Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Brown <broonie@kernel.org>

Ard Biesheuvel (8):
  efi: Add missing static initializer for efi_mm::cpus_allowed_lock
  efi/runtime: Return success/failure from arch_efi_call_virt_setup()
  efi/runtime: Deal with arch_efi_call_virt_setup() returning failure
  arm64/fpsimd: Permit kernel mode NEON with IRQs off
  arm64/fpsimd: Drop special handling for EFI runtime services
  arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
  arm64/efi: Move uaccess en/disable out of efi_set_pgd()
  arm64/efi: Call EFI runtime services without disabling preemption

 arch/arm/include/asm/efi.h              |   2 +-
 arch/arm64/include/asm/efi.h            |  15 +--
 arch/arm64/include/asm/fpsimd.h         |   4 -
 arch/arm64/include/asm/simd.h           |   2 +-
 arch/arm64/kernel/efi.c                 |  65 ++++++++--
 arch/arm64/kernel/fpsimd.c              | 137 ++------------------
 arch/loongarch/include/asm/efi.h        |   2 +-
 arch/riscv/include/asm/efi.h            |   2 +-
 arch/x86/include/asm/efi.h              |   2 +-
 arch/x86/platform/efi/efi_32.c          |   3 +-
 arch/x86/platform/efi/efi_64.c          |   3 +-
 arch/x86/platform/uv/bios_uv.c          |   3 +-
 drivers/firmware/efi/efi.c              |   3 +
 drivers/firmware/efi/riscv-runtime.c    |   3 +-
 drivers/firmware/efi/runtime-wrappers.c |  20 ++-
 include/linux/efi.h                     |   8 +-
 16 files changed, 104 insertions(+), 170 deletions(-)


base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 1/8] efi: Add missing static initializer for efi_mm::cpus_allowed_lock
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-18 10:30 ` [PATCH v3 2/8] efi/runtime: Return success/failure from arch_efi_call_virt_setup() Ard Biesheuvel
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown, stable

From: Ard Biesheuvel <ardb@kernel.org>

Initialize the cpus_allowed_lock struct member of efi_mm.

Cc: <stable@vger.kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/efi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 1ce428e2ac8a..fc407d891348 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -74,6 +74,9 @@ struct mm_struct efi_mm = {
 	.page_table_lock	= __SPIN_LOCK_UNLOCKED(efi_mm.page_table_lock),
 	.mmlist			= LIST_HEAD_INIT(efi_mm.mmlist),
 	.cpu_bitmap		= { [BITS_TO_LONGS(NR_CPUS)] = 0},
+#ifdef CONFIG_SCHED_MM_CID
+	.cpus_allowed_lock	= __RAW_SPIN_LOCK_UNLOCKED(efi_mm.cpus_allowed_lock),
+#endif
 };
 
 struct workqueue_struct *efi_rts_wq;
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 2/8] efi/runtime: Return success/failure from arch_efi_call_virt_setup()
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
  2025-09-18 10:30 ` [PATCH v3 1/8] efi: Add missing static initializer for efi_mm::cpus_allowed_lock Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-18 10:30 ` [PATCH v3 3/8] efi/runtime: Deal with arch_efi_call_virt_setup() returning failure Ard Biesheuvel
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

From: Ard Biesheuvel <ardb@kernel.org>

Permit the arch glue to signal failure from arch_efi_call_virt_setup().
This permits the use of sleeping locks in the call wrappers, and this
will allow EFI runtime services to be invoked without the need for
disabling preemption.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/include/asm/efi.h           | 2 +-
 arch/arm64/include/asm/efi.h         | 2 +-
 arch/arm64/kernel/efi.c              | 3 ++-
 arch/loongarch/include/asm/efi.h     | 2 +-
 arch/riscv/include/asm/efi.h         | 2 +-
 arch/x86/include/asm/efi.h           | 2 +-
 arch/x86/platform/efi/efi_32.c       | 3 ++-
 arch/x86/platform/efi/efi_64.c       | 3 ++-
 drivers/firmware/efi/riscv-runtime.c | 3 ++-
 9 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index e408399d5f0e..0809a69bb579 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -23,7 +23,7 @@ void arm_efi_init(void);
 int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
 int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md, bool);
 
-#define arch_efi_call_virt_setup()	efi_virtmap_load()
+#define arch_efi_call_virt_setup()	(efi_virtmap_load(), true)
 #define arch_efi_call_virt_teardown()	efi_virtmap_unload()
 
 #ifdef CONFIG_CPU_TTBR0_PAN
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index bcd5622aa096..decf87777f57 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -37,7 +37,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md,
 extern u64 *efi_rt_stack_top;
 efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
 
-void arch_efi_call_virt_setup(void);
+bool arch_efi_call_virt_setup(void);
 void arch_efi_call_virt_teardown(void);
 
 /*
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 6c371b158b99..9b03f3d77a25 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -167,11 +167,12 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
 
 static DEFINE_RAW_SPINLOCK(efi_rt_lock);
 
-void arch_efi_call_virt_setup(void)
+bool arch_efi_call_virt_setup(void)
 {
 	efi_virtmap_load();
 	raw_spin_lock(&efi_rt_lock);
 	__efi_fpsimd_begin();
+	return true;
 }
 
 void arch_efi_call_virt_teardown(void)
diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
index eddc8e79b3fa..84cf2151123f 100644
--- a/arch/loongarch/include/asm/efi.h
+++ b/arch/loongarch/include/asm/efi.h
@@ -14,7 +14,7 @@ void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
 
 #define ARCH_EFI_IRQ_FLAGS_MASK  0x00000004  /* Bit 2: CSR.CRMD.IE */
 
-#define arch_efi_call_virt_setup()
+#define arch_efi_call_virt_setup()	true
 #define arch_efi_call_virt_teardown()
 
 #define EFI_ALLOC_ALIGN		SZ_64K
diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
index 46a355913b27..a7b4d719e7be 100644
--- a/arch/riscv/include/asm/efi.h
+++ b/arch/riscv/include/asm/efi.h
@@ -40,7 +40,7 @@ static inline unsigned long efi_get_kimg_min_align(void)
 
 #define EFI_KIMG_PREFERRED_ADDRESS	efi_get_kimg_min_align()
 
-void arch_efi_call_virt_setup(void);
+bool arch_efi_call_virt_setup(void);
 void arch_efi_call_virt_teardown(void);
 
 unsigned long stext_offset(void);
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index f227a70ac91f..879c8402e024 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -140,7 +140,7 @@ extern void efi_delete_dummy_variable(void);
 extern void efi_crash_gracefully_on_page_fault(unsigned long phys_addr);
 extern void efi_free_boot_services(void);
 
-void arch_efi_call_virt_setup(void);
+bool arch_efi_call_virt_setup(void);
 void arch_efi_call_virt_teardown(void);
 
 extern u64 efi_setup;
diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index b2cc7b4552a1..215f16ce84ab 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -141,10 +141,11 @@ void __init efi_runtime_update_mappings(void)
 	}
 }
 
-void arch_efi_call_virt_setup(void)
+bool arch_efi_call_virt_setup(void)
 {
 	efi_fpu_begin();
 	firmware_restrict_branch_speculation_start();
+	return true;
 }
 
 void arch_efi_call_virt_teardown(void)
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index b4409df2105a..d4b1e70f41fa 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -443,12 +443,13 @@ static void efi_leave_mm(void)
 	unuse_temporary_mm(efi_prev_mm);
 }
 
-void arch_efi_call_virt_setup(void)
+bool arch_efi_call_virt_setup(void)
 {
 	efi_sync_low_kernel_mappings();
 	efi_fpu_begin();
 	firmware_restrict_branch_speculation_start();
 	efi_enter_mm();
+	return true;
 }
 
 void arch_efi_call_virt_teardown(void)
diff --git a/drivers/firmware/efi/riscv-runtime.c b/drivers/firmware/efi/riscv-runtime.c
index fa71cd898120..07e04b8f982a 100644
--- a/drivers/firmware/efi/riscv-runtime.c
+++ b/drivers/firmware/efi/riscv-runtime.c
@@ -142,10 +142,11 @@ static void efi_virtmap_unload(void)
 	preempt_enable();
 }
 
-void arch_efi_call_virt_setup(void)
+bool arch_efi_call_virt_setup(void)
 {
 	sync_kernel_mappings(efi_mm.pgd);
 	efi_virtmap_load();
+	return true;
 }
 
 void arch_efi_call_virt_teardown(void)
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 3/8] efi/runtime: Deal with arch_efi_call_virt_setup() returning failure
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
  2025-09-18 10:30 ` [PATCH v3 1/8] efi: Add missing static initializer for efi_mm::cpus_allowed_lock Ard Biesheuvel
  2025-09-18 10:30 ` [PATCH v3 2/8] efi/runtime: Return success/failure from arch_efi_call_virt_setup() Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-18 10:30 ` [PATCH v3 4/8] arm64/fpsimd: Permit kernel mode NEON with IRQs off Ard Biesheuvel
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

From: Ard Biesheuvel <ardb@kernel.org>

Deal with arch_efi_call_virt_setup() returning failure, by giving up and
returning an appropriate error code to the caller.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/platform/uv/bios_uv.c          |  3 ++-
 drivers/firmware/efi/runtime-wrappers.c | 20 +++++++++++++-------
 include/linux/efi.h                     |  8 ++++----
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c
index bf31af3d32d6..a442bbe5b1c2 100644
--- a/arch/x86/platform/uv/bios_uv.c
+++ b/arch/x86/platform/uv/bios_uv.c
@@ -32,7 +32,8 @@ static s64 __uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3,
 		 */
 		return BIOS_STATUS_UNIMPLEMENTED;
 
-	ret = efi_call_virt_pointer(tab, function, (u64)which, a1, a2, a3, a4, a5);
+	ret = efi_call_virt_pointer(tab, function, BIOS_STATUS_UNIMPLEMENTED,
+				    (u64)which, a1, a2, a3, a4, a5);
 
 	return ret;
 }
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 708b777857d3..82a27b414485 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -219,7 +219,10 @@ static void __nocfi efi_call_rts(struct work_struct *work)
 	efi_status_t status = EFI_NOT_FOUND;
 	unsigned long flags;
 
-	arch_efi_call_virt_setup();
+	if (!arch_efi_call_virt_setup()) {
+		status = EFI_NOT_READY;
+		goto out;
+	}
 	flags = efi_call_virt_save_flags();
 
 	switch (efi_rts_work.efi_rts_id) {
@@ -308,6 +311,7 @@ static void __nocfi efi_call_rts(struct work_struct *work)
 	efi_call_virt_check_flags(flags, efi_rts_work.caller);
 	arch_efi_call_virt_teardown();
 
+out:
 	efi_rts_work.status = status;
 	complete(&efi_rts_work.efi_rts_comp);
 }
@@ -444,8 +448,8 @@ virt_efi_set_variable_nb(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
 	if (down_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
-	status = efi_call_virt_pointer(efi.runtime, set_variable, name, vendor,
-				       attr, data_size, data);
+	status = efi_call_virt_pointer(efi.runtime, set_variable, EFI_NOT_READY,
+				       name, vendor, attr, data_size, data);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -481,9 +485,9 @@ virt_efi_query_variable_info_nb(u32 attr, u64 *storage_space,
 	if (down_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
-	status = efi_call_virt_pointer(efi.runtime, query_variable_info, attr,
-				       storage_space, remaining_space,
-				       max_variable_size);
+	status = efi_call_virt_pointer(efi.runtime, query_variable_info,
+				       EFI_NOT_READY, attr, storage_space,
+				       remaining_space, max_variable_size);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -509,12 +513,14 @@ virt_efi_reset_system(int reset_type, efi_status_t status,
 		return;
 	}
 
-	arch_efi_call_virt_setup();
+	if (!arch_efi_call_virt_setup())
+		goto out;
 	efi_rts_work.efi_rts_id = EFI_RESET_SYSTEM;
 	arch_efi_call_virt(efi.runtime, reset_system, reset_type, status,
 			   data_size, data);
 	arch_efi_call_virt_teardown();
 
+out:
 	up(&efi_runtime_lock);
 }
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a98cc39e7aaa..325d892e559b 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1181,19 +1181,19 @@ static inline void efi_check_for_embedded_firmwares(void) { }
  *    Restores the usual kernel environment once the call has returned.
  */
 
-#define efi_call_virt_pointer(p, f, args...)				\
+#define efi_call_virt_pointer(p, f, busy, args...)			\
 ({									\
-	typeof((p)->f(args)) __s;					\
+	typeof((p)->f(args)) __s = (busy);				\
 	unsigned long __flags;						\
 									\
-	arch_efi_call_virt_setup();					\
+	if (!arch_efi_call_virt_setup()) goto __out;			\
 									\
 	__flags = efi_call_virt_save_flags();				\
 	__s = arch_efi_call_virt(p, f, args);				\
 	efi_call_virt_check_flags(__flags, NULL);			\
 									\
 	arch_efi_call_virt_teardown();					\
-									\
+__out:									\
 	__s;								\
 })
 
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 4/8] arm64/fpsimd: Permit kernel mode NEON with IRQs off
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2025-09-18 10:30 ` [PATCH v3 3/8] efi/runtime: Deal with arch_efi_call_virt_setup() returning failure Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-19 11:33   ` Will Deacon
  2025-09-18 10:30 ` [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services Ard Biesheuvel
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

From: Ard Biesheuvel <ardb@kernel.org>

Currently, may_use_simd() will return false when called from a context
where IRQs are disabled. One notable case where this happens is when
calling the ResetSystem() EFI runtime service from the reboot/poweroff
code path. For this case alone, there is a substantial amount of FP/SIMD
support code to handle the corner case where a EFI runtime service is
invoked with IRQs disabled.

The only reason kernel mode SIMD is not allowed when IRQs are disabled
is that re-enabling softirqs in this case produces a noisy diagnostic
when lockdep is enabled. The warning is valid, in the sense that
delivering pending softirqs over the back of the call to
local_bh_enable() is problematic when IRQs are disabled.

While the API lacks a facility to simply mask and unmask softirqs
without triggering their delivery, disabling softirqs is not needed to
begin with when IRQs are disabled, given that softirqs are only every
taken asynchronously over the back of a hard IRQ.

So dis/enable softirq processing conditionally, based on whether IRQs
are enabled, and relax the check in may_use_simd().

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

diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
index 8e86c9e70e48..abd642c92f86 100644
--- a/arch/arm64/include/asm/simd.h
+++ b/arch/arm64/include/asm/simd.h
@@ -29,7 +29,7 @@ static __must_check inline bool may_use_simd(void)
 	 */
 	return !WARN_ON(!system_capabilities_finalized()) &&
 	       system_supports_fpsimd() &&
-	       !in_hardirq() && !irqs_disabled() && !in_nmi();
+	       !in_hardirq() && !in_nmi();
 }
 
 #else /* ! CONFIG_KERNEL_MODE_NEON */
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index c37f02d7194e..96a226316d1f 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -225,10 +225,12 @@ static void fpsimd_bind_task_to_cpu(void);
  */
 static void get_cpu_fpsimd_context(void)
 {
-	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
-		local_bh_disable();
-	else
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
+		if (!irqs_disabled())
+			local_bh_disable();
+	} else {
 		preempt_disable();
+	}
 }
 
 /*
@@ -240,10 +242,12 @@ static void get_cpu_fpsimd_context(void)
  */
 static void put_cpu_fpsimd_context(void)
 {
-	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
-		local_bh_enable();
-	else
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
+		if (!irqs_disabled())
+			local_bh_enable();
+	} else {
 		preempt_enable();
+	}
 }
 
 unsigned int task_get_vl(const struct task_struct *task, enum vec_type type)
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2025-09-18 10:30 ` [PATCH v3 4/8] arm64/fpsimd: Permit kernel mode NEON with IRQs off Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-18 11:57   ` Ard Biesheuvel
  2025-09-18 13:10   ` Mark Brown
  2025-09-18 10:30 ` [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state Ard Biesheuvel
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

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



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2025-09-18 10:30 ` [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-19 11:35   ` Will Deacon
  2025-09-18 10:30 ` [PATCH v3 7/8] arm64/efi: Move uaccess en/disable out of efi_set_pgd() Ard Biesheuvel
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

From: Ard Biesheuvel <ardb@kernel.org>

Replace the spinlock in the arm64 glue code with a mutex, so that
the CPU can preempted while running the EFI runtime service.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/efi.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 0d52414415f3..4372fafde8e9 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -166,15 +166,22 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
 	return s;
 }
 
-static DEFINE_RAW_SPINLOCK(efi_rt_lock);
+static DEFINE_MUTEX(efi_rt_lock);
 
 bool arch_efi_call_virt_setup(void)
 {
 	if (!may_use_simd())
 		return false;
 
+	/*
+	 * This might be called from a non-sleepable context so try to take the
+	 * lock but don't block on it. This should never fail in practice, as
+	 * all EFI runtime calls are serialized under the efi_runtime_lock.
+	 */
+	if (WARN_ON(!mutex_trylock(&efi_rt_lock)))
+		return false;
+
 	efi_virtmap_load();
-	raw_spin_lock(&efi_rt_lock);
 	kernel_neon_begin();
 	return true;
 }
@@ -182,8 +189,8 @@ bool arch_efi_call_virt_setup(void)
 void arch_efi_call_virt_teardown(void)
 {
 	kernel_neon_end();
-	raw_spin_unlock(&efi_rt_lock);
 	efi_virtmap_unload();
+	mutex_unlock(&efi_rt_lock);
 }
 
 asmlinkage u64 *efi_rt_stack_top __ro_after_init;
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 7/8] arm64/efi: Move uaccess en/disable out of efi_set_pgd()
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2025-09-18 10:30 ` [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-19 11:36   ` Will Deacon
  2025-09-18 10:30 ` [PATCH v3 8/8] arm64/efi: Call EFI runtime services without disabling preemption Ard Biesheuvel
  2025-09-18 11:33 ` [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
  8 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

From: Ard Biesheuvel <ardb@kernel.org>

efi_set_pgd() will no longer be called when invoking EFI runtime
services via the efi_rts_wq work queue, but the uaccess en/disable are
still needed when using PAN emulation using TTBR0 switching. So move
these into the callers.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/efi.h | 13 +++----------
 arch/arm64/kernel/efi.c      | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index decf87777f57..09650b2e15af 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -126,21 +126,14 @@ static inline void efi_set_pgd(struct mm_struct *mm)
 		if (mm != current->active_mm) {
 			/*
 			 * Update the current thread's saved ttbr0 since it is
-			 * restored as part of a return from exception. Enable
-			 * access to the valid TTBR0_EL1 and invoke the errata
-			 * workaround directly since there is no return from
-			 * exception when invoking the EFI run-time services.
+			 * restored as part of a return from exception.
 			 */
 			update_saved_ttbr0(current, mm);
-			uaccess_ttbr0_enable();
-			post_ttbr_update_workaround();
 		} else {
 			/*
-			 * Defer the switch to the current thread's TTBR0_EL1
-			 * until uaccess_enable(). Restore the current
-			 * thread's saved ttbr0 corresponding to its active_mm
+			 * Restore the current thread's saved ttbr0
+			 * corresponding to its active_mm
 			 */
-			uaccess_ttbr0_disable();
 			update_saved_ttbr0(current, current->active_mm);
 		}
 	}
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 4372fafde8e9..a60444dcec68 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -182,6 +182,15 @@ bool arch_efi_call_virt_setup(void)
 		return false;
 
 	efi_virtmap_load();
+
+	/*
+	 * Enable access to the valid TTBR0_EL1 and invoke the errata
+	 * workaround directly since there is no return from exception when
+	 * invoking the EFI run-time services.
+	 */
+	uaccess_ttbr0_enable();
+	post_ttbr_update_workaround();
+
 	kernel_neon_begin();
 	return true;
 }
@@ -189,6 +198,15 @@ bool arch_efi_call_virt_setup(void)
 void arch_efi_call_virt_teardown(void)
 {
 	kernel_neon_end();
+
+	/*
+	 * Defer the switch to the current thread's TTBR0_EL1 until
+	 * uaccess_enable(). Do so before efi_virtmap_unload() updates the
+	 * saved TTBR0 value, so the userland page tables are not activated
+	 * inadvertently over the back of an exception.
+	 */
+	uaccess_ttbr0_disable();
+
 	efi_virtmap_unload();
 	mutex_unlock(&efi_rt_lock);
 }
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 8/8] arm64/efi: Call EFI runtime services without disabling preemption
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
                   ` (6 preceding siblings ...)
  2025-09-18 10:30 ` [PATCH v3 7/8] arm64/efi: Move uaccess en/disable out of efi_set_pgd() Ard Biesheuvel
@ 2025-09-18 10:30 ` Ard Biesheuvel
  2025-09-19 11:36   ` Will Deacon
  2025-09-18 11:33 ` [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
  8 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 10:30 UTC (permalink / raw)
  To: linux-efi
  Cc: linux-kernel, linux-arm-kernel, Ard Biesheuvel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

From: Ard Biesheuvel <ardb@kernel.org>

The only remaining reason why EFI runtime services are invoked with
preemption disabled is the fact that the mm is swapped out behind the
back of the context switching code.

The kernel no longer disables preemption in kernel_neon_begin().
Furthermore, the EFI spec is being clarified to explicitly state that
only baseline FP/SIMD is permitted in EFI runtime service
implementations, and so the existing kernel mode NEON context switching
code is sufficient to preserve and restore the execution context of an
in-progress EFI runtime service call.

Most EFI calls are made from the efi_rts_wq, which is serviced by a
kthread. As kthreads never return to user space, they usually don't have
an mm, and so we can use the existing infrastructure to swap in the
efi_mm while the EFI call is in progress. This is visible to the
scheduler, which will therefore reactivate the selected mm when
switching out the kthread and back in again.

Given that the EFI spec explicitly permits runtime services to be called
with interrupts enabled, firmware code is already required to tolerate
interruptions. So rather than disable preemption, disable only migration
so that EFI runtime services are less likely to cause scheduling delays.
To avoid potential issues where runtime services are interrupted while
polling the secure firmware for async completions, keep migration
disabled so that a runtime service invocation does not resume on a
different CPU from the one it was started on.

Note, though, that the firmware executes at the same privilege level as
the kernel, and is therefore able to disable interrupts altogether.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/efi.c | 23 ++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index a60444dcec68..9b1603a69b69 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -10,6 +10,7 @@
 #include <linux/efi.h>
 #include <linux/init.h>
 #include <linux/kmemleak.h>
+#include <linux/kthread.h>
 #include <linux/screen_info.h>
 #include <linux/vmalloc.h>
 
@@ -181,7 +182,19 @@ bool arch_efi_call_virt_setup(void)
 	if (WARN_ON(!mutex_trylock(&efi_rt_lock)))
 		return false;
 
-	efi_virtmap_load();
+	if (preemptible() && (current->flags & PF_KTHREAD)) {
+		/*
+		 * Disable migration to ensure that a preempted EFI runtime
+		 * service call will be resumed on the same CPU. This avoids
+		 * potential issues with EFI runtime calls that are preempted
+		 * while polling for an asynchronous completion of a secure
+		 * firmware call, which may not permit the CPU to change.
+		 */
+		migrate_disable();
+		kthread_use_mm(&efi_mm);
+	} else {
+		efi_virtmap_load();
+	}
 
 	/*
 	 * Enable access to the valid TTBR0_EL1 and invoke the errata
@@ -207,7 +220,13 @@ void arch_efi_call_virt_teardown(void)
 	 */
 	uaccess_ttbr0_disable();
 
-	efi_virtmap_unload();
+	if (preemptible() && (current->flags & PF_KTHREAD)) {
+		kthread_unuse_mm(&efi_mm);
+		migrate_enable();
+	} else {
+		efi_virtmap_unload();
+	}
+
 	mutex_unlock(&efi_rt_lock);
 }
 
-- 
2.51.0.384.g4c02a37b29-goog



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 0/8] arm64: Make EFI calls preemptible
  2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
                   ` (7 preceding siblings ...)
  2025-09-18 10:30 ` [PATCH v3 8/8] arm64/efi: Call EFI runtime services without disabling preemption Ard Biesheuvel
@ 2025-09-18 11:33 ` Ard Biesheuvel
  2025-09-18 11:44   ` Will Deacon
  2025-09-19 11:38   ` Will Deacon
  8 siblings, 2 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 11:33 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, linux-kernel, linux-arm-kernel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, 18 Sept 2025 at 12:30, Ard Biesheuvel <ardb+git@google.com> wrote:
>
> From: Ard Biesheuvel <ardb@kernel.org>
>
> The arm64 port permits the use of the baseline FP/SIMD register file in
> kernel mode, and no longer requires preemption to be disabled. Now that
> the EFI spec is being clarified to state that EFI runtime services may
> only use baseline FP/SIMD, the fact that EFI may code may use FP/SIMD
> registers (while executing at the same privilege level as the kernel) is
> no longer a reason to disable preemption when invoking them.
>
> This means that the only remaining reason for disabling preemption is
> the fact that the active mm is swapped out and replaced with efi_mm in a
> way that is hidden from the scheduler, and so scheduling is not
> supported currently. However, given that virtually all (*) EFI runtime
> calls are made from the efi_rts_wq workqueue, the efi_mm can simply be
> loaded into the workqueue worker kthread while the call is in progress,
> and this does not require preemption to be disabled.
>
> Note that this is only a partial solution in terms of RT guarantees,
> given that the runtime services execute at the same privilege level as
> the kernel, and can therefore disable interrupts (and therefore
> preemption) directly. But it should prevent scheduling latency spikes
> for EFI calls that simply take a long time to run to completion.
>
> Changes since v2:
> - Permit ordinary kernel mode FP/SIMD with IRQs disabled, so that the
>   special EFI case only deals with invocations in hardirq or NMI context
> - Disallow EFI runtime calls in hardirq or NMI context, so that the
>   special FP/SIMD handling for EFI can be dropped entirely
> - Use a mutex rather than a semaphore for the arm64 EFI runtime lock,
>   now that it is never trylock()ed in IRQ or NMI context.
>
> Changes since v1/RFC:
> - Disable uaccess for SWPAN before updating the preserved TTBR0 value
> - Document why disabling migration is needed
> - Rebase onto v6.17-rc1
>
> (*) only efi_reset_system() and EFI pstore invoke EFI runtime services
>     without going through the workqueue, and the latter only when saving
>     a kernel oops log to the EFI varstore
>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
>
> Ard Biesheuvel (8):
>   efi: Add missing static initializer for efi_mm::cpus_allowed_lock
>   efi/runtime: Return success/failure from arch_efi_call_virt_setup()
>   efi/runtime: Deal with arch_efi_call_virt_setup() returning failure

Unless anyone objects, I am going to queue up these 3 patches ^^^ via
the EFI tree.

>   arm64/fpsimd: Permit kernel mode NEON with IRQs off
>   arm64/fpsimd: Drop special handling for EFI runtime services
>   arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
>   arm64/efi: Move uaccess en/disable out of efi_set_pgd()
>   arm64/efi: Call EFI runtime services without disabling preemption
>

... so the rest can go in via the arm64 tree in the next cycle.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 0/8] arm64: Make EFI calls preemptible
  2025-09-18 11:33 ` [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
@ 2025-09-18 11:44   ` Will Deacon
  2025-09-18 11:48     ` Ard Biesheuvel
  2025-09-19 11:38   ` Will Deacon
  1 sibling, 1 reply; 22+ messages in thread
From: Will Deacon @ 2025-09-18 11:44 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Ard Biesheuvel, linux-efi, linux-kernel, linux-arm-kernel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, Sep 18, 2025 at 01:33:48PM +0200, Ard Biesheuvel wrote:
> On Thu, 18 Sept 2025 at 12:30, Ard Biesheuvel <ardb+git@google.com> wrote:
> >
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > The arm64 port permits the use of the baseline FP/SIMD register file in
> > kernel mode, and no longer requires preemption to be disabled. Now that
> > the EFI spec is being clarified to state that EFI runtime services may
> > only use baseline FP/SIMD, the fact that EFI may code may use FP/SIMD
> > registers (while executing at the same privilege level as the kernel) is
> > no longer a reason to disable preemption when invoking them.
> >
> > This means that the only remaining reason for disabling preemption is
> > the fact that the active mm is swapped out and replaced with efi_mm in a
> > way that is hidden from the scheduler, and so scheduling is not
> > supported currently. However, given that virtually all (*) EFI runtime
> > calls are made from the efi_rts_wq workqueue, the efi_mm can simply be
> > loaded into the workqueue worker kthread while the call is in progress,
> > and this does not require preemption to be disabled.
> >
> > Note that this is only a partial solution in terms of RT guarantees,
> > given that the runtime services execute at the same privilege level as
> > the kernel, and can therefore disable interrupts (and therefore
> > preemption) directly. But it should prevent scheduling latency spikes
> > for EFI calls that simply take a long time to run to completion.
> >
> > Changes since v2:
> > - Permit ordinary kernel mode FP/SIMD with IRQs disabled, so that the
> >   special EFI case only deals with invocations in hardirq or NMI context
> > - Disallow EFI runtime calls in hardirq or NMI context, so that the
> >   special FP/SIMD handling for EFI can be dropped entirely
> > - Use a mutex rather than a semaphore for the arm64 EFI runtime lock,
> >   now that it is never trylock()ed in IRQ or NMI context.
> >
> > Changes since v1/RFC:
> > - Disable uaccess for SWPAN before updating the preserved TTBR0 value
> > - Document why disabling migration is needed
> > - Rebase onto v6.17-rc1
> >
> > (*) only efi_reset_system() and EFI pstore invoke EFI runtime services
> >     without going through the workqueue, and the latter only when saving
> >     a kernel oops log to the EFI varstore
> >
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Brown <broonie@kernel.org>
> >
> > Ard Biesheuvel (8):
> >   efi: Add missing static initializer for efi_mm::cpus_allowed_lock
> >   efi/runtime: Return success/failure from arch_efi_call_virt_setup()
> >   efi/runtime: Deal with arch_efi_call_virt_setup() returning failure
> 
> Unless anyone objects, I am going to queue up these 3 patches ^^^ via
> the EFI tree.
> 
> >   arm64/fpsimd: Permit kernel mode NEON with IRQs off
> >   arm64/fpsimd: Drop special handling for EFI runtime services
> >   arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
> >   arm64/efi: Move uaccess en/disable out of efi_set_pgd()
> >   arm64/efi: Call EFI runtime services without disabling preemption
> >
> 
> ... so the rest can go in via the arm64 tree in the next cycle.

I'm also happy to take the whole lot via arm64 this cycle, if you like?
I reviewed it a while ago and was happy with it then.

Up to you.

Will


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 0/8] arm64: Make EFI calls preemptible
  2025-09-18 11:44   ` Will Deacon
@ 2025-09-18 11:48     ` Ard Biesheuvel
  0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 11:48 UTC (permalink / raw)
  To: Will Deacon
  Cc: Ard Biesheuvel, linux-efi, linux-kernel, linux-arm-kernel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, 18 Sept 2025 at 13:44, Will Deacon <will@kernel.org> wrote:
>
> On Thu, Sep 18, 2025 at 01:33:48PM +0200, Ard Biesheuvel wrote:
> > On Thu, 18 Sept 2025 at 12:30, Ard Biesheuvel <ardb+git@google.com> wrote:
> > >
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > The arm64 port permits the use of the baseline FP/SIMD register file in
> > > kernel mode, and no longer requires preemption to be disabled. Now that
> > > the EFI spec is being clarified to state that EFI runtime services may
> > > only use baseline FP/SIMD, the fact that EFI may code may use FP/SIMD
> > > registers (while executing at the same privilege level as the kernel) is
> > > no longer a reason to disable preemption when invoking them.
> > >
> > > This means that the only remaining reason for disabling preemption is
> > > the fact that the active mm is swapped out and replaced with efi_mm in a
> > > way that is hidden from the scheduler, and so scheduling is not
> > > supported currently. However, given that virtually all (*) EFI runtime
> > > calls are made from the efi_rts_wq workqueue, the efi_mm can simply be
> > > loaded into the workqueue worker kthread while the call is in progress,
> > > and this does not require preemption to be disabled.
> > >
> > > Note that this is only a partial solution in terms of RT guarantees,
> > > given that the runtime services execute at the same privilege level as
> > > the kernel, and can therefore disable interrupts (and therefore
> > > preemption) directly. But it should prevent scheduling latency spikes
> > > for EFI calls that simply take a long time to run to completion.
> > >
> > > Changes since v2:
> > > - Permit ordinary kernel mode FP/SIMD with IRQs disabled, so that the
> > >   special EFI case only deals with invocations in hardirq or NMI context
> > > - Disallow EFI runtime calls in hardirq or NMI context, so that the
> > >   special FP/SIMD handling for EFI can be dropped entirely
> > > - Use a mutex rather than a semaphore for the arm64 EFI runtime lock,
> > >   now that it is never trylock()ed in IRQ or NMI context.
> > >
> > > Changes since v1/RFC:
> > > - Disable uaccess for SWPAN before updating the preserved TTBR0 value
> > > - Document why disabling migration is needed
> > > - Rebase onto v6.17-rc1
> > >
> > > (*) only efi_reset_system() and EFI pstore invoke EFI runtime services
> > >     without going through the workqueue, and the latter only when saving
> > >     a kernel oops log to the EFI varstore
> > >
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Mark Brown <broonie@kernel.org>
> > >
> > > Ard Biesheuvel (8):
> > >   efi: Add missing static initializer for efi_mm::cpus_allowed_lock
> > >   efi/runtime: Return success/failure from arch_efi_call_virt_setup()
> > >   efi/runtime: Deal with arch_efi_call_virt_setup() returning failure
> >
> > Unless anyone objects, I am going to queue up these 3 patches ^^^ via
> > the EFI tree.
> >
> > >   arm64/fpsimd: Permit kernel mode NEON with IRQs off
> > >   arm64/fpsimd: Drop special handling for EFI runtime services
> > >   arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
> > >   arm64/efi: Move uaccess en/disable out of efi_set_pgd()
> > >   arm64/efi: Call EFI runtime services without disabling preemption
> > >
> >
> > ... so the rest can go in via the arm64 tree in the next cycle.
>
> I'm also happy to take the whole lot via arm64 this cycle, if you like?
> I reviewed it a while ago and was happy with it then.
>

I've made some major changes this time, so please double check that
you're still ok with it.

In particular, I've ripped out all of the special EFI handling in fpsimd.c


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
  2025-09-18 10:30 ` [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services Ard Biesheuvel
@ 2025-09-18 11:57   ` Ard Biesheuvel
  2025-09-18 13:10   ` Mark Brown
  1 sibling, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-18 11:57 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, linux-kernel, linux-arm-kernel, Will Deacon,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

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();
 }

 /*


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
  2025-09-18 10:30 ` [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services Ard Biesheuvel
  2025-09-18 11:57   ` Ard Biesheuvel
@ 2025-09-18 13:10   ` Mark Brown
  2025-09-22  6:55     ` Ard Biesheuvel
  1 sibling, 1 reply; 22+ messages in thread
From: Mark Brown @ 2025-09-18 13:10 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, linux-kernel, linux-arm-kernel, Ard Biesheuvel,
	Will Deacon, Mark Rutland, Sebastian Andrzej Siewior,
	Peter Zijlstra, Catalin Marinas

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

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>

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 4/8] arm64/fpsimd: Permit kernel mode NEON with IRQs off
  2025-09-18 10:30 ` [PATCH v3 4/8] arm64/fpsimd: Permit kernel mode NEON with IRQs off Ard Biesheuvel
@ 2025-09-19 11:33   ` Will Deacon
  0 siblings, 0 replies; 22+ messages in thread
From: Will Deacon @ 2025-09-19 11:33 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, linux-kernel, linux-arm-kernel, Ard Biesheuvel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, Sep 18, 2025 at 12:30:15PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Currently, may_use_simd() will return false when called from a context
> where IRQs are disabled. One notable case where this happens is when
> calling the ResetSystem() EFI runtime service from the reboot/poweroff
> code path. For this case alone, there is a substantial amount of FP/SIMD
> support code to handle the corner case where a EFI runtime service is
> invoked with IRQs disabled.
> 
> The only reason kernel mode SIMD is not allowed when IRQs are disabled
> is that re-enabling softirqs in this case produces a noisy diagnostic
> when lockdep is enabled. The warning is valid, in the sense that
> delivering pending softirqs over the back of the call to
> local_bh_enable() is problematic when IRQs are disabled.
> 
> While the API lacks a facility to simply mask and unmask softirqs
> without triggering their delivery, disabling softirqs is not needed to
> begin with when IRQs are disabled, given that softirqs are only every
> taken asynchronously over the back of a hard IRQ.
> 
> So dis/enable softirq processing conditionally, based on whether IRQs
> are enabled, and relax the check in may_use_simd().
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/include/asm/simd.h |  2 +-
>  arch/arm64/kernel/fpsimd.c    | 16 ++++++++++------
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
> index 8e86c9e70e48..abd642c92f86 100644
> --- a/arch/arm64/include/asm/simd.h
> +++ b/arch/arm64/include/asm/simd.h
> @@ -29,7 +29,7 @@ static __must_check inline bool may_use_simd(void)
>  	 */
>  	return !WARN_ON(!system_capabilities_finalized()) &&
>  	       system_supports_fpsimd() &&
> -	       !in_hardirq() && !irqs_disabled() && !in_nmi();
> +	       !in_hardirq() && !in_nmi();
>  }
>  
>  #else /* ! CONFIG_KERNEL_MODE_NEON */
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index c37f02d7194e..96a226316d1f 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -225,10 +225,12 @@ static void fpsimd_bind_task_to_cpu(void);
>   */
>  static void get_cpu_fpsimd_context(void)
>  {
> -	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> -		local_bh_disable();
> -	else
> +	if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
> +		if (!irqs_disabled())
> +			local_bh_disable();
> +	} else {
>  		preempt_disable();
> +	}
>  }
>  
>  /*
> @@ -240,10 +242,12 @@ static void get_cpu_fpsimd_context(void)
>   */
>  static void put_cpu_fpsimd_context(void)
>  {
> -	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> -		local_bh_enable();
> -	else
> +	if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
> +		if (!irqs_disabled())
> +			local_bh_enable();

This is a little ugly, so I think a small comment summarising the
penultimate paragraph of your commit message would help to explain the dance
you're doing.

With that:

Acked-by: Will Deacon <will@kernel.org>

Will


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
  2025-09-18 10:30 ` [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state Ard Biesheuvel
@ 2025-09-19 11:35   ` Will Deacon
  2025-09-19 13:42     ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Will Deacon @ 2025-09-19 11:35 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, linux-kernel, linux-arm-kernel, Ard Biesheuvel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, Sep 18, 2025 at 12:30:17PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Replace the spinlock in the arm64 glue code with a mutex, so that
> the CPU can preempted while running the EFI runtime service.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/kernel/efi.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 0d52414415f3..4372fafde8e9 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -166,15 +166,22 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
>  	return s;
>  }
>  
> -static DEFINE_RAW_SPINLOCK(efi_rt_lock);
> +static DEFINE_MUTEX(efi_rt_lock);
>  
>  bool arch_efi_call_virt_setup(void)
>  {
>  	if (!may_use_simd())
>  		return false;
>  
> +	/*
> +	 * This might be called from a non-sleepable context so try to take the
> +	 * lock but don't block on it. This should never fail in practice, as
> +	 * all EFI runtime calls are serialized under the efi_runtime_lock.
> +	 */
> +	if (WARN_ON(!mutex_trylock(&efi_rt_lock)))
> +		return false;

If it will never fail in practice, why do we need the lock at all? Can we
just assert that the efi_runtime_lock is held instead and rely on that?

Will


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 7/8] arm64/efi: Move uaccess en/disable out of efi_set_pgd()
  2025-09-18 10:30 ` [PATCH v3 7/8] arm64/efi: Move uaccess en/disable out of efi_set_pgd() Ard Biesheuvel
@ 2025-09-19 11:36   ` Will Deacon
  0 siblings, 0 replies; 22+ messages in thread
From: Will Deacon @ 2025-09-19 11:36 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, linux-kernel, linux-arm-kernel, Ard Biesheuvel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, Sep 18, 2025 at 12:30:18PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> efi_set_pgd() will no longer be called when invoking EFI runtime
> services via the efi_rts_wq work queue, but the uaccess en/disable are
> still needed when using PAN emulation using TTBR0 switching. So move
> these into the callers.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/include/asm/efi.h | 13 +++----------
>  arch/arm64/kernel/efi.c      | 18 ++++++++++++++++++
>  2 files changed, 21 insertions(+), 10 deletions(-)

Thanks. This addresses the comment I made last time around:

Acked-by: Will Deacon <will@kernel.org>

Will


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 8/8] arm64/efi: Call EFI runtime services without disabling preemption
  2025-09-18 10:30 ` [PATCH v3 8/8] arm64/efi: Call EFI runtime services without disabling preemption Ard Biesheuvel
@ 2025-09-19 11:36   ` Will Deacon
  0 siblings, 0 replies; 22+ messages in thread
From: Will Deacon @ 2025-09-19 11:36 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, linux-kernel, linux-arm-kernel, Ard Biesheuvel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, Sep 18, 2025 at 12:30:19PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> The only remaining reason why EFI runtime services are invoked with
> preemption disabled is the fact that the mm is swapped out behind the
> back of the context switching code.
> 
> The kernel no longer disables preemption in kernel_neon_begin().
> Furthermore, the EFI spec is being clarified to explicitly state that
> only baseline FP/SIMD is permitted in EFI runtime service
> implementations, and so the existing kernel mode NEON context switching
> code is sufficient to preserve and restore the execution context of an
> in-progress EFI runtime service call.
> 
> Most EFI calls are made from the efi_rts_wq, which is serviced by a
> kthread. As kthreads never return to user space, they usually don't have
> an mm, and so we can use the existing infrastructure to swap in the
> efi_mm while the EFI call is in progress. This is visible to the
> scheduler, which will therefore reactivate the selected mm when
> switching out the kthread and back in again.
> 
> Given that the EFI spec explicitly permits runtime services to be called
> with interrupts enabled, firmware code is already required to tolerate
> interruptions. So rather than disable preemption, disable only migration
> so that EFI runtime services are less likely to cause scheduling delays.
> To avoid potential issues where runtime services are interrupted while
> polling the secure firmware for async completions, keep migration
> disabled so that a runtime service invocation does not resume on a
> different CPU from the one it was started on.
> 
> Note, though, that the firmware executes at the same privilege level as
> the kernel, and is therefore able to disable interrupts altogether.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/kernel/efi.c | 23 ++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)

Acked-by: Will Deacon <will@kernel.org>

Will


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 0/8] arm64: Make EFI calls preemptible
  2025-09-18 11:33 ` [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
  2025-09-18 11:44   ` Will Deacon
@ 2025-09-19 11:38   ` Will Deacon
  1 sibling, 0 replies; 22+ messages in thread
From: Will Deacon @ 2025-09-19 11:38 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Ard Biesheuvel, linux-efi, linux-kernel, linux-arm-kernel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Thu, Sep 18, 2025 at 01:33:48PM +0200, Ard Biesheuvel wrote:
> On Thu, 18 Sept 2025 at 12:30, Ard Biesheuvel <ardb+git@google.com> wrote:
> >
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > The arm64 port permits the use of the baseline FP/SIMD register file in
> > kernel mode, and no longer requires preemption to be disabled. Now that
> > the EFI spec is being clarified to state that EFI runtime services may
> > only use baseline FP/SIMD, the fact that EFI may code may use FP/SIMD
> > registers (while executing at the same privilege level as the kernel) is
> > no longer a reason to disable preemption when invoking them.
> >
> > This means that the only remaining reason for disabling preemption is
> > the fact that the active mm is swapped out and replaced with efi_mm in a
> > way that is hidden from the scheduler, and so scheduling is not
> > supported currently. However, given that virtually all (*) EFI runtime
> > calls are made from the efi_rts_wq workqueue, the efi_mm can simply be
> > loaded into the workqueue worker kthread while the call is in progress,
> > and this does not require preemption to be disabled.
> >
> > Note that this is only a partial solution in terms of RT guarantees,
> > given that the runtime services execute at the same privilege level as
> > the kernel, and can therefore disable interrupts (and therefore
> > preemption) directly. But it should prevent scheduling latency spikes
> > for EFI calls that simply take a long time to run to completion.
> >
> > Changes since v2:
> > - Permit ordinary kernel mode FP/SIMD with IRQs disabled, so that the
> >   special EFI case only deals with invocations in hardirq or NMI context
> > - Disallow EFI runtime calls in hardirq or NMI context, so that the
> >   special FP/SIMD handling for EFI can be dropped entirely
> > - Use a mutex rather than a semaphore for the arm64 EFI runtime lock,
> >   now that it is never trylock()ed in IRQ or NMI context.
> >
> > Changes since v1/RFC:
> > - Disable uaccess for SWPAN before updating the preserved TTBR0 value
> > - Document why disabling migration is needed
> > - Rebase onto v6.17-rc1
> >
> > (*) only efi_reset_system() and EFI pstore invoke EFI runtime services
> >     without going through the workqueue, and the latter only when saving
> >     a kernel oops log to the EFI varstore
> >
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Brown <broonie@kernel.org>
> >
> > Ard Biesheuvel (8):
> >   efi: Add missing static initializer for efi_mm::cpus_allowed_lock
> >   efi/runtime: Return success/failure from arch_efi_call_virt_setup()
> >   efi/runtime: Deal with arch_efi_call_virt_setup() returning failure
> 
> Unless anyone objects, I am going to queue up these 3 patches ^^^ via
> the EFI tree.
> 
> >   arm64/fpsimd: Permit kernel mode NEON with IRQs off
> >   arm64/fpsimd: Drop special handling for EFI runtime services
> >   arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
> >   arm64/efi: Move uaccess en/disable out of efi_set_pgd()
> >   arm64/efi: Call EFI runtime services without disabling preemption
> >
> 
> ... so the rest can go in via the arm64 tree in the next cycle.

As discussed off-list, that's probably the best plan for now. I've left
a few small comments on some of the arm64 bits and it would be good to
give them some soak time in -next.

Cheers,

Will


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
  2025-09-19 11:35   ` Will Deacon
@ 2025-09-19 13:42     ` Ard Biesheuvel
  2025-09-19 13:54       ` Will Deacon
  0 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-19 13:42 UTC (permalink / raw)
  To: Will Deacon
  Cc: Ard Biesheuvel, linux-efi, linux-kernel, linux-arm-kernel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Fri, 19 Sept 2025 at 13:35, Will Deacon <will@kernel.org> wrote:
>
> On Thu, Sep 18, 2025 at 12:30:17PM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > Replace the spinlock in the arm64 glue code with a mutex, so that
> > the CPU can preempted while running the EFI runtime service.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/kernel/efi.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 0d52414415f3..4372fafde8e9 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -166,15 +166,22 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
> >       return s;
> >  }
> >
> > -static DEFINE_RAW_SPINLOCK(efi_rt_lock);
> > +static DEFINE_MUTEX(efi_rt_lock);
> >
> >  bool arch_efi_call_virt_setup(void)
> >  {
> >       if (!may_use_simd())
> >               return false;
> >
> > +     /*
> > +      * This might be called from a non-sleepable context so try to take the
> > +      * lock but don't block on it. This should never fail in practice, as
> > +      * all EFI runtime calls are serialized under the efi_runtime_lock.
> > +      */
> > +     if (WARN_ON(!mutex_trylock(&efi_rt_lock)))
> > +             return false;
>
> If it will never fail in practice, why do we need the lock at all? Can we
> just assert that the efi_runtime_lock is held instead and rely on that?
>

Excellent point.

Do you mean a lockdep assert? efi_runtime_lock is a semaphore, so
there is no is_locked() API that we can BUG() on here.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
  2025-09-19 13:42     ` Ard Biesheuvel
@ 2025-09-19 13:54       ` Will Deacon
  0 siblings, 0 replies; 22+ messages in thread
From: Will Deacon @ 2025-09-19 13:54 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Ard Biesheuvel, linux-efi, linux-kernel, linux-arm-kernel,
	Mark Rutland, Sebastian Andrzej Siewior, Peter Zijlstra,
	Catalin Marinas, Mark Brown

On Fri, Sep 19, 2025 at 03:42:12PM +0200, Ard Biesheuvel wrote:
> On Fri, 19 Sept 2025 at 13:35, Will Deacon <will@kernel.org> wrote:
> >
> > On Thu, Sep 18, 2025 at 12:30:17PM +0200, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Replace the spinlock in the arm64 glue code with a mutex, so that
> > > the CPU can preempted while running the EFI runtime service.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  arch/arm64/kernel/efi.c | 13 ++++++++++---
> > >  1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > > index 0d52414415f3..4372fafde8e9 100644
> > > --- a/arch/arm64/kernel/efi.c
> > > +++ b/arch/arm64/kernel/efi.c
> > > @@ -166,15 +166,22 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
> > >       return s;
> > >  }
> > >
> > > -static DEFINE_RAW_SPINLOCK(efi_rt_lock);
> > > +static DEFINE_MUTEX(efi_rt_lock);
> > >
> > >  bool arch_efi_call_virt_setup(void)
> > >  {
> > >       if (!may_use_simd())
> > >               return false;
> > >
> > > +     /*
> > > +      * This might be called from a non-sleepable context so try to take the
> > > +      * lock but don't block on it. This should never fail in practice, as
> > > +      * all EFI runtime calls are serialized under the efi_runtime_lock.
> > > +      */
> > > +     if (WARN_ON(!mutex_trylock(&efi_rt_lock)))
> > > +             return false;
> >
> > If it will never fail in practice, why do we need the lock at all? Can we
> > just assert that the efi_runtime_lock is held instead and rely on that?
> >
> 
> Excellent point.
> 
> Do you mean a lockdep assert? efi_runtime_lock is a semaphore, so
> there is no is_locked() API that we can BUG() on here.

Yes, I was thinking of lockdep. Even though lockdep doesn't tend to be
enabled in production, just having it in the code is useful documentation
imo.

Will


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services
  2025-09-18 13:10   ` Mark Brown
@ 2025-09-22  6:55     ` Ard Biesheuvel
  0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2025-09-22  6:55 UTC (permalink / raw)
  To: Mark Brown
  Cc: Ard Biesheuvel, linux-efi, linux-kernel, linux-arm-kernel,
	Will Deacon, Mark Rutland, Sebastian Andrzej Siewior,
	Peter Zijlstra, Catalin Marinas

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.


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2025-09-22  6:55 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 1/8] efi: Add missing static initializer for efi_mm::cpus_allowed_lock Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 2/8] efi/runtime: Return success/failure from arch_efi_call_virt_setup() Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 3/8] efi/runtime: Deal with arch_efi_call_virt_setup() returning failure Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 4/8] arm64/fpsimd: Permit kernel mode NEON with IRQs off Ard Biesheuvel
2025-09-19 11:33   ` Will Deacon
2025-09-18 10:30 ` [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services Ard Biesheuvel
2025-09-18 11:57   ` Ard Biesheuvel
2025-09-18 13:10   ` Mark Brown
2025-09-22  6:55     ` Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state Ard Biesheuvel
2025-09-19 11:35   ` Will Deacon
2025-09-19 13:42     ` Ard Biesheuvel
2025-09-19 13:54       ` Will Deacon
2025-09-18 10:30 ` [PATCH v3 7/8] arm64/efi: Move uaccess en/disable out of efi_set_pgd() Ard Biesheuvel
2025-09-19 11:36   ` Will Deacon
2025-09-18 10:30 ` [PATCH v3 8/8] arm64/efi: Call EFI runtime services without disabling preemption Ard Biesheuvel
2025-09-19 11:36   ` Will Deacon
2025-09-18 11:33 ` [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
2025-09-18 11:44   ` Will Deacon
2025-09-18 11:48     ` Ard Biesheuvel
2025-09-19 11:38   ` Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox