linux-kernel - Re: [PATCH 2/3] remoteproc: imx_rproc: Add support for System Manager API

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEnQRZByPkxojykJ3w-Mxko08Nc5txsOCL4vy+P76HuTy3YTaA@mail.gmail.com>
Date: Wed, 4 Jun 2025 15:55:55 +0300
From: Daniel Baluta <daniel.baluta@...il.com>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
Cc: 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>, 
	Iuliana Prodan <iuliana.prodan@....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, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH 2/3] remoteproc: imx_rproc: Add support for System Manager API

Hi Peng,

Thanks a lot for the patches. Comments inline:

On Wed, Jun 4, 2025 at 5:36 AM Peng Fan (OSS) <peng.fan@....nxp.com> 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 couldn't control it.
couldn't -> can't

>  (2) M7 in a separate Logical Machine that Linux could control it using
>      LMM protocol
could -> can

>  (3) M7 runs in same Logical Machine as A55, so Linux could control it
>      using CPU protocol
could -> can

>
> So extend the driver to using LMM and CPU protocol to manage the M7 core.
>  - Add IMX_RPROC_SM to indicate the remotecores runs on a SoC that
>    has System Manager.

remotecores -> remote core

>  - Compare linux LM ID(got using scmi_imx_lmm_info) and M7 LM ID(got
>    from DTB), if same, 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.
>
> Current setup relies on pre-Linux software(U-Boot) to do
> M7 TCM ECC initialization. In future, we could add the support in Linux
> to decouple U-Boot and Linux.
>
> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
>  drivers/remoteproc/imx_rproc.c | 139 ++++++++++++++++++++++++++++++++++++++++-
>  drivers/remoteproc/imx_rproc.h |   2 +
>  2 files changed, 139 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 74299af1d7f10a0db794de494c52304b2323b89f..0649faa98725db99366946c65edf5b7daff78316 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -8,6 +8,7 @@
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  #include <linux/firmware/imx/sci.h>
> +#include <linux/firmware/imx/sm.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/mailbox_client.h>
> @@ -21,6 +22,7 @@
>  #include <linux/reboot.h>
>  #include <linux/regmap.h>
>  #include <linux/remoteproc.h>
> +#include <linux/scmi_imx_protocol.h>
>  #include <linux/workqueue.h>
>
>  #include "imx_rproc.h"
> @@ -91,6 +93,11 @@ struct imx_rproc_mem {
>  #define ATT_CORE_MASK   0xffff
>  #define ATT_CORE(I)     BIT((I))
>
> +/* Logical Machine Operation */
> +#define IMX_RPROC_FLAGS_SM_LMM_OP      BIT(0)
> +/* Linux has permission to handle the Logical Machine of remote cores */
> +#define IMX_RPROC_FLAGS_SM_LMM_AVAIL   BIT(1)
> +
>  static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block);
>  static void imx_rproc_free_mbox(struct rproc *rproc);
>
> @@ -115,6 +122,22 @@ struct imx_rproc {
>         u32                             entry;          /* cpu start address */
>         u32                             core_index;
>         struct dev_pm_domain_list       *pd_list;
> +       /* For i.MX System Manager based systems */
> +       u32                             cpuid;
> +       u32                             lmid;
> +       u32                             flags;
> +};
> +
> +static const struct imx_rproc_att imx_rproc_att_imx95_m7[] = {
> +       /* dev addr , sys addr  , size      , flags */
> +       /* TCM CODE NON-SECURE */
> +       { 0x00000000, 0x203C0000, 0x00040000, ATT_OWN | ATT_IOMEM },
> +
> +       /* TCM SYS NON-SECURE*/
> +       { 0x20000000, 0x20400000, 0x00040000, ATT_OWN | ATT_IOMEM },
> +
> +       /* DDR */
> +       { 0x80000000, 0x80000000, 0x50000000, 0 },
>  };

^ this belongs to patch 3/3

Otherwise, patch looks good to me.