| 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: <20250709090231.GB14535@nxa18884-linux>
Date: Wed, 9 Jul 2025 17:02:31 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Mathieu Poirier <mathieu.poirier@...aro.org>
Cc: Bjorn Andersson <andersson@...nel.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>, Frank Li <frank.li@....com>,
Daniel Baluta <daniel.baluta@....com>,
Iuliana Prodan <iuliana.prodan@....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, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH v3 2/5] remoteproc: imx_rproc: Add support for System
Manager API
Hi Mathieu,
On Tue, Jul 08, 2025 at 10:32:34AM -0600, Mathieu Poirier wrote:
>Good day,
Thanks, good day.
>
>On Wed, Jun 25, 2025 at 10:23:28AM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@....com>
>>
>> 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.
>>
>> There are three cases for M7:
>> (1) M7 in a separate Logical Machine(LM) that Linux can't control it.
>> (2) M7 in a separate Logical Machine that Linux can control it using
>> LMM protocol
>> (3) M7 runs in same Logical Machine as A55, so Linux can control it
>> using CPU protocol
>>
>> So extend the driver to using LMM and CPU protocol to manage the M7 core.
>> - Add IMX_RPROC_SM to indicate the remote core runs on a SoC that
>> has System Manager.
>> - Compare linux LM ID(got using scmi_imx_lmm_info) and M7 LM ID(the ID
>> is fixed as 1 in SM firmware if M7 is in a seprate LM),
>> if Linux LM ID equals M7 LM ID(linux and M7 in same LM), use CPU
>> protocol to start/stop. Otherwise, use LMM protocol to start/stop.
>> Whether using CPU or LMM protocol to start/stop, the M7 status
>> detection could use CPU protocol to detect started or not. So
>> in imx_rproc_detect_mode, use scmi_imx_cpu_started to check the
>> status of M7.
>> - For above case 1 and 2, Use SCMI_IMX_LMM_POWER_ON to detect whether
>> the M7 LM is under control of A55 LM.
>>
>
>Thanks for the context, it really helps.
Glad that helps.
>
[....]
>> @@ -592,6 +637,38 @@ static int imx_rproc_prepare(struct rproc *rproc)
>> rproc_add_carveout(rproc, mem);
>> }
>>
>> + switch (dcfg->method) {
>> + case IMX_RPROC_SM:
>> + if (!(priv->flags & IMX_RPROC_FLAGS_SM_LMM_OP))
>> + break;
>> + /*
>> + * Power on the Logical Machine to make sure TCM is available.
>> + * Also serve as permission check. If 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) {
>> + dev_info(priv->dev, "lmm(%d) powered on\n", dcfg->lmid);
>> + priv->flags |= IMX_RPROC_FLAGS_SM_LMM_AVAIL;
>
>This is set all the time imx_rproc_prepare() is called - isn't there a way to
>set it once at initialisation time?
Yeah. Moving this to probe path should be ok, such as in imx_rproc_detect_mode.
The drawback is M7 logical machine will be left in powered up state if
moving to probe path before user starts M7 LM. Leaving in here means M7 logcal
machine will be only powered up when user does
"echo start > /xx/remoteproc-y/state", but needs to set
IMX_RPROC_FLAGS_SM_LMM_AVAIL flags each time do preparing.
If you prefer moving this logic to probe, I could give a try to move
to imx_rproc_detect_mode which is probe path.
How do you think?
>
>> + } else if (ret == -EACCES) {
>> + dev_info(priv->dev, "lmm(%d) not under Linux Control\n", dcfg->lmid);
>> + /*
>> + * If remote cores boots up in detached mode, continue;
>> + * else linux has no permission, return -EACCES.
>> + */
>> + if (priv->rproc->state != RPROC_DETACHED)
>> + return -EACCES;
>> + } else if (ret) {
>> + dev_err(priv->dev, "Failed to power on lmm(%d): %d\n", ret, dcfg->lmid);
>> + return ret;
>> + }
>> +
>> + break;
>> + default:
>> + break;
>> + };
>> +
>
>Please put this in a function and get rid of the switch{}.
ok. Fix in v4
>
>> return 0;
>> }
>>
>> @@ -911,13 +988,41 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>> struct regmap_config config = { .name = "imx-rproc" };
>> const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>> struct device *dev = priv->dev;
>> + struct scmi_imx_lmm_info info;
>> struct regmap *regmap;
>> struct arm_smccc_res res;
>> + bool started = false;
>> int ret;
>> u32 val;
>> u8 pt;
>>
>> switch (dcfg->method) {
>> + case IMX_RPROC_SM:
>> + /* Get current Linux Logical Machine ID */
>> + ret = scmi_imx_lmm_info(LMM_ID_DISCOVER, &info);
>> + if (ret) {
>> + dev_err(dev, "Failed to get current LMM ID err: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /*
>> + * Check whether remote processor is in same Logical Machine as Linux.
>> + * If no, need use Logical Machine API to manage remote processor, and
>> + * set IMX_RPROC_FLAGS_SM_LMM_OP.
>> + * If yes, use CPU protocol API to manage remote processor.
>> + */
>> + if (dcfg->lmid != info.lmid) {
>> + priv->flags |= IMX_RPROC_FLAGS_SM_LMM_OP;
>> + dev_info(dev, "Using LMM Protocol OPS\n");
>> + } else {
>> + dev_info(dev, "Using CPU Protocol OPS\n");
>> + }
>> +
>> + scmi_imx_cpu_started(dcfg->cpuid, &started);
>
>Function scmi_imx_cpu_started() returns a value that is not taken into account.
>
>> + if (started)
I will update to "if (started || ret)" in v4, with ret assigned the return
value of scmi_imx_cpu_started.
Thanks,
Peng
Powered by blists - more mailing lists