[U-Boot] [PATCH v5 3/8] ARMv7: PSCI: update the place of saving target PC

[Chen-Yu Tsai]

On Tue, Jun 14, 2016 at 3:01 PM,  <macro.wave.z at gmail.com> wrote:
> From: Hongbo Zhang <hongbo.zhang at nxp.com>
>
> The legacy code saves target PC at stack top, this patch changes it to stack
> bottom, because we will save more contents for PSCI v1.0, by this way we don't
> need to adjust the stack pointer when more contents are saved.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang at nxp.com>
> Signed-off-by: Wang Dongsheng <dongsheng.wang at nxp.com>
> ---
>  arch/arm/cpu/armv7/psci.S   | 9 +++++----
>  arch/arm/include/asm/psci.h | 4 ++++
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S
> index 3b92f1d..5b235df 100644
> --- a/arch/arm/cpu/armv7/psci.S
> +++ b/arch/arm/cpu/armv7/psci.S
> @@ -259,7 +259,8 @@ ENTRY(psci_cpu_on_common)
>
>         mov     r0, r1
>         bl      psci_get_cpu_stack_top          @ get stack top of target CPU
> -       str     r2, [r0]                        @ store target PC at stack top
> +       sub     r5, r0, #PSCI_TARGET_PC_OFFSET
> +       str     r2, [r5]                        @ store target PC

IMO having a separate function for this would be better.
It would be clearer, and easier to reuse or replace.

Also you should save and restore r5 across this function.

>         dsb
>
>         pop     {pc}
> @@ -286,14 +287,13 @@ ENDPROC(psci_cpu_off_common)
>
>  @ expects CPU ID in r0 and returns stack top in r0
>  ENTRY(psci_get_cpu_stack_top)
> -       mov     r5, #0x400                      @ 1kB of stack per CPU
> +       mov     r5, #PSCI_PERCPU_STACK_SIZE     @ 1kB of stack per CPU
>         mul     r0, r0, r5
>
>         ldr     r5, =psci_text_end              @ end of monitor text
>         add     r5, r5, #0x2000                 @ Skip two pages
>         lsr     r5, r5, #12                     @ Align to start of page
>         lsl     r5, r5, #12
> -       sub     r5, r5, #4                      @ reserve 1 word for target PC
>         sub     r0, r5, r0                      @ here's our stack!

This does not apply. r5 was changed to r3.

>
>         bx      lr
> @@ -306,7 +306,8 @@ ENTRY(psci_cpu_entry)
>
>         bl      psci_get_cpu_id                 @ CPU ID => r0
>         bl      psci_get_cpu_stack_top          @ stack top => r0
> -       ldr     r0, [r0]                        @ target PC at stack top
> +       sub     r0, r0, #PSCI_TARGET_PC_OFFSET
> +       ldr     r0, [r0]                        @ get target PC
>         b       _do_nonsec_entry
>  ENDPROC(psci_cpu_entry)
>
> diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> index 2367ec0..cb08544 100644
> --- a/arch/arm/include/asm/psci.h
> +++ b/arch/arm/include/asm/psci.h
> @@ -63,6 +63,10 @@
>  #define ARM_PSCI_1_0_FN_STAT_RESIDENCY         ARM_PSCI_0_2_FN(16)
>  #define ARM_PSCI_1_0_FN_STAT_COUNT             ARM_PSCI_0_2_FN(17)
>
> +/* size of percpu stack, 1kB */
> +#define PSCI_PERCPU_STACK_SIZE         0x400
> +#define PSCI_TARGET_PC_OFFSET          (PSCI_PERCPU_STACK_SIZE - 4)

I think you want PSCI_PERCPU_STACK_SIZE?

A stack starts at 0x400 and goes down to 0x0. You want to store
the target PC at 0x0, not 0x4.

Regards
ChenYu

> +
>  #ifndef __ASSEMBLY__
>  int psci_update_dt(void *fdt);
>  void psci_board_init(void);
> --
> 2.1.4
>