[PATCH v5 08/12] xen/arm: Implement PSCI SYSTEM_SUSPEND call (host interface)

Mykola Kvach posted 12 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v5 08/12] xen/arm: Implement PSCI SYSTEM_SUSPEND call (host interface)
Posted by Mykola Kvach 2 months, 2 weeks ago
From: Mirela Simonovic <mirela.simonovic@aggios.com>

Invoke PSCI SYSTEM_SUSPEND to finalize Xen's suspend sequence on ARM64 platforms.
Pass the resume entry point (hyp_resume) as the first argument to EL3. The resume
handler is currently a stub and will be implemented later in assembly. Ignore the
context ID argument, as is done in Linux.

Only enable this path when CONFIG_SYSTEM_SUSPEND is set and
PSCI version is >= 1.0.

Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Changes in v4:
  - select the appropriate PSCI SYSTEM_SUSPEND function ID based on platform
  - update comments and commit message to reflect recent changes

Changes in v3:
  - return PSCI_NOT_SUPPORTED instead of a hardcoded 1 on ARM32
  - check PSCI version before invoking SYSTEM_SUSPEND in call_psci_system_suspend
---
 xen/arch/arm/arm64/head.S          |  8 ++++++++
 xen/arch/arm/include/asm/psci.h    |  1 +
 xen/arch/arm/include/asm/suspend.h |  2 ++
 xen/arch/arm/psci.c                | 23 ++++++++++++++++++++++-
 xen/arch/arm/suspend.c             |  5 +++++
 5 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 72c7b24498..3522c497c5 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -561,6 +561,14 @@ END(efi_xen_start)
 
 #endif /* CONFIG_ARM_EFI */
 
+#ifdef CONFIG_SYSTEM_SUSPEND
+
+FUNC(hyp_resume)
+        b .
+END(hyp_resume)
+
+#endif /* CONFIG_SYSTEM_SUSPEND */
+
 /*
  * Local variables:
  * mode: ASM
diff --git a/xen/arch/arm/include/asm/psci.h b/xen/arch/arm/include/asm/psci.h
index 48a93e6b79..bb3c73496e 100644
--- a/xen/arch/arm/include/asm/psci.h
+++ b/xen/arch/arm/include/asm/psci.h
@@ -23,6 +23,7 @@ int call_psci_cpu_on(int cpu);
 void call_psci_cpu_off(void);
 void call_psci_system_off(void);
 void call_psci_system_reset(void);
+int call_psci_system_suspend(void);
 
 /* Range of allocated PSCI function numbers */
 #define	PSCI_FNUM_MIN_VALUE                 _AC(0,U)
diff --git a/xen/arch/arm/include/asm/suspend.h b/xen/arch/arm/include/asm/suspend.h
index 78d0e2383b..55041a5d06 100644
--- a/xen/arch/arm/include/asm/suspend.h
+++ b/xen/arch/arm/include/asm/suspend.h
@@ -7,6 +7,8 @@
 
 int host_system_suspend(void);
 
+void hyp_resume(void);
+
 #endif /* CONFIG_SYSTEM_SUSPEND */
 
 #endif /* __ASM_ARM_SUSPEND_H__ */
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index b6860a7760..c9d126b195 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -17,17 +17,20 @@
 #include <asm/cpufeature.h>
 #include <asm/psci.h>
 #include <asm/acpi.h>
+#include <asm/suspend.h>
 
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for
  * some calls it is necessary to use SMC64 to pass or return 64-bit values.
- * For such calls PSCI_0_2_FN_NATIVE(x) will choose the appropriate
+ * For such calls PSCI_*_FN_NATIVE(x) will choose the appropriate
  * (native-width) function ID.
  */
 #ifdef CONFIG_ARM_64
 #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64_##name
+#define PSCI_1_0_FN_NATIVE(name)    PSCI_1_0_FN64_##name
 #else
 #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32_##name
+#define PSCI_1_0_FN_NATIVE(name)    PSCI_1_0_FN32_##name
 #endif
 
 uint32_t psci_ver;
@@ -60,6 +63,24 @@ void call_psci_cpu_off(void)
     }
 }
 
+int call_psci_system_suspend(void)
+{
+#ifdef CONFIG_SYSTEM_SUSPEND
+    struct arm_smccc_res res;
+
+    if ( psci_ver < PSCI_VERSION(1, 0) )
+        return PSCI_NOT_SUPPORTED;
+
+    /* 2nd argument (context ID) is not used */
+    arm_smccc_smc(PSCI_1_0_FN_NATIVE(SYSTEM_SUSPEND), __pa(hyp_resume), &res);
+    return PSCI_RET(res);
+#else
+    dprintk(XENLOG_WARNING,
+            "SYSTEM_SUSPEND not supported (CONFIG_SYSTEM_SUSPEND disabled)\n");
+    return PSCI_NOT_SUPPORTED;
+#endif
+}
+
 void call_psci_system_off(void)
 {
     if ( psci_ver > PSCI_VERSION(0, 1) )
diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
index abf4b928ce..11e86b7f51 100644
--- a/xen/arch/arm/suspend.c
+++ b/xen/arch/arm/suspend.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#include <asm/psci.h>
 #include <xen/console.h>
 #include <xen/cpu.h>
 #include <xen/llc-coloring.h>
@@ -70,6 +71,10 @@ static long system_suspend(void *data)
      */
     update_boot_mapping(true);
 
+    status = call_psci_system_suspend();
+    if ( status )
+        dprintk(XENLOG_WARNING, "PSCI system suspend failed, err=%d\n", status);
+
     system_state = SYS_STATE_resume;
     update_boot_mapping(false);
 
-- 
2.48.1
Re: [PATCH v5 08/12] xen/arm: Implement PSCI SYSTEM_SUSPEND call (host interface)
Posted by Volodymyr Babchuk 2 months, 1 week ago
Hi Mykola,

Sequence of next 3 patches (and previous one) really puzzles me. Can you
first implement hyp_resume() function, then add PSCI_SYSTEM_SUSPEND call
and only then implement system_suspend() function? Why do this backwards?

Mykola Kvach <xakep.amatop@gmail.com> writes:

> From: Mirela Simonovic <mirela.simonovic@aggios.com>
>
> Invoke PSCI SYSTEM_SUSPEND to finalize Xen's suspend sequence on ARM64 platforms.
> Pass the resume entry point (hyp_resume) as the first argument to EL3. The resume
> handler is currently a stub and will be implemented later in assembly. Ignore the
> context ID argument, as is done in Linux.
>
> Only enable this path when CONFIG_SYSTEM_SUSPEND is set and
> PSCI version is >= 1.0.
>
> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
> Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> Changes in v4:
>   - select the appropriate PSCI SYSTEM_SUSPEND function ID based on platform
>   - update comments and commit message to reflect recent changes
>
> Changes in v3:
>   - return PSCI_NOT_SUPPORTED instead of a hardcoded 1 on ARM32
>   - check PSCI version before invoking SYSTEM_SUSPEND in call_psci_system_suspend
> ---
>  xen/arch/arm/arm64/head.S          |  8 ++++++++
>  xen/arch/arm/include/asm/psci.h    |  1 +
>  xen/arch/arm/include/asm/suspend.h |  2 ++
>  xen/arch/arm/psci.c                | 23 ++++++++++++++++++++++-
>  xen/arch/arm/suspend.c             |  5 +++++
>  5 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 72c7b24498..3522c497c5 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -561,6 +561,14 @@ END(efi_xen_start)
>  
>  #endif /* CONFIG_ARM_EFI */
>  
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +
> +FUNC(hyp_resume)
> +        b .
> +END(hyp_resume)
> +
> +#endif /* CONFIG_SYSTEM_SUSPEND */
> +
>  /*
>   * Local variables:
>   * mode: ASM
> diff --git a/xen/arch/arm/include/asm/psci.h b/xen/arch/arm/include/asm/psci.h
> index 48a93e6b79..bb3c73496e 100644
> --- a/xen/arch/arm/include/asm/psci.h
> +++ b/xen/arch/arm/include/asm/psci.h
> @@ -23,6 +23,7 @@ int call_psci_cpu_on(int cpu);
>  void call_psci_cpu_off(void);
>  void call_psci_system_off(void);
>  void call_psci_system_reset(void);
> +int call_psci_system_suspend(void);
>  
>  /* Range of allocated PSCI function numbers */
>  #define	PSCI_FNUM_MIN_VALUE                 _AC(0,U)
> diff --git a/xen/arch/arm/include/asm/suspend.h b/xen/arch/arm/include/asm/suspend.h
> index 78d0e2383b..55041a5d06 100644
> --- a/xen/arch/arm/include/asm/suspend.h
> +++ b/xen/arch/arm/include/asm/suspend.h
> @@ -7,6 +7,8 @@
>  
>  int host_system_suspend(void);
>  
> +void hyp_resume(void);
> +
>  #endif /* CONFIG_SYSTEM_SUSPEND */
>  
>  #endif /* __ASM_ARM_SUSPEND_H__ */
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index b6860a7760..c9d126b195 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -17,17 +17,20 @@
>  #include <asm/cpufeature.h>
>  #include <asm/psci.h>
>  #include <asm/acpi.h>
> +#include <asm/suspend.h>
>  
>  /*
>   * While a 64-bit OS can make calls with SMC32 calling conventions, for
>   * some calls it is necessary to use SMC64 to pass or return 64-bit values.
> - * For such calls PSCI_0_2_FN_NATIVE(x) will choose the appropriate
> + * For such calls PSCI_*_FN_NATIVE(x) will choose the appropriate
>   * (native-width) function ID.
>   */
>  #ifdef CONFIG_ARM_64
>  #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64_##name
> +#define PSCI_1_0_FN_NATIVE(name)    PSCI_1_0_FN64_##name
>  #else
>  #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32_##name
> +#define PSCI_1_0_FN_NATIVE(name)    PSCI_1_0_FN32_##name
>  #endif
>  
>  uint32_t psci_ver;
> @@ -60,6 +63,24 @@ void call_psci_cpu_off(void)
>      }
>  }
>  
> +int call_psci_system_suspend(void)
> +{
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +    struct arm_smccc_res res;
> +
> +    if ( psci_ver < PSCI_VERSION(1, 0) )
> +        return PSCI_NOT_SUPPORTED;
> +
> +    /* 2nd argument (context ID) is not used */
> +    arm_smccc_smc(PSCI_1_0_FN_NATIVE(SYSTEM_SUSPEND), __pa(hyp_resume), &res);
> +    return PSCI_RET(res);
> +#else
> +    dprintk(XENLOG_WARNING,
> +            "SYSTEM_SUSPEND not supported (CONFIG_SYSTEM_SUSPEND disabled)\n");
> +    return PSCI_NOT_SUPPORTED;
> +#endif
> +}
> +
>  void call_psci_system_off(void)
>  {
>      if ( psci_ver > PSCI_VERSION(0, 1) )
> diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
> index abf4b928ce..11e86b7f51 100644
> --- a/xen/arch/arm/suspend.c
> +++ b/xen/arch/arm/suspend.c
> @@ -1,5 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  
> +#include <asm/psci.h>
>  #include <xen/console.h>
>  #include <xen/cpu.h>
>  #include <xen/llc-coloring.h>
> @@ -70,6 +71,10 @@ static long system_suspend(void *data)
>       */
>      update_boot_mapping(true);
>  
> +    status = call_psci_system_suspend();
> +    if ( status )
> +        dprintk(XENLOG_WARNING, "PSCI system suspend failed, err=%d\n", status);

So this is where missing call to PSCI_SYSTEM_SUSPEND is... 

> +
>      system_state = SYS_STATE_resume;
>      update_boot_mapping(false);

-- 
WBR, Volodymyr
Re: [PATCH v5 08/12] xen/arm: Implement PSCI SYSTEM_SUSPEND call (host interface)
Posted by Mykola Kvach 2 months ago
Hi Volodymyr,

On Sat, Aug 23, 2025 at 4:06 AM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
>
> Hi Mykola,
>
> Sequence of next 3 patches (and previous one) really puzzles me. Can you
> first implement hyp_resume() function, then add PSCI_SYSTEM_SUSPEND call
> and only then implement system_suspend() function? Why do this backwards?

This order comes from the first version of the patch series.
I'll reorder the commits in the next version.

>
> Mykola Kvach <xakep.amatop@gmail.com> writes:
>
> > From: Mirela Simonovic <mirela.simonovic@aggios.com>
> >
> > Invoke PSCI SYSTEM_SUSPEND to finalize Xen's suspend sequence on ARM64 platforms.
> > Pass the resume entry point (hyp_resume) as the first argument to EL3. The resume
> > handler is currently a stub and will be implemented later in assembly. Ignore the
> > context ID argument, as is done in Linux.
> >
> > Only enable this path when CONFIG_SYSTEM_SUSPEND is set and
> > PSCI version is >= 1.0.
> >
> > Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
> > Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com>
> > Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > Changes in v4:
> >   - select the appropriate PSCI SYSTEM_SUSPEND function ID based on platform
> >   - update comments and commit message to reflect recent changes
> >
> > Changes in v3:
> >   - return PSCI_NOT_SUPPORTED instead of a hardcoded 1 on ARM32
> >   - check PSCI version before invoking SYSTEM_SUSPEND in call_psci_system_suspend
> > ---
> >  xen/arch/arm/arm64/head.S          |  8 ++++++++
> >  xen/arch/arm/include/asm/psci.h    |  1 +
> >  xen/arch/arm/include/asm/suspend.h |  2 ++
> >  xen/arch/arm/psci.c                | 23 ++++++++++++++++++++++-
> >  xen/arch/arm/suspend.c             |  5 +++++
> >  5 files changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index 72c7b24498..3522c497c5 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -561,6 +561,14 @@ END(efi_xen_start)
> >
> >  #endif /* CONFIG_ARM_EFI */
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +
> > +FUNC(hyp_resume)
> > +        b .
> > +END(hyp_resume)
> > +
> > +#endif /* CONFIG_SYSTEM_SUSPEND */
> > +
> >  /*
> >   * Local variables:
> >   * mode: ASM
> > diff --git a/xen/arch/arm/include/asm/psci.h b/xen/arch/arm/include/asm/psci.h
> > index 48a93e6b79..bb3c73496e 100644
> > --- a/xen/arch/arm/include/asm/psci.h
> > +++ b/xen/arch/arm/include/asm/psci.h
> > @@ -23,6 +23,7 @@ int call_psci_cpu_on(int cpu);
> >  void call_psci_cpu_off(void);
> >  void call_psci_system_off(void);
> >  void call_psci_system_reset(void);
> > +int call_psci_system_suspend(void);
> >
> >  /* Range of allocated PSCI function numbers */
> >  #define      PSCI_FNUM_MIN_VALUE                 _AC(0,U)
> > diff --git a/xen/arch/arm/include/asm/suspend.h b/xen/arch/arm/include/asm/suspend.h
> > index 78d0e2383b..55041a5d06 100644
> > --- a/xen/arch/arm/include/asm/suspend.h
> > +++ b/xen/arch/arm/include/asm/suspend.h
> > @@ -7,6 +7,8 @@
> >
> >  int host_system_suspend(void);
> >
> > +void hyp_resume(void);
> > +
> >  #endif /* CONFIG_SYSTEM_SUSPEND */
> >
> >  #endif /* __ASM_ARM_SUSPEND_H__ */
> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> > index b6860a7760..c9d126b195 100644
> > --- a/xen/arch/arm/psci.c
> > +++ b/xen/arch/arm/psci.c
> > @@ -17,17 +17,20 @@
> >  #include <asm/cpufeature.h>
> >  #include <asm/psci.h>
> >  #include <asm/acpi.h>
> > +#include <asm/suspend.h>
> >
> >  /*
> >   * While a 64-bit OS can make calls with SMC32 calling conventions, for
> >   * some calls it is necessary to use SMC64 to pass or return 64-bit values.
> > - * For such calls PSCI_0_2_FN_NATIVE(x) will choose the appropriate
> > + * For such calls PSCI_*_FN_NATIVE(x) will choose the appropriate
> >   * (native-width) function ID.
> >   */
> >  #ifdef CONFIG_ARM_64
> >  #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64_##name
> > +#define PSCI_1_0_FN_NATIVE(name)    PSCI_1_0_FN64_##name
> >  #else
> >  #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32_##name
> > +#define PSCI_1_0_FN_NATIVE(name)    PSCI_1_0_FN32_##name
> >  #endif
> >
> >  uint32_t psci_ver;
> > @@ -60,6 +63,24 @@ void call_psci_cpu_off(void)
> >      }
> >  }
> >
> > +int call_psci_system_suspend(void)
> > +{
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +    struct arm_smccc_res res;
> > +
> > +    if ( psci_ver < PSCI_VERSION(1, 0) )
> > +        return PSCI_NOT_SUPPORTED;
> > +
> > +    /* 2nd argument (context ID) is not used */
> > +    arm_smccc_smc(PSCI_1_0_FN_NATIVE(SYSTEM_SUSPEND), __pa(hyp_resume), &res);
> > +    return PSCI_RET(res);
> > +#else
> > +    dprintk(XENLOG_WARNING,
> > +            "SYSTEM_SUSPEND not supported (CONFIG_SYSTEM_SUSPEND disabled)\n");
> > +    return PSCI_NOT_SUPPORTED;
> > +#endif
> > +}
> > +
> >  void call_psci_system_off(void)
> >  {
> >      if ( psci_ver > PSCI_VERSION(0, 1) )
> > diff --git a/xen/arch/arm/suspend.c b/xen/arch/arm/suspend.c
> > index abf4b928ce..11e86b7f51 100644
> > --- a/xen/arch/arm/suspend.c
> > +++ b/xen/arch/arm/suspend.c
> > @@ -1,5 +1,6 @@
> >  /* SPDX-License-Identifier: GPL-2.0-only */
> >
> > +#include <asm/psci.h>
> >  #include <xen/console.h>
> >  #include <xen/cpu.h>
> >  #include <xen/llc-coloring.h>
> > @@ -70,6 +71,10 @@ static long system_suspend(void *data)
> >       */
> >      update_boot_mapping(true);
> >
> > +    status = call_psci_system_suspend();
> > +    if ( status )
> > +        dprintk(XENLOG_WARNING, "PSCI system suspend failed, err=%d\n", status);
>
> So this is where missing call to PSCI_SYSTEM_SUSPEND is...
>
> > +
> >      system_state = SYS_STATE_resume;
> >      update_boot_mapping(false);
>
> --
> WBR, Volodymyr

Best regards,
Mykola