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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 12 Nov 2018, 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@xxxxxxxxxx>
> Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>
> 
> ---
> 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)
> +{
> +#ifdef CONFIG_ARM_64
> +    struct arm_smccc_res res;
> +
> +    /* 2nd argument (context ID) is not used */
> +    arm_smccc_smc(PSCI_1_0_FN64_SYSTEM_SUSPEND, __pa(hyp_resume), &res);
> +
> +    return PSCI_RET(res);
> +#else
> +    /* not supported */
> +    return 1;
> +#endif
> +}

Given that suspend is unimplemented on arm32, the #ifdef is OK. But
in that case return PSCI_NOT_SUPPORTED for arm32.

Otherwise you should be able to remove this #ifdef by introducing
something similar to PSCI_0_2_FN_NATIVE, but for PSCI_1_0 calls.


>  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();
> +    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);

I think it would be better to separate physical suspend from virtual
suspend, like Julien suggested. As you separate the C implementation,
please also introduce separate header files (for instance vsuspend.h and
suspend.h).


>  #endif
>  
> -- 
> 2.13.0
> 

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