Commit d2c48b23 authored by Pierre Gondois's avatar Pierre Gondois Committed by Will Deacon

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/5813b8c5-ae3e-87fd-fccc-94c9cd08816d@arm.com/Suggested-by: default avatarJames Morse <james.morse@arm.com>
Signed-off-by: default avatarPierre Gondois <pierre.gondois@arm.com>
Reviewed-by: default avatarJames Morse <james.morse@arm.com>
Link: https://lore.kernel.org/r/20230216084920.144064-1-pierre.gondois@arm.comSigned-off-by: default avatarWill Deacon <will@kernel.org>
parent e8d018dd
...@@ -43,6 +43,8 @@ static asmlinkage void (*sdei_firmware_call)(unsigned long function_id, ...@@ -43,6 +43,8 @@ static asmlinkage void (*sdei_firmware_call)(unsigned long function_id,
/* entry point from firmware to arch asm code */ /* entry point from firmware to arch asm code */
static unsigned long sdei_entry_point; static unsigned long sdei_entry_point;
static int sdei_hp_state;
struct sdei_event { struct sdei_event {
/* These three are protected by the sdei_list_lock */ /* These three are protected by the sdei_list_lock */
struct list_head list; struct list_head list;
...@@ -301,8 +303,6 @@ int sdei_mask_local_cpu(void) ...@@ -301,8 +303,6 @@ int sdei_mask_local_cpu(void)
{ {
int err; int err;
WARN_ON_ONCE(preemptible());
err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PE_MASK, 0, 0, 0, 0, 0, NULL); err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PE_MASK, 0, 0, 0, 0, 0, NULL);
if (err && err != -EIO) { if (err && err != -EIO) {
pr_warn_once("failed to mask CPU[%u]: %d\n", pr_warn_once("failed to mask CPU[%u]: %d\n",
...@@ -315,6 +315,7 @@ int sdei_mask_local_cpu(void) ...@@ -315,6 +315,7 @@ int sdei_mask_local_cpu(void)
static void _ipi_mask_cpu(void *ignored) static void _ipi_mask_cpu(void *ignored)
{ {
WARN_ON_ONCE(preemptible());
sdei_mask_local_cpu(); sdei_mask_local_cpu();
} }
...@@ -322,8 +323,6 @@ int sdei_unmask_local_cpu(void) ...@@ -322,8 +323,6 @@ int sdei_unmask_local_cpu(void)
{ {
int err; int err;
WARN_ON_ONCE(preemptible());
err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PE_UNMASK, 0, 0, 0, 0, 0, NULL); err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PE_UNMASK, 0, 0, 0, 0, 0, NULL);
if (err && err != -EIO) { if (err && err != -EIO) {
pr_warn_once("failed to unmask CPU[%u]: %d\n", pr_warn_once("failed to unmask CPU[%u]: %d\n",
...@@ -336,6 +335,7 @@ int sdei_unmask_local_cpu(void) ...@@ -336,6 +335,7 @@ int sdei_unmask_local_cpu(void)
static void _ipi_unmask_cpu(void *ignored) static void _ipi_unmask_cpu(void *ignored)
{ {
WARN_ON_ONCE(preemptible());
sdei_unmask_local_cpu(); sdei_unmask_local_cpu();
} }
...@@ -343,6 +343,8 @@ static void _ipi_private_reset(void *ignored) ...@@ -343,6 +343,8 @@ static void _ipi_private_reset(void *ignored)
{ {
int err; int err;
WARN_ON_ONCE(preemptible());
err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PRIVATE_RESET, 0, 0, 0, 0, 0, err = invoke_sdei_fn(SDEI_1_0_FN_SDEI_PRIVATE_RESET, 0, 0, 0, 0, 0,
NULL); NULL);
if (err && err != -EIO) if (err && err != -EIO)
...@@ -389,8 +391,6 @@ static void _local_event_enable(void *data) ...@@ -389,8 +391,6 @@ static void _local_event_enable(void *data)
int err; int err;
struct sdei_crosscall_args *arg = data; struct sdei_crosscall_args *arg = data;
WARN_ON_ONCE(preemptible());
err = sdei_api_event_enable(arg->event->event_num); err = sdei_api_event_enable(arg->event->event_num);
sdei_cross_call_return(arg, err); sdei_cross_call_return(arg, err);
...@@ -479,8 +479,6 @@ static void _local_event_unregister(void *data) ...@@ -479,8 +479,6 @@ static void _local_event_unregister(void *data)
int err; int err;
struct sdei_crosscall_args *arg = data; struct sdei_crosscall_args *arg = data;
WARN_ON_ONCE(preemptible());
err = sdei_api_event_unregister(arg->event->event_num); err = sdei_api_event_unregister(arg->event->event_num);
sdei_cross_call_return(arg, err); sdei_cross_call_return(arg, err);
...@@ -561,8 +559,6 @@ static void _local_event_register(void *data) ...@@ -561,8 +559,6 @@ static void _local_event_register(void *data)
struct sdei_registered_event *reg; struct sdei_registered_event *reg;
struct sdei_crosscall_args *arg = data; struct sdei_crosscall_args *arg = data;
WARN_ON(preemptible());
reg = per_cpu_ptr(arg->event->private_registered, smp_processor_id()); reg = per_cpu_ptr(arg->event->private_registered, smp_processor_id());
err = sdei_api_event_register(arg->event->event_num, sdei_entry_point, err = sdei_api_event_register(arg->event->event_num, sdei_entry_point,
reg, 0, 0); reg, 0, 0);
...@@ -717,6 +713,8 @@ static int sdei_pm_notifier(struct notifier_block *nb, unsigned long action, ...@@ -717,6 +713,8 @@ static int sdei_pm_notifier(struct notifier_block *nb, unsigned long action,
{ {
int rv; int rv;
WARN_ON_ONCE(preemptible());
switch (action) { switch (action) {
case CPU_PM_ENTER: case CPU_PM_ENTER:
rv = sdei_mask_local_cpu(); rv = sdei_mask_local_cpu();
...@@ -765,7 +763,7 @@ static int sdei_device_freeze(struct device *dev) ...@@ -765,7 +763,7 @@ static int sdei_device_freeze(struct device *dev)
int err; int err;
/* unregister private events */ /* unregister private events */
cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING); cpuhp_remove_state(sdei_entry_point);
err = sdei_unregister_shared(); err = sdei_unregister_shared();
if (err) if (err)
...@@ -786,12 +784,15 @@ static int sdei_device_thaw(struct device *dev) ...@@ -786,12 +784,15 @@ static int sdei_device_thaw(struct device *dev)
return err; return err;
} }
err = cpuhp_setup_state(CPUHP_AP_ARM_SDEI_STARTING, "SDEI", err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "SDEI",
&sdei_cpuhp_up, &sdei_cpuhp_down); &sdei_cpuhp_up, &sdei_cpuhp_down);
if (err) if (err < 0) {
pr_warn("Failed to re-register CPU hotplug notifier...\n"); pr_warn("Failed to re-register CPU hotplug notifier...\n");
return err; return err;
}
sdei_hp_state = err;
return 0;
} }
static int sdei_device_restore(struct device *dev) static int sdei_device_restore(struct device *dev)
...@@ -823,7 +824,7 @@ static int sdei_reboot_notifier(struct notifier_block *nb, unsigned long action, ...@@ -823,7 +824,7 @@ static int sdei_reboot_notifier(struct notifier_block *nb, unsigned long action,
* We are going to reset the interface, after this there is no point * We are going to reset the interface, after this there is no point
* doing work when we take CPUs offline. * doing work when we take CPUs offline.
*/ */
cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING); cpuhp_remove_state(sdei_hp_state);
sdei_platform_reset(); sdei_platform_reset();
...@@ -1003,13 +1004,15 @@ static int sdei_probe(struct platform_device *pdev) ...@@ -1003,13 +1004,15 @@ static int sdei_probe(struct platform_device *pdev)
goto remove_cpupm; goto remove_cpupm;
} }
err = cpuhp_setup_state(CPUHP_AP_ARM_SDEI_STARTING, "SDEI", err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "SDEI",
&sdei_cpuhp_up, &sdei_cpuhp_down); &sdei_cpuhp_up, &sdei_cpuhp_down);
if (err) { if (err < 0) {
pr_warn("Failed to register CPU hotplug notifier...\n"); pr_warn("Failed to register CPU hotplug notifier...\n");
goto remove_reboot; goto remove_reboot;
} }
sdei_hp_state = err;
return 0; return 0;
remove_reboot: remove_reboot:
......
...@@ -163,7 +163,6 @@ enum cpuhp_state { ...@@ -163,7 +163,6 @@ enum cpuhp_state {
CPUHP_AP_PERF_X86_CSTATE_STARTING, CPUHP_AP_PERF_X86_CSTATE_STARTING,
CPUHP_AP_PERF_XTENSA_STARTING, CPUHP_AP_PERF_XTENSA_STARTING,
CPUHP_AP_MIPS_OP_LOONGSON3_STARTING, CPUHP_AP_MIPS_OP_LOONGSON3_STARTING,
CPUHP_AP_ARM_SDEI_STARTING,
CPUHP_AP_ARM_VFP_STARTING, CPUHP_AP_ARM_VFP_STARTING,
CPUHP_AP_ARM64_DEBUG_MONITORS_STARTING, CPUHP_AP_ARM64_DEBUG_MONITORS_STARTING,
CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING, CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment