diff mbox series

[v2,2/2] arm64: sdei: abort running SDEI handlers during crash

Message ID 20230329202519.6110-2-scott@os.amperecomputing.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] arm64: sdei: Use alternative patching to select the conduit in sdei exit | expand

Commit Message

D Scott Phillips March 29, 2023, 8:25 p.m. UTC
Interrupts are blocked in SDEI context, per the SDEI spec: "The client
interrupts cannot preempt the event handler." If we crashed in the SDEI
handler-running context (as with ACPI's AGDI) then we need to clean up the
SDEI state before proceeding to the crash kernel so that the crash kernel
can have working interrupts.

Track the active SDEI handler per-cpu so that we can COMPLETE_AND_RESUME
the handler, discarding the interrupted context.

Fixes: f5df26961853 ("arm64: kernel: Add arch-specific SDEI entry code and CPU masking")
Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com>
---
Hi James, I added the per-cpu active event info instead of the stack pointer
comparison you suggested for the purpose of handling the case where a critical
event handler interrupts a normal event handler. I actually started adding it
to get at the interrupted stack pointer, but then realized I could just use the
active event info itself. This does leave a small gap where a critical event
could interrput a normal event before or after the setting or clearing of the
per-cpu active info. I suppose it could be covered by adding in _end labels to
__sdei_entry_trampoline, __sdei_exit_trampoline, and __sdei_asm_handler, and
checking for the critical event's interrupted pc within those functions. I
didn't do that here in the thinking that this is good enough, thoughts on that?

Changes since v1:
- Store the active SDEI event being handled per-cpu, use the per-cpu active
  handler information to know when to abort.
- Add prints before attempting to abort sdei handlers.
v1 Link: https://lore.kernel.org/linux-arm-kernel/20230204000851.3871-1-scott@os.amperecomputing.com/

 arch/arm64/include/asm/sdei.h |  8 ++++++++
 arch/arm64/kernel/entry.S     | 24 ++++++++++++++++++++++++
 arch/arm64/kernel/sdei.c      |  3 +++
 arch/arm64/kernel/smp.c       | 27 +++++++++++++++++++++++----
 4 files changed, 58 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/sdei.h b/arch/arm64/include/asm/sdei.h
index badb39510515..58bc9ebe4a6d 100644
--- a/arch/arm64/include/asm/sdei.h
+++ b/arch/arm64/include/asm/sdei.h
@@ -13,6 +13,11 @@ 
 
 #include <asm/virt.h>
 
+#ifdef CONFIG_ARM_SDE_INTERFACE
+DECLARE_PER_CPU(struct sdei_registered_event *, sdei_active_normal_event);
+DECLARE_PER_CPU(struct sdei_registered_event *, sdei_active_critical_event);
+#endif
+
 /* Software Delegated Exception entry point from firmware*/
 asmlinkage void __sdei_asm_handler(unsigned long event_num, unsigned long arg,
 				   unsigned long pc, unsigned long pstate);
@@ -23,6 +28,9 @@  asmlinkage void __sdei_asm_entry_trampoline(unsigned long event_num,
 						   unsigned long pc,
 						   unsigned long pstate);
 
+/* Abort a running handler. Context is discarded. */
+void sdei_handler_abort(void);
+
 /*
  * The above entry point does the minimum to call C code. This function does
  * anything else, before calling the driver.
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 80b6f67a67d4..253755bb33d7 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -998,6 +998,12 @@  SYM_CODE_START(__sdei_asm_handler)
 	ldrb	w4, [x19, #SDEI_EVENT_PRIORITY]
 #endif
 
+	cbnz	w4, 1f
+	adr_this_cpu dst=x5, sym=sdei_active_normal_event, tmp=x6
+	b	2f
+1:	adr_this_cpu dst=x5, sym=sdei_active_critical_event, tmp=x6
+2:	str	x19, [x5]
+
 #ifdef CONFIG_VMAP_STACK
 	/*
 	 * entry.S may have been using sp as a scratch register, find whether
@@ -1061,6 +1067,13 @@  SYM_CODE_START(__sdei_asm_handler)
 	mov_q	x3, SDEI_1_0_FN_SDEI_EVENT_COMPLETE_AND_RESUME
 	csel	x0, x2, x3, ls
 
+	ldrb	w3, [x4, #SDEI_EVENT_PRIORITY]
+	cbnz	w3, 1f
+	adr_this_cpu dst=x5, sym=sdei_active_normal_event, tmp=x6
+	b	2f
+1:	adr_this_cpu dst=x5, sym=sdei_active_critical_event, tmp=x6
+2:	str	xzr, [x5]
+
 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
 alternative_if ARM64_UNMAP_KERNEL_AT_EL0
 	b 1f
@@ -1075,4 +1088,15 @@  alternative_else_nop_endif
 #endif
 SYM_CODE_END(__sdei_asm_handler)
 NOKPROBE(__sdei_asm_handler)
+
+SYM_CODE_START(sdei_handler_abort)
+	mov_q	x0, SDEI_1_0_FN_SDEI_EVENT_COMPLETE_AND_RESUME
+	adr	x1, 1f
+	sdei_handler_exit
+	// exit the handler and jump to the next instruction.
+	// Exit will stomp x0-x17, PSTATE, ELR_ELx, and SPSR_ELx.
+1:	ret
+SYM_CODE_END(sdei_handler_abort)
+NOKPROBE(sdei_handler_abort)
+
 #endif /* CONFIG_ARM_SDE_INTERFACE */
diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index be7f6ea49956..07fa9630f0ce 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -45,6 +45,9 @@  DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr);
 DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr);
 #endif
 
+DEFINE_PER_CPU(struct sdei_registered_event *, sdei_active_normal_event);
+DEFINE_PER_CPU(struct sdei_registered_event *, sdei_active_critical_event);
+
 static void _free_sdei_stack(unsigned long * __percpu *ptr, int cpu)
 {
 	unsigned long *p;
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index ffc5d76cf695..311430990175 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -1047,10 +1047,8 @@  void crash_smp_send_stop(void)
 	 * If this cpu is the only one alive at this point in time, online or
 	 * not, there are no stop messages to be sent around, so just back out.
 	 */
-	if (num_other_online_cpus() == 0) {
-		sdei_mask_local_cpu();
-		return;
-	}
+	if (num_other_online_cpus() == 0)
+		goto skip_ipi;
 
 	cpumask_copy(&mask, cpu_online_mask);
 	cpumask_clear_cpu(smp_processor_id(), &mask);
@@ -1069,7 +1067,28 @@  void crash_smp_send_stop(void)
 		pr_warn("SMP: failed to stop secondary CPUs %*pbl\n",
 			cpumask_pr_args(&mask));
 
+skip_ipi:
 	sdei_mask_local_cpu();
+
+#ifdef CONFIG_ARM_SDE_INTERFACE
+	/*
+	 * If the crash happened in an SDEI event handler then we need to
+	 * finish the handler with the firmware so that we can have working
+	 * interrupts in the crash kernel.
+	 */
+	if (__this_cpu_read(sdei_active_critical_event)) {
+		pr_warn("SMP: stopped CPUS from SDEI critical event handler "
+			"context, attempting to finish handler.\n");
+		sdei_handler_abort();
+		__this_cpu_write(sdei_active_critical_event, NULL);
+	}
+	if (__this_cpu_read(sdei_active_normal_event)) {
+		pr_warn("SMP: stopped CPUS from SDEI normal event handler "
+			"context, attempting to finish handler.\n");
+		sdei_handler_abort();
+		__this_cpu_write(sdei_active_normal_event, NULL);
+	}
+#endif
 }
 
 bool smp_crash_stop_failed(void)