Hi Volodymyr,
On Sat, Aug 23, 2025 at 4:06 AM Volodymyr Babchuk
<volodymyr_babc...@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.ama...@gmail.com> writes:
>
> > From: Mirela Simonovic <mirela.simono...@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.simono...@aggios.com>
> > Signed-off-by: Saeed Nowshadi <saeed.nowsh...@xilinx.com>
> > Signed-off-by: Mykyta Poturai <mykyta_potu...@epam.com>
> > Signed-off-by: Mykola Kvach <mykola_kv...@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
