mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers
@ 2026-11-07 17:52 Vijendar Mukunda
  2026-11-07 17:52 ` [PATCH 1/4] ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler Vijendar Mukunda
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Vijendar Mukunda @ 2026-11-07 17:52 UTC (permalink / raw)
  To: broonie
  Cc: vkoul, alsa-devel, lgirdwood, perex, tiwai, yung-chuan.liao,
	Basavaraj.Hiregoudar, Sunil-kumar.Dommati, venkataprasad.potturu,
	Syed.SabaKareem, Mario.Limonciello, Richard.Gong, Poojitha.N,
	lee, Vijendar.Mukunda, linux-sound, linux-kernel,
	sound-open-firmware

These four patches fix two related race conditions in the AMD SoundWire
IRQ handlers for the PS (ACP63) and SOF platforms.

Patches 1 and 2 guard against a NULL dereference in the ACP external IRQ
handler.  The handler is registered before sdw_amd_probe() assigns
adata->sdw, so a stray interrupt in the probe window, or one that fires
after sdw_amd_exit() frees the context, can reach the adata->sdw->pdev[i]
dereference with adata->sdw still NULL.

Patches 3 and 4 guard schedule_work() calls with the interrupt_enabled
flag so that an in-flight IRQ arriving while the SoundWire manager is
being torn down does not queue new work after amd_disable_sdw_interrupts()
has cleared the flag.

NOTE: Patches 3 and 4 depend on the following patch which introduces the
interrupt_enabled field in the AMD SoundWire manager:

  https://lore.kernel.org/linux-sound/20261107154659.3358223-18-Vijendar.Mukunda@amd.com/

Until that patch is merged, patches 3 and 4 should not be picked up
independently.

Vijendar Mukunda (4):
  ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ
    handler
  ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ
    handler
  ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled
    flag
  ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled
    flag

 sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
 sound/soc/sof/amd/acp.c   | 32 ++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 24 deletions(-)

--
2.53.0


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

* [PATCH 1/4] ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler
  2026-11-07 17:52 [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Vijendar Mukunda
@ 2026-11-07 17:52 ` Vijendar Mukunda
  2026-11-07 17:52 ` [PATCH 2/4] ASoC: SOF: amd: " Vijendar Mukunda
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Vijendar Mukunda @ 2026-11-07 17:52 UTC (permalink / raw)
  To: broonie
  Cc: vkoul, alsa-devel, lgirdwood, perex, tiwai, yung-chuan.liao,
	Basavaraj.Hiregoudar, Sunil-kumar.Dommati, venkataprasad.potturu,
	Syed.SabaKareem, Mario.Limonciello, Richard.Gong, Poojitha.N,
	lee, Vijendar.Mukunda, linux-sound, linux-kernel,
	sound-open-firmware

The ACP external IRQ handler is registered before sdw_amd_probe() assigns
adata->sdw, so a stray interrupt in the probe window could reach the
adata->sdw->pdev[i] dereference with adata->sdw still NULL.

The same window exists on the teardown side: after sdw_amd_exit() frees
the context but before adata->sdw is cleared by the caller.

Add NULL checks for adata->sdw in acp63_irq_handler() before dereferencing
->pdev[0] and ->pdev[1], and in handle_acp70_sdw_wake_event() for the same
reason.  The interrupt-status register write that acknowledges the IRQ is
kept outside the guard so the hardware bit is always cleared regardless of
whether the SoundWire context is live.

Fixes: e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
index b604db4942dd..d0b66df10157 100644
--- a/sound/soc/amd/ps/pci-ps.c
+++ b/sound/soc/amd/ps/pci-ps.c
@@ -26,16 +26,20 @@ static void handle_acp70_sdw_wake_event(struct acp63_dev_data *adata)
 	struct amd_sdw_manager *amd_manager;
 
 	if (adata->acp70_sdw0_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw0_wake_event = 0;
 	}
 
 	if (adata->acp70_sdw1_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw1_wake_event = 0;
 	}
 }
@@ -209,18 +213,22 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 	ext_intr_stat = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
 	if (ext_intr_stat & ACP_SDW0_STAT) {
 		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
 
 	ext_intr_stat1 = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
 	if (ext_intr_stat1 & ACP_SDW1_STAT) {
 		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
 
-- 
2.53.0


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

* [PATCH 2/4] ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ handler
  2026-11-07 17:52 [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Vijendar Mukunda
  2026-11-07 17:52 ` [PATCH 1/4] ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler Vijendar Mukunda
@ 2026-11-07 17:52 ` Vijendar Mukunda
  2026-11-07 17:52 ` [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag Vijendar Mukunda
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Vijendar Mukunda @ 2026-11-07 17:52 UTC (permalink / raw)
  To: broonie
  Cc: vkoul, alsa-devel, lgirdwood, perex, tiwai, yung-chuan.liao,
	Basavaraj.Hiregoudar, Sunil-kumar.Dommati, venkataprasad.potturu,
	Syed.SabaKareem, Mario.Limonciello, Richard.Gong, Poojitha.N,
	lee, Vijendar.Mukunda, linux-sound, linux-kernel,
	sound-open-firmware

The ACP external IRQ handler is registered before sdw_amd_probe() assigns
adata->sdw, so a stray interrupt in the probe window could reach the
adata->sdw->pdev[i] dereference with adata->sdw still NULL.

The same window exists on the teardown side: after sdw_amd_exit() frees
the context but before adata->sdw is cleared by the caller.

Add NULL checks for adata->sdw in acp_irq_handler() before dereferencing
->pdev[0] and ->pdev[1], and in amd_sof_handle_acp70_sdw_wake_event() for
the same reason.  The interrupt-status register write that acknowledges the
IRQ is kept outside the guard so the hardware bit is always cleared
regardless of whether the SoundWire context is live.

Fixes: 96eb81851012 ("ASoC: SOF: amd: add interrupt handling for SoundWire manager devices")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/sof/amd/acp.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
index 86e2402b8ffd..623e016f38b8 100644
--- a/sound/soc/sof/amd/acp.c
+++ b/sound/soc/sof/amd/acp.c
@@ -478,16 +478,20 @@ static void amd_sof_handle_acp70_sdw_wake_event(struct acp_dev_data *adata)
 	struct amd_sdw_manager *amd_manager;
 
 	if (adata->acp70_sdw0_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw0_wake_event = 0;
 	}
 
 	if (adata->acp70_sdw1_wake_event) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-		if (amd_manager)
-			pm_request_resume(amd_manager->dev);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+			if (amd_manager)
+				pm_request_resume(amd_manager->dev);
+		}
 		adata->acp70_sdw1_wake_event = 0;
 	}
 }
@@ -579,10 +583,12 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 
 	val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat);
 	if (val & ACP_SDW0_IRQ_MASK) {
-		amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
 		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
-		if (amd_manager)
-			schedule_work(&amd_manager->amd_sdw_irq_thread);
+		if (adata->sdw) {
+			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
+			if (amd_manager)
+				schedule_work(&amd_manager->amd_sdw_irq_thread);
+		}
 		irq_flag = 1;
 	}
 
@@ -608,11 +614,13 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 	if (desc->ext_intr_stat1) {
 		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->ext_intr_stat1);
 		if (val & ACP_SDW1_IRQ_MASK) {
-			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
 			snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat1,
 					  ACP_SDW1_IRQ_MASK);
-			if (amd_manager)
-				schedule_work(&amd_manager->amd_sdw_irq_thread);
+			if (adata->sdw) {
+				amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
+				if (amd_manager)
+					schedule_work(&amd_manager->amd_sdw_irq_thread);
+			}
 			irq_flag = 1;
 		}
 		switch (adata->pci_rev) {
-- 
2.53.0


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

* [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag
  2026-11-07 17:52 [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Vijendar Mukunda
  2026-11-07 17:52 ` [PATCH 1/4] ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler Vijendar Mukunda
  2026-11-07 17:52 ` [PATCH 2/4] ASoC: SOF: amd: " Vijendar Mukunda
@ 2026-11-07 17:52 ` Vijendar Mukunda
  2026-11-07 17:52 ` [PATCH 4/4] ASoC: amd: ps: " Vijendar Mukunda
  2026-11-07 19:39 ` [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Mukunda,Vijendar
  4 siblings, 0 replies; 7+ messages in thread
From: Vijendar Mukunda @ 2026-11-07 17:52 UTC (permalink / raw)
  To: broonie
  Cc: vkoul, alsa-devel, lgirdwood, perex, tiwai, yung-chuan.liao,
	Basavaraj.Hiregoudar, Sunil-kumar.Dommati, venkataprasad.potturu,
	Syed.SabaKareem, Mario.Limonciello, Richard.Gong, Poojitha.N,
	lee, Vijendar.Mukunda, linux-sound, linux-kernel,
	sound-open-firmware

acp_irq_handler() in sound/soc/sof/amd/acp.c called schedule_work()
for the SoundWire manager workqueue guarded only by a NULL check on
amd_manager, the same gap fixed in the PS platform IRQ handler.

Add the interrupt_enabled check so that an in-flight IRQ arriving while
the SoundWire manager is being torn down does not queue new work after
amd_disable_sdw_interrupts() has cleared the flag.

Reported-by: Lee Jones <lee@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-ab458b2e-f505-4a41-8238-a93ceb5d9c77
Fixes: 96eb81851012 ("ASoC: SOF: amd: add interrupt handling for SoundWire manager devices")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/sof/amd/acp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
index 623e016f38b8..5c4eab0be497 100644
--- a/sound/soc/sof/amd/acp.c
+++ b/sound/soc/sof/amd/acp.c
@@ -586,7 +586,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_SDW0_IRQ_MASK);
 		if (adata->sdw) {
 			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-			if (amd_manager)
+			if (amd_manager && amd_manager->interrupt_enabled)
 				schedule_work(&amd_manager->amd_sdw_irq_thread);
 		}
 		irq_flag = 1;
@@ -618,7 +618,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
 					  ACP_SDW1_IRQ_MASK);
 			if (adata->sdw) {
 				amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-				if (amd_manager)
+				if (amd_manager && amd_manager->interrupt_enabled)
 					schedule_work(&amd_manager->amd_sdw_irq_thread);
 			}
 			irq_flag = 1;
-- 
2.53.0


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

* [PATCH 4/4] ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled flag
  2026-11-07 17:52 [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Vijendar Mukunda
                   ` (2 preceding siblings ...)
  2026-11-07 17:52 ` [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag Vijendar Mukunda
@ 2026-11-07 17:52 ` Vijendar Mukunda
  2026-11-07 19:39 ` [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Mukunda,Vijendar
  4 siblings, 0 replies; 7+ messages in thread
From: Vijendar Mukunda @ 2026-11-07 17:52 UTC (permalink / raw)
  To: broonie
  Cc: vkoul, alsa-devel, lgirdwood, perex, tiwai, yung-chuan.liao,
	Basavaraj.Hiregoudar, Sunil-kumar.Dommati, venkataprasad.potturu,
	Syed.SabaKareem, Mario.Limonciello, Richard.Gong, Poojitha.N,
	lee, Vijendar.Mukunda, linux-sound, linux-kernel,
	sound-open-firmware

acp63_irq_handler() called schedule_work() for the SoundWire manager
workqueue guarded only by a NULL pointer check on amd_manager. Add a
check for amd_manager->interrupt_enabled so that an in-flight interrupt
that fires while the SoundWire manager is being torn down does not queue
new work after amd_disable_sdw_interrupts() has cleared the flag.

Reported-by: Lee Jones <lee@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-ab458b2e-f505-4a41-8238-a93ceb5d9c77
Fixes: e1cb350610ce ("ASoC: amd: ps: handle SoundWire interrupts in acp pci driver")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/ps/pci-ps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
index d0b66df10157..0442010e969c 100644
--- a/sound/soc/amd/ps/pci-ps.c
+++ b/sound/soc/amd/ps/pci-ps.c
@@ -215,7 +215,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 		writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT);
 		if (adata->sdw) {
 			amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);
-			if (amd_manager)
+			if (amd_manager && amd_manager->interrupt_enabled)
 				schedule_work(&amd_manager->amd_sdw_irq_thread);
 		}
 		irq_flag = 1;
@@ -226,7 +226,7 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
 		writel(ACP_SDW1_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT1);
 		if (adata->sdw) {
 			amd_manager = dev_get_drvdata(&adata->sdw->pdev[1]->dev);
-			if (amd_manager)
+			if (amd_manager && amd_manager->interrupt_enabled)
 				schedule_work(&amd_manager->amd_sdw_irq_thread);
 		}
 		irq_flag = 1;
-- 
2.53.0


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

* Re: [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers
  2026-11-07 17:52 [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Vijendar Mukunda
                   ` (3 preceding siblings ...)
  2026-11-07 17:52 ` [PATCH 4/4] ASoC: amd: ps: " Vijendar Mukunda
@ 2026-11-07 19:39 ` Mukunda,Vijendar
  2026-11-07 19:46   ` Mukunda,Vijendar
  4 siblings, 1 reply; 7+ messages in thread
From: Mukunda,Vijendar @ 2026-11-07 19:39 UTC (permalink / raw)
  To: broonie, vinod.koul
  Cc: alsa-devel, lgirdwood, perex, tiwai, yung-chuan.liao,
	Basavaraj.Hiregoudar, Sunil-kumar.Dommati, venkataprasad.potturu,
	Syed.SabaKareem, Mario.Limonciello, Richard.Gong, Poojitha.N,
	lee, linux-sound, linux-kernel, sound-open-firmware

On 07/11/26 23:22, Vijendar Mukunda wrote:
> These four patches fix two related race conditions in the AMD SoundWire
> IRQ handlers for the PS (ACP63) and SOF platforms.
>
> Patches 1 and 2 guard against a NULL dereference in the ACP external IRQ
> handler.  The handler is registered before sdw_amd_probe() assigns
> adata->sdw, so a stray interrupt in the probe window, or one that fires
> after sdw_amd_exit() frees the context, can reach the adata->sdw->pdev[i]
> dereference with adata->sdw still NULL.
>
> Patches 3 and 4 guard schedule_work() calls with the interrupt_enabled
> flag so that an in-flight IRQ arriving while the SoundWire manager is
> being torn down does not queue new work after amd_disable_sdw_interrupts()
> has cleared the flag.
>
> NOTE: Patches 3 and 4 depend on the following patch which introduces the
> interrupt_enabled field in the AMD SoundWire manager:
>
>   https://lore.kernel.org/linux-sound/20261107154659.3358223-18-Vijendar.Mukunda@amd.com/
>
> Until that patch is merged, patches 3 and 4 should not be picked up
> independently.
Hi Mark, Vinod,
 
The following two series have a build and correctness dependency on
each other:
 
SoundWire fix series (25 patches):
https://lore.kernel.org/linux-sound/20261107154659.3358223-1-Vijendar.Mukunda@amd.com/
 
Companion ASoC series (4 patches):
https://lore.kernel.org/alsa-devel/20261107175339.3547918-2-Vijendar.Mukunda@amd.com/
 
ASoC patches 1/4 and 2/4 reference the interrupt_enabled member of
struct amd_sdw_manager introduced by SoundWire patch 17/25, so the
ASoC patches will not build without the SoundWire series. SoundWire
patch 18/25 delays *sdw_ctx publication until after startup succeeds,
opening a probe-window race in acp63_irq_handler() and
acp_irq_handler() that ASoC patches 3/4 and 4/4 close with adata->sdw
NULL checks.
 
Because the two series are split across trees, the Sashiko AI review
tool sees each series in isolation and raises the cross-tree dependency
as unresolved races:
 
https://sashiko.dev/#/patchset/20261107154659.3358223-1-Vijendar.Mukunda%40amd.com
https://sashiko.dev/#/patchset/20261107175339.3547918-1-Vijendar.Mukunda%40amd.com
 
When both series were combined and run through Sashiko as a single
patch set, none of these cross-dependency issues were reported,
confirming that the concerns are an artifact of the split rather than
real bugs. Reposting as a single combined series would give Sashiko
the full context and make the review cycle easier. Please let us know
if that is the preferred approach.
 
Thanks,
Vijendar



> Vijendar Mukunda (4):
>   ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ
>     handler
>   ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ
>     handler
>   ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled
>     flag
>   ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled
>     flag
>
>  sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
>  sound/soc/sof/amd/acp.c   | 32 ++++++++++++++++++++------------
>  2 files changed, 40 insertions(+), 24 deletions(-)
>
> --
> 2.53.0


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

* Re: [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers
  2026-11-07 19:39 ` [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Mukunda,Vijendar
@ 2026-11-07 19:46   ` Mukunda,Vijendar
  0 siblings, 0 replies; 7+ messages in thread
From: Mukunda,Vijendar @ 2026-11-07 19:46 UTC (permalink / raw)
  To: broonie, vinod.koul
  Cc: alsa-devel, lgirdwood, perex, tiwai, yung-chuan.liao,
	Basavaraj.Hiregoudar, Sunil-kumar.Dommati, venkataprasad.potturu,
	Syed.SabaKareem, Mario.Limonciello, Richard.Gong, Poojitha.N,
	lee, linux-sound, linux-kernel, sound-open-firmware

On 08/11/26 01:09, Mukunda,Vijendar wrote:
> On 07/11/26 23:22, Vijendar Mukunda wrote:
>> These four patches fix two related race conditions in the AMD SoundWire
>> IRQ handlers for the PS (ACP63) and SOF platforms.
>>
>> Patches 1 and 2 guard against a NULL dereference in the ACP external IRQ
>> handler.  The handler is registered before sdw_amd_probe() assigns
>> adata->sdw, so a stray interrupt in the probe window, or one that fires
>> after sdw_amd_exit() frees the context, can reach the adata->sdw->pdev[i]
>> dereference with adata->sdw still NULL.
>>
>> Patches 3 and 4 guard schedule_work() calls with the interrupt_enabled
>> flag so that an in-flight IRQ arriving while the SoundWire manager is
>> being torn down does not queue new work after amd_disable_sdw_interrupts()
>> has cleared the flag.
>>
>> NOTE: Patches 3 and 4 depend on the following patch which introduces the
>> interrupt_enabled field in the AMD SoundWire manager:
>>
>>   https://lore.kernel.org/linux-sound/20261107154659.3358223-18-Vijendar.Mukunda@amd.com/
>>
>> Until that patch is merged, patches 3 and 4 should not be picked up
>> independently.
> Hi Mark, Vinod,
>  
> The following two series have a build and correctness dependency on
> each other:
>  
> SoundWire fix series (25 patches):
> https://lore.kernel.org/linux-sound/20261107154659.3358223-1-Vijendar.Mukunda@amd.com/
>  
> Companion ASoC series (4 patches):
> https://lore.kernel.org/alsa-devel/20261107175339.3547918-2-Vijendar.Mukunda@amd.com/
>  
> ASoC patches 1/4 and 2/4 reference the interrupt_enabled member of
I was referring to the patch ordering in my local copy, which differs from the
series that was posted. In the posted series, the correct references are 3/4 and
4/4 for the |interrupt_enabled| dependency, and 1/4 and 2/4 for the |adata->sdw|
NULL checks.
> struct amd_sdw_manager introduced by SoundWire patch 17/25, so the
> ASoC patches will not build without the SoundWire series. SoundWire
> patch 18/25 delays *sdw_ctx publication until after startup succeeds,
> opening a probe-window race in acp63_irq_handler() and
> acp_irq_handler() that ASoC patches 3/4 and 4/4 close with adata->sdw
> NULL checks.
Correction to my previous reply: I quoted the wrong patch numbers.

In the posted series, patches 3/4 and 4/4 have the interrupt_enabled
dependency, while patches 1/4 and 2/4 contain the adata->sdw NULL checks.

Sorry for the confusion.
> Because the two series are split across trees, the Sashiko AI review
> tool sees each series in isolation and raises the cross-tree dependency
> as unresolved races:
>  
> https://sashiko.dev/#/patchset/20261107154659.3358223-1-Vijendar.Mukunda%40amd.com
> https://sashiko.dev/#/patchset/20261107175339.3547918-1-Vijendar.Mukunda%40amd.com
>  
> When both series were combined and run through Sashiko as a single
> patch set, none of these cross-dependency issues were reported,
> confirming that the concerns are an artifact of the split rather than
> real bugs. Reposting as a single combined series would give Sashiko
> the full context and make the review cycle easier. Please let us know
> if that is the preferred approach.
>  
> Thanks,
> Vijendar
>
>
>
>> Vijendar Mukunda (4):
>>   ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ
>>     handler
>>   ASoC: SOF: amd: guard adata->sdw dereferences with NULL check in IRQ
>>     handler
>>   ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled
>>     flag
>>   ASoC: amd: ps: guard SDW IRQ work scheduling with interrupt_enabled
>>     flag
>>
>>  sound/soc/amd/ps/pci-ps.c | 32 ++++++++++++++++++++------------
>>  sound/soc/sof/amd/acp.c   | 32 ++++++++++++++++++++------------
>>  2 files changed, 40 insertions(+), 24 deletions(-)
>>
>> --
>> 2.53.0


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

end of thread, other threads:[~2026-11-07 19:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-07 17:52 [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Vijendar Mukunda
2026-11-07 17:52 ` [PATCH 1/4] ASoC: amd: ps: guard adata->sdw dereferences with NULL check in IRQ handler Vijendar Mukunda
2026-11-07 17:52 ` [PATCH 2/4] ASoC: SOF: amd: " Vijendar Mukunda
2026-11-07 17:52 ` [PATCH 3/4] ASoC: SOF: amd: guard SDW IRQ work scheduling with interrupt_enabled flag Vijendar Mukunda
2026-11-07 17:52 ` [PATCH 4/4] ASoC: amd: ps: " Vijendar Mukunda
2026-11-07 19:39 ` [PATCH 0/4] ASoC: amd: fix IRQ handler races in AMD SoundWire drivers Mukunda,Vijendar
2026-11-07 19:46   ` Mukunda,Vijendar

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

all inboxes | Powered by JetHome®