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

On 14-Aug-26 18:25, 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, NoC QoS and BBSM tamper status.  There
> is no public specification for these calls; the function IDs are taken
> from the upstream and vendor kernels.
> 
> Forward this reviewed set of calls from the hardware domain, following
> the whitelist model of the i.MX8QM platform.  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  | 116 ++++++++++++++++++++++++++++++++
>  2 files changed, 117 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..dc49e754ee
> --- /dev/null
> +++ b/xen/arch/arm/platforms/imx8m.c
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
Any specific reason for 2+? Can it be just GPLv2 only? We tend to prefer the
former as some companies are wary of v3.

> +/*
> + * xen/arch/arm/platforms/imx8m.c
Please remove this line. It does not add any useful information and can quickly
go stale.

> + *
> + * i.MX 8M family setup
> + *
> + * Copyright 2026 Open-EP (E-Paper) Community
> + */
> +
> +#include <xen/sched.h>
> +#include <asm/platform.h>
> +#include <asm/smccc.h>
> +
> +static const char * const imx8m_dt_compat[] __initconst =
> +{
> +    "fsl,imx8mp",
> +    "fsl,imx8mq",
> +    "fsl,imx8mm",
> +    "fsl,imx8mn",
> +    NULL
> +};
> +
> +/*
> + * SiP SMC function IDs used by the i.MX8M Linux drivers.  There is no
> + * public specification for these; the IDs and names are extracted from
> + * the upstream and vendor kernels (see drivers/soc/imx, drivers/devfreq,
> + * drivers/remoteproc, drivers/firmware/imx).
> + */
> +#define IMX_SIP_GPC         0xC2000000  /* GPC power-domain control */
Please create a macro utilizing ARM_SMCCC_CALL_VAL same as i.MX 8QM platform
driver (or EEMI) so that these values denote actual functions

> +#define IMX_SIP_CPUFREQ     0xC2000001
> +#define IMX_SIP_DDR_DVFS    0xC2000004  /* DRAM frequency scaling */
> +#define IMX_SIP_SRC         0xC2000005  /* SRC: M-core remoteproc start/stop */
> +#define IMX_SIP_GET_SOC_INFO 0xC2000006
All values should be equally indented

> +#define IMX_SIP_NOC         0xC2000008  /* NoC QoS priority setup */
> +#define IMX_SIP_BBSM        0xC200000D  /* BBSM tamper status */
> +
> +static bool imx8m_smc(struct cpu_user_regs *regs)
> +{
> +    uint32_t function_id = get_user_reg(regs, 0);
> +    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;
> +    }
> +
> +    switch ( function_id )
> +    {
> +    /*
> +     * All of these manage hardware that belongs to the hardware domain
> +     * (power domains, DRAM controller, M-core, NoC, secure RTC) or are
You mention RTC but there's no call for it. Did you miss it?

> +     * read-only queries.  Forward them.
Stray space before `Forward`

> +     */
> +    case IMX_SIP_GPC:
> +    case IMX_SIP_DDR_DVFS:
> +    case IMX_SIP_SRC:
> +    case IMX_SIP_GET_SOC_INFO:
> +    case IMX_SIP_NOC:
> +    case IMX_SIP_BBSM:
> +        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_CPUFREQ:
> +        return false;
> +
> +    default:
> +        gprintk(XENLOG_WARNING, "imx8m: smc: Unknown function id %x\n",
> +                function_id);
> +        return false;
> +    }
The difference between this driver and QM is that the latter whitelists
subfunctions whereas you whitelist whole services which seems like a lot. I
don't want to go into details of TF-A, Linux but I would recommend you looking
at subfunctions dom0 actually needs.

~Michal

> +
> +    arm_smccc_1_1_smc(function_id,
> +                      get_user_reg(regs, 1),
> +                      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;
> +}
> +
> +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:
> + */