[U-Boot,5/8] ARMv8: PCSI: Add generic ARMv8 PSCI code

Implement core support for PSCI. As this is generic code, it doesn't
implement anything really useful (all the functions are returning
Not Implemented).

This is largely ported from the similar code that exists for ARMv7

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/cpu/armv8/Makefile |    1 +
 arch/arm/cpu/armv8/psci.S   |  171 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/cpu/armv8/psci.S

Hi Arnab,

On Wed, Aug 27, 2014 at 09:29:58PM +0100, Arnab Basu wrote:
> Implement core support for PSCI. As this is generic code, it doesn't
> implement anything really useful (all the functions are returning
> Not Implemented).

This is really nice to see! Thanks for working on this.

Some functions which return NOT_IMPLEMENTED below are requried to be
implemented per the PSCI 0.2 spec.

I hope that the plan is to implement all the required functions before
we turn this on?

Otherwise, comments below.

> This is largely ported from the similar code that exists for ARMv7
> 
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/cpu/armv8/Makefile |    1 +
>  arch/arm/cpu/armv8/psci.S   |  171 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 172 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/cpu/armv8/psci.S
> 
> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> index 4f0ea87..8f6988d 100644
> --- a/arch/arm/cpu/armv8/Makefile
> +++ b/arch/arm/cpu/armv8/Makefile
> @@ -15,3 +15,4 @@ obj-y	+= cache.o
>  obj-y	+= tlb.o
>  obj-y	+= transition.o
>  obj-y	+= cpu-dt.o
> +obj-$(CONFIG_ARMV8_PSCI)	+= psci.o
> diff --git a/arch/arm/cpu/armv8/psci.S b/arch/arm/cpu/armv8/psci.S
> new file mode 100644
> index 0000000..5f4e3b2
> --- /dev/null
> +++ b/arch/arm/cpu/armv8/psci.S
> @@ -0,0 +1,171 @@
> +/*
> + * (C) Copyright 2014
> + * Arnab Basu <arnab.basu@freescale.com>
> + *
> + * Based on arch/arm/cpu/armv7/psci.S
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/psci.h>
> +
> +.pushsection ._secure.text, "ax"
> +
> +ENTRY(psci_0_2_cpu_suspend_64)
> +ENTRY(psci_0_2_cpu_on_64)
> +ENTRY(psci_0_2_affinity_info_64)
> +ENTRY(psci_0_2_migrate_64)
> +ENTRY(psci_0_2_migrate_info_up_cpu_64)
> +	mov	x0, #ARM_PSCI_RET_NI	/* Return -1 (Not Implemented) */
> +	ret
> +ENDPROC(psci_0_2_cpu_suspend_64)
> +ENDPROC(psci_0_2_cpu_on_64)
> +ENDPROC(psci_0_2_affinity_info_64)
> +ENDPROC(psci_0_2_migrate_64)
> +ENDPROC(psci_0_2_migrate_info_up_cpu_64)
> +.weak psci_0_2_cpu_suspend_64
> +.weak psci_0_2_cpu_on_64
> +.weak psci_0_2_affinity_info_64
> +.weak psci_0_2_migrate_64
> +.weak psci_0_2_migrate_info_up_cpu_64
> +
> +ENTRY(psci_0_2_psci_version)
> +	mov	x0, #2			/* Return Major = 0, Minor = 2*/
> +	ret
> +ENDPROC(psci_0_2_psci_version)
> +
> +.align 4
> +_psci_0_2_table:
> +	.quad	PSCI_0_2_FN_PSCI_VERSION
> +	.quad	psci_0_2_psci_version
> +	.quad	PSCI_0_2_FN64_CPU_SUSPEND
> +	.quad	psci_0_2_cpu_suspend_64
> +	.quad	PSCI_0_2_FN64_CPU_ON
> +	.quad	psci_0_2_cpu_on_64
> +	.quad	PSCI_0_2_FN64_AFFINITY_INFO
> +	.quad	psci_0_2_affinity_info_64
> +	.quad	PSCI_0_2_FN64_MIGRATE
> +	.quad	psci_0_2_migrate_64
> +	.quad	PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU
> +	.quad	psci_0_2_migrate_info_up_cpu_64
> +	.quad	0
> +	.quad	0

It would be nice if we could reorganise this something like:

	.quad PSCI_0_2_FN_PSCI_VERSION,		psci_0_2_psci_version
	.quad PSCI_0_2_FN64_CPU_SUSPEND,	psci_0_2_cpu_suspend_64
	.quad PSCI_0_2_FN64_CPU_ON,		psci_0_2_cpu_on_64
	.quad PSCI_0_2_FN64_AFFINITY_INFO,	psci_0_2_affinity_info_64
	.quad PSCI_0_2_FN64_MIGRATE,		psci_0_2_migrate_64
	.quad PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU,	psci_0_2_migrate_info_up_cpu_64
	.quad 0, 				0

As that would make the relationship between IDs and functions clearer
(at least to me). Maybe a macro could make this less painful.

> +.macro	psci_enter
> +	stp	x29, x30, [sp, #-16]!
> +	stp	x27, x28, [sp, #-16]!
> +	stp	x25, x26, [sp, #-16]!
> +	stp	x23, x24, [sp, #-16]!
> +	stp	x21, x22, [sp, #-16]!
> +	stp	x19, x20, [sp, #-16]!
> +	stp	x17, x18, [sp, #-16]!
> +	stp	x15, x16, [sp, #-16]!
> +	stp	x13, x14, [sp, #-16]!
> +	stp	x11, x12, [sp, #-16]!
> +	stp	x9, x10, [sp, #-16]!
> +	stp	x7, x8, [sp, #-16]!
> +	stp	x5, x6, [sp, #-16]!
> +	mrs	x5, elr_el3
> +	stp	x5, x4, [sp, #-16]!
> +
> +	/* EL0 and El1 will execute in secure */

I think this would be better as:

	/* U-Boot will run on the secure side */

> +	mrs	x4, scr_el3
> +	bic	x4, x4, #1
> +	msr	scr_el3, x4
> +.endm
> +
> +.macro	psci_return
> +	/* EL0 and El1 will execute in non-secure */

Similarly:

	/* The OS will run on the non-secure side */

> +	mrs	x4, scr_el3
> +	orr	x4, x4, #1
> +	msr	scr_el3, x4
> +
> +	ldp	x5, x4, [sp], #16
> +	msr	elr_el3, x5
> +	ldp	x5, x6, [sp], #16
> +	ldp	x7, x8, [sp], #16
> +	ldp	x9, x10, [sp], #16
> +	ldp	x11, x12, [sp], #16
> +	ldp	x13, x14, [sp], #16
> +	ldp	x15, x16, [sp], #16
> +	ldp	x17, x18, [sp], #16
> +	ldp	x19, x20, [sp], #16
> +	ldp	x21, x22, [sp], #16
> +	ldp	x23, x24, [sp], #16
> +	ldp	x25, x26, [sp], #16
> +	ldp	x27, x28, [sp], #16
> +	ldp	x29, x30, [sp], #16
> +	eret
> +.endm

PSCI 0.2 follows the SMC calling convention, where x0-x17 are
caller-saved as with the usual AArch64 calling conventions, so we don't
need to save/restore them here for AArch64 callers.

We do need to ensure we don't clobber x18-x30, sp_elx and sp_el0. The
calling convention is ambiguous as to whether we can clobber sp_el1, but
for the moment I'd assume we shouldn't.

For AArch32 callers, only x0-x3 are caller-saved.

> +ENTRY(_smc_psci)
> +	psci_enter
> +	adr	x4, _psci_0_2_table
> +1:	ldr	x5, [x4]		/* Load PSCI function ID */
> +	ldr	x6, [x4, #8]		/* Load target PC */

1:	ldp	x5, x6, [x4]		/* Load function ID and handler addr */

> +	cmp	x5, #0			/* If reach the end, bail out */
> +	b.eq	fn_not_found

cbz	x5, fn_not_found		/* If reach the end, bail out */

> +	cmp	x0, x5			/* If not matching, try next entry */
> +	b.eq	fn_call
> +	add	x4, x4, #16
> +	b	1b
> +
> +fn_call:
> +	blr	x6
> +	psci_return
> +
> +fn_not_found:
> +	mov	x0, #ARM_PSCI_RET_INVAL  /* Return -2 (Invalid) */

I believe this should be NOT_SUPPORTED (-1), which states that the PSCI
implementation doesn't implement this function rather than the arguments
to the function were invalid.

> +	psci_return
> +ENDPROC(_smc_psci)
> +
> +ENTRY(default_psci_vector)
> +	eret
> +ENDPROC(default_psci_vector)

This looks misnamed. This is the vector for exceptions triggered by
something other than PSCI.

That said I think we need to do something else here. If we haven't
handled the exception we're likely to just return to whatever caused it
and take the same exception again.

A failstop behaviour would seem better if the only other exceptions we
expect to take are fatal anyway.

> +.align 2

I don't think this is necessary; we had code immediately before.

> +__handle_sync:
> +	str 	x4, [sp, #-8]!
> +	mrs	x4, esr_el3
> +	ubfx	x4, x4, #26, #6
> +	cmp	x4, #0x17

Could we not have a macro like ESR_EC_SMC64 instead of the #17?

Thanks,
Mark.

> +	b.eq	smc_found
> +	ldr	x4, [sp], #8
> +	b	default_psci_vector
> +smc_found:
> +	ldr     x4, [sp], #8
> +	b	_smc_psci
> +
> +/*
> + * PSCI Exception vectors.
> + */
> +	.align	11
> +	.globl	psci_vectors
> +psci_vectors:
> +	.align	7
> +	b	default_psci_vector	/* Current EL Synchronous Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL IRQ Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL FIQ Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL Error Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL Synchronous Handler */
> +	.align	7
> +	b	default_psci_vector	/* Current EL IRQ Handler */
> +	.align	7
> +	b	default_psci_vector	/* Current EL FIQ Handler */
> +	.align	7
> +	b	default_psci_vector	/* Current EL Error Handler */
> +	.align	7
> +	b	__handle_sync		/* Lower EL Synchronous (64b) */
> +	.align	7
> +	b	default_psci_vector	/* Lower EL IRQ (64b) */
> +	.align	7
> +	b	default_psci_vector	/* Lower EL FIQ (64b) */
> +	.align	7
> +	b	default_psci_vector	/* Lower EL Error (64b) */
> +
> +.popsection
> -- 
> 1.7.7.4
> 
>