Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/boards/intel_adsp_ace40_nvl.conf
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ CONFIG_PM_DEVICE_RUNTIME_ASYNC=n
# Zephyr / logging
CONFIG_LOG_BACKEND_ADSP=n
CONFIG_WINSTREAM_CONSOLE=n
CONFIG_LOG_RUNTIME_FILTERING=y
CONFIG_LOG_RUNTIME_DEFAULT_LEVEL=1

# Zephyr / debug: temporary, until fixed
CONFIG_GDBSTUB=n
2 changes: 2 additions & 0 deletions app/boards/intel_adsp_ace40_nvls.conf
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ CONFIG_PM_DEVICE_RUNTIME_ASYNC=n
# Zephyr / logging
CONFIG_LOG_BACKEND_ADSP=n
CONFIG_WINSTREAM_CONSOLE=n
CONFIG_LOG_RUNTIME_FILTERING=y
CONFIG_LOG_RUNTIME_DEFAULT_LEVEL=1

# Zephyr / debug: temporary, until fixed
CONFIG_GDBSTUB=n
7 changes: 7 additions & 0 deletions app/debug_overlay.conf
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,10 @@ CONFIG_COLD_STORE_EXECUTE_DEBUG=y

CONFIG_GDBSTUB=y
CONFIG_GDBSTUB_ENTER_IMMEDIATELY=n

# Logging options
# Set default runtime log level to INFO (3) to maintain expected behavior in SOF CI.
# This ensures firmware starts with INFO level logging, same as previous configuration.
# Testing with runtime filtering enabled ensures the same feature set is validated.
# Note: This setting has no effect if CONFIG_LOG_RUNTIME_FILTERING is disabled.
CONFIG_LOG_RUNTIME_DEFAULT_LEVEL=3
9 changes: 9 additions & 0 deletions app/os_linux_overlay.conf
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,12 @@
# state, so context save can be disabled
CONFIG_ADSP_IMR_CONTEXT_SAVE=n
CONFIG_SOF_USERSPACE_PROXY=n

# Logging options

# Set default runtime log level to INFO (3).
# Note: This setting only has effect when CONFIG_LOG_RUNTIME_FILTERING is enabled.
# By default, the runtime level equals the compile-time level, but it is explicitly
# set here to INFO to ensure expected behavior if platform configurations set a
# lower default level (e.g., ERROR).
CONFIG_LOG_RUNTIME_DEFAULT_LEVEL=3
3 changes: 3 additions & 0 deletions src/audio/drc/drc_log.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ SOF_DEFINE_REG_UUID(drc);
LOG_MODULE_REGISTER(drc, CONFIG_SOF_LOG_LEVEL);
DECLARE_TR_CTX(drc_tr, SOF_UUID(drc_uuid), LOG_LEVEL_INFO);
EXPORT_SYMBOL(log_const_drc);
#ifdef CONFIG_LOG_RUNTIME_FILTERING
EXPORT_SYMBOL(log_dynamic_drc);
#endif
45 changes: 42 additions & 3 deletions src/ipc/ipc4/logging.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,53 @@ int ipc4_logging_enable_logs(bool first_block,
log_state = (const struct ipc4_log_state_info *)data;

if (log_state->enable) {
uint32_t log_level = LOG_LEVEL_NONE; /* Default if no bits set */
uint32_t mask = log_state->logs_mask[0];

/* TODO: Improve mask handling for better code maintainability
* The logs_mask bits should be defined using proper macros or a struct
* to improve readability and maintainability. Current hardcoded bit
* positions are sufficient for now but should be refactored in the future.
* Possible improvements:
* - Define IPC4_LOG_MASK_* macros for each bit position
* - Create a struct with bitfields for each priority level
* - Add proper documentation in IPC4 specification headers
*/

/* Determine log level from mask bits 0-4 (priority levels)
* bit 0: critical & error -> LOG_LEVEL_ERR
* bit 1: high & warning -> LOG_LEVEL_WRN
* bit 2: medium -> LOG_LEVEL_INF
* bit 3: low & info -> LOG_LEVEL_INF
* bit 4: verbose & debug -> LOG_LEVEL_DBG
* Check highest bit set to determine maximum log level
*/
if (mask & BIT(4))
log_level = LOG_LEVEL_DBG;
else if (mask & (BIT(3) | BIT(2)))
log_level = LOG_LEVEL_INF;
else if (mask & BIT(1))
log_level = LOG_LEVEL_WRN;
else if (mask & BIT(0))
log_level = LOG_LEVEL_ERR;
Copy link
Collaborator

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

Copy link
Contributor Author

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmleman I wasn't claiming that. Just saying this effectively breaks mtrace for existing systems. By default logs are disabled (no enable sent by host), but we have documented flow to set a kernel module parameter. If set, kernel will enable mtrace (no log mask) and mtrace is expected to work (also in release firmware files). As I understand this change, after this PR, there's no way to get mtrace working with current Linux kernel and to get FW logs out, one needs a new kernel that can set the log_mask to some non-zero value. Or did I miss something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its probably best to have a Kconfig=level for Linux based OS to boot with current level and then have kernel tune after FW boot if required. This can be a debugFS file @ujfalusi
Other OSes/targets can set the level as needed in their Kconfigs.


adsp_mtrace_log_init(mtrace_log_hook);
/* Initialize work queue if not already initialized */
if (!log_work.work.handler)
k_work_init_delayable(&log_work, log_work_handler);

/* 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);
/* Enable backend with determined log level
*
* Note: If CONFIG_LOG_RUNTIME_FILTERING is not enabled, the log_level
* parameter has no effect - all logs are filtered at compile-time only.
*
* Note: Setting log_level to LOG_LEVEL_NONE will result in no logs being
* output, as all runtime filters will be set to NONE. This behavior will
* be useful in the future when per-source filtering can be specified via
* IPC, allowing selective enabling of specific log sources while keeping
* others disabled.
*/
log_backend_enable(log_backend, mtrace_log_hook, log_level);

mtrace_aging_timer = log_state->aging_timer_period;
if (mtrace_aging_timer < IPC4_MTRACE_AGING_TIMER_MIN_MS) {
Expand Down
2 changes: 1 addition & 1 deletion west.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ manifest:

- name: zephyr
repo-path: zephyr
revision: f6a32b27dc17bdb4e1ba4ad84de0c56ca71e257d
revision: pull/103820/head
remote: zephyrproject

# Import some projects listed in zephyr/west.yml@revision
Expand Down