From 1871d588205e5e3fde38d943967f7fcff815266c Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 14 Sep 2021 09:32:37 +0800 Subject: [PATCH 1/3] Revert "dma-trace: add check to avoid dereference from NULL" This reverts commit 89ec377cb55de04270dd6fa2bd8672c3dea3e83d. --- src/ipc/dma-copy.c | 8 -------- src/platform/intel/cavs/platform.c | 4 +--- src/trace/dma-trace.c | 6 ++++-- src/trace/trace.c | 3 ++- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/ipc/dma-copy.c b/src/ipc/dma-copy.c index fd2730f732a1..ac2ecc345f48 100644 --- a/src/ipc/dma-copy.c +++ b/src/ipc/dma-copy.c @@ -64,10 +64,6 @@ int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, { int ret; - /* return if DMA channel is not get yet */ - if (!dc->chan) - return -EINVAL; - /* tell gateway to copy */ ret = dma_copy(dc->chan, size, 0); if (ret < 0) @@ -89,10 +85,6 @@ int dma_copy_to_host_nowait(struct dma_copy *dc, struct dma_sg_config *host_sg, int32_t err; int32_t offset = host_offset; - /* return if DMA channel is not get yet */ - if (!dc->chan) - return -EINVAL; - if (size <= 0) return 0; diff --git a/src/platform/intel/cavs/platform.c b/src/platform/intel/cavs/platform.c index 3d7db4b2b5f2..66093ac2ab43 100644 --- a/src/platform/intel/cavs/platform.c +++ b/src/platform/intel/cavs/platform.c @@ -524,9 +524,7 @@ int platform_init(struct sof *sof) #elif CONFIG_TRACE /* Initialize DMA for Trace*/ trace_point(TRACE_BOOT_PLATFORM_DMA_TRACE); - ret = dma_trace_init_complete(sof->dmat); - if (ret < 0) - return ret; + dma_trace_init_complete(sof->dmat); #endif /* show heap status */ diff --git a/src/trace/dma-trace.c b/src/trace/dma-trace.c index 779480e49468..2d8dc4cd8b71 100644 --- a/src/trace/dma-trace.c +++ b/src/trace/dma-trace.c @@ -476,8 +476,9 @@ void dma_trace_on(void) { struct dma_trace_data *trace_data = dma_trace_data_get(); - if (!trace_data || trace_data->enabled) + if (trace_data->enabled) { return; + } trace_data->enabled = 1; schedule_task(&trace_data->dmat_work, DMA_TRACE_PERIOD, @@ -489,8 +490,9 @@ void dma_trace_off(void) { struct dma_trace_data *trace_data = dma_trace_data_get(); - if (!trace_data || !trace_data->enabled) + if (!trace_data->enabled) { return; + } schedule_task_cancel(&trace_data->dmat_work); trace_data->enabled = 0; diff --git a/src/trace/trace.c b/src/trace/trace.c index fb21dc5ce229..b1a378a1c045 100644 --- a/src/trace/trace.c +++ b/src/trace/trace.c @@ -272,8 +272,9 @@ void trace_log_unfiltered(bool send_atomic, const void *log_entry, const struct struct trace *trace = trace_get(); va_list vl; - if (!trace || !trace->enable) + if (!trace->enable) { return; + } va_start(vl, arg_count); vatrace_log(send_atomic, (uint32_t)log_entry, ctx, lvl, id_1, id_2, arg_count, vl); From 975af8c44428e8643c2b834e0063cd3d9bd6e2ac Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 14 Sep 2021 09:32:42 +0800 Subject: [PATCH 2/3] Revert "trace: enable trace after it is ready" This reverts commit 7df367451d0fa638b530c1c92f02d645845dbe53. --- src/init/init.c | 5 ++--- src/trace/dma-trace.c | 1 + src/trace/trace.c | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/init/init.c b/src/init/init.c index cef4bfa64e38..eb379746129f 100644 --- a/src/init/init.c +++ b/src/init/init.c @@ -134,8 +134,10 @@ static int primary_core_init(int argc, char *argv[], struct sof *sof) interrupt_init(sof); #endif /* __ZEPHYR__ */ +#if CONFIG_TRACE trace_point(TRACE_BOOT_SYS_TRACES); trace_init(sof); +#endif trace_point(TRACE_BOOT_SYS_NOTIFIER); init_system_notify(sof); @@ -149,9 +151,6 @@ static int primary_core_init(int argc, char *argv[], struct sof *sof) trace_point(TRACE_BOOT_PLATFORM); - /* now start the trace */ - trace_on(); - #if CONFIG_NO_SECONDARY_CORE_ROM lp_sram_unpack(); #endif diff --git a/src/trace/dma-trace.c b/src/trace/dma-trace.c index 2d8dc4cd8b71..d113443f282e 100644 --- a/src/trace/dma-trace.c +++ b/src/trace/dma-trace.c @@ -200,6 +200,7 @@ int dma_trace_init_complete(struct dma_trace_data *d) SOF_TASK_PRI_MED, trace_work, d, 0, 0); out: + return ret; } diff --git a/src/trace/trace.c b/src/trace/trace.c index b1a378a1c045..f31fb3a1c3e8 100644 --- a/src/trace/trace.c +++ b/src/trace/trace.c @@ -290,8 +290,9 @@ void trace_log_filtered(bool send_atomic, const void *log_entry, const struct tr uint64_t current_ts; #endif /* CONFIG_TRACE_FILTERING_ADAPTIVE */ - if (!trace || !trace->enable) + if (!trace->enable) { return; + } #if CONFIG_TRACE_FILTERING_VERBOSITY if (!trace_filter_verbosity(lvl, ctx)) @@ -511,6 +512,7 @@ void trace_off(void) void trace_init(struct sof *sof) { sof->trace = rzalloc(SOF_MEM_ZONE_SYS_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(*sof->trace)); + sof->trace->enable = 1; sof->trace->pos = 0; #if CONFIG_TRACE_FILTERING_ADAPTIVE sof->trace->user_filter_override = false; From 0c004756475e83300d07cf9e3e775b6b1fbfb080 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 14 Sep 2021 09:41:48 +0800 Subject: [PATCH 3/3] Debug: demonstrate trace crash issue to @marc-hb @marc-hb, see how the early trace failure blocking the FW boot here. Signed-off-by: Keyon Jie --- src/arch/xtensa/configs/apollolake_defconfig | 1 + src/lib/alloc.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/arch/xtensa/configs/apollolake_defconfig b/src/arch/xtensa/configs/apollolake_defconfig index 2c2aa1b231f5..6d30e2d4e8b9 100644 --- a/src/arch/xtensa/configs/apollolake_defconfig +++ b/src/arch/xtensa/configs/apollolake_defconfig @@ -8,3 +8,4 @@ CONFIG_PERFORMANCE_COUNTERS=y CONFIG_COMP_SRC_SMALL=y CONFIG_COMP_TDFB=n CONFIG_COMP_TONE=n +CONFIG_DEBUG_HEAP=y diff --git a/src/lib/alloc.c b/src/lib/alloc.c index b7de858c0fb6..600517ff2ab5 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -652,6 +652,10 @@ static void alloc_trace_heap(enum mem_zone zone, uint32_t caps, size_t bytes) bytes, zone, caps, flags); \ alloc_trace_heap(zone, caps, bytes); \ } \ + else { \ + tr_info(&mem_tr, "allocated ptr %p 0x%x bytes at zone 0x%x", \ + ptr, bytes, zone); \ + } \ } while (0) #else #define DEBUG_TRACE_PTR(ptr, bytes, zone, caps, flags)