diff --git a/CMakeLists.txt b/CMakeLists.txt index 3304d94fdf8..85fc82f5b95 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1698,7 +1698,11 @@ list(APPEND if(CONFIG_LOG_DICTIONARY_SUPPORT) set(log_dict_db_output --json=${PROJECT_BINARY_DIR}/log_dictionary.json) +elseif(CONFIG_LOG_MIPI_SYST_USE_CATALOG) + set(log_dict_db_output --syst=${PROJECT_BINARY_DIR}/mipi_syst_collateral.xml) +endif() +if(log_dict_db_output) list(APPEND post_build_commands COMMAND diff --git a/subsys/logging/Kconfig.formatting b/subsys/logging/Kconfig.formatting index 69265beb1f0..5464c6ca6bb 100644 --- a/subsys/logging/Kconfig.formatting +++ b/subsys/logging/Kconfig.formatting @@ -27,6 +27,23 @@ config LOG_MIPI_SYST_ENABLE help Enable MIPI SyS-T format output for the logger system. +config LOG_MIPI_SYST_USE_CATALOG + bool "Use MIPI Sys-T Catalog for logging" + depends on LOG2 && LOG_MIPI_SYST_ENABLE + select LOG2_FMT_SECTION + select LOG2_MSG_PKG_ALWAYS_ADD_RO_STRING_IDXS + help + Use MIPI Sys-T Catalog for logging instead of plain text. + +config LOG_MIPI_SYST_CATALOG_ARGS_BUFFER_SIZE + int "Size of temporary arguments buffer when using Sys-T Catalog" + depends on LOG_MIPI_SYST_USE_CATALOG + default 1024 + help + The size (in bytes) of the temporary buffer to store the expanded + argument list needed for the MIPI Sys-T library for processing + catalog messages. + config LOG_DICTIONARY_SUPPORT bool depends on LOG2 diff --git a/subsys/logging/log_output_syst.c b/subsys/logging/log_output_syst.c index 37cd7048906..ae1f3e84807 100644 --- a/subsys/logging/log_output_syst.c +++ b/subsys/logging/log_output_syst.c @@ -9,7 +9,10 @@ #include #include #include +#include +#include #include +#include #include #include #include @@ -770,7 +773,8 @@ static void hexdump2_print(const uint8_t *data, uint32_t length, } } -static int mipi_formatter(cbprintf_cb out, void *ctx, +#ifndef CONFIG_LOG_MIPI_SYST_USE_CATALOG +static int mipi_vprintf_formatter(cbprintf_cb out, void *ctx, const char *fmt, va_list ap) { struct log_msg2 *msg = ctx; @@ -780,6 +784,394 @@ static int mipi_formatter(cbprintf_cb out, void *ctx, return 0; } +#else + +/* + * TODO: Big endian support. + * + * MIPI Sys-T catalog messages require arguments to be in little endian. + * Currently, if the format strings are removed (which is very highly + * probable with usage of catalog messages), there is no way to + * distinguish arguments in va_list anymore, and thus no longer able + * to convert endianness. So assert that we only support little endian + * machines for now. + */ +BUILD_ASSERT(__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__, + "Does not support big endian machines at the moment!"); + +/* + * TODO: Support x86-32 and long double. + * + * The argument of long double on x86 32-bit is 3 bytes. However, + * MIPI Sys-T requires 4 bytes for this. Currently, since we are + * copying the argument list as-is, and no way to know which + * argument is long double, a long double argument cannot be + * expanded to 4 bytes. So error out here to prevent it being + * used at all. + */ +#if defined(CONFIG_X86) && !defined(CONFIG_X86_64) && defined(CONFIG_CBPRINTF_PACKAGE_LONGDOUBLE) +#error "x86-32 and CONFIG_CBPRINTF_PACKAGE_LONGDOUBLE is not supported!" +#endif + +/* + * TODO: Support integer arguments under 64-bit systems. + * + * On 64-bit systems, an integer argument is of 8 bytes even though + * sizeof(int) == 4. MIPI Sys-T expects an integer argument to be 4 bytes. + * Currently, since we are copying argument list as-is, and no way to + * know which argument is integer, a 32-bit integer cannot be + * shrunk to 4 bytes for output. So error out here to prevent it + * being used at all. + */ +#ifdef CONFIG_64BIT +#error "64-bit is not support at the moment!" +#endif + +#ifdef CONFIG_64BIT +#define MIPI_SYST_CATMSG_ARGS_COPY MIPI_SYST_CATALOG64_ARGS_COPY +#else +#define MIPI_SYST_CATMSG_ARGS_COPY MIPI_SYST_CATALOG32_ARGS_COPY +#endif + +static inline bool is_in_log_strings_section(const void *addr) +{ + extern const char __log_strings_start[]; + extern const char __log_strings_end[]; + + if (((const char *)addr >= (const char *)__log_strings_start) && + ((const char *)addr < (const char *)__log_strings_end)) { + return true; + } + + return false; +} + +static struct k_spinlock payload_lock; +static uint8_t payload_buf[CONFIG_LOG_MIPI_SYST_CATALOG_ARGS_BUFFER_SIZE]; +static const uint8_t * const payload_buf_end = + &payload_buf[CONFIG_LOG_MIPI_SYST_CATALOG_ARGS_BUFFER_SIZE]; + +enum string_list { + NO_STRING_LIST, + RO_STR_IDX_LIST, + RW_STR_IDX_LIST, + APPENDED_STR_LIST, +}; + +/* + * @brief Figure out where is the next string argument. + * + * @param[out] pos Offset in byte of string argument from beginning of package. + * + * @param[in] ros_remaining How many read-only strings left + * @param[in] ros_str_pos Pointer to the read-only string indexes + * + * @param[in] rws_remaining How many read-write strings left + * @param[in] rws_str_pos Pointer to the read-write string indexes + * + * @param[in] s_remaining How many appended strings left + * @param[in] str_pos Pointer to the appended string list + * + * @retval NO_STRING_LIST No string picked. Usually means there is + * no more strings remaining in lists. + * @retval RO_STR_IDX_LIST Position coming from read-only string list. + * @retval RW_STR_IDX_LIST Position coming from read-write string list. + * @retval APPENDED_STR_LIST Position coming from appended string list. + */ +static inline +enum string_list get_next_string_arg(uint16_t *pos, + uint8_t ros_remaining, uint8_t *ro_str_pos, + uint8_t rws_remaining, uint8_t *rw_str_pos, + uint8_t s_remaining, uint8_t *str_pos) +{ + enum string_list which_list = NO_STRING_LIST; + + if (ros_remaining > 0) { + *pos = ro_str_pos[0]; + which_list = RO_STR_IDX_LIST; + } + + if (rws_remaining > 0) { + if ((which_list == NO_STRING_LIST) || (rw_str_pos[0] < *pos)) { + *pos = rw_str_pos[0]; + which_list = RW_STR_IDX_LIST; + } + } + + if (s_remaining > 0) { + /* + * The first uint8_t in the appended string list for + * each string is its supposed position in + * the argument list. + */ + if ((which_list == NO_STRING_LIST) || (str_pos[0] < *pos)) { + *pos = str_pos[0]; + which_list = APPENDED_STR_LIST; + } + } + + if (which_list != NO_STRING_LIST) { + /* + * Note that the stored position value is in + * multiple of uint32_t. So need to convert it + * back to number of bytes. + */ + *pos *= sizeof(uint32_t); + } + + return which_list; +} + +/** + * @brief Build the catalog message payload and send it out. + * + * This builds the catalog message payload to be sent through MIPI Sys-T + * interface. + * + * For format strings without any string arguments, the argument list + * is provided to the MIPI library as-is without any processing. + * Otherwise, the strings replace the arguments in the argument list + * by replacing the string arguments with the full strings. + * + * @param[in] severity Severity of log message. + * @param[in] fmt Format string. + * @param[in] pkg Pointer to log message package. + * @param[in] pkg_sz Log message package size in bytes. + * @param[in] arg Pointer to argument list inside log message package. + * @param[in] arg_sz Argument list size in bytes inside log message package. + * @param[in] s_nbr Number of appended strings in log message package. + * @param[in] ros_nbr Number of read-only string indexes in log message package. + * @param[in] rws_nbr Number of read-write string indexes in log message package. + * + * @retval 0 Success + * @retval -ENOSPC Payload buffer size is too small. + */ +static int build_catalog_payload(uint32_t severity, const char *fmt, + uint8_t *pkg, size_t pkg_sz, + uint8_t *arg, size_t arg_sz, + uint8_t s_nbr, uint8_t ros_nbr, uint8_t rws_nbr) +{ + uint8_t *cur_str_arg_pos = NULL; + uint8_t *payload = payload_buf; + enum string_list which_str_list = NO_STRING_LIST; + int ret = 0; + + /* End of argument list. */ + uint8_t * const arg_end = arg + arg_sz; + + /* + * Start of read-only strings indexes, which is + * after the argument list. Skip the first ro-string + * index as it points to the format string. + */ + uint8_t *ro_str_pos = arg_end + 1; + + /* + * Start of read-write strings indexes, which is + * after the argument list, and read-only string + * indexes. + */ + uint8_t *rw_str_pos = arg_end + ros_nbr; + + /* Start of appended strings in package */ + uint8_t *str_pos = arg_end + ros_nbr + rws_nbr; + + /* Number of strings remaining to be processed */ + uint8_t ros_remaining = ros_nbr - 1; + uint8_t rws_remaining = rws_nbr; + uint8_t s_remaining = s_nbr; + + k_spinlock_key_t key = k_spin_lock(&payload_lock); + + do { + if (payload == payload_buf_end) { + /* + * No space left in payload buffer but there are + * still arguments left. So bail out. + */ + ret = -ENOSPC; + goto out; + } + + if (cur_str_arg_pos == NULL) { + uint16_t str_arg_pos; + + which_str_list = get_next_string_arg(&str_arg_pos, + ros_remaining, ro_str_pos, + rws_remaining, rw_str_pos, + s_remaining, str_pos); + + if (which_str_list != NO_STRING_LIST) { + cur_str_arg_pos = pkg + str_arg_pos; + } else { + /* + * No more strings remaining from the lists. + * Set the pointer to the end of argument list, + * which it will never match the incrementally + * moving arg, and also string selection will + * no longer be carried out. + */ + cur_str_arg_pos = arg + arg_sz; + } + } + + if (arg == cur_str_arg_pos) { + /* + * The current argument is a string pointer. + * So need to copy the string into the payload. + */ + + size_t str_sz = 0; + + /* Extract the string pointer from package */ + uint8_t *s = *((uint8_t **)arg); + + /* Skip the string pointer for next argument */ + arg += sizeof(void *); + + /* Copy the string over. */ + while (s[0] != '\0') { + payload[0] = s[0]; + payload++; + str_sz++; + s++; + + if (payload == payload_buf_end) { + /* + * No space left in payload buffer but there are + * still characters to be copied. So bail out. + */ + ret = -ENOSPC; + goto out; + } + } + + /* Need to terminate the string */ + payload[0] = '\0'; + payload++; + + if (which_str_list == RO_STR_IDX_LIST) { + /* Move to next read-only string index */ + ro_str_pos++; + ros_remaining--; + } else if (which_str_list == RW_STR_IDX_LIST) { + /* Move to next read-write string index */ + rw_str_pos++; + rws_remaining--; + } else if (which_str_list == APPENDED_STR_LIST) { + /* + * str_pos needs to skip the string index, + * the string itself, and the NULL character. + * So next time it points to another position + * index. + */ + str_pos += str_sz + 2; + s_remaining--; + } + + cur_str_arg_pos = NULL; + } else { + /* + * Copy until the next string argument + * (or until the end of argument list). + */ + while (arg < cur_str_arg_pos) { + payload[0] = arg[0]; + payload++; + arg++; + + if (payload == payload_buf_end) { + /* + * No space left in payload buffer but there are + * still characters to be copied. So bail out. + */ + ret = -ENOSPC; + goto out; + } + } + } + } while (arg < arg_end); + + MIPI_SYST_CATMSG_ARGS_COPY(&log_syst_handle, severity, + (uintptr_t)fmt, + payload_buf, + (size_t)(payload - payload_buf)); +out: + k_spin_unlock(&payload_lock, key); + return ret; +} + +static int mipi_catalog_formatter(cbprintf_cb out, void *ctx, + const char *fmt, va_list ap) +{ + int ret = 0; + struct log_msg2 *msg = ctx; + uint32_t severity = level_to_syst_severity(log_msg2_get_level(msg)); + + if (is_in_log_strings_section(fmt)) { + /* + * Note that only format strings that are in + * the log_strings_section are processed as + * catalog messages because only these strings + * are in the collateral XML file. + */ + + size_t pkg_sz, arg_sz; + uint8_t s_nbr, ros_nbr, rws_nbr, total_str; + uint8_t *arg; + + uint8_t *pkg = log_msg2_get_package(msg, &pkg_sz); + + /* + * Need to skip the package header (of pointer size), + * and the pointer to format string to get to + * the argument list. + */ + arg = pkg + 2 * sizeof(void *); + arg_sz = pkg[0] * sizeof(uint32_t) - 2 * sizeof(void *); + + /* Number of appended strings already in the package. */ + s_nbr = pkg[1]; + + /* Number of string indexes, for both read-only and read-write strings. */ + ros_nbr = pkg[2]; + rws_nbr = pkg[3]; + + total_str = s_nbr + rws_nbr; + if (ros_nbr > 0) { + /* + * If there are read-only string indexes, the first + * is the format string. So we can ignore that as + * we are not copying it to the catalog message + * payload. + */ + total_str += ros_nbr - 1; + } + + if (total_str == 0) { + /* + * There are no string arguments so the argument list + * inside the package can be used as-is. The first + * read-only string pointer is the the format string so + * we can ignore that. + */ + MIPI_SYST_CATMSG_ARGS_COPY(&log_syst_handle, severity, + (uintptr_t)fmt, + arg, + arg_sz); + } else { + ret = build_catalog_payload(severity, fmt, pkg, pkg_sz, + arg, arg_sz, + s_nbr, ros_nbr, rws_nbr); + } + + } else { + MIPI_SYST_VPRINTF(&log_syst_handle, severity, fmt, ap); + } + + return ret; +} +#endif /* !CONFIG_LOG_MIPI_SYST_USE_CATALOG */ void log_output_msg2_syst_process(const struct log_output *output, struct log_msg2 *msg, uint32_t flag) @@ -791,7 +1183,13 @@ void log_output_msg2_syst_process(const struct log_output *output, uint8_t *data = log_msg2_get_package(msg, &len); if (len) { - (void)cbpprintf_external(NULL, mipi_formatter, msg, data); + (void)cbpprintf_external(NULL, +#ifdef CONFIG_LOG_MIPI_SYST_USE_CATALOG + mipi_catalog_formatter, +#else + mipi_vprintf_formatter, +#endif + msg, data); } data = log_msg2_get_data(msg, &hexdump_len);