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

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 18-Aug-26 17:39, 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, 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, and
> the SoC info call is a read-only query.  CPU and DRAM frequency scaling
> are denied because the hardware domain cannot make an informed decision
> about resources shared with the other domains, and any unknown function
> ID is rejected.
> 
> Signed-off-by: Wig Cheng <onlywig@xxxxxxxxx>
> ---
> Changes in v3:
> - Include <asm/regs.h> for get/set_user_reg().
> - Add a description for the CPUFREQ function id.
> - Drop the unused SRC M4_START and NoC LCDIF subfunction macros.
> - Order the switch cases by function id.
> - Deny DDR DVFS as well, for the same reason CPU frequency scaling is
>   denied: the hardware domain cannot make an informed decision about
>   DRAM shared with the other domains.  Previously it was forwarded.
> - Return false directly on a denied subfunction instead of goto plus a
>   redundant printk (vsmccc_handle_call() already logs the rejection).
> 
>  xen/arch/arm/platforms/Makefile |   1 +
>  xen/arch/arm/platforms/imx8m.c  | 139 ++++++++++++++++++++++++++++++++
>  2 files changed, 140 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..0aceed9d43
> --- /dev/null
> +++ b/xen/arch/arm/platforms/imx8m.c
> @@ -0,0 +1,139 @@
> +/* 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/regs.h>
> +#include <asm/smccc.h>
> +
> +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   /* CPU frequency scaling */
> +#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_STOP      0x02
> +
> +#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, 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 )
> +            return false;
> +        break;
> +
> +    /*
> +     * CPU and DRAM frequency scaling: the hardware domain does not see the
> +     * whole system and cannot make an informed decision about resources
> +     * shared with the other domains, so deny both (CPU frequency scaling
> +     * is denied on the i.MX8QM platform for the same reason).
> +     */
> +    case IMX_SIP_FID(IMX_SIP_F_CPUFREQ):
> +    case IMX_SIP_FID(IMX_SIP_F_DDR_DVFS):
> +        return false;
> +
> +    case IMX_SIP_FID(IMX_SIP_F_SRC):
> +        if ( subfunction_id > IMX_SIP_SRC_SF_M4_STOP )
This is a range check rather than a white list. I think it's very important to
be able to see what exact subfunctions are allowed i.e. switch ( subfunction_id 
)

> +            return false;
> +        break;
> +
> +    case IMX_SIP_FID(IMX_SIP_F_SOC_INFO):
> +        break;
> +
> +    case IMX_SIP_FID(IMX_SIP_F_NOC):
> +        if ( subfunction_id > IMX_SIP_NOC_SF_PRIORITY )
Same here.

> +            return false;
> +        break;
> +
> +    default:
> +        gprintk(XENLOG_WARNING, "imx8m: smc: Unknown function id %x\n",
> +                function_id);
Worth printing also subfunction_id.

With the remarks addressed:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal