-
Notifications
You must be signed in to change notification settings - Fork 350
ipc4: ace: Add dynamic log level control via IPC with runtime filtering #10536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
ipc4: ace: Add dynamic log level control via IPC with runtime filtering #10536
Conversation
This commit will be replaced with actual zephyr update after necessary changes are merged. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds IPC4-driven runtime log level control so the host can adjust firmware logging verbosity dynamically, and enables Zephyr runtime log filtering on Intel ADSP ACE boards with a quiet default.
Changes:
- Parse
logs_maskfromIPC4_ENABLE_LOGSand derive a Zephyr log level used when enabling the backend. - Enable
CONFIG_LOG_RUNTIME_FILTERINGand setCONFIG_LOG_RUNTIME_DEFAULT_LEVEL=1(ERROR) across multiple Intel ADSP ACE board configs. - Point the Zephyr west manifest to an upstream PR branch providing
CONFIG_LOG_RUNTIME_DEFAULT_LEVEL.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| west.yml | Switch Zephyr revision to the upstream PR providing CONFIG_LOG_RUNTIME_DEFAULT_LEVEL. |
| src/ipc/ipc4/logging.c | Compute log level from IPC4 logs_mask and pass it to log_backend_enable(). |
| app/debug_overlay.conf | Sets runtime filtering to n in the debug overlay config. |
| app/boards/intel_adsp_ace40_nvls.conf | Enables runtime filtering and sets default runtime level to ERROR. |
| app/boards/intel_adsp_ace40_nvl.conf | Enables runtime filtering and sets default runtime level to ERROR. |
| app/boards/intel_adsp_ace30_wcl.conf | Enables runtime filtering and sets default runtime level to ERROR. |
| app/boards/intel_adsp_ace30_ptl.conf | Enables runtime filtering and sets default runtime level to ERROR. |
| app/boards/intel_adsp_ace20_lnl.conf | Enables runtime filtering and sets default runtime level to ERROR. |
| app/boards/intel_adsp_ace15_mtpm.conf | Enables runtime filtering and sets default runtime level to ERROR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Enable backend if not already active */ | ||
| if (!log_backend_is_active(log_backend)) | ||
| log_backend_enable(log_backend, mtrace_log_hook, CONFIG_SOF_LOG_LEVEL); | ||
| log_backend_enable(log_backend, mtrace_log_hook, log_level); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to enable backend for LOG_LEVEL_NONE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, this doesn't seem to be illegal (pointless, but not illegal).
This IPC properly implemented using what Zephyr logger offers will allow, for example, to silence all logs for a specified backend and then set them to a higher level only for a specified source. And in such a scenario, enabling with level set to NONE makes perfect sense.
|
@kv2019i what does Linux support for this IPC look like? Presumably it needs to be implemented? Is it fine to immediately switch the default log filtering state? |
| CONFIG_LOG_FLUSH_SLEEP_US=5000 | ||
| CONFIG_WINSTREAM_CONSOLE=n | ||
| CONFIG_LOG_RUNTIME_FILTERING=y | ||
| CONFIG_LOG_RUNTIME_DEFAULT_LEVEL=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either I don't understand something or something isn't working. This means, that only ERR level logs should appear in mtrace, right? But if you check this PR logs it's still the usual INFO. If indeed this is trying to set log level to ERR, is this a good idea? We need INF level logs in CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this moment, according to my knowledge, everything works as I intended. The default log filtering level is set to ERROR at startup. This doesn't change the log level that gets compiled in, it still remains INFO. The fact that CI assumes that logs will appear in the memory window without sending IPC which is supposed to enable them, I've known for some time (I once wanted to disable logger backend autostart). The FW used in CI is built with the app/debug_overlay.conf and there runtime filtering is disabled to maintain the existing behavior expected by CI.
Implement dynamic log level selection based on the logs_mask field from the IPC4_ENABLE_LOGS command, allowing the host to control firmware logging verbosity at runtime. Previously, logging was enabled with a fixed compile-time level (CONFIG_SOF_LOG_LEVEL), preventing dynamic control. This change parses the logs_mask[0] bits 0-4 to determine the requested log level and passes it to the backend. Mask bit mapping to Zephyr log levels: - bit 0 (critical/error) -> LOG_LEVEL_ERR (1) - bit 1 (high/warning) -> LOG_LEVEL_WRN (2) - bit 2 (medium) -> LOG_LEVEL_INF (3) - bit 3 (low/info) -> LOG_LEVEL_INF (3) - bit 4 (verbose/debug) -> LOG_LEVEL_DBG (4) The highest set bit determines the maximum log level. This allows the host to request ERROR-only logging for quiet operation, or DEBUG for verbose troubleshooting, without firmware recompilation. Additionally, log_backend_enable() is now always called (it is idempotent and safe for reconfiguration), removing the previous check that prevented level changes on already-active backends. A TODO comment documents that the mask handling should be improved with proper macros or structs in the future for better code maintainability, but the current implementation is sufficient. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
21ebebb to
e55ea21
Compare
kv2019i
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good new feature, but we need to figure out how to introduce without breaking existing systems. Some comments inline.
| else if (mask & BIT(1)) | ||
| log_level = LOG_LEVEL_WRN; | ||
| else if (mask & BIT(0)) | ||
| log_level = LOG_LEVEL_ERR; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to think about how to introduce this. Current shipping Linux kernels do not set log_mask and we cannot retroactively change shipped kernels (update of FW must not break user systems with older kernel), so this would effectively break mtrace for all existing systems as log level is forced to LOG_LEVEL_NONE. I think this needs to be a a build option (so we can keep the old behaviour in Linux overlay) and/or the default needs to be LOG_LEVEL_INF (like it is now). Looping in our kernel folks @bardliao @ujfalusi @ranj063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does FW that doesn't unnecessarily spam with logs mean breaking the user's system?
Enable CONFIG_LOG_RUNTIME_FILTERING and set CONFIG_LOG_RUNTIME_DEFAULT_LEVEL to 1 (ERROR) for all Intel ADSP ACE platforms. This provides quiet firmware boot with minimal logging overhead while maintaining full DEBUG compile-time capability for dynamic log level control via IPC. Platforms configured: - intel_adsp_ace15_mtpm - intel_adsp_ace20_lnl - intel_adsp_ace30_ptl - intel_adsp_ace30_wcl - intel_adsp_ace40_nvl - intel_adsp_ace40_nvls With these settings: - Firmware boots showing only ERROR level logs (quiet startup) - Host can dynamically increase verbosity to INFO or DEBUG via IPC4_ENABLE_LOGS without recompilation - All log macros (LOG_ERR, LOG_INF, etc.) remain compiled in Runtime filtering is explicitly disabled in debug_overlay.conf to maintain current debug build behavior where logs are not filtered at runtime. This works in conjunction with the IPC4 logging handler changes that parse logs_mask to set the appropriate runtime filter level. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
Export log_dynamic_drc symbol when CONFIG_LOG_RUNTIME_FILTERING is enabled to fix LLEXT module linking failure. When runtime filtering is enabled, LOG_MODULE_REGISTER() creates both log_const_<module> and log_dynamic_<module> structures. LLEXT modules need to link against these symbols at load time. Previously, only log_const_drc was exported, causing the LLEXT linker to fail with: <wrn> llext: llext_link_plt: PLT: cannot find idx 46 name log_dynamic_drc <err> llext: do_llext_load: Failed to link, ret -2 <err> lib_manager: llext_manager_link_single: linking failed: -2 This prevented the DRC module from loading when built as LLEXT with runtime log filtering enabled. The fix conditionally exports log_dynamic_drc when runtime filtering is configured, allowing the LLEXT module to resolve all required logging symbols. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
e55ea21 to
4ac3119
Compare
This PR adds support for dynamic firmware log level control through the IPC4_ENABLE_LOGS command, allowing the host to adjust logging verbosity at runtime without firmware recompilation.
Dependencies:
This PR depends on Zephyr upstream PR that adds
CONFIG_LOG_RUNTIME_DEFAULT_LEVEL:Changes Overview:
IPC4 logging handler enhancement - Parse logs_mask field from IPC4_ENABLE_LOGS to determine requested log level and pass it to the backend, enabling dynamic control (ERROR/WARNING/INFO/DEBUG) based on host request
Platform configuration updates - Enable runtime filtering with ERROR default level for all Intel ADSP ACE platforms (MTL/LNL/PTL/WCL/NVL/NVLS), providing quiet boot while maintaining full debug capability