From c4744ab1348861981ff533a970c22845e0358af6 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 29 Jul 2021 12:36:44 -0700 Subject: [PATCH 1/2] trace: avoid passing va_list Passing va_list seems to result in issues with stopping the DMA. Call log_func() and mtrace_dict_entry() in the _log_message() macro instead to prevent this. Signed-off-by: Ranjani Sridharan --- src/include/sof/trace/trace.h | 35 ++++++++++++++++++--------- src/trace/trace.c | 44 ++++++++++------------------------ test/cmocka/src/common_mocks.c | 10 +------- 3 files changed, 37 insertions(+), 52 deletions(-) diff --git a/src/include/sof/trace/trace.h b/src/include/sof/trace/trace.h index 155964698c56..c152aebb92e4 100644 --- a/src/include/sof/trace/trace.h +++ b/src/include/sof/trace/trace.h @@ -147,13 +147,10 @@ void trace_off(void); void trace_init(struct sof *sof); /* All tracing macros in this file end up calling these functions in the end. */ -typedef void (*log_func_t)(bool send_atomic, const void *log_entry, const struct tr_ctx *ctx, - uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, va_list args); - void trace_log_filtered(bool send_atomic, const void *log_entry, const struct tr_ctx *ctx, - uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, va_list args); + uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, ...); void trace_log_unfiltered(bool send_atomic, const void *log_entry, const struct tr_ctx *ctx, - uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, va_list args); + uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, ...); struct sof_ipc_trace_filter_elem *trace_filter_fill(struct sof_ipc_trace_filter_elem *elem, struct sof_ipc_trace_filter_elem *end, struct trace_filter *filter); @@ -217,11 +214,6 @@ void mtrace_event(const char *complete_packet, uint32_t length); # define MTRACE_DUPLICATION_LEVEL LOG_LEVEL_ERROR #endif /* CONFIG_TRACEM */ -/* This function is _not_ passed the format string to save space */ -void _log_sofdict(log_func_t sofdict_logf, bool atomic, const void *log_entry, - const struct tr_ctx *ctx, const uint32_t lvl, - uint32_t id_1, uint32_t id_2, int arg_count, ...); - /* _log_message() */ #ifdef CONFIG_LIBRARY @@ -264,6 +256,26 @@ _thrown_from_macro_BASE_LOG_in_trace_h * linker section and replaced by a &log_entry pointer to it. This must * be a macro for the source location to be meaningful. */ +#ifndef __ZEPHYR__ +#define _log_message(log_func, atomic, lvl, comp_class, ctx, id_1, id_2, format, ...) \ +do { \ + _DECLARE_LOG_ENTRY(lvl, format, comp_class, \ + META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__)); \ + STATIC_ASSERT_ARG_SIZE(__VA_ARGS__); \ + STATIC_ASSERT(_TRACE_EVENT_MAX_ARGUMENT_COUNT >= \ + META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__), \ + BASE_LOG_ASSERT_FAIL_MSG \ + ); \ + (lvl <= MTRACE_DUPLICATION_LEVEL ? \ + mtrace_dict_entry(true, (uint32_t)&log_entry, \ + META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__), \ + ##__VA_ARGS__) : 0); \ + log_func(atomic, &log_entry, ctx, lvl, id_1, id_2, \ + META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__), ##__VA_ARGS__); \ + _log_nodict(atomic, META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__), \ + lvl, format, ##__VA_ARGS__); \ +} while (0) +#else #define _log_message(log_func, atomic, lvl, comp_class, ctx, id_1, id_2, format, ...) \ do { \ _DECLARE_LOG_ENTRY(lvl, format, comp_class, \ @@ -273,11 +285,12 @@ do { \ META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__), \ BASE_LOG_ASSERT_FAIL_MSG \ ); \ - _log_sofdict(log_func, atomic, &log_entry, ctx, lvl, id_1, id_2, \ + log_func(atomic, &log_entry, ctx, lvl, id_1, id_2, \ META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__), ##__VA_ARGS__); \ _log_nodict(atomic, META_COUNT_VARAGS_BEFORE_COMPILE(__VA_ARGS__), \ lvl, format, ##__VA_ARGS__); \ } while (0) +#endif #ifdef __ZEPHYR__ /* Just like XTOS, only the most urgent messages go to limited diff --git a/src/trace/trace.c b/src/trace/trace.c index f311d6021950..51d6c157ef27 100644 --- a/src/trace/trace.c +++ b/src/trace/trace.c @@ -249,21 +249,25 @@ static void dma_trace_log(bool send_atomic, uint32_t log_entry, const struct tr_ } void trace_log_unfiltered(bool send_atomic, const void *log_entry, const struct tr_ctx *ctx, - uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, va_list vl) + uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, ...) { struct trace *trace = trace_get(); + va_list vl; if (!trace->enable) { return; } + va_start(vl, arg_count); dma_trace_log(send_atomic, (uint32_t)log_entry, ctx, lvl, id_1, id_2, arg_count, vl); + va_end(vl); } void trace_log_filtered(bool send_atomic, const void *log_entry, const struct tr_ctx *ctx, - uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, va_list vl) + uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, ...) { struct trace *trace = trace_get(); + va_list vl; #if CONFIG_TRACE_FILTERING_ADAPTIVE uint64_t current_ts; @@ -289,7 +293,9 @@ void trace_log_filtered(bool send_atomic, const void *log_entry, const struct tr } #endif /* CONFIG_TRACE_FILTERING_ADAPTIVE */ + va_start(vl, arg_count); dma_trace_log(send_atomic, (uint32_t)log_entry, ctx, lvl, id_1, id_2, arg_count, vl); + va_end(vl); } struct sof_ipc_trace_filter_elem *trace_filter_fill(struct sof_ipc_trace_filter_elem *elem, @@ -509,9 +515,9 @@ void trace_init(struct sof *sof) dma_trace_init_early(sof); } -static void mtrace_dict_entry_vl(bool atomic_context, uint32_t dict_entry_address, - int n_args, va_list ap) +void mtrace_dict_entry(bool atomic_context, uint32_t dict_entry_address, int n_args, ...) { + va_list ap; int i; char packet[MESSAGE_SIZE(_TRACE_EVENT_MAX_ARGUMENT_COUNT)]; uint32_t *args = (uint32_t *)&packet[MESSAGE_SIZE(0)]; @@ -520,8 +526,10 @@ static void mtrace_dict_entry_vl(bool atomic_context, uint32_t dict_entry_addres put_header(packet, dt_tr.uuid_p, _TRACE_INV_ID, _TRACE_INV_ID, dict_entry_address, tstamp); + va_start(ap, n_args); for (i = 0; i < MIN(n_args, _TRACE_EVENT_MAX_ARGUMENT_COUNT); i++) args[i] = va_arg(ap, uint32_t); + va_end(ap); if (atomic_context) { mtrace_event(packet, MESSAGE_SIZE(n_args)); @@ -534,31 +542,3 @@ static void mtrace_dict_entry_vl(bool atomic_context, uint32_t dict_entry_addres spin_unlock_irq(&trace->lock, saved_flags); } } - -void mtrace_dict_entry(bool atomic_context, uint32_t dict_entry_address, int n_args, ...) -{ - va_list ap; - - va_start(ap, n_args); - mtrace_dict_entry_vl(atomic_context, dict_entry_address, n_args, ap); - va_end(ap); -} - -void _log_sofdict(log_func_t sofdict_logf, bool atomic, const void *log_entry, - const struct tr_ctx *ctx, const uint32_t lvl, - uint32_t id_1, uint32_t id_2, int arg_count, ...) -{ - va_list ap; - -#ifndef __ZEPHYR__ /* for Zephyr see _log_nodict() in trace.h */ - if (lvl <= MTRACE_DUPLICATION_LEVEL) { - va_start(ap, arg_count); - mtrace_dict_entry_vl(atomic, (uint32_t)log_entry, arg_count, ap); - va_end(ap); - } -#endif - - va_start(ap, arg_count); - sofdict_logf(atomic, log_entry, ctx, lvl, id_1, id_2, arg_count, ap); - va_end(ap); -} diff --git a/test/cmocka/src/common_mocks.c b/test/cmocka/src/common_mocks.c index ddabdad353d6..e362f4b953bd 100644 --- a/test/cmocka/src/common_mocks.c +++ b/test/cmocka/src/common_mocks.c @@ -102,8 +102,7 @@ void WEAK __panic(uint32_t p, char *filename, uint32_t linenum) #if CONFIG_TRACE void WEAK trace_log_filtered(bool send_atomic, const void *log_entry, const struct tr_ctx *ctx, - uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, - va_list args) + uint32_t lvl, uint32_t id_1, uint32_t id_2, int arg_count, ...) { (void) send_atomic; (void) log_entry; @@ -112,13 +111,6 @@ void WEAK trace_log_filtered(bool send_atomic, const void *log_entry, const stru (void) id_1; (void) id_2; (void) arg_count; - (void) args; -} - -void WEAK _log_sofdict(log_func_t sofdict_logf, bool atomic, const void *log_entry, - const struct tr_ctx *ctx, const uint32_t lvl, - uint32_t id_1, uint32_t id_2, int arg_count, ...) -{ } void WEAK trace_flush_dma_to_mbox(void) From 757755a0ed924977c2b4a660baeb62323e82081d Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 29 Jul 2021 15:14:54 -0700 Subject: [PATCH 2/2] APL defconfig: remove some unnecessary components to allow the build to succeed. Signed-off-by: Ranjani Sridharan --- src/arch/xtensa/configs/apollolake_defconfig | 4 ++++ src/include/sof/trace/trace.h | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/arch/xtensa/configs/apollolake_defconfig b/src/arch/xtensa/configs/apollolake_defconfig index 2c2aa1b231f5..e373aeffce39 100644 --- a/src/arch/xtensa/configs/apollolake_defconfig +++ b/src/arch/xtensa/configs/apollolake_defconfig @@ -8,3 +8,7 @@ CONFIG_PERFORMANCE_COUNTERS=y CONFIG_COMP_SRC_SMALL=y CONFIG_COMP_TDFB=n CONFIG_COMP_TONE=n +CONFIG_COMP_KPB=n +CONFIG_COMP_SEL=n +CONFIG_COMP_CROSSOVER=n +CONFIG_COMP_DRC=n diff --git a/src/include/sof/trace/trace.h b/src/include/sof/trace/trace.h index c152aebb92e4..2ac506b1b14f 100644 --- a/src/include/sof/trace/trace.h +++ b/src/include/sof/trace/trace.h @@ -97,7 +97,6 @@ struct trace_filter { #if CONFIG_TRACE -#include #include /* LOG_LEVEL_... */ /*