Re: [PATCH v2 3/4] xen/arm: add i.MX8M platform support

On 18-Aug-26 04:52, Wig Cheng wrote:
> Add platform glue for the NXP i.MX8M family (i.MX8MP/MQ/MM/MN).
> 
> When Linux is used as dom0 a number of drivers make SiP SMC calls into
> TF-A to manage hardware: GPC power domains, DDR frequency scaling, SRC
> (M-core remoteproc), SoC info and NoC QoS.  There is no public
> specification for these calls; the function IDs and their subfunctions
> are taken from the vendor kernel call sites.
> 
> Forward only the specific subfunctions the hardware domain issues,
> following the whitelist model of the i.MX8QM platform.  Where a service
> has a fixed set of subfunctions (GPC, SRC, NoC) they are filtered; DDR
> DVFS is left at service level because its reg1 is a frequency setpoint
> rather than a fixed subfunction id, and the SoC info call is a read-only
> query.  CPU frequency scaling is denied because the hardware domain
> cannot make an informed decision, and any unknown function ID is
> rejected.
> 
> Signed-off-by: Wig Cheng <[email protected]>
> ---
>  xen/arch/arm/platforms/Makefile |   1 +
>  xen/arch/arm/platforms/imx8m.c  | 152 ++++++++++++++++++++++++++++++++
>  2 files changed, 153 insertions(+)
>  create mode 100644 xen/arch/arm/platforms/imx8m.c
> 
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index bec6e55d1f..cdf936c50d 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ALL_PLAT)   += sunxi.o
>  obj-$(CONFIG_ALL64_PLAT) += thunderx.o
>  obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o
>  obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o
> +obj-$(CONFIG_ALL64_PLAT) += imx8m.o
>  obj-$(CONFIG_ALL64_PLAT) += imx8qm.o
>  obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp.o
>  obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp-eemi.o
> diff --git a/xen/arch/arm/platforms/imx8m.c b/xen/arch/arm/platforms/imx8m.c
> new file mode 100644
> index 0000000000..fcf01298d8
> --- /dev/null
> +++ b/xen/arch/arm/platforms/imx8m.c
> @@ -0,0 +1,152 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * i.MX 8M family setup
> + *
> + * Copyright 2026 Open-EP (E-Paper) Community
> + */
> +
> +#include <xen/sched.h>
> +#include <asm/platform.h>
> +#include <asm/smccc.h>
You should also include asm/regs.h for `get/set_user_regs()`

> +
> +static const char * const imx8m_dt_compat[] __initconst =
> +{
> +    "fsl,imx8mp",
> +    "fsl,imx8mq",
> +    "fsl,imx8mm",
> +    "fsl,imx8mn",
> +    NULL
> +};
> +
> +#define IMX_SIP_FID(fid) \
> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> +                       ARM_SMCCC_CONV_64, \
> +                       ARM_SMCCC_OWNER_SIP, \
> +                       (fid))
> +
> +/*
> + * SiP SMC function IDs used by the i.MX8M Linux drivers.  There is no
> + * public specification for these; the IDs and their subfunctions are
> + * extracted from the vendor kernel call sites (see drivers/soc/imx,
> + * drivers/devfreq, drivers/remoteproc).
> + */
> +#define IMX_SIP_F_GPC       0x0   /* GPC power-domain control */
> +#define IMX_SIP_F_CPUFREQ   0x1
Why no description?

> +#define IMX_SIP_F_DDR_DVFS  0x4   /* DRAM frequency scaling */
> +#define IMX_SIP_F_SRC       0x5   /* SRC: M-core remoteproc start/stop */
> +#define IMX_SIP_F_SOC_INFO  0x6   /* read-only SoC info query */
> +#define IMX_SIP_F_NOC       0x8   /* NoC QoS priority setup */
> +
> +#define IMX_SIP_GPC_SF_PM_DOMAIN    0x03
> +
> +#define IMX_SIP_SRC_SF_M4_START     0x00
This seems to be unused. Why?

> +#define IMX_SIP_SRC_SF_M4_STOP      0x02
> +
> +#define IMX_SIP_NOC_SF_LCDIF        0x00
This seems to be unused. Why?

> +#define IMX_SIP_NOC_SF_PRIORITY     0x01
> +
> +static bool imx8m_smc(struct cpu_user_regs *regs)
> +{
> +    uint32_t function_id = get_user_reg(regs, 0);
> +    uint32_t subfunction_id = get_user_reg(regs, 1);
> +    struct arm_smccc_res res;
> +
> +    if ( !cpus_have_const_cap(ARM_SMCCC_1_1) )
> +    {
> +        printk_once(XENLOG_WARNING
> +                    "imx8m: smc: no SMCCC 1.1 support. Disabling firmware calls\n");
> +
> +        return false;
> +    }
> +
> +    /* Only the hardware domain may use the SiP calls */
> +    if ( !is_hardware_domain(current->domain) )
> +    {
> +        gprintk(XENLOG_WARNING, "imx8m: smc: No access\n");
> +        return false;
> +    }
> +
> +    /*
> +     * Forward only the subfunctions the dom0 kernel actually issues.  All
> +     * of these manage hardware that belongs to the hardware domain (power
> +     * domains, DRAM controller, M-core, NoC) or are read-only queries.
> +     */
> +    switch ( function_id )
> +    {
> +    case IMX_SIP_FID(IMX_SIP_F_GPC):
> +        if ( subfunction_id != IMX_SIP_GPC_SF_PM_DOMAIN )
> +            goto deny_subfunction;
> +        break;
> +
> +    case IMX_SIP_FID(IMX_SIP_F_SRC):
Please list the cases in an order (GPC, CPUFREQ ...)

> +        if ( subfunction_id > IMX_SIP_SRC_SF_M4_STOP )
> +            goto deny_subfunction;
> +        break;
> +
> +    case IMX_SIP_FID(IMX_SIP_F_NOC):
> +        if ( subfunction_id > IMX_SIP_NOC_SF_PRIORITY )
> +            goto deny_subfunction;
> +        break;
> +
> +    case IMX_SIP_FID(IMX_SIP_F_DDR_DVFS):
IMO the stated reason for CPUFREQ applies here as well and we should rather deny
this call. What makes it safe in your opinion to allow this call?

> +        /*
> +         * For DDR DVFS reg1 is a frequency setpoint index (or the
> +         * GET_FREQ_COUNT / GET_FREQ_INFO query), not a fixed subfunction
> +         * id, so it is not filtered here.
> +         */
> +        break;
> +
> +    case IMX_SIP_FID(IMX_SIP_F_SOC_INFO):
> +        break;
> +
> +    /*
> +     * CPU frequency scaling: the hardware domain does not see the whole
> +     * system and cannot make an informed decision, so deny it (matches
> +     * the i.MX8QM platform).
> +     */
> +    case IMX_SIP_FID(IMX_SIP_F_CPUFREQ):
> +        return false;
> +
> +    default:
> +        gprintk(XENLOG_WARNING, "imx8m: smc: Unknown function id %x\n",
> +                function_id);
> +        return false;
> +    }
> +
> +    arm_smccc_1_1_smc(function_id,
> +                      subfunction_id,
> +                      get_user_reg(regs, 2),
> +                      get_user_reg(regs, 3),
> +                      get_user_reg(regs, 4),
> +                      get_user_reg(regs, 5),
> +                      get_user_reg(regs, 6),
> +                      get_user_reg(regs, 7),
> +                      &res);
> +
> +    set_user_reg(regs, 0, res.a0);
> +    set_user_reg(regs, 1, res.a1);
> +    set_user_reg(regs, 2, res.a2);
> +    set_user_reg(regs, 3, res.a3);
> +
> +    return true;
> +
> + deny_subfunction:
> +    gprintk(XENLOG_WARNING,
> +            "imx8m: smc: function %x: denied subfunction %x\n",
> +            function_id, subfunction_id);
IMO just return false directly on deny, no need for goto and printk, given that
Xen will also print Unhandled SMC/HVC from `vsmccc_handle_call()`.

> +    return false;
> +}
> +
> +PLATFORM_START(imx8m, "i.MX 8M")
> +    .compatible = imx8m_dt_compat,
> +    .smc = imx8m_smc,
> +PLATFORM_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */