linux-kernel - Re: [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8ea4ce87-ea35-56d3-31e3-40e570425653@nxp.com>
Date:   Thu, 19 Jan 2023 01:55:14 +0200
From:   Iuliana Prodan <iuliana.prodan@....com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        "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>,
        linux-imx <linux-imx@....com>, 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 v3] remoteproc: imx_dsp_rproc: add module parameter to
 ignore ready flag from remote processor

On 1/18/2023 7:24 PM, Mathieu Poirier wrote:
> Hi Iuliana,
>
> On Tue, Jan 17, 2023 at 01:03:57PM +0200, Iuliana Prodan (OSS) wrote:
>> From: Iuliana Prodan <iuliana.prodan@....com>
>>
>> There are cases when we want to test a simple "hello world"
>> application on the DSP and we don't have IPC between the cores.
>> Therefore, skip the wait for remote processor to start.
>>
>> Added "ignore_dsp_ready" flag while inserting the module to ignore
>> remote processor reply after start.
>> By default, this is off - do not ignore reply from rproc.
>>
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@....com>
>>
>> ---
>> Changes since v2
>> - s/ignoreready/ignore_dsp_ready
>>
>> Changes since v1
>> - change BIT(31) to BIT(1) for REMOTE_SKIP_WAIT
>>
>> ---
>>   drivers/remoteproc/imx_dsp_rproc.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
>> index 95da1cbefacf..22e2ef068c67 100644
>> --- a/drivers/remoteproc/imx_dsp_rproc.c
>> +++ b/drivers/remoteproc/imx_dsp_rproc.c
>> @@ -26,9 +26,20 @@
>>   #include "remoteproc_elf_helpers.h"
>>   #include "remoteproc_internal.h"
>>   
>> +#define IMX_DSP_IGNORE_REMOTE_READY		0
>> +
>> +/*
>> + * Module parameters
>> + */
>> +static unsigned int imx_dsp_rproc_ignore_ready = IMX_DSP_IGNORE_REMOTE_READY;
> Static variables are initialised to '0' and as such this is not needed.
>
>> +module_param_named(ignore_dsp_ready, imx_dsp_rproc_ignore_ready, int, 0644);
>> +MODULE_PARM_DESC(ignore_dsp_ready,
>> +		 "Ignore remote proc reply after start, default is 0 (off).");
>> +
>>   #define DSP_RPROC_CLK_MAX			5
>>   
>>   #define REMOTE_IS_READY				BIT(0)
>> +#define REMOTE_SKIP_WAIT			BIT(1)
>>   #define REMOTE_READY_WAIT_MAX_RETRIES		500
>>   
>>   /* att flags */
>> @@ -285,6 +296,9 @@ static int imx_dsp_rproc_ready(struct rproc *rproc)
>>   	if (!priv->rxdb_ch)
>>   		return 0;
>>   
>> +	if (priv->flags & REMOTE_SKIP_WAIT)
>> +		return 0;
>> +
> This looks very hackish to me...
>
> Here priv->rxdb_ch is valid and as such the DB mailbox has been setup, which
> contradicts the commit log where it is stated that "we don't have IPC between
> cores".  Moreover, the commit log mentions to "skip the wait for remote
> processor to start".  How can the remote processor executed an sample
> application if it is not ready?
>
> Lastly, is there even a need to call imx_dsp_rproc_mbox_init() if an IPC is not
> needed?
>
> I'm fine with the module parameter but would much rather see a solution that
> does not configure any kind of IPC related mechanic when it is not needed.
>
> Thanks,
> Mathieu
Hi Mathieu,

I've tested this with hello_world sample from Zephyr.
This was loaded on a hifi4 core from Linux using remoteproc. And, with 
this patch is working, otherwise I get "can't start rproc imx-dsp-rproc: 
-110:" because the ARM core is waiting for a reply from the hifi4.

I agree, I shouldn't initialize mbox if there is no IPC between the cores.
I'll fix this in a v4.

Thanks,
Iulia