From e6940da09dabf49be1ea6eeeb3ae9dc35607a656 Mon Sep 17 00:00:00 2001 From: Marcin Maka Date: Mon, 17 Feb 2020 11:21:32 +0100 Subject: [PATCH] abi: define debug abi version for user space dbg interfaces The sof-logger and potentially other debug API clients perform ABI compatibility check using the single FW ABI version. The same one is used by the primary FW client which is the kernel driver. If there is a change made to the debug API, the main ABI has to be updated to protect integrity of the debug tools while such a change may not affect the kernel driver at all. This patch introduces new debug ABI version to be increased when changing user space debug interfaces while the the main ABI is not affected. Recompilation and installation of the new driver every time the tunneled debug protocol is upgraded may be avoided. Signed-off-by: Marcin Maka --- rimage/file_simple.c | 98 +++++++++++++++++++++-------- src/include/ipc/info.h | 8 +++ src/include/kernel/abi.h | 2 +- src/include/sof/fw-ready-metadata.h | 1 + src/include/user/abi_dbg.h | 29 +++++++++ src/ipc/CMakeLists.txt | 3 +- src/ipc/user_abi_version.c | 18 ++++++ src/platform/baytrail/platform.c | 4 ++ src/platform/haswell/platform.c | 4 ++ src/platform/imx8/platform.c | 4 ++ src/platform/imx8m/platform.c | 4 ++ src/platform/intel/cavs/platform.c | 4 ++ tools/logger/convert.c | 22 ++++--- 13 files changed, 162 insertions(+), 39 deletions(-) create mode 100644 src/include/user/abi_dbg.h create mode 100644 src/ipc/user_abi_version.c diff --git a/rimage/file_simple.c b/rimage/file_simple.c index 4391c4a72649..428c8b7d68a4 100644 --- a/rimage/file_simple.c +++ b/rimage/file_simple.c @@ -2,6 +2,7 @@ // // Copyright(c) 2015 Intel Corporation. All rights reserved. +#include #include #include #include @@ -72,6 +73,73 @@ static int get_mem_zone_type(struct image *image, Elf32_Shdr *section) return SOF_FW_BLK_TYPE_INVALID; } +static int fw_version_copy(struct snd_sof_logs_header *header, + const struct module *module) +{ + Elf32_Shdr *section = NULL; + struct sof_ipc_ext_data_hdr *ext_hdr = NULL; + void *buffer = NULL; + + if (module->fw_ready_index <= 0) + return 0; + + section = &module->section[module->fw_ready_index]; + + buffer = calloc(1, section->size); + if (!buffer) + return -ENOMEM; + + fseek(module->fd, section->off, SEEK_SET); + size_t count = fread(buffer, 1, + section->size, module->fd); + + if (count != section->size) { + fprintf(stderr, "error: can't read ready section %d\n", -errno); + free(buffer); + return -errno; + } + + memcpy(&header->version, + &((struct sof_ipc_fw_ready *)buffer)->version, + sizeof(header->version)); + + /* fw_ready structure contains main (primarily kernel) + * ABI version. + */ + + fprintf(stdout, "fw abi main version: %d:%d:%d\n", + SOF_ABI_VERSION_MAJOR(header->version.abi_version), + SOF_ABI_VERSION_MINOR(header->version.abi_version), + SOF_ABI_VERSION_PATCH(header->version.abi_version)); + + /* let's find dbg abi version, which the log client + * is interested in and override the kernel's one. + * + * skip the base fw-ready record and begin from the first extension. + */ + ext_hdr = buffer + ((struct sof_ipc_fw_ready *)buffer)->hdr.size; + while ((uintptr_t)ext_hdr < (uintptr_t)buffer + section->size) { + if (ext_hdr->type == SOF_IPC_EXT_USER_ABI_INFO) { + header->version.abi_version = + ((struct sof_ipc_user_abi_version *) + ext_hdr)->abi_dbg_version; + break; + } + //move to the next entry + ext_hdr = (struct sof_ipc_ext_data_hdr *) + ((uint8_t *)ext_hdr + ext_hdr->hdr.size); + } + + fprintf(stdout, "fw abi dbg version: %d:%d:%d\n", + SOF_ABI_VERSION_MAJOR(header->version.abi_version), + SOF_ABI_VERSION_MINOR(header->version.abi_version), + SOF_ABI_VERSION_PATCH(header->version.abi_version)); + + free(buffer); + + return 0; +} + static int block_idx; static int write_block(struct image *image, struct module *module, @@ -389,33 +457,9 @@ int write_logs_dictionary(struct image *image) /* extract fw_version from fw_ready message located * in .fw_ready section */ - if (module->fw_ready_index > 0) { - Elf32_Shdr *section = - &module->section[module->fw_ready_index]; - - buffer = calloc(1, sizeof(struct sof_ipc_fw_ready)); - if (!buffer) - return -ENOMEM; - - fseek(module->fd, section->off, SEEK_SET); - size_t count = fread(buffer, 1, - sizeof(struct sof_ipc_fw_ready), module->fd); - - if (count != sizeof(struct sof_ipc_fw_ready)) { - fprintf(stderr, - "error: can't read ready section %d\n", - -errno); - ret = -errno; - goto out; - } - - memcpy(&header.version, - &((struct sof_ipc_fw_ready *)buffer)->version, - sizeof(header.version)); - - free(buffer); - buffer = NULL; - } + ret = fw_version_copy(&header, module); + if (ret < 0) + goto out; if (module->logs_index > 0) { Elf32_Shdr *section = diff --git a/src/include/ipc/info.h b/src/include/ipc/info.h index 8cccd3da3695..7f88cf598c84 100644 --- a/src/include/ipc/info.h +++ b/src/include/ipc/info.h @@ -40,6 +40,7 @@ enum sof_ipc_ext_data { SOF_IPC_EXT_WINDOW, SOF_IPC_EXT_CC_INFO, SOF_IPC_EXT_PROBE_INFO, + SOF_IPC_EXT_USER_ABI_INFO, }; /* FW version - SOF_IPC_GLB_VERSION */ @@ -151,4 +152,11 @@ struct sof_ipc_probe_support { uint32_t reserved[2]; } __attribute__((packed)); +/* extended data: user abi version(s) */ +struct sof_ipc_user_abi_version { + struct sof_ipc_ext_data_hdr ext_hdr; + + uint32_t abi_dbg_version; +} __attribute__((packed)); + #endif /* __IPC_INFO_H__ */ diff --git a/src/include/kernel/abi.h b/src/include/kernel/abi.h index bf7697287a8c..a442b760ab3e 100644 --- a/src/include/kernel/abi.h +++ b/src/include/kernel/abi.h @@ -29,7 +29,7 @@ /** \brief SOF ABI version major, minor and patch numbers */ #define SOF_ABI_MAJOR 3 -#define SOF_ABI_MINOR 13 +#define SOF_ABI_MINOR 14 #define SOF_ABI_PATCH 0 /** \brief SOF ABI version number. Format within 32bit word is MMmmmppp */ diff --git a/src/include/sof/fw-ready-metadata.h b/src/include/sof/fw-ready-metadata.h index 1a1591f71dc3..87888c148b57 100644 --- a/src/include/sof/fw-ready-metadata.h +++ b/src/include/sof/fw-ready-metadata.h @@ -12,5 +12,6 @@ extern const struct sof_ipc_cc_version cc_version; extern const struct sof_ipc_probe_support probe_support; +extern const struct sof_ipc_user_abi_version user_abi_version; #endif /* __IPC_FW_READY_METADATA_H__ */ diff --git a/src/include/user/abi_dbg.h b/src/include/user/abi_dbg.h new file mode 100644 index 000000000000..8c44ad5f7c7b --- /dev/null +++ b/src/include/user/abi_dbg.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * + * Copyright(c) 2020 Intel Corporation. All rights reserved. + * + * Author: Marcin Maka + */ + +/** + * \file include/user/abi_dbg.h + * \brief ABI definitions for debug interfaces accessed by user space apps. + * \author Marcin Maka + * + * Follows the rules documented in include/user/abi.h. + */ + +#include + +#ifndef __USER_ABI_DBG_H__ +#define __USER_ABI_DBG_H__ + +#define SOF_ABI_DBG_MAJOR 3 +#define SOF_ABI_DBG_MINOR 14 +#define SOF_ABI_DBG_PATCH 0 + +#define SOF_ABI_DBG_VERSION SOF_ABI_VER(SOF_ABI_DBG_MAJOR, \ + SOF_ABI_DBG_MINOR, \ + SOF_ABI_DBG_PATCH) + +#endif /* __USER_ABI_DBG_H__ */ diff --git a/src/ipc/CMakeLists.txt b/src/ipc/CMakeLists.txt index d81aa40bf474..7423a2eac72b 100644 --- a/src/ipc/CMakeLists.txt +++ b/src/ipc/CMakeLists.txt @@ -37,7 +37,8 @@ set_property(TARGET data_structs APPEND add_local_sources(data_structs cc_version.c - probe_support.c) + probe_support.c + user_abi_version.c) target_link_libraries(data_structs sof_options) target_link_libraries(sof_static_libraries INTERFACE data_structs) diff --git a/src/ipc/user_abi_version.c b/src/ipc/user_abi_version.c new file mode 100644 index 000000000000..b63c1ea66c37 --- /dev/null +++ b/src/ipc/user_abi_version.c @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: BSD-3-Clause +// +// Copyright(c) 2020 Intel Corporation. All rights reserved. +// +// Author: Marcin Maka + +#include +#include + +const struct sof_ipc_user_abi_version user_abi_version + __attribute__((section(".fw_ready_metadata"))) = { + .ext_hdr = { + .hdr.cmd = SOF_IPC_FW_READY, + .hdr.size = sizeof(struct sof_ipc_user_abi_version), + .type = SOF_IPC_EXT_USER_ABI_INFO, + }, + .abi_dbg_version = SOF_ABI_DBG_VERSION, +}; diff --git a/src/platform/baytrail/platform.c b/src/platform/baytrail/platform.c index 8bcbbd4c825a..4ee8a6b24787 100644 --- a/src/platform/baytrail/platform.c +++ b/src/platform/baytrail/platform.c @@ -145,6 +145,10 @@ int platform_boot_complete(uint32_t boot_message) mailbox_dspbox_write(mb_offset, &probe_support, probe_support.ext_hdr.hdr.size); + mb_offset = mb_offset + probe_support.ext_hdr.hdr.size; + + mailbox_dspbox_write(mb_offset, &user_abi_version, + user_abi_version.ext_hdr.hdr.size); /* now interrupt host to tell it we are done booting */ shim_write(SHIM_IPCDL, SOF_IPC_FW_READY | outbox); diff --git a/src/platform/haswell/platform.c b/src/platform/haswell/platform.c index 91c6d3988f31..a8223f193781 100644 --- a/src/platform/haswell/platform.c +++ b/src/platform/haswell/platform.c @@ -141,6 +141,10 @@ int platform_boot_complete(uint32_t boot_message) mailbox_dspbox_write(mb_offset, &probe_support, probe_support.ext_hdr.hdr.size); + mb_offset = mb_offset + probe_support.ext_hdr.hdr.size; + + mailbox_dspbox_write(mb_offset, &user_abi_version, + user_abi_version.ext_hdr.hdr.size); /* now interrupt host to tell it we are done booting */ shim_write(SHIM_IPCD, outbox | SHIM_IPCD_BUSY); diff --git a/src/platform/imx8/platform.c b/src/platform/imx8/platform.c index 9bef7c599d29..c6111263cf85 100644 --- a/src/platform/imx8/platform.c +++ b/src/platform/imx8/platform.c @@ -139,6 +139,10 @@ int platform_boot_complete(uint32_t boot_message) mailbox_dspbox_write(mb_offset, &probe_support, probe_support.ext_hdr.hdr.size); + mb_offset = mb_offset + probe_support.ext_hdr.hdr.size; + + mailbox_dspbox_write(mb_offset, &user_abi_version, + user_abi_version.ext_hdr.hdr.size); /* now interrupt host to tell it we are done booting */ imx_mu_xcr_rmw(IMX_MU_xCR_GIRn(1), 0); diff --git a/src/platform/imx8m/platform.c b/src/platform/imx8m/platform.c index f28bd422d0f5..95d7886ea0e5 100644 --- a/src/platform/imx8m/platform.c +++ b/src/platform/imx8m/platform.c @@ -138,6 +138,10 @@ int platform_boot_complete(uint32_t boot_message) mailbox_dspbox_write(mb_offset, &probe_support, probe_support.ext_hdr.hdr.size); + mb_offset = mb_offset + probe_support.ext_hdr.hdr.size; + + mailbox_dspbox_write(mb_offset, &user_abi_version, + user_abi_version.ext_hdr.hdr.size); /* now interrupt host to tell it we are done booting */ imx_mu_xcr_rmw(IMX_MU_xCR_GIRn(1), 0); diff --git a/src/platform/intel/cavs/platform.c b/src/platform/intel/cavs/platform.c index 7298110e2af3..f838e15ad354 100644 --- a/src/platform/intel/cavs/platform.c +++ b/src/platform/intel/cavs/platform.c @@ -306,6 +306,10 @@ int platform_boot_complete(uint32_t boot_message) mailbox_dspbox_write(mb_offset, &probe_support, probe_support.ext_hdr.hdr.size); + mb_offset = mb_offset + probe_support.ext_hdr.hdr.size; + + mailbox_dspbox_write(mb_offset, &user_abi_version, + user_abi_version.ext_hdr.hdr.size); /* tell host we are ready */ #if CAVS_VERSION == CAVS_VERSION_1_5 diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 239b6c0eb528..efb465dc7b3c 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include "convert.h" @@ -445,15 +445,16 @@ int convert(const struct convert_config *config) { return -EINVAL; } - /* logger and version_file abi verification */ - if SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_VERSION, ver.abi_version) { + /* logger and version_file abi dbg verification */ + if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_DBG_VERSION, + ver.abi_version)) { fprintf(stderr, "Error: abi version in %s file " "does not coincide with abi version used " "by logger.\n", config->version_file); fprintf(stderr, "logger ABI Version is %d:%d:%d\n", - SOF_ABI_VERSION_MAJOR(SOF_ABI_VERSION), - SOF_ABI_VERSION_MINOR(SOF_ABI_VERSION), - SOF_ABI_VERSION_PATCH(SOF_ABI_VERSION)); + SOF_ABI_VERSION_MAJOR(SOF_ABI_DBG_VERSION), + SOF_ABI_VERSION_MINOR(SOF_ABI_DBG_VERSION), + SOF_ABI_VERSION_PATCH(SOF_ABI_DBG_VERSION)); fprintf(stderr, "version_file ABI Version is %d:%d:%d\n", SOF_ABI_VERSION_MAJOR(ver.abi_version), SOF_ABI_VERSION_MINOR(ver.abi_version), @@ -463,14 +464,15 @@ int convert(const struct convert_config *config) { } /* default logger and ldc_file abi verification */ - if SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_VERSION, snd.version.abi_version) { + if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_DBG_VERSION, + snd.version.abi_version)) { fprintf(stderr, "Error: abi version in %s file " "does not coincide with abi version used " "by logger.\n", config->ldc_file); fprintf(stderr, "logger ABI Version is %d:%d:%d\n", - SOF_ABI_VERSION_MAJOR(SOF_ABI_VERSION), - SOF_ABI_VERSION_MINOR(SOF_ABI_VERSION), - SOF_ABI_VERSION_PATCH(SOF_ABI_VERSION)); + SOF_ABI_VERSION_MAJOR(SOF_ABI_DBG_VERSION), + SOF_ABI_VERSION_MINOR(SOF_ABI_DBG_VERSION), + SOF_ABI_VERSION_PATCH(SOF_ABI_DBG_VERSION)); fprintf(stderr, "ldc_file ABI Version is %d:%d:%d\n", SOF_ABI_VERSION_MAJOR(snd.version.abi_version), SOF_ABI_VERSION_MINOR(snd.version.abi_version),