[PATCH 0/4] SOF FW loader fixes and updates

[PATCH 0/4] SOF FW loader fixes and updates

[Ranjani Sridharan]

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(-)

-- 
2.25.1

[PATCH 1/4] ASoC: SOF: IPC: make sof_ipc_window monosized

[Ranjani Sridharan]

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;
 
-- 
2.25.1

[PATCH 2/4] ASoC: SOF: loader: fix memory leak in get_ext_windows

[Ranjani Sridharan]

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;
 
-- 
2.25.1

[PATCH 3/4] ASoC: SOF: ext_manifest: Parse debug ABI version

[Ranjani Sridharan]

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);
-- 
2.25.1

[PATCH 4/4] ASoC: SOF: loader: Add debug box region

[Ranjani Sridharan]

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 */
-- 
2.25.1

Re: [PATCH 0/4] SOF FW loader fixes and updates

[Mark Brown]

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