diff options
author | Cristian Ciocaltea <cristian.ciocaltea@collabora.com> | 2024-02-09 01:43:14 +0200 |
---|---|---|
committer | Mark Brown <broonie@kernel.org> | 2024-02-11 16:58:50 +0000 |
commit | c4b603c6e2df3a17831731d8bbec5c16fa7bbdf8 (patch) | |
tree | 87b3433530581e828b206561667046042947d321 /sound/soc/sof/amd | |
parent | 6ef5d5b92f7117b324efaac72b3db27ae8bb3082 (diff) |
ASoC: SOF: amd: Fix locking in ACP IRQ handler
A recent change in acp_irq_thread() was meant to address a potential race
condition while trying to acquire the hardware semaphore responsible for
the synchronization between firmware and host IPC interrupts.
This resulted in an improper use of the IPC spinlock, causing normal
kernel memory allocations (which may sleep) inside atomic contexts:
1707255557.133976 kernel: BUG: sleeping function called from invalid context at include/linux/sched/mm.h:315
...
1707255557.134757 kernel: sof_ipc3_rx_msg+0x70/0x130 [snd_sof]
1707255557.134793 kernel: acp_sof_ipc_irq_thread+0x1e0/0x550 [snd_sof_amd_acp]
1707255557.134855 kernel: acp_irq_thread+0xa3/0x130 [snd_sof_amd_acp]
1707255557.134904 kernel: ? irq_thread+0xb5/0x1e0
1707255557.134947 kernel: ? __pfx_irq_thread_fn+0x10/0x10
1707255557.134985 kernel: irq_thread_fn+0x23/0x60
Moreover, there are attempts to lock a mutex from the same atomic
context:
1707255557.136357 kernel: =============================
1707255557.136393 kernel: [ BUG: Invalid wait context ]
1707255557.136413 kernel: 6.8.0-rc3-next-20240206-audio-next #9 Tainted: G W
1707255557.136432 kernel: -----------------------------
1707255557.136451 kernel: irq/66-AudioDSP/502 is trying to lock:
1707255557.136470 kernel: ffff965152f26af8 (&sb->s_type->i_mutex_key#2){+.+.}-{3:3}, at: start_creating.part.0+0x5f/0x180
...
1707255557.137429 kernel: start_creating.part.0+0x5f/0x180
1707255557.137457 kernel: __debugfs_create_file+0x61/0x210
1707255557.137475 kernel: snd_sof_debugfs_io_item+0x75/0xc0 [snd_sof]
1707255557.137494 kernel: sof_ipc3_do_rx_work+0x7cf/0x9f0 [snd_sof]
1707255557.137513 kernel: sof_ipc3_rx_msg+0xb3/0x130 [snd_sof]
1707255557.137532 kernel: acp_sof_ipc_irq_thread+0x1e0/0x550 [snd_sof_amd_acp]
1707255557.137551 kernel: acp_irq_thread+0xa3/0x130 [snd_sof_amd_acp]
Fix the issues by reducing the lock scope in acp_irq_thread(), so that
it guards only the hardware semaphore acquiring attempt. Additionally,
restore the initial locking in acp_sof_ipc_irq_thread() to synchronize
the handling of immediate replies from DSP core.
Fixes: 802134c8c2c8 ("ASoC: SOF: amd: Refactor spinlock_irq(&sdev->ipc_lock) sequence in irq_handler")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Link: https://lore.kernel.org/r/20240208234315.2182048-1-cristian.ciocaltea@collabora.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Diffstat (limited to 'sound/soc/sof/amd')
-rw-r--r-- | sound/soc/sof/amd/acp-ipc.c | 2 | ||||
-rw-r--r-- | sound/soc/sof/amd/acp.c | 17 |
2 files changed, 10 insertions, 9 deletions
diff --git a/sound/soc/sof/amd/acp-ipc.c b/sound/soc/sof/amd/acp-ipc.c index 2743f07a5e08..b44b1b1adb6e 100644 --- a/sound/soc/sof/amd/acp-ipc.c +++ b/sound/soc/sof/amd/acp-ipc.c @@ -188,11 +188,13 @@ irqreturn_t acp_sof_ipc_irq_thread(int irq, void *context) dsp_ack = snd_sof_dsp_read(sdev, ACP_DSP_BAR, ACP_SCRATCH_REG_0 + dsp_ack_write); if (dsp_ack) { + spin_lock_irq(&sdev->ipc_lock); /* handle immediate reply from DSP core */ acp_dsp_ipc_get_reply(sdev); snd_sof_ipc_reply(sdev, 0); /* set the done bit */ acp_dsp_ipc_dsp_done(sdev); + spin_unlock_irq(&sdev->ipc_lock); ipc_irq = true; } diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c index 32a741fcb84f..07632ae6ccf5 100644 --- a/sound/soc/sof/amd/acp.c +++ b/sound/soc/sof/amd/acp.c @@ -355,21 +355,20 @@ static irqreturn_t acp_irq_thread(int irq, void *context) unsigned int count = ACP_HW_SEM_RETRY_COUNT; spin_lock_irq(&sdev->ipc_lock); - while (snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->hw_semaphore_offset)) { - /* Wait until acquired HW Semaphore lock or timeout */ - count--; - if (!count) { - dev_err(sdev->dev, "%s: Failed to acquire HW lock\n", __func__); - spin_unlock_irq(&sdev->ipc_lock); - return IRQ_NONE; - } + /* Wait until acquired HW Semaphore lock or timeout */ + while (snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->hw_semaphore_offset) && --count) + ; + spin_unlock_irq(&sdev->ipc_lock); + + if (!count) { + dev_err(sdev->dev, "%s: Failed to acquire HW lock\n", __func__); + return IRQ_NONE; } sof_ops(sdev)->irq_thread(irq, sdev); /* Unlock or Release HW Semaphore */ snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->hw_semaphore_offset, 0x0); - spin_unlock_irq(&sdev->ipc_lock); return IRQ_HANDLED; }; |