All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue
  2024-03-28 16:34 [PATCH v3 0/2] Xen: ARM: Improved NXP iMX8 platform support John Ernberg
  2024-03-28 16:34 ` [PATCH v3 2/3] xen/drivers: imx-lpuart: Replace iMX8QM compatible with iMX8QXP John Ernberg
@ 2024-03-28 16:34 ` John Ernberg
  2024-04-02  7:24   ` Michal Orzel
  2024-03-28 16:34 ` [PATCH v3 3/3] MAINTAINERS: Become a reviewer of iMX8Q{M,XP} related patches John Ernberg
  2 siblings, 1 reply; 7+ messages in thread
From: John Ernberg @ 2024-03-28 16:34 UTC (permalink / raw)
  To: Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk
  Cc: Andrew Cooper, George Dunlap, Jan Beulich, xen-devel, Peng Fan,
	Jonas Blixt, John Ernberg

When using Linux for dom0 there are a bunch of drivers that need to do SMC
SIP calls into the firmware to enable certain hardware bits like the
watchdog.

Provide a basic platform glue that implements the needed SMC forwarding.

The format of these calls are as follows:
 - reg 0: function ID
 - reg 1: subfunction ID (when there's a subfunction)
 remaining regs: args

For now we only allow Dom0 to make these calls as they are all managing
hardware. There is no specification for these SIP calls, the IDs and names
have been extracted from the upstream linux kernel and the vendor kernel.

Most of the SIP calls are only available for the iMX8M series of SoCs, so
they are easy to reject and they need to be revisited when iMX8M series
support is added.

From the other calls we can reject CPUFREQ because Dom0 cannot make an
informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake
up from suspend, which Xen doesn't support at this time.

This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which
for now are allowed to Dom0.

NOTE: This code 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

Signed-off-by: Peng Fan <peng.fan@nxp.com>
[jernberg: Add SIP call filtering]
Signed-off-by: John Ernberg <john.ernberg@actia.se>

---

v3:
 - Adhere to style guidelines for line length and label indentation (Michal Orzel)
 - Use smccc macros to build the SIP function identifier (Michal Orzel)
 - Adjust platform name to be specific to QM and QXP variants (Michal Orzel)
 - Pick up additional SIPs which may be used by other Linux versions - for review purposes
 - Changes to the commit message due to above

v2:
 - Reword the commit message to be a bit clearer
 - Include the link previously added as a context note to the commit message (Julien Grall)
 - Add Pengs signed off (Julien Grall, Peng Fan)
 - Add basic SIP call filter (Julien Grall)
 - Expand the commit message a whole bunch because of the changes to the code
---
 xen/arch/arm/platforms/Makefile |   1 +
 xen/arch/arm/platforms/imx8qm.c | 168 ++++++++++++++++++++++++++++++++
 2 files changed, 169 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..0992475474
--- /dev/null
+++ b/xen/arch/arm/platforms/imx8qm.c
@@ -0,0 +1,168 @@
+/* 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 <xen/sched.h>
+#include <asm/platform.h>
+#include <asm/smccc.h>
+
+static const char * const imx8qm_dt_compat[] __initconst =
+{
+    "fsl,imx8qm",
+    "fsl,imx8qxp",
+    NULL
+};
+
+#define IMX_SIP_FID(fid) \
+    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+                       ARM_SMCCC_CONV_64, \
+                       ARM_SMCCC_OWNER_SIP, \
+                       fid)
+
+#define IMX_SIP_F_GPC            0x0000
+#define IMX_SIP_F_CPUFREQ        0x0001
+#define IMX_SIP_F_TIME           0x0002
+#define IMX_SIP_F_BUILDINFO      0x0003
+#define IMX_SIP_F_DDR_DVFS       0x0004
+#define IMX_SIP_F_SRC            0x0005
+#define IMX_SIP_F_GET_SOC_INFO   0x0006
+#define IMX_SIP_F_NOC            0x0008
+#define IMX_SIP_F_WAKEUP_SRC     0x0009
+#define IMX_SIP_F_OTP_READ       0x000A
+#define IMX_SIP_F_OTP_WRITE      0x000B
+#define IMX_SIP_F_SET_TEMP_ALARM 0x000C
+
+#define IMX_SIP_TIME_SF_RTC_SET_TIME     0x00
+#define IMX_SIP_TIME_SF_WDOG_START       0x01
+#define IMX_SIP_TIME_SF_WDOG_STOP        0x02
+#define IMX_SIP_TIME_SF_WDOG_SET_ACT     0x03
+#define IMX_SIP_TIME_SF_WDOG_PING        0x04
+#define IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT 0x05
+#define IMX_SIP_TIME_SF_WDOG_GET_STAT    0x06
+#define IMX_SIP_TIME_SF_WDOG_SET_PRETIME 0x07
+
+static bool imx8qm_is_sip_time_call_ok(uint32_t subfunction_id)
+{
+    switch ( subfunction_id )
+    {
+    case IMX_SIP_TIME_SF_RTC_SET_TIME:
+        return true;
+    case IMX_SIP_TIME_SF_WDOG_START:
+    case IMX_SIP_TIME_SF_WDOG_STOP:
+    case IMX_SIP_TIME_SF_WDOG_SET_ACT:
+    case IMX_SIP_TIME_SF_WDOG_PING:
+    case IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT:
+    case IMX_SIP_TIME_SF_WDOG_GET_STAT:
+    case IMX_SIP_TIME_SF_WDOG_SET_PRETIME:
+        return true;
+    default:
+        printk(XENLOG_WARNING "imx8qm: smc: time: Unknown subfunction id %x\n",
+               subfunction_id);
+        return false;
+    }
+}
+
+static bool imx8qm_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 "no SMCCC 1.1 support. Disabling firmware calls\n");
+
+        return false;
+    }
+
+    /* Only hardware domain may use the SIP calls */
+    if ( !is_hardware_domain(current->domain) )
+    {
+        gprintk(XENLOG_WARNING, "imx8qm: smc: No access\n");
+        return false;
+    }
+
+    switch ( function_id )
+    {
+    /* Only available on imx8m series */
+    case IMX_SIP_FID(IMX_SIP_F_GPC):
+        return false;
+    case IMX_SIP_FID(IMX_SIP_F_CPUFREQ):
+        /* Hardware domain can't take any informed decision here */
+        return false;
+    case IMX_SIP_FID(IMX_SIP_F_BUILDINFO):
+        goto allow_call;
+    case IMX_SIP_FID(IMX_SIP_F_TIME):
+        if ( imx8qm_is_sip_time_call_ok(subfunction_id) )
+            goto allow_call;
+        return false;
+    /* Only available on imx8m series */
+    case IMX_SIP_FID(IMX_SIP_F_DDR_DVFS):
+        return false;
+    /* Only available on imx8m series */
+    case IMX_SIP_FID(IMX_SIP_F_SRC):
+        return false;
+    /* Only available on imx8m series */
+    case IMX_SIP_FID(IMX_SIP_F_GET_SOC_INFO):
+        return false;
+    /* Only available on imx8m series */
+    case IMX_SIP_FID(IMX_SIP_F_NOC):
+        return false;
+    /* Xen doesn't have suspend support */
+    case IMX_SIP_FID(IMX_SIP_F_WAKEUP_SRC):
+        return false;
+    case IMX_SIP_FID(IMX_SIP_F_OTP_READ):
+        /* subfunction_id is the fuse number, no sensible check possible */
+        goto allow_call;
+    case IMX_SIP_FID(IMX_SIP_F_OTP_WRITE):
+        /* subfunction_id is the fuse number, no sensible check possible */
+        goto allow_call;
+    case IMX_SIP_FID(IMX_SIP_F_SET_TEMP_ALARM):
+        goto allow_call;
+    default:
+        printk(XENLOG_WARNING "imx8qm: smc: Unknown function id %x\n",
+               function_id);
+        return false;
+    }
+
+ allow_call:
+    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;
+}
+
+PLATFORM_START(imx8qm, "i.MX 8Q{M,XP}")
+    .compatible = imx8qm_dt_compat,
+    .smc = imx8qm_smc,
+PLATFORM_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/3] xen/drivers: imx-lpuart: Replace iMX8QM compatible with iMX8QXP
  2024-03-28 16:34 [PATCH v3 0/2] Xen: ARM: Improved NXP iMX8 platform support John Ernberg
@ 2024-03-28 16:34 ` John Ernberg
  2024-03-28 16:34 ` [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue John Ernberg
  2024-03-28 16:34 ` [PATCH v3 3/3] MAINTAINERS: Become a reviewer of iMX8Q{M,XP} related patches John Ernberg
  2 siblings, 0 replies; 7+ messages in thread
From: John Ernberg @ 2024-03-28 16:34 UTC (permalink / raw)
  To: Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk
  Cc: Andrew Cooper, George Dunlap, Jan Beulich, xen-devel, Peng Fan,
	Jonas Blixt, John Ernberg, Julien Grall

Allow the uart to probe also with iMX8QXP. The ip-block is the same as in
the QM.

Since the fsl,imx8qm-lpuart compatible in Linux exists in name only and is
not used in the driver any iMX8QM device tree that can boot Linux must set
fsl,imx8qxp-lpuart compatible as well as the QM one.

Thus we replace the compatible rather than adding just another one.

Signed-off-by: John Ernberg <john.ernberg@actia.se>
Acked-by: Julien Grall <jgrall@amazon.com>

---

v3:
 - no changes

v2:
 - Replace the compatible rather than adding to the list (Julien Grall)
 - Reword commit message to reflect the above.
 - Collect Julien's ack
---
 xen/drivers/char/imx-lpuart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/char/imx-lpuart.c b/xen/drivers/char/imx-lpuart.c
index 522680a25c..9d0e0871f7 100644
--- a/xen/drivers/char/imx-lpuart.c
+++ b/xen/drivers/char/imx-lpuart.c
@@ -255,7 +255,7 @@ static int __init imx_lpuart_init(struct dt_device_node *dev,
 
 static const struct dt_device_match imx_lpuart_dt_compat[] __initconst =
 {
-    DT_MATCH_COMPATIBLE("fsl,imx8qm-lpuart"),
+    DT_MATCH_COMPATIBLE("fsl,imx8qxp-lpuart"),
     { /* sentinel */ },
 };
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/3] MAINTAINERS: Become a reviewer of iMX8Q{M,XP} related patches
  2024-03-28 16:34 [PATCH v3 0/2] Xen: ARM: Improved NXP iMX8 platform support John Ernberg
  2024-03-28 16:34 ` [PATCH v3 2/3] xen/drivers: imx-lpuart: Replace iMX8QM compatible with iMX8QXP John Ernberg
  2024-03-28 16:34 ` [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue John Ernberg
@ 2024-03-28 16:34 ` John Ernberg
  2024-04-05  0:23   ` Stefano Stabellini
  2 siblings, 1 reply; 7+ messages in thread
From: John Ernberg @ 2024-03-28 16:34 UTC (permalink / raw)
  To: Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk
  Cc: Andrew Cooper, George Dunlap, Jan Beulich, xen-devel, Peng Fan,
	Jonas Blixt, John Ernberg

I have experience with the IMX8QXP, and the supported parts of the IMX8QM
are identical.

Help review patches touching these areas.

---

v3:
 - New patch (Bertrand Marquis)
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1bd22fd75f..09982241b3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -337,6 +337,11 @@ F:	tools/misc/xenhypfs.c
 F:	xen/common/hypfs.c
 F:	xen/include/xen/hypfs.h
 
+IMX8QM/QXP SUPPORT
+R:	John Ernberg <john.ernberg@actia.se>
+F:	xen/arch/arm/platforms/imx8qm.c
+F:	xen/drivers/char/imx-lpuart.c
+
 INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
 R:	Lukasz Hawrylko <lukasz@hawrylko.pl>
 R:	Daniel P. Smith <dpsmith@apertussolutions.com>
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 0/2] Xen: ARM: Improved NXP iMX8 platform support
@ 2024-03-28 16:34 John Ernberg
  2024-03-28 16:34 ` [PATCH v3 2/3] xen/drivers: imx-lpuart: Replace iMX8QM compatible with iMX8QXP John Ernberg
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: John Ernberg @ 2024-03-28 16:34 UTC (permalink / raw)
  To: Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk
  Cc: Andrew Cooper, George Dunlap, Jan Beulich, xen-devel, Peng Fan,
	Jonas Blixt, John Ernberg

The iMX lpuart driver added at 44e17aa60d47 ("xen/arm: Add i.MX lpuart driver")
is not enough to boot a Linux based dom0 when certain drivers, such as the
watchdog driver, are enabled.

We're also fixing compatibles in imx-lpuart to allow Xen to use the UART
on the QXP variant as well.

When it comes to the watchdog we're currently only implementing the support
for letting Dom0 manage it. There is also a desire for another approach
where Xen kicks the watchdog (dom0-less use-cases etc). This approach is
not covered by this patch set.

NOTE: There is still an open point about the iMX8M* related SIP calls in
the driver and the value of documenting them. There was also a few other
calls noted that weren't implemented because Linux today aren't using them.
I added these as they were part of the discussion if they should be
documented or not.


v3: (see individual patches for detailed changelog)
 - Added a few more SIP calls
 - Become a reviewer of IMX8Q{M,XP} related patches (Bertrand Marquis)

v2: https://lore.kernel.org/xen-devel/20240214160644.3418228-1-john.ernberg@actia.se/
 - Added SIP call filtering (Julien Grall)
 - Replace lpuart compatible instead (Julien Grall)

v1: https://lore.kernel.org/xen-devel/20240131114952.305805-1-john.ernberg@actia.se

John Ernberg (3):
  xen/arm: Add imx8q{m,x} platform glue
  xen/drivers: imx-lpuart: Replace iMX8QM compatible with iMX8QXP
  MAINTAINERS: Become a reviewer of iMX8Q{M,XP} related patches

 MAINTAINERS                     |   5 +
 xen/arch/arm/platforms/Makefile |   1 +
 xen/arch/arm/platforms/imx8qm.c | 168 ++++++++++++++++++++++++++++++++
 xen/drivers/char/imx-lpuart.c   |   2 +-
 4 files changed, 175 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/arm/platforms/imx8qm.c

-- 
2.44.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue
  2024-03-28 16:34 ` [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue John Ernberg
@ 2024-04-02  7:24   ` Michal Orzel
  2024-04-08  7:13     ` John Ernberg
  0 siblings, 1 reply; 7+ messages in thread
From: Michal Orzel @ 2024-04-02  7:24 UTC (permalink / raw)
  To: John Ernberg, Julien Grall, Stefano Stabellini, Bertrand Marquis,
	Volodymyr Babchuk
  Cc: Andrew Cooper, George Dunlap, Jan Beulich, xen-devel, Peng Fan,
	Jonas Blixt

Hi John,

On 28/03/2024 17:34, John Ernberg wrote:
> 
> 
> When using Linux for dom0 there are a bunch of drivers that need to do SMC
> SIP calls into the firmware to enable certain hardware bits like the
> watchdog.
> 
> Provide a basic platform glue that implements the needed SMC forwarding.
> 
> The format of these calls are as follows:
>  - reg 0: function ID
>  - reg 1: subfunction ID (when there's a subfunction)
>  remaining regs: args
> 
> For now we only allow Dom0 to make these calls as they are all managing
> hardware. There is no specification for these SIP calls, the IDs and names
> have been extracted from the upstream linux kernel and the vendor kernel.
> 
> Most of the SIP calls are only available for the iMX8M series of SoCs, so
> they are easy to reject and they need to be revisited when iMX8M series
> support is added.
I just realized that you pinged me in v2 for clarification, sorry I missed that.
I still believe we shouldn't list FIDs that are meant for IMX8M, given that
the driver is written for IMX8QM/QXP.

> 
> From the other calls we can reject CPUFREQ because Dom0 cannot make an
> informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake
> up from suspend, which Xen doesn't support at this time.
> 
> This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which
> for now are allowed to Dom0.
> 
> NOTE: This code 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
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> [jernberg: Add SIP call filtering]
> Signed-off-by: John Ernberg <john.ernberg@actia.se>
> 
> ---
> 
> v3:
>  - Adhere to style guidelines for line length and label indentation (Michal Orzel)
>  - Use smccc macros to build the SIP function identifier (Michal Orzel)
>  - Adjust platform name to be specific to QM and QXP variants (Michal Orzel)
>  - Pick up additional SIPs which may be used by other Linux versions - for review purposes
>  - Changes to the commit message due to above
> 
> v2:
>  - Reword the commit message to be a bit clearer
>  - Include the link previously added as a context note to the commit message (Julien Grall)
>  - Add Pengs signed off (Julien Grall, Peng Fan)
>  - Add basic SIP call filter (Julien Grall)
>  - Expand the commit message a whole bunch because of the changes to the code
> ---
>  xen/arch/arm/platforms/Makefile |   1 +
>  xen/arch/arm/platforms/imx8qm.c | 168 ++++++++++++++++++++++++++++++++
>  2 files changed, 169 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..0992475474
> --- /dev/null
> +++ b/xen/arch/arm/platforms/imx8qm.c
> @@ -0,0 +1,168 @@
> +/* 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 <xen/sched.h>
> +#include <asm/platform.h>
> +#include <asm/smccc.h>
> +
> +static const char * const imx8qm_dt_compat[] __initconst =
> +{
> +    "fsl,imx8qm",
> +    "fsl,imx8qxp",
> +    NULL
> +};
> +
> +#define IMX_SIP_FID(fid) \
> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> +                       ARM_SMCCC_CONV_64, \
> +                       ARM_SMCCC_OWNER_SIP, \
> +                       fid)
macro parameter should be enclosed within parenthesis

> +
> +#define IMX_SIP_F_GPC            0x0000
> +#define IMX_SIP_F_CPUFREQ        0x0001
> +#define IMX_SIP_F_TIME           0x0002
> +#define IMX_SIP_F_BUILDINFO      0x0003
> +#define IMX_SIP_F_DDR_DVFS       0x0004
> +#define IMX_SIP_F_SRC            0x0005
> +#define IMX_SIP_F_GET_SOC_INFO   0x0006
> +#define IMX_SIP_F_NOC            0x0008
> +#define IMX_SIP_F_WAKEUP_SRC     0x0009
> +#define IMX_SIP_F_OTP_READ       0x000A
> +#define IMX_SIP_F_OTP_WRITE      0x000B
> +#define IMX_SIP_F_SET_TEMP_ALARM 0x000C
Is there specific reason for leading zeros?

> +
> +#define IMX_SIP_TIME_SF_RTC_SET_TIME     0x00
> +#define IMX_SIP_TIME_SF_WDOG_START       0x01
> +#define IMX_SIP_TIME_SF_WDOG_STOP        0x02
> +#define IMX_SIP_TIME_SF_WDOG_SET_ACT     0x03
> +#define IMX_SIP_TIME_SF_WDOG_PING        0x04
> +#define IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT 0x05
> +#define IMX_SIP_TIME_SF_WDOG_GET_STAT    0x06
> +#define IMX_SIP_TIME_SF_WDOG_SET_PRETIME 0x07
> +
> +static bool imx8qm_is_sip_time_call_ok(uint32_t subfunction_id)
> +{
> +    switch ( subfunction_id )
> +    {
> +    case IMX_SIP_TIME_SF_RTC_SET_TIME:
> +        return true;
> +    case IMX_SIP_TIME_SF_WDOG_START:
> +    case IMX_SIP_TIME_SF_WDOG_STOP:
> +    case IMX_SIP_TIME_SF_WDOG_SET_ACT:
> +    case IMX_SIP_TIME_SF_WDOG_PING:
> +    case IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT:
> +    case IMX_SIP_TIME_SF_WDOG_GET_STAT:
> +    case IMX_SIP_TIME_SF_WDOG_SET_PRETIME:
> +        return true;
> +    default:
> +        printk(XENLOG_WARNING "imx8qm: smc: time: Unknown subfunction id %x\n",
gprintk

> +               subfunction_id);
> +        return false;
> +    }
> +}
> +
> +static bool imx8qm_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 "no SMCCC 1.1 support. Disabling firmware calls\n");
NIT: you can move string within quotes to the next line to avoid exceeding 80 chars.
Also, all the other messages are prepended with "imx8qm: smc:" so better be consistent.

> +
> +        return false;
> +    }
> +
> +    /* Only hardware domain may use the SIP calls */
> +    if ( !is_hardware_domain(current->domain) )
> +    {
> +        gprintk(XENLOG_WARNING, "imx8qm: smc: No access\n");
Here you use gprintk, but ...

> +        return false;
> +    }
> +
> +    switch ( function_id )
> +    {
> +    /* Only available on imx8m series */
> +    case IMX_SIP_FID(IMX_SIP_F_GPC):
> +        return false;
> +    case IMX_SIP_FID(IMX_SIP_F_CPUFREQ):
> +        /* Hardware domain can't take any informed decision here */
> +        return false;
> +    case IMX_SIP_FID(IMX_SIP_F_BUILDINFO):
> +        goto allow_call;
> +    case IMX_SIP_FID(IMX_SIP_F_TIME):
> +        if ( imx8qm_is_sip_time_call_ok(subfunction_id) )
> +            goto allow_call;
> +        return false;
> +    /* Only available on imx8m series */
> +    case IMX_SIP_FID(IMX_SIP_F_DDR_DVFS):
> +        return false;
> +    /* Only available on imx8m series */
> +    case IMX_SIP_FID(IMX_SIP_F_SRC):
> +        return false;
> +    /* Only available on imx8m series */
> +    case IMX_SIP_FID(IMX_SIP_F_GET_SOC_INFO):
> +        return false;
> +    /* Only available on imx8m series */
> +    case IMX_SIP_FID(IMX_SIP_F_NOC):
> +        return false;
> +    /* Xen doesn't have suspend support */
> +    case IMX_SIP_FID(IMX_SIP_F_WAKEUP_SRC):
> +        return false;
> +    case IMX_SIP_FID(IMX_SIP_F_OTP_READ):
> +        /* subfunction_id is the fuse number, no sensible check possible */
> +        goto allow_call;
> +    case IMX_SIP_FID(IMX_SIP_F_OTP_WRITE):
> +        /* subfunction_id is the fuse number, no sensible check possible */
> +        goto allow_call;
> +    case IMX_SIP_FID(IMX_SIP_F_SET_TEMP_ALARM):
> +        goto allow_call;
> +    default:
> +        printk(XENLOG_WARNING "imx8qm: smc: Unknown function id %x\n",
... here you don't.

> +               function_id);
> +        return false;
> +    }
> +
> + allow_call:
> +    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;
> +}
> +
> +PLATFORM_START(imx8qm, "i.MX 8Q{M,XP}")
> +    .compatible = imx8qm_dt_compat,
> +    .smc = imx8qm_smc,
> +PLATFORM_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --
> 2.44.0


~Michal


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 3/3] MAINTAINERS: Become a reviewer of iMX8Q{M,XP} related patches
  2024-03-28 16:34 ` [PATCH v3 3/3] MAINTAINERS: Become a reviewer of iMX8Q{M,XP} related patches John Ernberg
@ 2024-04-05  0:23   ` Stefano Stabellini
  0 siblings, 0 replies; 7+ messages in thread
From: Stefano Stabellini @ 2024-04-05  0:23 UTC (permalink / raw)
  To: John Ernberg
  Cc: Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
	xen-devel, Peng Fan, Jonas Blixt

On Thu, 28 Mar 2024, John Ernberg wrote:
> I have experience with the IMX8QXP, and the supported parts of the IMX8QM
> are identical.
> 
> Help review patches touching these areas.

You need to add a signed-off-by. With that:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
> 
> v3:
>  - New patch (Bertrand Marquis)
> ---
>  MAINTAINERS | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1bd22fd75f..09982241b3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -337,6 +337,11 @@ F:	tools/misc/xenhypfs.c
>  F:	xen/common/hypfs.c
>  F:	xen/include/xen/hypfs.h
>  
> +IMX8QM/QXP SUPPORT
> +R:	John Ernberg <john.ernberg@actia.se>
> +F:	xen/arch/arm/platforms/imx8qm.c
> +F:	xen/drivers/char/imx-lpuart.c
> +
>  INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
>  R:	Lukasz Hawrylko <lukasz@hawrylko.pl>
>  R:	Daniel P. Smith <dpsmith@apertussolutions.com>
> -- 
> 2.44.0
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue
  2024-04-02  7:24   ` Michal Orzel
@ 2024-04-08  7:13     ` John Ernberg
  0 siblings, 0 replies; 7+ messages in thread
From: John Ernberg @ 2024-04-08  7:13 UTC (permalink / raw)
  To: Michal Orzel, Julien Grall, Stefano Stabellini, Bertrand Marquis,
	Volodymyr Babchuk
  Cc: Andrew Cooper, George Dunlap, Jan Beulich, xen-devel, Peng Fan,
	Jonas Blixt

Hi Michel,

Apologies for the slow reply.

On 4/2/24 9:24 AM, Michal Orzel wrote:
> Hi John,
> 
> On 28/03/2024 17:34, John Ernberg wrote:
>>
>>
>> When using Linux for dom0 there are a bunch of drivers that need to do SMC
>> SIP calls into the firmware to enable certain hardware bits like the
>> watchdog.
>>
>> Provide a basic platform glue that implements the needed SMC forwarding.
>>
>> The format of these calls are as follows:
>>   - reg 0: function ID
>>   - reg 1: subfunction ID (when there's a subfunction)
>>   remaining regs: args
>>
>> For now we only allow Dom0 to make these calls as they are all managing
>> hardware. There is no specification for these SIP calls, the IDs and names
>> have been extracted from the upstream linux kernel and the vendor kernel.
>>
>> Most of the SIP calls are only available for the iMX8M series of SoCs, so
>> they are easy to reject and they need to be revisited when iMX8M series
>> support is added.
> I just realized that you pinged me in v2 for clarification, sorry I missed that.
> I still believe we shouldn't list FIDs that are meant for IMX8M, given that
> the driver is written for IMX8QM/QXP.

Ack. Will take them all out and only add the known required set.

> 
>>
>>  From the other calls we can reject CPUFREQ because Dom0 cannot make an
>> informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake
>> up from suspend, which Xen doesn't support at this time.
>>
>> This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which
>> for now are allowed to Dom0.
>>
>> NOTE: This code 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
>>
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> [jernberg: Add SIP call filtering]
>> Signed-off-by: John Ernberg <john.ernberg@actia.se>
>>
>> ---
>>
>> v3:
>>   - Adhere to style guidelines for line length and label indentation (Michal Orzel)
>>   - Use smccc macros to build the SIP function identifier (Michal Orzel)
>>   - Adjust platform name to be specific to QM and QXP variants (Michal Orzel)
>>   - Pick up additional SIPs which may be used by other Linux versions - for review purposes
>>   - Changes to the commit message due to above
>>
>> v2:
>>   - Reword the commit message to be a bit clearer
>>   - Include the link previously added as a context note to the commit message (Julien Grall)
>>   - Add Pengs signed off (Julien Grall, Peng Fan)
>>   - Add basic SIP call filter (Julien Grall)
>>   - Expand the commit message a whole bunch because of the changes to the code
>> ---
>>   xen/arch/arm/platforms/Makefile |   1 +
>>   xen/arch/arm/platforms/imx8qm.c | 168 ++++++++++++++++++++++++++++++++
>>   2 files changed, 169 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..0992475474
>> --- /dev/null
>> +++ b/xen/arch/arm/platforms/imx8qm.c
>> @@ -0,0 +1,168 @@
>> +/* 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 <xen/sched.h>
>> +#include <asm/platform.h>
>> +#include <asm/smccc.h>
>> +
>> +static const char * const imx8qm_dt_compat[] __initconst =
>> +{
>> +    "fsl,imx8qm",
>> +    "fsl,imx8qxp",
>> +    NULL
>> +};
>> +
>> +#define IMX_SIP_FID(fid) \
>> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
>> +                       ARM_SMCCC_CONV_64, \
>> +                       ARM_SMCCC_OWNER_SIP, \
>> +                       fid)
> macro parameter should be enclosed within parenthesis

Ack.
> 
>> +
>> +#define IMX_SIP_F_GPC            0x0000
>> +#define IMX_SIP_F_CPUFREQ        0x0001
>> +#define IMX_SIP_F_TIME           0x0002
>> +#define IMX_SIP_F_BUILDINFO      0x0003
>> +#define IMX_SIP_F_DDR_DVFS       0x0004
>> +#define IMX_SIP_F_SRC            0x0005
>> +#define IMX_SIP_F_GET_SOC_INFO   0x0006
>> +#define IMX_SIP_F_NOC            0x0008
>> +#define IMX_SIP_F_WAKEUP_SRC     0x0009
>> +#define IMX_SIP_F_OTP_READ       0x000A
>> +#define IMX_SIP_F_OTP_WRITE      0x000B
>> +#define IMX_SIP_F_SET_TEMP_ALARM 0x000C
> Is there specific reason for leading zeros?

Nope. I'll drop them.
> 
>> +
>> +#define IMX_SIP_TIME_SF_RTC_SET_TIME     0x00
>> +#define IMX_SIP_TIME_SF_WDOG_START       0x01
>> +#define IMX_SIP_TIME_SF_WDOG_STOP        0x02
>> +#define IMX_SIP_TIME_SF_WDOG_SET_ACT     0x03
>> +#define IMX_SIP_TIME_SF_WDOG_PING        0x04
>> +#define IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT 0x05
>> +#define IMX_SIP_TIME_SF_WDOG_GET_STAT    0x06
>> +#define IMX_SIP_TIME_SF_WDOG_SET_PRETIME 0x07
>> +
>> +static bool imx8qm_is_sip_time_call_ok(uint32_t subfunction_id)
>> +{
>> +    switch ( subfunction_id )
>> +    {
>> +    case IMX_SIP_TIME_SF_RTC_SET_TIME:
>> +        return true;
>> +    case IMX_SIP_TIME_SF_WDOG_START:
>> +    case IMX_SIP_TIME_SF_WDOG_STOP:
>> +    case IMX_SIP_TIME_SF_WDOG_SET_ACT:
>> +    case IMX_SIP_TIME_SF_WDOG_PING:
>> +    case IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT:
>> +    case IMX_SIP_TIME_SF_WDOG_GET_STAT:
>> +    case IMX_SIP_TIME_SF_WDOG_SET_PRETIME:
>> +        return true;
>> +    default:
>> +        printk(XENLOG_WARNING "imx8qm: smc: time: Unknown subfunction id %x\n",
> gprintk

Ack.
> 
>> +               subfunction_id);
>> +        return false;
>> +    }
>> +}
>> +
>> +static bool imx8qm_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 "no SMCCC 1.1 support. Disabling firmware calls\n");
> NIT: you can move string within quotes to the next line to avoid exceeding 80 chars.
> Also, all the other messages are prepended with "imx8qm: smc:" so better be consistent.

Ack.
> 
>> +
>> +        return false;
>> +    }
>> +
>> +    /* Only hardware domain may use the SIP calls */
>> +    if ( !is_hardware_domain(current->domain) )
>> +    {
>> +        gprintk(XENLOG_WARNING, "imx8qm: smc: No access\n");
> Here you use gprintk, but ...
> 
>> +        return false;
>> +    }
>> +
>> +    switch ( function_id )
>> +    {
>> +    /* Only available on imx8m series */
>> +    case IMX_SIP_FID(IMX_SIP_F_GPC):
>> +        return false;
>> +    case IMX_SIP_FID(IMX_SIP_F_CPUFREQ):
>> +        /* Hardware domain can't take any informed decision here */
>> +        return false;
>> +    case IMX_SIP_FID(IMX_SIP_F_BUILDINFO):
>> +        goto allow_call;
>> +    case IMX_SIP_FID(IMX_SIP_F_TIME):
>> +        if ( imx8qm_is_sip_time_call_ok(subfunction_id) )
>> +            goto allow_call;
>> +        return false;
>> +    /* Only available on imx8m series */
>> +    case IMX_SIP_FID(IMX_SIP_F_DDR_DVFS):
>> +        return false;
>> +    /* Only available on imx8m series */
>> +    case IMX_SIP_FID(IMX_SIP_F_SRC):
>> +        return false;
>> +    /* Only available on imx8m series */
>> +    case IMX_SIP_FID(IMX_SIP_F_GET_SOC_INFO):
>> +        return false;
>> +    /* Only available on imx8m series */
>> +    case IMX_SIP_FID(IMX_SIP_F_NOC):
>> +        return false;
>> +    /* Xen doesn't have suspend support */
>> +    case IMX_SIP_FID(IMX_SIP_F_WAKEUP_SRC):
>> +        return false;
>> +    case IMX_SIP_FID(IMX_SIP_F_OTP_READ):
>> +        /* subfunction_id is the fuse number, no sensible check possible */
>> +        goto allow_call;
>> +    case IMX_SIP_FID(IMX_SIP_F_OTP_WRITE):
>> +        /* subfunction_id is the fuse number, no sensible check possible */
>> +        goto allow_call;
>> +    case IMX_SIP_FID(IMX_SIP_F_SET_TEMP_ALARM):
>> +        goto allow_call;
>> +    default:
>> +        printk(XENLOG_WARNING "imx8qm: smc: Unknown function id %x\n",
> ... here you don't.

Still getting used gprintk.
> 
>> +               function_id);
>> +        return false;
>> +    }
>> +
>> + allow_call:
>> +    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;
>> +}
>> +
>> +PLATFORM_START(imx8qm, "i.MX 8Q{M,XP}")
>> +    .compatible = imx8qm_dt_compat,
>> +    .smc = imx8qm_smc,
>> +PLATFORM_END
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> --
>> 2.44.0
> 
> 
> ~Michal

Thanks! // John Ernberg

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-04-08  7:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-28 16:34 [PATCH v3 0/2] Xen: ARM: Improved NXP iMX8 platform support John Ernberg
2024-03-28 16:34 ` [PATCH v3 2/3] xen/drivers: imx-lpuart: Replace iMX8QM compatible with iMX8QXP John Ernberg
2024-03-28 16:34 ` [PATCH v3 1/3] xen/arm: Add imx8q{m,x} platform glue John Ernberg
2024-04-02  7:24   ` Michal Orzel
2024-04-08  7:13     ` John Ernberg
2024-03-28 16:34 ` [PATCH v3 3/3] MAINTAINERS: Become a reviewer of iMX8Q{M,XP} related patches John Ernberg
2024-04-05  0:23   ` Stefano Stabellini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.