Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue

[Julien Grall <julien@xen.org>]

Hi,

On 31/01/2024 11:50, John Ernberg wrote:
> When using Linux for dom0 there are a bunch of drivers that need to do SMC
> SIP calls into the PSCI provider to enable certain hardware bits like the
> watchdog.

Do you know which protocol this is under the hood. Is this SCMI?

> 
> Provide a basic platform glue that implements the needed SMC forwarding.
> 
> Signed-off-by: John Ernberg <john.ernberg@actia.se>
> ---
> NOTE: This is based on code found in NXP Xen tree located here:
> https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c

Anything after --- will be removed while applied to the three. I think 
this NOTE should be written down in the commit message.

You also possibly want a signed-off-by from Peng as this is his code.

> 
>   xen/arch/arm/platforms/Makefile |  1 +
>   xen/arch/arm/platforms/imx8qm.c | 65 +++++++++++++++++++++++++++++++++
>   2 files changed, 66 insertions(+)
>   create mode 100644 xen/arch/arm/platforms/imx8qm.c
> 
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index 8632f4115f..bec6e55d1f 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -9,5 +9,6 @@ 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) += imx8qm.o
>   obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp.o
>   obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp-eemi.o
> diff --git a/xen/arch/arm/platforms/imx8qm.c b/xen/arch/arm/platforms/imx8qm.c
> new file mode 100644
> index 0000000000..a9cd9c3615
> --- /dev/null
> +++ b/xen/arch/arm/platforms/imx8qm.c
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * xen/arch/arm/platforms/imx8qm.c
> + *
> + * i.MX 8QM setup
> + *
> + * Copyright (c) 2016 Freescale Inc.
> + * Copyright 2018-2019 NXP
> + *
> + *
> + * Peng Fan <peng.fan@nxp.com>
> + */
> +
> +#include <asm/platform.h>
> +#include <asm/smccc.h>
> +
> +static const char * const imx8qm_dt_compat[] __initconst =
> +{
> +    "fsl,imx8qm",
> +    "fsl,imx8qxp",
> +    NULL
> +};
> +
> +static bool imx8qm_smc(struct cpu_user_regs *regs)
> +{

Your implementation below will not only forward SMC for dom0 but also 
for any non-trusted domains. Have you investigated that all the SIP 
calls are safe to be called by anyone?

But even if we restrict to dom0, have you checked that none of the SMCs 
use buffers?

Rather than providing a blanket forward, to me it sounds more like you 
want to provide an allowlist of the SMCs. This is more futureproof and 
avoid the risk to expose unsafe SMCs to any domain.

For an example, you can have a look at the EEMI mediator for Xilinx.

Cheers,

-- 
Julien Grall