Re: [PATCH v8 12/13] xen/arm: Implement PSCI SYSTEM_SUSPEND call (host interface)

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Luca,

Thank you for the review.

On Mon, Apr 27, 2026 at 7:23 PM Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
>
> Hi Mykola,
>
> > On 2 Apr 2026, at 11:45, Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
> >
> > From: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>
> > Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> > Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> > ---
> > Changes in v7:
> > - no changes
> > ---
> > xen/arch/arm/include/asm/psci.h |  1 +
> > xen/arch/arm/psci.c             | 23 ++++++++++++++++++++++-
> > 2 files changed, 23 insertions(+), 1 deletion(-)
> >
> > 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/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);
>
> I think Linux is passing 0 as context ID, probably to mark that it’s not 
> used, I think we should do the
> same

Yes, agreed.

SYSTEM_SUSPEND takes context_id as the second PSCI argument, and Xen does
not use it. I will pass it explicitly as 0 instead of relying on the SMCCC
wrapper/default register state.

Best regards,
Mykola

>
> > +    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) )
> >
>
> Cheers,
> Luca
>
>
>