From 809f3485abcd11ccc869c002526fd5ef7842f2e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Chru=C5=9Bci=C5=84ski?= Date: Tue, 4 Feb 2025 07:59:33 +0100 Subject: [PATCH] [nrf fromlist] logging: frontend_stmesp: Fix string addresses from remote core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When decoding logs from a remote core with memory that APP can access, wrong address of an array with string addresses was used. Log message contains index of a string and APP strings array was used instead of remote core. Extend STMESP logging so that address of string array of a remote core is send during startup to the APP and APP is using this array to decode strings from remote cores. Bug applies only to PPR and FLPR as APP has no access to RAD memory. Upstream PR #: 86242 Signed-off-by: Krzysztof Chruściński --- drivers/misc/coresight/nrf_etr.c | 10 +-- include/zephyr/logging/log_frontend_stmesp.h | 9 +-- .../logging/log_frontend_stmesp_demux.h | 1 + .../logging/frontends/log_frontend_stmesp.c | 3 + .../frontends/log_frontend_stmesp_demux.c | 71 +++++++++++++++---- 5 files changed, 70 insertions(+), 24 deletions(-) diff --git a/drivers/misc/coresight/nrf_etr.c b/drivers/misc/coresight/nrf_etr.c index 11d2240d861..bf1aecd6f5b 100644 --- a/drivers/misc/coresight/nrf_etr.c +++ b/drivers/misc/coresight/nrf_etr.c @@ -203,13 +203,13 @@ static void trace_point_process(struct log_frontend_stmesp_demux_trace_point *pa static const char *tp_d32 = "%d %08x"; const char *dname = stm_m_name[packet->major]; static const char *sname = "tp"; - const char **lptr; + const char *lptr; if (packet->id >= CONFIG_LOG_FRONTEND_STMESP_TURBO_LOG_BASE) { - TYPE_SECTION_GET(const char *, log_stmesp_ptr, - packet->id - CONFIG_LOG_FRONTEND_STMESP_TURBO_LOG_BASE, &lptr); - uint8_t level = (uint8_t)((*lptr)[0]) - (uint8_t)'0'; - const char *ptr = *lptr + 1; + lptr = log_frontend_stmesp_demux_str_get(packet->major, + packet->id - CONFIG_LOG_FRONTEND_STMESP_TURBO_LOG_BASE); + uint8_t level = (uint8_t)(lptr[0]) - (uint8_t)'0'; + const char *ptr = lptr + 1; static const union cbprintf_package_hdr desc0 = { .desc = {.len = 2 /* hdr + fmt */}}; static const union cbprintf_package_hdr desc1 = { diff --git a/include/zephyr/logging/log_frontend_stmesp.h b/include/zephyr/logging/log_frontend_stmesp.h index e2b7a2159a5..38f33bf62f1 100644 --- a/include/zephyr/logging/log_frontend_stmesp.h +++ b/include/zephyr/logging/log_frontend_stmesp.h @@ -106,16 +106,17 @@ TYPE_SECTION_START_EXTERN(const char *, log_stmesp_ptr); * @param _source Pointer to the source structure. * @param ... String. */ + #define LOG_FRONTEND_STMESP_LOG0(_source, ...) \ do { \ static const char _str[] __in_section(_log_stmesp_str, static, _) \ __used __noasan __aligned(sizeof(uint32_t)) = GET_ARG_N(1, __VA_ARGS__); \ static const char *_str_ptr __in_section(_log_stmesp_ptr, static, _) \ __used __noasan = _str; \ - uint32_t idx = \ + uint32_t _idx = \ ((uintptr_t)&_str_ptr - (uintptr_t)TYPE_SECTION_START(log_stmesp_ptr)) / \ sizeof(void *); \ - log_frontend_stmesp_log0(_source, idx); \ + log_frontend_stmesp_log0(_source, _idx); \ } while (0) /** @brief Macro for handling a turbo log message with one argument. @@ -129,10 +130,10 @@ TYPE_SECTION_START_EXTERN(const char *, log_stmesp_ptr); __used __noasan __aligned(sizeof(uint32_t)) = GET_ARG_N(1, __VA_ARGS__); \ static const char *_str_ptr __in_section(_log_stmesp_ptr, static, _) \ __used __noasan = _str; \ - uint32_t idx = \ + uint32_t _idx = \ ((uintptr_t)&_str_ptr - (uintptr_t)TYPE_SECTION_START(log_stmesp_ptr)) / \ sizeof(void *); \ - log_frontend_stmesp_log1(_source, idx, (uintptr_t)(GET_ARG_N(2, __VA_ARGS__))); \ + log_frontend_stmesp_log1(_source, _idx, (uintptr_t)(GET_ARG_N(2, __VA_ARGS__))); \ } while (0) #ifdef __cplusplus diff --git a/include/zephyr/logging/log_frontend_stmesp_demux.h b/include/zephyr/logging/log_frontend_stmesp_demux.h index 728ce4011ff..1a32ff27218 100644 --- a/include/zephyr/logging/log_frontend_stmesp_demux.h +++ b/include/zephyr/logging/log_frontend_stmesp_demux.h @@ -290,6 +290,7 @@ void log_frontend_stmesp_demux_free(union log_frontend_stmesp_demux_packet packe * cannot be retrieved. */ const char *log_frontend_stmesp_demux_sname_get(uint32_t m_id, uint16_t s_id); +const char *log_frontend_stmesp_demux_str_get(uint32_t m_id, uint16_t s_id); /** @brief Check if there are any started but not completed log messages. * diff --git a/subsys/logging/frontends/log_frontend_stmesp.c b/subsys/logging/frontends/log_frontend_stmesp.c index 4c6dd93dfe7..f9bfc4ddc07 100644 --- a/subsys/logging/frontends/log_frontend_stmesp.c +++ b/subsys/logging/frontends/log_frontend_stmesp.c @@ -589,10 +589,13 @@ void log_frontend_init(void) TYPE_SECTION_START_EXTERN(struct log_source_const_data, log_const); STMESP_Type *stm_esp; uintptr_t log_const_start; + uintptr_t log_str_start; (void)stmesp_get_port(CONFIG_LOG_FRONTEND_STPESP_TURBO_SOURCE_PORT_ID, &stm_esp); log_const_start = (uintptr_t)TYPE_SECTION_START(log_const); + log_str_start = (uintptr_t)TYPE_SECTION_START(log_stmesp_ptr); STM_D32(stm_esp, log_const_start, false, true); + STM_D32(stm_esp, log_str_start, false, true); #endif } diff --git a/subsys/logging/frontends/log_frontend_stmesp_demux.c b/subsys/logging/frontends/log_frontend_stmesp_demux.c index e46dadc652f..3f922b7417c 100644 --- a/subsys/logging/frontends/log_frontend_stmesp_demux.c +++ b/subsys/logging/frontends/log_frontend_stmesp_demux.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -43,9 +44,25 @@ struct log_frontend_stmesp_demux_active_entry { int off; }; -struct log_frontend_stmesp_coop_sources { +/* Coprocessors (FLPR, PPR) sends location where APP can find strings and logging + * source names utilizing the fact that APP has access to FLPR/PPR memory if it is + * an owner of that coprocessor. During the initialization FLPR/PPR sends 2 DMTS32 + * to the specific channel. First word is an address where logging source constant + * data section is located and second is where a section with addresses to constant + * strings used for logging is located. + */ +struct log_frontend_stmesp_coproc_sources { uint32_t m_id; - const struct log_source_const_data *log_const; + uint32_t data_cnt; + union { + struct { + const struct log_source_const_data *log_const; + uintptr_t *log_str_section; + } data; + struct { + uintptr_t data[2]; + } raw_data; + }; }; struct log_frontend_stmesp_demux { @@ -72,7 +89,7 @@ struct log_frontend_stmesp_demux { uint32_t dropped; - struct log_frontend_stmesp_coop_sources coop_sources[2]; + struct log_frontend_stmesp_coproc_sources coproc_sources[2]; }; struct log_frontend_stmesp_entry_source_pair { @@ -404,10 +421,33 @@ const char *log_frontend_stmesp_demux_sname_get(uint32_t m_id, uint16_t s_id) if (demux.m_ids[m_id] == APP_M_ID) { return log_source_name_get(0, s_id); - } else if (m_id == demux.coop_sources[0].m_id) { - return demux.coop_sources[0].log_const[s_id].name; - } else if (m_id == demux.coop_sources[1].m_id) { - return demux.coop_sources[1].log_const[s_id].name; + } else if (m_id == demux.coproc_sources[0].m_id) { + return demux.coproc_sources[0].data.log_const[s_id].name; + } else if (m_id == demux.coproc_sources[1].m_id) { + return demux.coproc_sources[1].data.log_const[s_id].name; + } + + return "unknown"; +} + +const char *log_frontend_stmesp_demux_str_get(uint32_t m_id, uint16_t s_id) +{ + if (!IS_ENABLED(CONFIG_LOG_FRONTEND_STMESP_TURBO_LOG)) { + return ""; + } + + uintptr_t *log_str_start = NULL; + + if (demux.m_ids[m_id] == APP_M_ID) { + log_str_start = (uintptr_t *)TYPE_SECTION_START(log_stmesp_ptr); + } else if (m_id == demux.coproc_sources[0].m_id) { + log_str_start = demux.coproc_sources[0].data.log_str_section; + } else if (m_id == demux.coproc_sources[1].m_id) { + log_str_start = demux.coproc_sources[1].data.log_str_section; + } + + if (log_str_start) { + return (const char *)log_str_start[s_id]; } return "unknown"; @@ -439,15 +479,16 @@ int log_frontend_stmesp_demux_packet_start(uint32_t *data, uint64_t *ts) if (IS_ENABLED(CONFIG_LOG_FRONTEND_STMESP_TURBO_LOG) && (ch == CONFIG_LOG_FRONTEND_STPESP_TURBO_SOURCE_PORT_ID)) { - if (demux.m_ids[m] == FLPR_M_ID) { - demux.coop_sources[0].m_id = m; - demux.coop_sources[0].log_const = - (const struct log_source_const_data *)(uintptr_t)*data; - } else if (demux.m_ids[m] == PPR_M_ID) { - demux.coop_sources[1].m_id = m; - demux.coop_sources[1].log_const = - (const struct log_source_const_data *)(uintptr_t)*data; + struct log_frontend_stmesp_coproc_sources *src = + &demux.coproc_sources[demux.m_ids[m] == FLPR_M_ID ? 0 : 1]; + + if (src->data_cnt >= 2) { + /* Unexpected packet. */ + return -EINVAL; } + + src->m_id = m; + src->raw_data.data[src->data_cnt++] = (uintptr_t)*data; return 0; }