Re: [PATCH 13/18] xen/arm: Implement PSCI SYSTEM_SUSPEND call (physical interface)

[Julien Grall <julien.grall@arm.com>]

Hi,

On 11/12/18 11:30 AM, Mirela Simonovic wrote:
> PSCI system suspend function shall be invoked to finalize Xen suspend
> procedure. Resume entry point, which needs to be passed via 1st argument
> of PSCI system suspend call to the EL3, is hyp_resume. For now, hyp_resume
> is just a placeholder that will be implemented in assembly. Context ID,
> which is 2nd argument of system suspend PSCI call, is unused, as in Linux.
> 
> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
> Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xilinx.com>
> 
> ---
> Changes in v2:
> 
> -The commit message was stale - referring to the do_suspend function
> that has been renamed long time ago. Fixed commit message
> ---
>   xen/arch/arm/arm64/entry.S    |  3 +++
>   xen/arch/arm/psci.c           | 16 ++++++++++++++++
>   xen/arch/arm/suspend.c        |  4 ++++
>   xen/include/asm-arm/psci.h    |  1 +
>   xen/include/asm-arm/suspend.h |  1 +
>   5 files changed, 25 insertions(+)
> 
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 97b05f53ea..dbc4717903 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -533,6 +533,9 @@ ENTRY(__context_switch)
>           mov     sp, x9
>           ret
>   
> +ENTRY(hyp_resume)
> +        b .
> +
>   /*
>    * Local variables:
>    * mode: ASM
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index a93121f43b..b100bd8ad2 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -24,6 +24,7 @@
>   #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
> @@ -67,6 +68,21 @@ void call_psci_cpu_off(void)
>       }
>   }
>   
> +int call_psci_system_suspend(void)
> +{

SYSTEM_SUSPEND was introduced by PSCI 1.0 and optional. So you need to 
check the PSCI version and use PSCI_FEATURES to check if it was implemented.

> +#ifdef CONFIG_ARM_64
> +    struct arm_smccc_res res;
> +
> +    /* 2nd argument (context ID) is not used */

It still needs to be defined to some known values rather than whatever 
is in x2 at that time.

But I would suggest to make good use of it to catch implementation not 
doing the right thing. We could define it to 0xdeadbeef and shout at 
anyone not preserving the value.

> +    arm_smccc_smc(PSCI_1_0_FN64_SYSTEM_SUSPEND, __pa(hyp_resume), &res);
> +
> +    return PSCI_RET(res);
> +#else
> +    /* not supported */
> +    return 1;
> +#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 d1b48c339a..37926374bc 100644
> --- a/xen/arch/arm/suspend.c
> +++ b/xen/arch/arm/suspend.c
> @@ -141,6 +141,10 @@ static long system_suspend(void *data)
>           goto resume_irqs;
>       }
>   
> +    status = call_psci_system_suspend();

Some platform may not support PSCI at all. So this need to be check.

> +    if ( status )
> +        dprintk(XENLOG_ERR, "PSCI system suspend failed, err=%d\n", status);
> +
>       system_state = SYS_STATE_resume;
>   
>       gic_resume();
> diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
> index 26462d0c47..9f6116a224 100644
> --- a/xen/include/asm-arm/psci.h
> +++ b/xen/include/asm-arm/psci.h
> @@ -20,6 +20,7 @@ extern uint32_t psci_ver;
>   
>   int psci_init(void);
>   int call_psci_cpu_on(int cpu);
> +int call_psci_system_suspend(void);
>   void call_psci_cpu_off(void);
>   void call_psci_system_off(void);
>   void call_psci_system_reset(void);
> diff --git a/xen/include/asm-arm/suspend.h b/xen/include/asm-arm/suspend.h
> index de787d296a..7604e2e2e2 100644
> --- a/xen/include/asm-arm/suspend.h
> +++ b/xen/include/asm-arm/suspend.h
> @@ -2,6 +2,7 @@
>   #define __ASM_ARM_SUSPEND_H__
>   
>   int32_t domain_suspend(register_t epoint, register_t cid);
> +void hyp_resume(void);
>   
>   #endif
>   
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel