| Message ID | 1465887683-27492-5-git-send-email-hongbo.zhang@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | York Sun |
| Headers | show |
Hi, On Tue, Jun 14, 2016 at 3:01 PM, <macro.wave.z@gmail.com> wrote: > From: Hongbo Zhang <hongbo.zhang@nxp.com> > > According to latest PSCI specification, the context ID is needed by CPU_ON. > This patch saves context ID to the second lowest address of the stack (next to > where target PC is saved), and restores it to r0 when needed while target CPU > booting up. Interesting. This doesn't seem to be used by Linux yet. This led me using r3 as a scratch register. See below. > > Signed-off-by: Hongbo Zhang <hongbo.zhang@nxp.com> > Signed-off-by: Wang Dongsheng <dongsheng.wang@nxp.com> > --- > arch/arm/cpu/armv7/nonsec_virt.S | 7 +++++++ > arch/arm/cpu/armv7/psci.S | 4 +++- > arch/arm/include/asm/psci.h | 1 + > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S > index b7563ed..6566643 100644 > --- a/arch/arm/cpu/armv7/nonsec_virt.S > +++ b/arch/arm/cpu/armv7/nonsec_virt.S > @@ -11,6 +11,7 @@ > #include <asm/gic.h> > #include <asm/armv7.h> > #include <asm/proc-armv/ptrace.h> > +#include <asm/psci.h> > > .arch_extension sec > .arch_extension virt > @@ -89,6 +90,12 @@ _secure_monitor: > movne r4, #0 > mcrrne p15, 4, r4, r4, c14 @ Reset CNTVOFF to zero > 1: > +#ifdef CONFIG_ARMV7_PSCI > + bl psci_get_cpu_id > + bl psci_get_cpu_stack_top > + sub r0, r0, #PSCI_CONTEXT_ID_OFFSET > + ldr r0, [r0] @ get Context ID in r0 You should do this in psci_cpu_entry, for a couple of reasons: - That is also where the target PC is loaded. - It is PSCI specific. - All first time SMC calls would go through _secure_monitor. This includes when U-boot jumps into the kernel through _do_nonsec_entry, at which point you don't have a proper value stored, while U-boot passes 0 here. See: http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/lib/bootm.c;h=0838d89907b9a2eb81f4ebb31d8c045e031c5e11;hb=HEAD#l324 > +#endif > mov lr, ip > mov ip, #(F_BIT | I_BIT | A_BIT) @ Set A, I and F > tst lr, #1 @ Check for Thumb PC > diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S > index 5b235df..3ba9e51 100644 > --- a/arch/arm/cpu/armv7/psci.S > +++ b/arch/arm/cpu/armv7/psci.S > @@ -253,7 +253,7 @@ ENTRY(psci_enable_smp) > ENDPROC(psci_enable_smp) > .weak psci_enable_smp > > -/* expects target CPU in r1, target PC in r2 */ > +/* expects target CPU in r1, target PC in r2, target conetxt ID in r3 */ > ENTRY(psci_cpu_on_common) > push {lr} > > @@ -261,6 +261,8 @@ ENTRY(psci_cpu_on_common) > bl psci_get_cpu_stack_top @ get stack top of target CPU r3 get overwritten in psci_get_cpu_stack_top in http://git.denx.de/?p=u-boot.git;a=commitdiff;h=dae08d228122e4ad296077106520a4db3ca17872 Some of the functions now follow the ARM calling conventions. You should save r3 and possibly other registers across any function calls. Regards ChenYu > sub r5, r0, #PSCI_TARGET_PC_OFFSET > str r2, [r5] @ store target PC > + sub r5, r0, #PSCI_CONTEXT_ID_OFFSET > + str r3, [r5] @ store target context ID > dsb > > pop {pc} > diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h > index cb08544..bedcd30 100644 > --- a/arch/arm/include/asm/psci.h > +++ b/arch/arm/include/asm/psci.h > @@ -66,6 +66,7 @@ > /* size of percpu stack, 1kB */ > #define PSCI_PERCPU_STACK_SIZE 0x400 > #define PSCI_TARGET_PC_OFFSET (PSCI_PERCPU_STACK_SIZE - 4) > +#define PSCI_CONTEXT_ID_OFFSET (PSCI_PERCPU_STACK_SIZE - 8) > > #ifndef __ASSEMBLY__ > int psci_update_dt(void *fdt); > -- > 2.1.4 >
On Tue, Jun 28, 2016 at 11:15 AM, Chen-Yu Tsai <wens@csie.org> wrote: > Hi, > > On Tue, Jun 14, 2016 at 3:01 PM, <macro.wave.z@gmail.com> wrote: >> From: Hongbo Zhang <hongbo.zhang@nxp.com> >> >> According to latest PSCI specification, the context ID is needed by CPU_ON. >> This patch saves context ID to the second lowest address of the stack (next to >> where target PC is saved), and restores it to r0 when needed while target CPU >> booting up. > > Interesting. This doesn't seem to be used by Linux yet. This led me using r3 > as a scratch register. See below. > Context ID is introduced by PSCI v1.0, since we are adding v1.0 support, I think we should add this although Linux kernel doesn't use this till now, it is spec. And what's more, r0, r1, r2 are already used by current codes, and in the spec paragraph 5.2.1: "For PSCI functions that use only 32-bit parameters, the arguments are passed in R0 to R3 (AArch32) or W0 to W3 (AArch64), with return values in R0 or W0." All of these make the r3 the only best register for context ID I think. >> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@nxp.com> >> Signed-off-by: Wang Dongsheng <dongsheng.wang@nxp.com> >> --- >> arch/arm/cpu/armv7/nonsec_virt.S | 7 +++++++ >> arch/arm/cpu/armv7/psci.S | 4 +++- >> arch/arm/include/asm/psci.h | 1 + >> 3 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S >> index b7563ed..6566643 100644 >> --- a/arch/arm/cpu/armv7/nonsec_virt.S >> +++ b/arch/arm/cpu/armv7/nonsec_virt.S >> @@ -11,6 +11,7 @@ >> #include <asm/gic.h> >> #include <asm/armv7.h> >> #include <asm/proc-armv/ptrace.h> >> +#include <asm/psci.h> >> >> .arch_extension sec >> .arch_extension virt >> @@ -89,6 +90,12 @@ _secure_monitor: >> movne r4, #0 >> mcrrne p15, 4, r4, r4, c14 @ Reset CNTVOFF to zero >> 1: >> +#ifdef CONFIG_ARMV7_PSCI >> + bl psci_get_cpu_id >> + bl psci_get_cpu_stack_top >> + sub r0, r0, #PSCI_CONTEXT_ID_OFFSET >> + ldr r0, [r0] @ get Context ID in r0 > > You should do this in psci_cpu_entry, for a couple of reasons: > Yes, thanks. > - That is also where the target PC is loaded. > > - It is PSCI specific. > > - All first time SMC calls would go through _secure_monitor. > This includes when U-boot jumps into the kernel through > _do_nonsec_entry, at which point you don't have a proper value > stored, while U-boot passes 0 here. See: > > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/lib/bootm.c;h=0838d89907b9a2eb81f4ebb31d8c045e031c5e11;hb=HEAD#l324 > >> +#endif >> mov lr, ip >> mov ip, #(F_BIT | I_BIT | A_BIT) @ Set A, I and F >> tst lr, #1 @ Check for Thumb PC >> diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S >> index 5b235df..3ba9e51 100644 >> --- a/arch/arm/cpu/armv7/psci.S >> +++ b/arch/arm/cpu/armv7/psci.S >> @@ -253,7 +253,7 @@ ENTRY(psci_enable_smp) >> ENDPROC(psci_enable_smp) >> .weak psci_enable_smp >> >> -/* expects target CPU in r1, target PC in r2 */ >> +/* expects target CPU in r1, target PC in r2, target conetxt ID in r3 */ >> ENTRY(psci_cpu_on_common) >> push {lr} >> >> @@ -261,6 +261,8 @@ ENTRY(psci_cpu_on_common) >> bl psci_get_cpu_stack_top @ get stack top of target CPU > > r3 get overwritten in psci_get_cpu_stack_top in > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=dae08d228122e4ad296077106520a4db3ca17872 > > Some of the functions now follow the ARM calling conventions. > You should save r3 and possibly other registers across any function > calls. > OK, will consider register saving. > Regards > ChenYu > >> sub r5, r0, #PSCI_TARGET_PC_OFFSET >> str r2, [r5] @ store target PC >> + sub r5, r0, #PSCI_CONTEXT_ID_OFFSET >> + str r3, [r5] @ store target context ID >> dsb >> >> pop {pc} >> diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h >> index cb08544..bedcd30 100644 >> --- a/arch/arm/include/asm/psci.h >> +++ b/arch/arm/include/asm/psci.h >> @@ -66,6 +66,7 @@ >> /* size of percpu stack, 1kB */ >> #define PSCI_PERCPU_STACK_SIZE 0x400 >> #define PSCI_TARGET_PC_OFFSET (PSCI_PERCPU_STACK_SIZE - 4) >> +#define PSCI_CONTEXT_ID_OFFSET (PSCI_PERCPU_STACK_SIZE - 8) >> >> #ifndef __ASSEMBLY__ >> int psci_update_dt(void *fdt); >> -- >> 2.1.4 >>
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index b7563ed..6566643 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -11,6 +11,7 @@ #include <asm/gic.h> #include <asm/armv7.h> #include <asm/proc-armv/ptrace.h> +#include <asm/psci.h> .arch_extension sec .arch_extension virt @@ -89,6 +90,12 @@ _secure_monitor: movne r4, #0 mcrrne p15, 4, r4, r4, c14 @ Reset CNTVOFF to zero 1: +#ifdef CONFIG_ARMV7_PSCI + bl psci_get_cpu_id + bl psci_get_cpu_stack_top + sub r0, r0, #PSCI_CONTEXT_ID_OFFSET + ldr r0, [r0] @ get Context ID in r0 +#endif mov lr, ip mov ip, #(F_BIT | I_BIT | A_BIT) @ Set A, I and F tst lr, #1 @ Check for Thumb PC diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S index 5b235df..3ba9e51 100644 --- a/arch/arm/cpu/armv7/psci.S +++ b/arch/arm/cpu/armv7/psci.S @@ -253,7 +253,7 @@ ENTRY(psci_enable_smp) ENDPROC(psci_enable_smp) .weak psci_enable_smp -/* expects target CPU in r1, target PC in r2 */ +/* expects target CPU in r1, target PC in r2, target conetxt ID in r3 */ ENTRY(psci_cpu_on_common) push {lr} @@ -261,6 +261,8 @@ ENTRY(psci_cpu_on_common) bl psci_get_cpu_stack_top @ get stack top of target CPU sub r5, r0, #PSCI_TARGET_PC_OFFSET str r2, [r5] @ store target PC + sub r5, r0, #PSCI_CONTEXT_ID_OFFSET + str r3, [r5] @ store target context ID dsb pop {pc} diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h index cb08544..bedcd30 100644 --- a/arch/arm/include/asm/psci.h +++ b/arch/arm/include/asm/psci.h @@ -66,6 +66,7 @@ /* size of percpu stack, 1kB */ #define PSCI_PERCPU_STACK_SIZE 0x400 #define PSCI_TARGET_PC_OFFSET (PSCI_PERCPU_STACK_SIZE - 4) +#define PSCI_CONTEXT_ID_OFFSET (PSCI_PERCPU_STACK_SIZE - 8) #ifndef __ASSEMBLY__ int psci_update_dt(void *fdt);