Skip to content

Commit d2c48b2

Browse files
pierregondoiswilldeacon
authored andcommitted
firmware: arm_sdei: Fix sleep from invalid context BUG
Running a preempt-rt (v6.2-rc3-rt1) based kernel on an Ampere Altra triggers: BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46 in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 24, name: cpuhp/0 preempt_count: 0, expected: 0 RCU nest depth: 0, expected: 0 3 locks held by cpuhp/0/24: #0: ffffda30217c70d0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x5c/0x248 #1: ffffda30217c7120 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x5c/0x248 #2: ffffda3021c711f0 (sdei_list_lock){....}-{3:3}, at: sdei_cpuhp_up+0x3c/0x130 irq event stamp: 36 hardirqs last enabled at (35): [<ffffda301e85b7bc>] finish_task_switch+0xb4/0x2b0 hardirqs last disabled at (36): [<ffffda301e812fec>] cpuhp_thread_fun+0x21c/0x248 softirqs last enabled at (0): [<ffffda301e80b184>] copy_process+0x63c/0x1ac0 softirqs last disabled at (0): [<0000000000000000>] 0x0 CPU: 0 PID: 24 Comm: cpuhp/0 Not tainted 5.19.0-rc3-rt5-[...] Hardware name: WIWYNN Mt.Jade Server [...] Call trace: dump_backtrace+0x114/0x120 show_stack+0x20/0x70 dump_stack_lvl+0x9c/0xd8 dump_stack+0x18/0x34 __might_resched+0x188/0x228 rt_spin_lock+0x70/0x120 sdei_cpuhp_up+0x3c/0x130 cpuhp_invoke_callback+0x250/0xf08 cpuhp_thread_fun+0x120/0x248 smpboot_thread_fn+0x280/0x320 kthread+0x130/0x140 ret_from_fork+0x10/0x20 sdei_cpuhp_up() is called in the STARTING hotplug section, which runs with interrupts disabled. Use a CPUHP_AP_ONLINE_DYN entry instead to execute the cpuhp cb later, with preemption enabled. SDEI originally got its own cpuhp slot to allow interacting with perf. It got superseded by pNMI and this early slot is not relevant anymore. [1] Some SDEI calls (e.g. SDEI_1_0_FN_SDEI_PE_MASK) take actions on the calling CPU. It is checked that preemption is disabled for them. _ONLINE cpuhp cb are executed in the 'per CPU hotplug thread'. Preemption is enabled in those threads, but their cpumask is limited to 1 CPU. Move 'WARN_ON_ONCE(preemptible())' statements so that SDEI cpuhp cb don't trigger them. Also add a check for the SDEI_1_0_FN_SDEI_PRIVATE_RESET SDEI call which acts on the calling CPU. [1]: https://lore.kernel.org/all/[email protected]/ Suggested-by: James Morse <[email protected]> Signed-off-by: Pierre Gondois <[email protected]> Reviewed-by: James Morse <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Will Deacon <[email protected]>
1 parent e8d018d commit d2c48b2

File tree

2 files changed

+20
-18
lines changed

2 files changed

+20
-18
lines changed

drivers/firmware/arm_sdei.c

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ static asmlinkage void (*sdei_firmware_call)(unsigned long function_id,
4343
/* entry point from firmware to arch asm code */
4444
static unsigned long sdei_entry_point;
4545

46+
static int sdei_hp_state;
47+
4648
struct sdei_event {
4749
/* These three are protected by the sdei_list_lock */
4850
struct list_head list;
@@ -301,8 +303,6 @@ int sdei_mask_local_cpu(void)
301303
{
302304
int err;
303305

304-
WARN_ON_ONCE(preemptible());
305-
306306
err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PE_MASK, 0, 0, 0, 0, 0, NULL);
307307
if (err && err != -EIO) {
308308
pr_warn_once("failed to mask CPU[%u]: %d\n",
@@ -315,15 +315,14 @@ int sdei_mask_local_cpu(void)
315315

316316
static void _ipi_mask_cpu(void *ignored)
317317
{
318+
WARN_ON_ONCE(preemptible());
318319
sdei_mask_local_cpu();
319320
}
320321

321322
int sdei_unmask_local_cpu(void)
322323
{
323324
int err;
324325

325-
WARN_ON_ONCE(preemptible());
326-
327326
err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PE_UNMASK, 0, 0, 0, 0, 0, NULL);
328327
if (err && err != -EIO) {
329328
pr_warn_once("failed to unmask CPU[%u]: %d\n",
@@ -336,13 +335,16 @@ int sdei_unmask_local_cpu(void)
336335

337336
static void _ipi_unmask_cpu(void *ignored)
338337
{
338+
WARN_ON_ONCE(preemptible());
339339
sdei_unmask_local_cpu();
340340
}
341341

342342
static void _ipi_private_reset(void *ignored)
343343
{
344344
int err;
345345

346+
WARN_ON_ONCE(preemptible());
347+
346348
err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PRIVATE_RESET, 0, 0, 0, 0, 0,
347349
NULL);
348350
if (err && err != -EIO)
@@ -389,8 +391,6 @@ static void _local_event_enable(void *data)
389391
int err;
390392
struct sdei_crosscall_args *arg = data;
391393

392-
WARN_ON_ONCE(preemptible());
393-
394394
err = sdei_api_event_enable(arg->event->event_num);
395395

396396
sdei_cross_call_return(arg, err);
@@ -479,8 +479,6 @@ static void _local_event_unregister(void *data)
479479
int err;
480480
struct sdei_crosscall_args *arg = data;
481481

482-
WARN_ON_ONCE(preemptible());
483-
484482
err = sdei_api_event_unregister(arg->event->event_num);
485483

486484
sdei_cross_call_return(arg, err);
@@ -561,8 +559,6 @@ static void _local_event_register(void *data)
561559
struct sdei_registered_event *reg;
562560
struct sdei_crosscall_args *arg = data;
563561

564-
WARN_ON(preemptible());
565-
566562
reg = per_cpu_ptr(arg->event->private_registered, smp_processor_id());
567563
err = sdei_api_event_register(arg->event->event_num, sdei_entry_point,
568564
reg, 0, 0);
@@ -717,6 +713,8 @@ static int sdei_pm_notifier(struct notifier_block *nb, unsigned long action,
717713
{
718714
int rv;
719715

716+
WARN_ON_ONCE(preemptible());
717+
720718
switch (action) {
721719
case CPU_PM_ENTER:
722720
rv = sdei_mask_local_cpu();
@@ -765,7 +763,7 @@ static int sdei_device_freeze(struct device *dev)
765763
int err;
766764

767765
/* unregister private events */
768-
cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING);
766+
cpuhp_remove_state(sdei_entry_point);
769767

770768
err = sdei_unregister_shared();
771769
if (err)
@@ -786,12 +784,15 @@ static int sdei_device_thaw(struct device *dev)
786784
return err;
787785
}
788786

789-
err = cpuhp_setup_state(CPUHP_AP_ARM_SDEI_STARTING, "SDEI",
787+
err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "SDEI",
790788
&sdei_cpuhp_up, &sdei_cpuhp_down);
791-
if (err)
789+
if (err < 0) {
792790
pr_warn("Failed to re-register CPU hotplug notifier...\n");
791+
return err;
792+
}
793793

794-
return err;
794+
sdei_hp_state = err;
795+
return 0;
795796
}
796797

797798
static int sdei_device_restore(struct device *dev)
@@ -823,7 +824,7 @@ static int sdei_reboot_notifier(struct notifier_block *nb, unsigned long action,
823824
* We are going to reset the interface, after this there is no point
824825
* doing work when we take CPUs offline.
825826
*/
826-
cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING);
827+
cpuhp_remove_state(sdei_hp_state);
827828

828829
sdei_platform_reset();
829830

@@ -1003,13 +1004,15 @@ static int sdei_probe(struct platform_device *pdev)
10031004
goto remove_cpupm;
10041005
}
10051006

1006-
err = cpuhp_setup_state(CPUHP_AP_ARM_SDEI_STARTING, "SDEI",
1007+
err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "SDEI",
10071008
&sdei_cpuhp_up, &sdei_cpuhp_down);
1008-
if (err) {
1009+
if (err < 0) {
10091010
pr_warn("Failed to register CPU hotplug notifier...\n");
10101011
goto remove_reboot;
10111012
}
10121013

1014+
sdei_hp_state = err;
1015+
10131016
return 0;
10141017

10151018
remove_reboot:

include/linux/cpuhotplug.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ enum cpuhp_state {
163163
CPUHP_AP_PERF_X86_CSTATE_STARTING,
164164
CPUHP_AP_PERF_XTENSA_STARTING,
165165
CPUHP_AP_MIPS_OP_LOONGSON3_STARTING,
166-
CPUHP_AP_ARM_SDEI_STARTING,
167166
CPUHP_AP_ARM_VFP_STARTING,
168167
CPUHP_AP_ARM64_DEBUG_MONITORS_STARTING,
169168
CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING,

0 commit comments

Comments
 (0)