Linux EFI development
 help / color / mirror / Atom feed
* [PATCH] x86/efi: Restore IRQ state in EFI page fault handler
@ 2026-05-01  9:03 Ard Biesheuvel
  2026-05-04 16:39 ` Ard Biesheuvel
  2026-05-04 18:01 ` Eric Biggers
  0 siblings, 2 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2026-05-01  9:03 UTC (permalink / raw)
  To: linux-efi; +Cc: x86, Ard Biesheuvel, Eric Biggers, Ivan Hu

From: Ard Biesheuvel <ardb@kernel.org>

The kernel's softirq API does not permit re-enabling softirqs while IRQs
are disabled. The reason for this is that local_bh_enable() will not
only re-enable delivery of softirqs over the back of IRQs, it will also
handle any pending softirqs immediately, regardless of whether IRQs are
enabled at that point.

For this reason, commit

  d02198550423 ("x86/fpu: Improve crypto performance by making kernel-mode FPU reliably usable in softirqs")

disables softirqs only when IRQs are enabled, as it is not permitted
otherwise, but also unnecessary, given that asynchronous softirq
delivery never happens to begin with while IRQs are disabled.

However, this does mean that entering a kernel mode FPU section with
IRQs enabled and leaving it with IRQs disabled leads to problems, as
identified by Sashiko [0]: the EFI page fault handler is called from
page_fault_oops() with IRQs disabled, and thus ends the kernel mode FPU
section with IRQs disabled as well, regardless of whether IRQs were
enabled when it was started. This may result in schedule() being called
with a non-zero preempt_count, causing a BUG().

So take care to re-enable IRQs when handling any EFI page faults if they
were taken with IRQs enabled.

[0] https://sashiko.dev/#/patchset/20260430074107.27051-1-ivan.hu%40canonical.com

Cc: Eric Biggers <ebiggers@kernel.org>
Cc: Ivan Hu <ivan.hu@canonical.com>
Fixes: d02198550423 ("x86/fpu: Improve crypto performance by making kernel-mode FPU reliably usable in softirqs")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/include/asm/efi.h     |  3 ++-
 arch/x86/mm/fault.c            |  2 +-
 arch/x86/platform/efi/quirks.c | 11 ++++++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index dc8fe1361c18..be58b7f5c806 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -137,7 +137,8 @@ extern void __init efi_dump_pagetable(void);
 extern void __init efi_apply_memmap_quirks(void);
 extern int __init efi_reuse_config(u64 tables, int nr_tables);
 extern void efi_delete_dummy_variable(void);
-extern void efi_crash_gracefully_on_page_fault(unsigned long phys_addr);
+extern void efi_crash_gracefully_on_page_fault(unsigned long phys_addr,
+					       const struct pt_regs *regs);
 extern void efi_unmap_boot_services(void);
 
 void arch_efi_call_virt_setup(void);
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index f0e77e084482..63de8e8684f2 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -686,7 +686,7 @@ page_fault_oops(struct pt_regs *regs, unsigned long error_code,
 	 * avoid hanging the system.
 	 */
 	if (IS_ENABLED(CONFIG_EFI))
-		efi_crash_gracefully_on_page_fault(address);
+		efi_crash_gracefully_on_page_fault(address, regs);
 
 	/* Only not-present faults should be handled by KFENCE. */
 	if (!(error_code & X86_PF_PROT) &&
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 7475405119ce..425552e4ec36 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -761,7 +761,8 @@ int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
  * @return: Returns, if the page fault is not handled. This function
  * will never return if the page fault is handled successfully.
  */
-void efi_crash_gracefully_on_page_fault(unsigned long phys_addr)
+void efi_crash_gracefully_on_page_fault(unsigned long phys_addr,
+					const struct pt_regs *regs)
 {
 	if (!IS_ENABLED(CONFIG_X86_64))
 		return;
@@ -810,6 +811,14 @@ void efi_crash_gracefully_on_page_fault(unsigned long phys_addr)
 		return;
 	}
 
+	/*
+	 * The API does not permit entering a kernel mode FPU section with
+	 * interrupts enabled and leaving it with interrupts disabled.  So
+	 * re-enable interrupts now if they were enabled when the page fault
+	 * occurred.
+	 */
+	local_irq_restore(regs->flags);
+
 	/*
 	 * Before calling EFI Runtime Service, the kernel has switched the
 	 * calling process to efi_mm. Hence, switch back to task_mm.
-- 
2.54.0.545.g6539524ca2-goog


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

* Re: [PATCH] x86/efi: Restore IRQ state in EFI page fault handler
  2026-05-01  9:03 [PATCH] x86/efi: Restore IRQ state in EFI page fault handler Ard Biesheuvel
@ 2026-05-04 16:39 ` Ard Biesheuvel
  2026-05-04 18:01 ` Eric Biggers
  1 sibling, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2026-05-04 16:39 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-efi; +Cc: x86, Eric Biggers, Ivan Hu



On Fri, 1 May 2026, at 11:03, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> The kernel's softirq API does not permit re-enabling softirqs while IRQs
> are disabled. The reason for this is that local_bh_enable() will not
> only re-enable delivery of softirqs over the back of IRQs, it will also
> handle any pending softirqs immediately, regardless of whether IRQs are
> enabled at that point.
>
> For this reason, commit
>
>   d02198550423 ("x86/fpu: Improve crypto performance by making 
> kernel-mode FPU reliably usable in softirqs")
>
> disables softirqs only when IRQs are enabled, as it is not permitted
> otherwise, but also unnecessary, given that asynchronous softirq
> delivery never happens to begin with while IRQs are disabled.
>
> However, this does mean that entering a kernel mode FPU section with
> IRQs enabled and leaving it with IRQs disabled leads to problems, as
> identified by Sashiko [0]: the EFI page fault handler is called from
> page_fault_oops() with IRQs disabled, and thus ends the kernel mode FPU
> section with IRQs disabled as well, regardless of whether IRQs were
> enabled when it was started. This may result in schedule() being called
> with a non-zero preempt_count, causing a BUG().
>
> So take care to re-enable IRQs when handling any EFI page faults if they
> were taken with IRQs enabled.
>
> [0] 
> https://sashiko.dev/#/patchset/20260430074107.27051-1-ivan.hu%40canonical.com
>
> Cc: Eric Biggers <ebiggers@kernel.org>
> Cc: Ivan Hu <ivan.hu@canonical.com>
> Fixes: d02198550423 ("x86/fpu: Improve crypto performance by making 
> kernel-mode FPU reliably usable in softirqs")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/x86/include/asm/efi.h     |  3 ++-
>  arch/x86/mm/fault.c            |  2 +-
>  arch/x86/platform/efi/quirks.c | 11 ++++++++++-
>  3 files changed, 13 insertions(+), 3 deletions(-)
>

If nobody minds, I am taking this via the EFI fixes branch.


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

* Re: [PATCH] x86/efi: Restore IRQ state in EFI page fault handler
  2026-05-01  9:03 [PATCH] x86/efi: Restore IRQ state in EFI page fault handler Ard Biesheuvel
  2026-05-04 16:39 ` Ard Biesheuvel
@ 2026-05-04 18:01 ` Eric Biggers
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2026-05-04 18:01 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, x86, Ard Biesheuvel, Ivan Hu

On Fri, May 01, 2026 at 11:03:12AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> The kernel's softirq API does not permit re-enabling softirqs while IRQs
> are disabled. The reason for this is that local_bh_enable() will not
> only re-enable delivery of softirqs over the back of IRQs, it will also
> handle any pending softirqs immediately, regardless of whether IRQs are
> enabled at that point.
> 
> For this reason, commit
> 
>   d02198550423 ("x86/fpu: Improve crypto performance by making kernel-mode FPU reliably usable in softirqs")
> 
> disables softirqs only when IRQs are enabled, as it is not permitted
> otherwise, but also unnecessary, given that asynchronous softirq
> delivery never happens to begin with while IRQs are disabled.
> 
> However, this does mean that entering a kernel mode FPU section with
> IRQs enabled and leaving it with IRQs disabled leads to problems, as
> identified by Sashiko [0]: the EFI page fault handler is called from
> page_fault_oops() with IRQs disabled, and thus ends the kernel mode FPU
> section with IRQs disabled as well, regardless of whether IRQs were
> enabled when it was started. This may result in schedule() being called
> with a non-zero preempt_count, causing a BUG().
> 
> So take care to re-enable IRQs when handling any EFI page faults if they
> were taken with IRQs enabled.
> 
> [0] https://sashiko.dev/#/patchset/20260430074107.27051-1-ivan.hu%40canonical.com
> 
> Cc: Eric Biggers <ebiggers@kernel.org>
> Cc: Ivan Hu <ivan.hu@canonical.com>
> Fixes: d02198550423 ("x86/fpu: Improve crypto performance by making kernel-mode FPU reliably usable in softirqs")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/x86/include/asm/efi.h     |  3 ++-
>  arch/x86/mm/fault.c            |  2 +-
>  arch/x86/platform/efi/quirks.c | 11 ++++++++++-
>  3 files changed, 13 insertions(+), 3 deletions(-)

Reviewed-by: Eric Biggers <ebiggers@kernel.org>

- Eric

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

end of thread, other threads:[~2026-05-04 18:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-01  9:03 [PATCH] x86/efi: Restore IRQ state in EFI page fault handler Ard Biesheuvel
2026-05-04 16:39 ` Ard Biesheuvel
2026-05-04 18:01 ` Eric Biggers

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