Re: [PATCH] ASoC: SOF: Intel: hda: Stop the controller before freeing its DMA buffers

[Péter Ujfalusi]

On 28/10/2026 16:44, Sasha Levin wrote:
> hda_dsp_probe() starts the CORB/RIRB DMA engines through
> hda_dsp_ctrl_init_chip(), but neither hda_dsp_remove() nor the error
> path of hda_dsp_probe() stops them before hda_dsp_stream_free() frees
> the CORB/RIRB ring buffer and the position buffer.  Unsolicited
> responses also stay enabled, so the controller can keep writing RIRB
> entries into the freed page.
> 
> Until commit 9e6966f70404 ("ASoC: SOF: Intel: hda: Keep non-alt mlinks
> powered at probe on ACE2+"), hda_init_caps() dropped the initial
> references to the non-alternate links, and dropping the last one
> stopped the command DMA, so a probe that failed later, e.g. because
> the firmware is missing, did not leave it running.  Now it does, and
> as the probe runs from a work item, the device stays bound after the
> failure, with bus mastering enabled and the DMA engines pointed at
> freed memory.
> 
> Normal removal resumes the device first, which restarts the command
> DMA, so there the ring buffer is freed while the DMA is running until
> pci_disable_device() clears bus mastering.
> 
> Stop the controller with hda_dsp_ctrl_stop_chip() before freeing the
> buffers, as is done on suspend, and export it for use from hda.c.
> 
> This was reproduced by binding SOF in DSPless mode to a QEMU
> ich9-intel-hda: after the probe fails with "SOF firmware and/or
> topology file not found", CORB and RIRB DMA are still running with
> unsolicited responses and bus mastering enabled, and the page they use
> is soon reused as a page table.

This is indeed a possibility which I have overlooked.
FWIW, Li Yifan spotted the same issue earlier:
https://github.com/thesofproject/linux/pull/5976

but using hda_dsp_ctrl_stop_chip() is the better way.
> Fixes: dd96daca6c83 ("ASoC: SOF: Intel: Add APL/CNL HW DSP support")

Can you drop this Fixes tag, the hda-ctrl.c hunk will not apply nicely
to older stable kernels
With the removed tag:
Acked-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>

> Fixes: 9e6966f70404 ("ASoC: SOF: Intel: hda: Keep non-alt mlinks powered at probe on ACE2+")
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>



> ---
>  sound/soc/sof/intel/hda-ctrl.c | 1 +
>  sound/soc/sof/intel/hda.c      | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c
> index aeb34310eebd6..333f6bbea7539 100644
> --- a/sound/soc/sof/intel/hda-ctrl.c
> +++ b/sound/soc/sof/intel/hda-ctrl.c
> @@ -349,6 +349,7 @@ void hda_dsp_ctrl_stop_chip(struct snd_sof_dev *sdev)
>  
>  	bus->chip_init = false;
>  }
> +EXPORT_SYMBOL_NS(hda_dsp_ctrl_stop_chip, "SND_SOC_SOF_INTEL_HDA_COMMON");
>  
>  MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_DESCRIPTION("SOF helpers for HDaudio platforms");
> diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
> index 4dbba9186b29d..2a40cb9bf24d9 100644
> --- a/sound/soc/sof/intel/hda.c
> +++ b/sound/soc/sof/intel/hda.c
> @@ -939,6 +939,7 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
>  		hda_dsp_ctrl_ppcap_enable(sdev, false);
>  	}
>  free_ipc_irq:
> +	hda_dsp_ctrl_stop_chip(sdev);
>  	free_irq(sdev->ipc_irq, sdev);
>  free_irq_vector:
>  	if (sdev->msi_enabled)
> @@ -1006,6 +1007,9 @@ void hda_dsp_remove(struct snd_sof_dev *sdev)
>  		snd_dma_free_pages(&hda->iccmax_dmab);
>  
>  skip_disable_dsp:
> +	/* stop CORB/RIRB and position buffer DMA before the buffers are freed */
> +	hda_dsp_ctrl_stop_chip(sdev);
> +
>  	free_irq(sdev->ipc_irq, sdev);
>  	if (sdev->msi_enabled)
>  		pci_free_irq_vectors(pci);

-- 
Péter