[PATCH 0/4] SOF FW loader fixes and updates
This series includes fixes and updates to the SOF firmware loader.
Iulian Olaru (1): ASoC: SOF: loader: Add debug box region
Karol Trzcinski (2): ASoC: SOF: IPC: make sof_ipc_window monosized ASoC: SOF: ext_manifest: Parse debug ABI version
Pierre-Louis Bossart (1): ASoC: SOF: loader: fix memory leak in get_ext_windows
include/sound/sof/ext_manifest.h | 7 +++++++ include/sound/sof/info.h | 2 +- include/uapi/sound/sof/abi.h | 2 +- sound/soc/sof/loader.c | 34 +++++++++++++++++++++++++++++--- sound/soc/sof/sof-priv.h | 1 + 5 files changed, 41 insertions(+), 5 deletions(-)
From: Karol Trzcinski karolx.trzcinski@linux.intel.com
This step is needed to add possibility to pack sof_ipc_window inside another one in used FW build tools - for example in extended manifest. Structure reusability leads to easy parsing function reuse, so source code is shorter and easier to maintain.
Using structures with constant size is less tricky and properly supported by each toolchain by contrast to variable size elements.
This is minor ABI change - backward compatibility is kept.
Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- include/sound/sof/info.h | 2 +- include/uapi/sound/sof/abi.h | 2 +- sound/soc/sof/loader.c | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/sound/sof/info.h b/include/sound/sof/info.h index 5a55ba8b7e56..313e3e70c630 100644 --- a/include/sound/sof/info.h +++ b/include/sound/sof/info.h @@ -99,7 +99,7 @@ struct sof_ipc_window_elem { struct sof_ipc_window { struct sof_ipc_ext_data_hdr ext_hdr; uint32_t num_windows; - struct sof_ipc_window_elem window[]; + struct sof_ipc_window_elem window[SOF_IPC_MAX_ELEMS]; } __packed;
struct sof_ipc_cc_version { diff --git a/include/uapi/sound/sof/abi.h b/include/uapi/sound/sof/abi.h index d54be303090f..6af32f82fb99 100644 --- a/include/uapi/sound/sof/abi.h +++ b/include/uapi/sound/sof/abi.h @@ -26,7 +26,7 @@
/* SOF ABI version major, minor and patch numbers */ #define SOF_ABI_MAJOR 3 -#define SOF_ABI_MINOR 16 +#define SOF_ABI_MINOR 17 #define SOF_ABI_PATCH 0
/* SOF ABI version number. Format within 32bit word is MMmmmppp */ diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index b94fa5f5d480..25dc28ebafb7 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -20,13 +20,12 @@ static int get_ext_windows(struct snd_sof_dev *sdev, { const struct sof_ipc_window *w = container_of(ext_hdr, struct sof_ipc_window, ext_hdr); - size_t w_size = struct_size(w, window, w->num_windows);
if (w->num_windows == 0 || w->num_windows > SOF_IPC_MAX_ELEMS) return -EINVAL;
if (sdev->info_window) { - if (memcmp(sdev->info_window, w, w_size)) { + if (memcmp(sdev->info_window, w, ext_hdr->hdr.size)) { dev_err(sdev->dev, "error: mismatch between window descriptor from extended manifest and mailbox"); return -EINVAL; } @@ -34,7 +33,7 @@ static int get_ext_windows(struct snd_sof_dev *sdev, }
/* keep a local copy of the data */ - sdev->info_window = kmemdup(w, w_size, GFP_KERNEL); + sdev->info_window = kmemdup(w, ext_hdr->hdr.size, GFP_KERNEL); if (!sdev->info_window) return -ENOMEM;
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sdev->info_window is allocated with kmemdup and never freed, use devm_ version since this is only used for first boot.
Fixes: 8d809c15acf23 ('ASoC: SOF: ext_manifest: parse windows') Cc: Karol Trzcinski karolx.trzcinski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Daniel Baluta daniel.baluta@gmail.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Rander Wang rander.wang@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/loader.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index 25dc28ebafb7..42e0e3e58869 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -33,7 +33,8 @@ static int get_ext_windows(struct snd_sof_dev *sdev, }
/* keep a local copy of the data */ - sdev->info_window = kmemdup(w, ext_hdr->hdr.size, GFP_KERNEL); + sdev->info_window = devm_kmemdup(sdev->dev, w, ext_hdr->hdr.size, + GFP_KERNEL); if (!sdev->info_window) return -ENOMEM;
From: Karol Trzcinski karolx.trzcinski@linux.intel.com
The debug ABI can be extracted from the extended manifest content. This information known at build time does not need to be provided in a mailbox.
Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- include/sound/sof/ext_manifest.h | 7 +++++++ sound/soc/sof/loader.c | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+)
diff --git a/include/sound/sof/ext_manifest.h b/include/sound/sof/ext_manifest.h index 04359cda92dc..342e86e54db5 100644 --- a/include/sound/sof/ext_manifest.h +++ b/include/sound/sof/ext_manifest.h @@ -60,6 +60,7 @@ enum sof_ext_man_elem_type { SOF_EXT_MAN_ELEM_FW_VERSION = 0, SOF_EXT_MAN_ELEM_WINDOW = SOF_IPC_EXT_WINDOW, SOF_EXT_MAN_ELEM_CC_VERSION = SOF_IPC_EXT_CC_INFO, + SOF_EXT_MAN_ELEM_DBG_ABI = SOF_IPC_EXT_USER_ABI_INFO, };
/* extended manifest element header */ @@ -92,4 +93,10 @@ struct sof_ext_man_cc_version { struct sof_ipc_cc_version cc_version; } __packed;
+struct ext_man_dbg_abi { + struct sof_ext_man_elem_header hdr; + /* use sof_ipc struct because of code re-use */ + struct sof_ipc_user_abi_version dbg_abi; +} __packed; + #endif /* __SOF_FIRMWARE_EXT_MANIFEST_H__ */ diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index 42e0e3e58869..b8e72084dfeb 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -176,6 +176,22 @@ static int ext_man_get_cc_info(struct snd_sof_dev *sdev, return get_cc_info(sdev, &cc->cc_version.ext_hdr); }
+static int ext_man_get_dbg_abi_info(struct snd_sof_dev *sdev, + const struct sof_ext_man_elem_header *hdr) +{ + const struct ext_man_dbg_abi *dbg_abi = + container_of(hdr, struct ext_man_dbg_abi, hdr); + + if (sdev->first_boot) + dev_dbg(sdev->dev, + "Firmware: DBG_ABI %d:%d:%d\n", + SOF_ABI_VERSION_MAJOR(dbg_abi->dbg_abi.abi_dbg_version), + SOF_ABI_VERSION_MINOR(dbg_abi->dbg_abi.abi_dbg_version), + SOF_ABI_VERSION_PATCH(dbg_abi->dbg_abi.abi_dbg_version)); + + return 0; +} + static ssize_t snd_sof_ext_man_size(const struct firmware *fw) { const struct sof_ext_man_header *head; @@ -255,6 +271,9 @@ static int snd_sof_fw_ext_man_parse(struct snd_sof_dev *sdev, case SOF_EXT_MAN_ELEM_CC_VERSION: ret = ext_man_get_cc_info(sdev, elem_hdr); break; + case SOF_EXT_MAN_ELEM_DBG_ABI: + ret = ext_man_get_dbg_abi_info(sdev, elem_hdr); + break; default: dev_warn(sdev->dev, "warning: unknown sof_ext_man header type %d size 0x%X\n", elem_hdr->type, elem_hdr->size);
From: Iulian Olaru iulianolaru249@yahoo.com
This patch adds an IPC initiated debug box region in the snd_sof_dev structure, defined in soc/sof/sof-priv.h. It is initialized at loading, in the sof_get_windows function from soc/sof/loader.c, in a similar manner with the stream box and host box.
This region is useful because the firmware will put an error message here so the kernel can read it in case of a dsp oops.
Signed-off-by: Iulian Olaru iulianolaru249@yahoo.com Reviewed-by: Daniel Baluta daniel.baluta@gmail.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/loader.c | 9 +++++++++ sound/soc/sof/sof-priv.h | 1 + 2 files changed, 10 insertions(+)
diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index b8e72084dfeb..68ed454f7ddf 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -310,6 +310,8 @@ static void sof_get_windows(struct snd_sof_dev *sdev) u32 outbox_size = 0; u32 stream_size = 0; u32 inbox_size = 0; + u32 debug_size = 0; + u32 debug_offset = 0; int window_offset; int bar; int i; @@ -363,6 +365,8 @@ static void sof_get_windows(struct snd_sof_dev *sdev) SOF_DEBUGFS_ACCESS_D0_ONLY); break; case SOF_IPC_REGION_DEBUG: + debug_offset = window_offset + elem->offset; + debug_size = elem->size; snd_sof_debugfs_io_item(sdev, sdev->bar[bar] + window_offset + @@ -412,12 +416,17 @@ static void sof_get_windows(struct snd_sof_dev *sdev) sdev->stream_box.offset = stream_offset; sdev->stream_box.size = stream_size;
+ sdev->debug_box.offset = debug_offset; + sdev->debug_box.size = debug_size; + dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", inbox_offset, inbox_size); dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", outbox_offset, outbox_size); dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", stream_offset, stream_size); + dev_dbg(sdev->dev, " debug region 0x%x - size 0x%x\n", + debug_offset, debug_size); }
/* check for ABI compatibility and create memory windows on first boot */ diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index e2070072791c..53d26be88f64 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -383,6 +383,7 @@ struct snd_sof_dev { struct snd_sof_mailbox dsp_box; /* DSP initiated IPC */ struct snd_sof_mailbox host_box; /* Host initiated IPC */ struct snd_sof_mailbox stream_box; /* Stream position update */ + struct snd_sof_mailbox debug_box; /* Debug info updates */ struct snd_sof_ipc_msg *msg; int ipc_irq; u32 next_comp_id; /* monotonic - reset during S3 */
On Tue, 25 Aug 2020 16:58:50 -0700, Ranjani Sridharan wrote:
This series includes fixes and updates to the SOF firmware loader.
Iulian Olaru (1): ASoC: SOF: loader: Add debug box region
Karol Trzcinski (2): ASoC: SOF: IPC: make sof_ipc_window monosized ASoC: SOF: ext_manifest: Parse debug ABI version
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: SOF: IPC: make sof_ipc_window monosized commit: 76ab546cd8f0c64d4603b2faad4558c5b670561e [2/4] ASoC: SOF: loader: fix memory leak in get_ext_windows commit: e9157a449aa3f72fdbded9ce780c666bf0951cf3 [3/4] ASoC: SOF: ext_manifest: Parse debug ABI version commit: 60b7c1ba289b8ebe4f275b0b381f711e5b60184b [4/4] ASoC: SOF: loader: Add debug box region commit: e17b7389dcc4239b806cd8789a812ee1b8b7b134
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Ranjani Sridharan