[U-Boot] [PATCH 3/9] ARM: ARMv7: PSCI: move target PC in each CPU stack no longer is shared

[Wang Dongsheng]

Hi Hongbo,

On 03/16/2016 03:47 PM, Hongbo Zhang wrote:
> Hi Dongsheng,
>
>> -----Original Message-----
>> From: Chenhui Zhao
>> Sent: Wednesday, March 16, 2016 3:05 PM
>> To: Hongbo Zhang <hongbo.zhang at nxp.com>
>> Cc: trini at konsulko.com; sbabic at denx.de; ijc at hellion.org.uk;
>> hdegoede at redhat.com; twarren at nvidia.com; Huan Wang
>> <alison.wang at nxp.com>; york sun <york.sun at nxp.com>;
>> jan.kiszka at siemens.com; Frank Li <frank.li at nxp.com>; Peng Fan
>> <peng.fan at nxp.com>; Zhengxiong Jin <jason.jin at nxp.com>;
>> oss at buserror.net; u-boot at lists.denx.de; albert.u.boot at aribaud.net
>> Subject: Re: [PATCH 3/9] ARM: ARMv7: PSCI: move target PC in each CPU stack
>> no longer is shared
>>
>> Add Hongbo.
>>
>> Thanks,
>> Chenhui
>>
>> ________________________________________
>> From: Dongsheng Wang <dongsheng.wang at nxp.com>
>> Sent: Monday, January 18, 2016 12:27 PM
>> To: albert.u.boot at aribaud.net
>> Cc: trini at konsulko.com; sbabic at denx.de; ijc at hellion.org.uk;
>> hdegoede at redhat.com; twarren at nvidia.com; Huan Wang; york sun;
>> jan.kiszka at siemens.com; Frank Li; Peng Fan; Zhengxiong Jin; Chenhui Zhao;
>> oss at buserror.net; u-boot at lists.denx.de; Dongsheng Wang
>> Subject: [PATCH 3/9] ARM: ARMv7: PSCI: move target PC in each CPU stack no
>> longer is shared
>>
>> From: Wang Dongsheng <dongsheng.wang at nxp.com>
>>
>> All of cpu share the same targetPC space that is unsafe. So move target PC save
>> space into CPU stack.
>>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang at nxp.com>
>> ---
>>   arch/arm/cpu/armv7/ls102xa/psci.S     |  4 +--
>>   arch/arm/cpu/armv7/mx7/psci.S         |  4 +--
>>   arch/arm/cpu/armv7/psci.S             | 51 ++++++++++++++++++++++++++++----
>> ---
>>   arch/arm/cpu/armv7/sunxi/psci_sun6i.S |  4 +--
>> arch/arm/cpu/armv7/sunxi/psci_sun7i.S |  4 +--
>>   arch/arm/cpu/armv7/virt-dt.c          |  3 ++-
>>   arch/arm/include/asm/psci.h           |  4 +++
>>   arch/arm/mach-tegra/psci.S            |  4 +--
>>   8 files changed, 53 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/ls102xa/psci.S
>> b/arch/arm/cpu/armv7/ls102xa/psci.S
>> index 0b067d9..3a34064 100644
>> --- a/arch/arm/cpu/armv7/ls102xa/psci.S
>> +++ b/arch/arm/cpu/armv7/ls102xa/psci.S
>> @@ -36,9 +36,7 @@ psci_cpu_on:
>>          and     r1, r1, #0xff
>>
>>          mov     r0, r1
>> -       bl      psci_get_cpu_stack_top
>> -       str     r2, [r0]
>> -       dsb
>> +       bl      psci_save_target_pc
>>
>>          @ Get DCFG base address
>>          movw    r4, #(CONFIG_SYS_FSL_GUTS_ADDR & 0xffff)
>> diff --git a/arch/arm/cpu/armv7/mx7/psci.S b/arch/arm/cpu/armv7/mx7/psci.S
>> index 34c6ab3..cb39f27 100644
>> --- a/arch/arm/cpu/armv7/mx7/psci.S
>> +++ b/arch/arm/cpu/armv7/mx7/psci.S
>> @@ -30,9 +30,7 @@ psci_cpu_on:
>>          push    {lr}
>>
>>          mov     r0, r1
>> -       bl      psci_get_cpu_stack_top
>> -       str     r2, [r0]
>> -       dsb
>> +       bl      psci_save_target_pc
>>
>>          ldr     r2, =psci_cpu_entry
>>          bl      imx_cpu_on
>> diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S index
>> 2425f6a..0e0f98e 100644
>> --- a/arch/arm/cpu/armv7/psci.S
>> +++ b/arch/arm/cpu/armv7/psci.S
>> @@ -20,6 +20,8 @@
>>   #include <asm/macro.h>
>>   #include <asm/psci.h>
>>
>> +#define SAVE_SPACE_TARGET_PC_OFFSET    0x0
>> +
>>          .pushsection ._secure.text, "ax"
>>
>>          .arch_extension sec
>> @@ -196,27 +198,58 @@ 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
>> -       mul     r0, r0, r5
>> -
>> +       @ Align psci_text_end minumum page size. Even if page is greater than
>> +       @ 4K, we still can ensure that data is always safe access by the CPU.
>>          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
>> +       @ Detect page border
>> +       lsl     r4, r5, #20
> Why do you shift it 20 bits? I guess you wanted to shift 12 bits instead.

Shift left 20bits will get the low of 12bits that is page border. if the 
12 bits of value is 0,
this means that we stand in the page border. And not need to do align page.

>> +       cmp     r4, #0
>> +       beq     out_psci_stack_align
>> +       add     r5, r5, #PSCI_STACK_ALIGN_SIZE
>> +       lsr     r5, r5, #12
>>          lsl     r5, r5, #12
>> -       sub     r5, r5, #4                      @ reserve 1 word for target PC
>> -       sub     r0, r5, r0                      @ here's our stack!
>> +
>> +out_psci_stack_align:
>> +       mov     r4, #PSCI_PERCPU_STACK_SIZE
>> +       add     r0, r0, #1
>> +       mul     r0, r0, r4
>> +       add     r0, r0, r5
>>
>>          bx      lr
>>   ENDPROC(psci_get_cpu_stack_top)
>>
>> +@ Save space in percpu stack tail.
>> +@ expects CPU ID in r0 and returns save space address in r0.
>> +ENTRY(psci_get_cpu_save_space)
> It is better to use term "reserve" here instead of "save"
s/save/reserve/, thanks.

>> +       mov     r10, lr
>> +
>> +       bl      psci_get_cpu_stack_top
>> +       sub     r0, r0, #PSCI_PERCPU_STACK_SIZE
> (r0 - PSCI_PERCPU_STACK_SIZE ) is stack top of next CPU's stack.
> (r0 - PSCI_PERCPU_STACK_SIZE +1 ) is the stack tail you want, right?
Yes, :)

Regards,
-Dongsheng