| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z-GEEVMXUysX1Hhu@p14s>
Date: Mon, 24 Mar 2025 10:10:57 -0600
From: Mathieu Poirier <mathieu.poirier@...aro.org>
To: "Iuliana Prodan (OSS)" <iuliana.prodan@....nxp.com>
Cc: Bjorn Andersson <andersson@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
"S.J. Wang" <shengjiu.wang@....com>,
Fabio Estevam <festevam@...il.com>,
Daniel Baluta <daniel.baluta@....com>,
Mpuaudiosw <Mpuaudiosw@....com>,
Iuliana Prodan <iuliana.prodan@....com>, imx@...ts.linux.dev,
linux-remoteproc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Pengutronix Kernel Team <kernel@...gutronix.de>
Subject: Re: [PATCH v2] remoteproc: imx_dsp_rproc: Add support for
DSP-specific features
Good day Iuliana,
On Tue, Mar 18, 2025 at 11:50:07PM +0200, Iuliana Prodan (OSS) wrote:
> From: Iuliana Prodan <iuliana.prodan@....com>
>
> Some DSP firmware requires a FW_READY signal before proceeding, while
> others do not.
> Therefore, add support to handle i.MX DSP-specific features.
>
> Implement handle_rsc callback to handle resource table parsing and to
> process DSP-specific resource, to determine if waiting is needed.
>
> Update imx_dsp_rproc_start() to handle this condition accordingly.
>
> Signed-off-by: Iuliana Prodan <iuliana.prodan@....com>
> ---
> Changes in v2:
> - Reviews from Mathieu Poirier:
> - Use vendor-specific resource table entry.
> - Implement resource handler specific to the i.MX DSP.
> - Revise commit message to include recent updates.
> - Link to v1: https://lore.kernel.org/all/20250305123923.514386-1-iuliana.prodan@oss.nxp.com/
>
> drivers/remoteproc/imx_dsp_rproc.c | 59 +++++++++++++++++++++++++++++-
> 1 file changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
> index b9bb15970966..1729bfbc602e 100644
> --- a/drivers/remoteproc/imx_dsp_rproc.c
> +++ b/drivers/remoteproc/imx_dsp_rproc.c
> @@ -35,9 +35,17 @@ module_param_named(no_mailboxes, no_mailboxes, int, 0644);
> MODULE_PARM_DESC(no_mailboxes,
> "There is no mailbox between cores, so ignore remote proc reply after start, default is 0 (off).");
>
> +/* Flag indicating that the remote is up and running */
> #define REMOTE_IS_READY BIT(0)
> +/* Flag indicating that the host should wait for a firmware-ready response */
> +#define HOST_WAIT_FW_READY BIT(1)
Right now the default behavior is to wait for a ready message from the remote
processor. We need to keep this implementation in order to remain backward
compatible with older firwmare images. As such the above should be:
#define HOST_DONT_WAIT_FW_READY BIT(1)
> #define REMOTE_READY_WAIT_MAX_RETRIES 500
>
> +/* This flag is set in the DSP resource table's features field to indicate
> + * that the firmware requires the host to wait for a FW_READY response.
> + */
> +#define WAIT_FW_READY BIT(0)
> +
> /* att flags */
> /* DSP own area */
> #define ATT_OWN BIT(31)
> @@ -136,6 +144,19 @@ struct imx_dsp_rproc_dcfg {
> int (*reset)(struct imx_dsp_rproc *priv);
> };
>
> +/**
> + * struct fw_rsc_imx_dsp - i.MX DSP specific info
> + * @len: length of the resource entry
> + * @features: feature flags supported by the i.MX DSP firmware
> + *
> + * This represents a DSP-specific resource in the firmware's
> + * resource table, providing information on supported features.
> + */
> +struct fw_rsc_imx_dsp {
> + uint32_t len;
> + uint32_t features;
> +};
> +
This needs a version field and magic number. Have a look at what Xilinx did for
inspiration [1] (both magic and complement of the magic number). You will also
need a "__packed" attribute for the structure.
[1].
https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/remoteproc/xlnx_r5_remoteproc.c#L103
> static const struct imx_rproc_att imx_dsp_rproc_att_imx8qm[] = {
> /* dev addr , sys addr , size , flags */
> { 0x596e8000, 0x556e8000, 0x00008000, ATT_OWN },
> @@ -300,6 +321,39 @@ static int imx_dsp_rproc_ready(struct rproc *rproc)
> return -ETIMEDOUT;
> }
>
> +/**
> + * imx_dsp_rproc_handle_rsc() - Handle DSP-specific resource table entries
> + * @rproc: remote processor instance
> + * @rsc_type: resource type identifier
> + * @rsc: pointer to the resource entry
> + * @offset: offset of the resource entry
> + * @avail: available space in the resource table
> + *
> + * Parse the DSP-specific resource entry and update flags accordingly.
> + * If the WAIT_FW_READY feature is set, the host must wait for the firmware
> + * to signal readiness before proceeding with execution.
> + *
> + * Return: RSC_HANDLED if processed successfully, RSC_IGNORED otherwise.
> + */
> +static int imx_dsp_rproc_handle_rsc(struct rproc *rproc, u32 rsc_type,
> + void *rsc, int offset, int avail)
> +{
> + struct imx_dsp_rproc *priv = rproc->priv;
> + struct fw_rsc_imx_dsp *imx_dsp_rsc = rsc;
> +
> + if (!imx_dsp_rsc || imx_dsp_rsc->len != sizeof(imx_dsp_rsc->features)) {
> + priv->flags |= HOST_WAIT_FW_READY;
Unless I am missing something, this way of enforcing the default behavior won't
work because in older images, function imx_dsp_rproc_handle_rsc() will never be
called.
To fix this I suggest renaming HOST_WAIT_FW_READY to
HOST_DONT_WAIT_FOR_FW_READY. That way the host can skip waiting for a FW ready
message only when it was specifically told to do so.
Thanks,
Mathieu
> + return RSC_IGNORED;
> + }
> +
> + if (imx_dsp_rsc->features & WAIT_FW_READY)
> + priv->flags |= HOST_WAIT_FW_READY;
> + else
> + priv->flags &= ~HOST_WAIT_FW_READY;
> +
> + return RSC_HANDLED;
> +}
> +
> /*
> * Start function for rproc_ops
> *
> @@ -335,8 +389,8 @@ static int imx_dsp_rproc_start(struct rproc *rproc)
>
> if (ret)
> dev_err(dev, "Failed to enable remote core!\n");
> - else
> - ret = imx_dsp_rproc_ready(rproc);
> + else if (priv->flags & HOST_WAIT_FW_READY)
> + return imx_dsp_rproc_ready(rproc);
>
> return ret;
> }
> @@ -936,6 +990,7 @@ static const struct rproc_ops imx_dsp_rproc_ops = {
> .kick = imx_dsp_rproc_kick,
> .load = imx_dsp_rproc_elf_load_segments,
> .parse_fw = imx_dsp_rproc_parse_fw,
> + .handle_rsc = imx_dsp_rproc_handle_rsc,
> .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> .sanity_check = rproc_elf_sanity_check,
> .get_boot_addr = rproc_elf_get_boot_addr,
> --
> 2.25.1
>
Powered by blists - more mailing lists