From 2c6c3fe779bb8db59ae3bcc79fb4a370d13a4b19 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 24 Nov 2021 11:45:15 +0200 Subject: [PATCH] ASoC: SOF: rework FW ABI version handling on kernel side The definitions in sof/abi.h have been a verbatim copy of the firmware header definition. The header also defines a version and this has caused issues: - kernel declares one version but as the interface supports backwards-compatible minor version updates, many firmware versions can be supported by any one kernel - a single firmware MINOR version bump can cover multiple changes to the interface and this can cause complex dependencies to kernel patch backporting as kernel will have multiple commits to support extensions identified by a single MAJOR.MINOR version of firmware Simplify the driver side definitions by removing the version information from kernel side abi.h header. Also remove strict version checks as these only make sense if kernel is updated in an atomic way for each firmware release (which is not the case especially in backported trees). Only leave check on the major version number in place. In control IPC implementation, kernel needs to send the firmware version it implements. This is not widely used, but to keep the few existing firmware components happy, declare a local SOF_CONTROL_ABI_VERSION that matches the control implementation and send that in the IPC message. If the firmware and its topology are newer than kernel, error may occur in topology parsing. E.g. if a new DAI type is added, kernel will return an error and is unable to pass this information to the firmware. A warning was emitted in this case prior to this patch, but that is no longer possible. This is considered an acceptable compromise to simplify the version management. BugLink: https://github.com/thesofproject/sof/issues/4986 Signed-off-by: Kai Vehmanen --- include/uapi/sound/sof/abi.h | 22 ---------------------- sound/soc/sof/Kconfig | 15 --------------- sound/soc/sof/control.c | 10 +++++++--- sound/soc/sof/ipc.c | 14 ++------------ sound/soc/sof/sof-priv.h | 3 +++ sound/soc/sof/topology.c | 16 ++-------------- 6 files changed, 14 insertions(+), 66 deletions(-) diff --git a/include/uapi/sound/sof/abi.h b/include/uapi/sound/sof/abi.h index fe2cfae94b45fd..85283a3ca093a7 100644 --- a/include/uapi/sound/sof/abi.h +++ b/include/uapi/sound/sof/abi.h @@ -6,29 +6,9 @@ * Copyright(c) 2018 Intel Corporation. All rights reserved. */ -/** - * SOF ABI versioning is based on Semantic Versioning where we have a given - * MAJOR.MINOR.PATCH version number. See https://semver.org/ - * - * Rules for incrementing or changing version :- - * - * 1) Increment MAJOR version if you make incompatible API changes. MINOR and - * PATCH should be reset to 0. - * - * 2) Increment MINOR version if you add backwards compatible features or - * changes. PATCH should be reset to 0. - * - * 3) Increment PATCH version if you add backwards compatible bug fixes. - */ - #ifndef __INCLUDE_UAPI_SOUND_SOF_ABI_H__ #define __INCLUDE_UAPI_SOUND_SOF_ABI_H__ -/* SOF ABI version major, minor and patch numbers */ -#define SOF_ABI_MAJOR 3 -#define SOF_ABI_MINOR 18 -#define SOF_ABI_PATCH 0 - /* SOF ABI version number. Format within 32bit word is MMmmmppp */ #define SOF_ABI_MAJOR_SHIFT 24 #define SOF_ABI_MAJOR_MASK 0xff @@ -54,8 +34,6 @@ SOF_ABI_VERSION_MAJOR((client_ver)) \ ) -#define SOF_ABI_VERSION SOF_ABI_VER(SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH) - /* SOF ABI magic number "SOF\0". */ #define SOF_ABI_MAGIC 0x00464F53 diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index a8b6f15f493568..e768a768dc1bc7 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -111,21 +111,6 @@ config SND_SOC_SOF_NOCODEC_SUPPORT Say Y if you need this nocodec fallback option. If unsure select "N". -config SND_SOC_SOF_STRICT_ABI_CHECKS - bool "SOF strict ABI checks" - help - This option enables strict ABI checks for firmware and topology - files. - When these files are more recent than the kernel, the kernel - will handle the functionality it supports and may report errors - during topology creation or run-time usage if new functionality - is invoked. - This option will stop topology creation and firmware load upfront. - It is intended for SOF CI/releases and not for users or distros. - Say Y if you want strict ABI checks for an SOF release. - If you are not involved in SOF releases and CI development, - select "N". - config SND_SOC_SOF_DEBUG bool "SOF debugging features" help diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c index ef61936dad5945..7e50befea36352 100644 --- a/sound/soc/sof/control.c +++ b/sound/soc/sof/control.c @@ -15,6 +15,9 @@ #include "sof-priv.h" #include "sof-audio.h" +/* Interface version implemented for controls IPC */ +#define SOF_CONTROL_ABI_VERSION SOF_ABI_VER(3, 18, 0) + static void update_mute_led(struct snd_sof_control *scontrol, struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -79,7 +82,7 @@ static void snd_sof_refresh_control(struct snd_sof_control *scontrol) /* set the ABI header values */ cdata->data->magic = SOF_ABI_MAGIC; - cdata->data->abi = SOF_ABI_VERSION; + cdata->data->abi = SOF_CONTROL_ABI_VERSION; /* refresh the component data from DSP */ scontrol->comp_data_dirty = false; @@ -433,7 +436,8 @@ int snd_sof_bytes_ext_volatile_get(struct snd_kcontrol *kcontrol, unsigned int _ /* set the ABI header values */ cdata->data->magic = SOF_ABI_MAGIC; - cdata->data->abi = SOF_ABI_VERSION; + cdata->data->abi = SOF_CONTROL_ABI_VERSION; + /* get all the component data from DSP */ ret = snd_sof_ipc_set_get_comp_data(scontrol, false); if (ret < 0) @@ -500,7 +504,7 @@ int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol, /* set the ABI header values */ cdata->data->magic = SOF_ABI_MAGIC; - cdata->data->abi = SOF_ABI_VERSION; + cdata->data->abi = SOF_CONTROL_ABI_VERSION; /* check data size doesn't exceed max coming from topology */ if (cdata->data->size > be->max - sizeof(struct sof_abi_hdr)) { diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index a9836e47567dc9..94d6e10b807a11 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -947,26 +947,16 @@ int snd_sof_ipc_valid(struct snd_sof_dev *sdev) "Firmware info: version %d:%d:%d-%s\n", v->major, v->minor, v->micro, v->tag); dev_info(sdev->dev, - "Firmware: ABI %d:%d:%d Kernel ABI %d:%d:%d\n", + "Firmware: ABI %d:%d:%d\n", SOF_ABI_VERSION_MAJOR(v->abi_version), SOF_ABI_VERSION_MINOR(v->abi_version), - SOF_ABI_VERSION_PATCH(v->abi_version), - SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH); + SOF_ABI_VERSION_PATCH(v->abi_version)); if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_VERSION, v->abi_version)) { dev_err(sdev->dev, "error: incompatible FW ABI version\n"); return -EINVAL; } - if (SOF_ABI_VERSION_MINOR(v->abi_version) > SOF_ABI_MINOR) { - if (!IS_ENABLED(CONFIG_SND_SOC_SOF_STRICT_ABI_CHECKS)) { - dev_warn(sdev->dev, "warn: FW ABI is more recent than kernel\n"); - } else { - dev_err(sdev->dev, "error: FW ABI is more recent than kernel\n"); - return -EINVAL; - } - } - if (ready->flags & SOF_IPC_INFO_BUILD) { dev_info(sdev->dev, "Firmware debug build %d on %s-%s - options:\n" diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 3222454385891a..c73ea8417ba42d 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -73,6 +73,9 @@ bool sof_debug_check_flag(int mask); /* max number of DSP cores */ #define SOF_MAX_DSP_NUM_CORES 8 +/* IPC interface version implemented in driver */ +#define SOF_ABI_VERSION SOF_ABI_VER(3, 0, 0) + struct sof_dsp_power_state { u32 state; u32 substate; /* platform-specific */ diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index e72dcae5e7ee7a..3e3e48b78bc269 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -3580,11 +3580,8 @@ static int sof_manifest(struct snd_soc_component *scomp, int index, return -EINVAL; } - dev_info(scomp->dev, - "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n", - man->priv.data[0], man->priv.data[1], - man->priv.data[2], SOF_ABI_MAJOR, SOF_ABI_MINOR, - SOF_ABI_PATCH); + dev_info(scomp->dev, "Topology: ABI %d:%d:%d\n", + man->priv.data[0], man->priv.data[1], man->priv.data[2]); abi_version = SOF_ABI_VER(man->priv.data[0], man->priv.data[1], @@ -3595,15 +3592,6 @@ static int sof_manifest(struct snd_soc_component *scomp, int index, return -EINVAL; } - if (SOF_ABI_VERSION_MINOR(abi_version) > SOF_ABI_MINOR) { - if (!IS_ENABLED(CONFIG_SND_SOC_SOF_STRICT_ABI_CHECKS)) { - dev_warn(scomp->dev, "warn: topology ABI is more recent than kernel\n"); - } else { - dev_err(scomp->dev, "error: topology ABI is more recent than kernel\n"); - return -EINVAL; - } - } - return 0; }