| 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: <20251029035221.GB7297@nxa18884-linux.ap.freescale.net>
Date: Wed, 29 Oct 2025 11:52:21 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Frank Li <Frank.li@....com>
Cc: Peng Fan <peng.fan@....com>, Bjorn Andersson <andersson@...nel.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Daniel Baluta <daniel.baluta@....com>,
linux-remoteproc@...r.kernel.org, devicetree@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] remoteproc: imx_rproc: Add support for System
Manager API
Hi Frank,
On Tue, Oct 28, 2025 at 12:16:56PM -0400, Frank Li wrote:
>On Tue, Oct 28, 2025 at 04:18:03PM +0800, Peng Fan wrote:
>> i.MX95 features a Cortex-M33 core, six Cortex-A55 cores, and
>> one Cortex-M7 core. The System Control Management Interface(SCMI)
>> firmware runs on the M33 core. The i.MX95 SCMI firmware named System
>> Manager(SM) includes vendor extension protocols, Logical Machine
>> Management(LMM) protocol and CPU protocol and etc.
>>
...
>> + depends on IMX_SCMI_CPU_DRV || !IMX_SCMI_CPU_DRV
>> + depends on IMX_SCMI_LMM_DRV || !IMX_SCMI_LMM_DRV
>
>what's means IMX_SCMI_LMM_DRV || !IMX_SCMI_LMM_DRV here, which is the same
>config
This is to make sure when IMX_SCMI_CPU_DRV or IMX_SCMI_LMM_DRV is module built,
imx_rproc should also be built as module.
>
>> select MAILBOX
>> help
...
>> + */
>> + if (priv->flags & IMX_RPROC_FLAGS_SM_CPU_OP)
>> + return 0;
>
>You already have imx_rproc_sm_lmm_stat[stop](),
>why not use imx_rproc_sm_lmm_prepare();
>
>imx_rproc_sm_cpu_prepare() should be empty. So needn't check
>IMX_RPROC_FLAGS_SM_CPU_OP.
I will add below in detect_mode(), and no need imx_rproc_sm_cpu_prepare.
priv->ops.prepare = &imx_rproc_sm_lmm_prepare;
>
>> +
>> + /*
>> static int imx_rproc_prepare(struct rproc *rproc)
>> {
>> struct imx_rproc *priv = rproc->priv;
>> @@ -532,7 +639,10 @@ static int imx_rproc_prepare(struct rproc *rproc)
>> rproc_add_carveout(rproc, mem);
>> }
>>
>> - return 0;
>> + if (priv->ops.detect_mode == imx_rproc_sm_detect_mode)
>> + return imx_rproc_sm_prepare(rproc);
Here I will change to:
if (priv->ops.prepare)
return priv->ops.prepare();
>> +
>> + return 0;
>> }
>>
>> static int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
>> @@ -991,6 +1101,83 @@ static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
>> return 0;
>> }
>>
>> + */
>> + if (dcfg->lmid == info.lmid) {
>> + priv->ops.start = &imx_rproc_sm_cpu_start;
>> + priv->ops.stop = &imx_rproc_sm_cpu_stop;
>
>you have switch callback here, also swtich imx_rproc_sm_prepare()
Yes, as above.
priv->ops.prepare = &imx_rproc_sm_lmm_prepare;
>
>> + priv->flags |= IMX_RPROC_FLAGS_SM_CPU_OP;
>> + dev_info(dev, "Using CPU Protocol OPS\n");
>> +
>> + return 0;
>> + }
>> +
>> + dev_info(dev, "Using LMM Protocol OPS\n");
>> + priv->ops.start = &imx_rproc_sm_lmm_start;
>> + priv->ops.stop = &imx_rproc_sm_lmm_stop;
>> + priv->flags |= IMX_RPROC_FLAGS_SM_LMM_OP;
>
>Most likely
>
> bool b = (dcfg->lmid == info.lmid);
> priv->ops.start = b ? &imx_rproc_sm_cpu_start : &imx_rproc_sm_lmm_start;
> ...
>or
> if (b) {
> } else {
> }
>
> to do ops switch.
ok.
>
>> +
>> + /*
>> + * Use power on to do permission check. If rproc is in different Logical Machine,
>> + * and linux has permission to handle the Logical Machine, set
>> + * IMX_RPROC_FLAGS_SM_LMM_AVAIL.
>> + */
>> + ret = scmi_imx_lmm_operation(dcfg->lmid, SCMI_IMX_LMM_POWER_ON, 0);
>> + if (ret != 0) {
>> + if (ret == -EACCES) {
>> + /* Not under Linux Control, so only do IPC between rproc and Linux */
>> + dev_info(priv->dev, "lmm(%d) not under Linux Control\n", dcfg->lmid);
>> + return 0;
>> + }
>> +
>> + dev_info(priv->dev, "power on lmm(%d) failed: %d\n", dcfg->lmid, ret);
>> + return ret;
>> + }
>> +
>> + priv->flags |= IMX_RPROC_FLAGS_SM_LMM_AVAIL;
>> +
>> + /* rproc is started before boot Linux, so do NOT shutdown the LM */
>> + if (started)
>> + return 0;
>> +
>> + /* Permission check finished, shutdown the LM to save power */
>> + ret = scmi_imx_lmm_operation(dcfg->lmid, SCMI_IMX_LMM_SHUTDOWN, 0);
>> + if (ret) {
>> + dev_err(priv->dev, "shutdown lmm(%d) failed: %d\n", dcfg->lmid, ret);
>> + return ret;
>> + }
>
>look like this block should be in imx_rproc_sm_prepare()?
This should be kept here. permission check flow:
power on m7 lm
|
| success(indicating linux control rproc LM)
-> check m7 booted by U-Boot
|-> If yes, keep power.
|-> If no, shutdown to save power.
Thanks,
Peng
>
>Frank
>> +
>> + return 0;
>> +}
>> +
>> static int imx_rproc_detect_mode(struct imx_rproc *priv)
>> {
>> /*
>> @@ -1166,6 +1353,10 @@ static const struct imx_rproc_plat_ops imx_rproc_ops_scu_api = {
>> .detect_mode = imx_rproc_scu_api_detect_mode,
>> };
>>
>> +static const struct imx_rproc_plat_ops imx_rproc_ops_sm = {
>> + .detect_mode = imx_rproc_sm_detect_mode,
>> +};
>> +
>> static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
>> .src_reg = IMX7D_SRC_SCR,
>> .src_mask = IMX7D_M4_RST_MASK,
>> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
>> index aeed08bdfb5619c7afd7201589f417cfd6745818..b8a30df92d3bdeb915e33551235d555b947d0b16 100644
>> --- a/drivers/remoteproc/imx_rproc.h
>> +++ b/drivers/remoteproc/imx_rproc.h
>> @@ -51,6 +51,9 @@ struct imx_rproc_dcfg {
>> enum imx_rproc_method method;
>> u32 flags;
>> const struct imx_rproc_plat_ops *ops;
>> + /* For System Manager(SM) based SoCs, the IDs are from SM firmware */
>> + u32 cpuid;
>> + u32 lmid;
>> };
>>
>> #endif /* _IMX_RPROC_H */
>>
>> --
>> 2.37.1
>>
Powered by blists - more mailing lists