Repository navigation
Conversation
|
Can one of the admins verify this patch?
|
There was a problem hiding this comment.
🟢 Approval recommended
The teardown ordering correctly prevents DMA access to freed ring buffers with no identified regressions.
0 open findings
What changed in this PR
Stops HDA CORB/RIRB DMA during device removal to prevent writes into freed ring buffers after SOF probe failure.
Changes:
- Stops codec command I/O before IRQ and DMA-buffer teardown.
- Prevents repeated DMAR faults and possible memory corruption.
| File | Description |
|---|---|
sound/soc/sof/intel/hda.c |
Stops CORB/RIRB DMA before freeing stream resources. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
test this please |
|
Hi @bardliao , Please let me know if any work required from my side for get it upstreamed. Thanks, |
|
@yifanli-intel I would like to wait for @kv2019i's review as he is the HDA expert. However, he is on leave now. |
Noted and thanks a lot for your quick reply :) -Yifan |
|
Sorry @yifanli-intel @bardliao , I had not seen the ping, will review this today. |
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Thanks @yifanli-intel ! This seems valid and probably unintentional side-effect of 9e6966f . Given that commit is from @ujfalusi , let's have him look at this as well, but looks good for me. Probably same side-effect happens without IOMMU as well, but in this case the problem just becomes easier to detect due to IOMMU errors.
Btw, @bardliao @yifanli-intel what's the latest on assisted-by usage in upstream. I think Torvalds acked removal of the assisted-by tags (in 7.3.0).
Thanks! Yes assisted-by is removed, I will update this patch. -Yifan |
If the deferred probe fails after hda_dsp_probe(), e.g. because the SOF firmware is not installed, the IOMMU reports a continuous stream of DMA write faults from the audio controller: [ 13.155983] sof-audio-pci-intel-nvl 0000:80:1f.3: error: sof_probe_work failed err: -2 [ 15.056446] DMAR: DRHD: handling fault status reg 2 [ 15.056457] DMAR: [DMA Write NO_PASID] Request device [0000:80:1f.3] fault addr 0xffffe000 [fault reason 0x71] SM: Present bit in first-level paging entry is clear hda_dsp_remove() frees the CORB/RIRB ring buffer in hda_dsp_stream_free() without stopping the controller, so RIRB DMA and unsolicited responses stay enabled. sof_pci_probe() has already returned success at this point, so the PCI device stays bound with bus mastering enabled, and every unsolicited response from the codec is written to the freed buffer. Without an IOMMU these writes would go to memory that may already have been reused. This was masked until commit 9e6966f ("ASoC: SOF: Intel: hda: Keep non-alt mlinks powered at probe on ACE2+"): hda_bus_ml_put_all() at the end of hda_init_caps() dropped the last link reference, and snd_hdac_ext_bus_link_put() then stopped CORB/RIRB DMA as a side effect. Call hda_codec_stop_cmd_io() before the buffers are freed, to disable CORB/RIRB DMA and unsolicited responses. Fixes: 9e6966f ("ASoC: SOF: Intel: hda: Keep non-alt mlinks powered at probe on ACE2+") Suggested-by: Bard Liao <yung-chuan.liao@linux.intel.com> Signed-off-by: Li Yifan <yifan2.li@intel.com>
eebb7a7 to
57d3546
Compare
|
Updated v2: removed the 'Assisted-by' tag, no change to patch and rest part of commit message. -Yifan |
|
I'll revert this patch @yifanli-intel , Sahsa sent a broader fix : https://lore.kernel.org/linux-sound/20261028144446.992948-1-sashal@kernel.org/ |
Thanks @ujfalusi for mentioning this PR in your reply to Sasha's patch, and agreed, his fix is the better one. I tested Sasha's patch on NovaLake-S, replied on the list with a Tested-by, and asked Sasha to add Reported-by/Closes pointing to this PR when he sends v2: https://lore.kernel.org/linux-sound/MN7PR11MB17084628AB71D0EB1CCCA56D23AF6E2@MN7PR11MB170846.namprd11.prod.outlook.com/ Fine with reverting this one. Also thanks @bardliao and @kv2019i for the reviews! -Yifan |
…fers 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 9e6966f ("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. Fixes: 9e6966f ("ASoC: SOF: Intel: hda: Keep non-alt mlinks powered at probe on ACE2+") Signed-off-by: Sasha Levin <sashal@kernel.org> Acked-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Tested-by: Li Yifan <yifan2.li@intel.com> Reported-by: Li Yifan <yifan2.li@intel.com> Closes: thesofproject/linux#5976 Link: https://patch.msgid.link/20261028144446.992948-1-sashal@kernel.org Signed-off-by: Mark Brown <broonie@kernel.org>
…fers 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 9e6966f ("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. Fixes: 9e6966f ("ASoC: SOF: Intel: hda: Keep non-alt mlinks powered at probe on ACE2+") Signed-off-by: Sasha Levin <sashal@kernel.org> Acked-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Tested-by: Li Yifan <yifan2.li@intel.com> Reported-by: Li Yifan <yifan2.li@intel.com> Closes: thesofproject#5976 Link: https://patch.msgid.link/20261028144446.992948-1-sashal@kernel.org Signed-off-by: Mark Brown <broonie@kernel.org>
We found this issue when testing v7.3-rc3 based kernel with Ubuntu 26.04 on some new platform. Before we manually install the sof firmware package to the device, we saw DMAR error after the sof probe failure:
[ 13.155983] sof-audio-pci-intel-nvl 0000:80:1f.3: error: sof_probe_work failed err: -2
[ 15.056446] DMAR: DRHD: handling fault status reg 2
[ 15.056457] DMAR: [DMA Write NO_PASID] Request device [0000:80:1f.3] fault addr 0xffffe000 [fault reason 0x71] SM: Present bit in first-level paging entry is clear
The DMAR log keeps printing in dmesg, and we did a bisect with v7.2 kernel and found no issues on v7.2.
In debugging, we found hda_dsp_remove() frees the CORB/RIRB ring buffer in hda_dsp_stream_free() without stopping the controller, so RIRB DMA and unsolicited responses stay enabled. sof_pci_probe() has already returned success at this point, so the PCI device stays bound with bus mastering enabled, and every unsolicited response from the codec is written to the freed buffer. Without an IOMMU these writes would go to memory that may already have been reused.
This was masked until commit 9e6966f ("ASoC: SOF: Intel: hda: Keep non-alt mlinks powered at probe on ACE2+"): hda_bus_ml_put_all() at the end of hda_init_caps() dropped the last link reference, and snd_hdac_ext_bus_link_put() then stopped CORB/RIRB DMA as a side effect.
With suggestion from Bard, we add hda_codec_stop_cmd_io() before the buffers are freed, to disable CORB/RIRB DMA and unsolicited responses.
Test executed with this patch:
compile on our v7.3-rc3 kernel with make x86_64 allyes/allmod config, no issue.
compile on topic/sof-dev kernel branch(current) with make x86_64 def config, no issue.
tested with our issued platform(NVL) with the patched kernel, the DMAR logs disappeared when sof probe failure, also tested bind/unbind audio device on the platform, no issue found
-Yifan