ipc4: harden IPC4 pipeline triggering logic#10893
ipc4: harden IPC4 pipeline triggering logic#10893serhiy-katsyuba-intel wants to merge 2 commits into
Conversation
Using IPC4_MOD_ID() is not the best way to check if IPC4 is enabled. For module ID == 0, IPC4_MOD_ID() returns 0 for both IPC3 and IPC4. Module ID 0 is a valid IPC4 BASEFW ID. Since BASEFW is never added to a pipeline, this change doesn't fix any real problem. However, it's just more appropriate and safer to use IS_ENABLED(CONFIG_IPC_MAJOR_4): if module ID data becomes corrupted (zeroed) at runtime, this shouldn't make debugging even harder by causing unexpected pipeline behavior. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens IPC4 pipeline trigger handling by removing reliance on IPC4_MOD_ID() as an IPC4/IPC3 discriminator (since module ID 0 is valid in IPC4) and by adding an IPC4-specific assertion to ensure the “single pipeline per trigger” assumption holds.
Changes:
- Replace
IPC4_MOD_ID(... )checks withIS_ENABLED(CONFIG_IPC_MAJOR_4)in several pipeline traversal/trigger/prepare/reset paths to avoid mis-detecting IPC4 when module ID is 0. - Add an IPC4-only assertion in
pipeline_schedule_triggered()to validate that at most one pipeline is present in the scheduled pipelines list for IPC4.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/audio/pipeline/pipeline-stream.c | Avoid IPC4 mis-detection by using a compile-time IPC4 enable check when deciding whether to traverse/trigger connected pipelines. |
| src/audio/pipeline/pipeline-schedule.c | Use compile-time IPC4 check for STOP-reset behavior; add IPC4-only assertion to validate single-pipeline triggering assumptions. |
| src/audio/pipeline/pipeline-params.c | Prevent cross-pipeline prepare propagation under IPC4 using IS_ENABLED(CONFIG_IPC_MAJOR_4). |
| src/audio/pipeline/pipeline-graph.c | Prevent cross-pipeline reset propagation under IPC4 using IS_ENABLED(CONFIG_IPC_MAJOR_4). |
tmleman
left a comment
There was a problem hiding this comment.
https://github.com/thesofproject/sof/actions/runs/27404363632/job/80989689260?pr=10893
This looks unrelated and I think I've already seen it before (I believe I have a fix on a branch with some other changes). But I will verify it on Monday.
|
@serhiy-katsyuba-intel can you check CI + fuzz, not sure if related to this PR ? |
4e4549d to
6c8929a
Compare
|
Pushed same changes just to re-trigger CI. |
With IPC4, each pipeline is triggered separately. Exactly 1 pipeline is expected in the pipelines list in pipeline_schedule_triggered(). Unfortunately, we still have considerable complex IPC3 pipeline triggering code being used with IPC4. This assertion ensures that the code works correctly for the IPC4 case. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
6c8929a to
66e41b0
Compare
Move two commits out of #10868 as it's not yet clear what to do with the revert there.
ipc4: harden IPC4 pipeline triggering logic
Using IPC4_MOD_ID() is not the best way to check if IPC4 is enabled. For module ID == 0, IPC4_MOD_ID() returns 0 for both IPC3 and IPC4. Module ID 0 is a valid IPC4 BASEFW ID. Since BASEFW is never added to a pipeline, this change doesn't fix any real problem. However, it's just more appropriate and safer to use IS_ENABLED(CONFIG_IPC_MAJOR_4): if module ID data becomes corrupted (zeroed) at runtime, this shouldn't make debugging even harder by causing unexpected pipeline behavior.
ipc4: assert IPC4 pipeline triggering logic
With IPC4, each pipeline is triggered separately. Exactly 1 pipeline is expected in the pipelines list in pipeline_schedule_triggered(). Unfortunately, we still have considerable complex IPC3 pipeline triggering code being used with IPC4. This assertion ensures that the code works correctly for the IPC4 case.