IADK: fix module configuration sizing and error handling - #11118
IADK: fix module configuration sizing and error handling#11118abonislawski wants to merge 4 commits into
Conversation
Reject malformed initialization data before deriving the extended module configuration. Signed-off-by: Adrian Bonislawski <adrian.bonislawski@intel.com>
Stop module initialization when the external factory reports a creation error. Signed-off-by: Adrian Bonislawski <adrian.bonislawski@intel.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens robustness of the IADK module instantiation path by validating module configuration IPC payloads before use, correctly sizing compound configuration blobs, and ensuring module factory failures do not lead to using an invalid module instance.
Changes:
- Fix compound INIT_INSTANCE size calculation by using
OutputPinFormatsizing for output pins. - Reject unparsable module configuration messages during
SystemAgent::CheckIn. - Stop proceeding after module factory creation failures (return error instead of continuing initialization).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/audio/module_adapter/iadk/system_agent.cpp | Adds config parse validation and checks factory creation result before continuing module initialization. |
| src/audio/module_adapter/iadk/module_initial_settings_concrete.cpp | Corrects compound configuration size computation for output pin formats. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ErrorCode::Type error; | ||
| IoPinsInfo pins_info; | ||
| const dsp_fw::DwordArray& cfg_ipc_msg = | ||
| *reinterpret_cast<const dsp_fw::DwordArray*>(obfuscated_mod_cfg); | ||
| ModuleInitialSettingsConcrete settings(cfg_ipc_msg); | ||
| if (!settings.IsParsable()) | ||
| return -EINVAL; |
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Just one open, otherwise LGTM
| error = module_factory.Create(*this, module_placeholder, ModuleInitialSettings(settings), | ||
| pins_info); | ||
| if (error != ErrorCode::NO_ERROR) | ||
| return -EINVAL; |
There was a problem hiding this comment.
Should we return the actual error or an IPC4 error code here if its called via IPC i.e. on module new ?
There was a problem hiding this comment.
@abonislawski in fact line 110 above seems wrong - it returns -1 while lines 96 and this one return errno codes, so line 110 happens to return -EPERM which is supposedly not what was intended there
There was a problem hiding this comment.
@lgirdwood we should return a regular errno here, not an IPC4 status because CheckIn() is below the IPC boundary and it will be not propagated to the host.
@lyakh yes, this should be einval too, commit added.
| error = module_factory.Create(*this, module_placeholder, ModuleInitialSettings(settings), | ||
| pins_info); | ||
| if (error != ErrorCode::NO_ERROR) | ||
| return -EINVAL; |
There was a problem hiding this comment.
@abonislawski in fact line 110 above seems wrong - it returns -1 while lines 96 and this one return errno codes, so line 110 happens to return -EPERM which is supposedly not what was intended there
Use the matching output pin type when validating compound configuration sizes. Signed-off-by: Adrian Bonislawski <adrian.bonislawski@intel.com>
Return -EINVAL instead when module prerequisites specify an unsupported number of input or output pins. Signed-off-by: Adrian Bonislawski <adrian.bonislawski@intel.com>
ebd0f17 to
fd009d4
Compare
|
@abonislawski Ready to merge one requried checks are passing. We can ignore the testbench failure, it will be fixed by #11128 |
OutputPinFormatfor output pins.SystemAgent::CheckIn.