| Message ID | 1467872700-45942-1-git-send-email-b18965@freescale.com |
|---|---|
| State | Rejected |
| Delegated to: | York Sun |
| Headers | show |
On 7 July 2016 at 07:25, Alison Wang <b18965@freescale.com> wrote: > As CONFIG_ARMV8_SWITCH_TO_EL1 is not used now, this patch is to remove > CONFIG_ARMV8_SWITCH_TO_EL1 and the corresponding functions. > > Signed-off-by: Alison Wang <alison.wang@nxp.com> I haven't reviewed the code changes, but I have tested it and everything still works for me with this change. I tested on FVP AEMv8 Base and FVP Foundation models and Juno R0, R1 and R2 development boards; booting BusyBox, OpenEmbedded (not on R0 or R2) and Android environments (not on R1). Tested-by: Ryan Harkin <ryan.harkin@linaro.org> > --- > arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 13 -------- > arch/arm/cpu/armv8/start.S | 5 +-- > arch/arm/cpu/armv8/transition.S | 6 ---- > arch/arm/include/asm/macro.h | 47 ---------------------------- > arch/arm/include/asm/system.h | 1 - > arch/arm/lib/bootm.c | 3 -- > arch/arm/mach-exynos/soc.c | 1 - > include/configs/vexpress_aemv8a.h | 1 - > include/configs/xilinx_zynqmp.h | 2 -- > 9 files changed, 1 insertion(+), 78 deletions(-) > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S > index 5af6b73..d3a0117 100644 > --- a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S > +++ b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S > @@ -325,19 +325,12 @@ ENTRY(secondary_boot_func) > #endif > > bl secondary_switch_to_el2 > -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > - bl secondary_switch_to_el1 > -#endif > > slave_cpu: > wfe > ldr x0, [x11] > cbz x0, slave_cpu > -#ifndef CONFIG_ARMV8_SWITCH_TO_EL1 > mrs x1, sctlr_el2 > -#else > - mrs x1, sctlr_el1 > -#endif > tbz x1, #25, cpu_is_le > rev x0, x0 /* BE to LE conversion */ > cpu_is_le: > @@ -350,12 +343,6 @@ ENTRY(secondary_switch_to_el2) > 1: armv8_switch_to_el2_m x0 > ENDPROC(secondary_switch_to_el2) > > -ENTRY(secondary_switch_to_el1) > - switch_el x0, 0f, 1f, 0f > -0: ret > -1: armv8_switch_to_el1_m x0, x1 > -ENDPROC(secondary_switch_to_el1) > - > /* Ensure that the literals used by the secondary boot code are > * assembled within it (this is required so that we can protect > * this area with a single memreserve region > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S > index 670e323..ab434a6 100644 > --- a/arch/arm/cpu/armv8/start.S > +++ b/arch/arm/cpu/armv8/start.S > @@ -242,12 +242,9 @@ WEAK(lowlevel_init) > #endif > > /* > - * All slaves will enter EL2 and optionally EL1. > + * All slaves will enter EL2. > */ > bl armv8_switch_to_el2 > -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > - bl armv8_switch_to_el1 > -#endif > > #endif /* CONFIG_ARMV8_MULTIENTRY */ > > diff --git a/arch/arm/cpu/armv8/transition.S b/arch/arm/cpu/armv8/transition.S > index 253a39b..209d5c8 100644 > --- a/arch/arm/cpu/armv8/transition.S > +++ b/arch/arm/cpu/armv8/transition.S > @@ -15,9 +15,3 @@ ENTRY(armv8_switch_to_el2) > 0: ret > 1: armv8_switch_to_el2_m x0 > ENDPROC(armv8_switch_to_el2) > - > -ENTRY(armv8_switch_to_el1) > - switch_el x0, 0f, 1f, 0f > -0: ret > -1: armv8_switch_to_el1_m x0, x1 > -ENDPROC(armv8_switch_to_el1) > diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h > index 9bb0efa..c1b3452 100644 > --- a/arch/arm/include/asm/macro.h > +++ b/arch/arm/include/asm/macro.h > @@ -167,53 +167,6 @@ lr .req x30 > eret > .endm > > -.macro armv8_switch_to_el1_m, xreg1, xreg2 > - /* Initialize Generic Timers */ > - mrs \xreg1, cnthctl_el2 > - orr \xreg1, \xreg1, #0x3 /* Enable EL1 access to timers */ > - msr cnthctl_el2, \xreg1 > - msr cntvoff_el2, xzr > - > - /* Initilize MPID/MPIDR registers */ > - mrs \xreg1, midr_el1 > - mrs \xreg2, mpidr_el1 > - msr vpidr_el2, \xreg1 > - msr vmpidr_el2, \xreg2 > - > - /* Disable coprocessor traps */ > - mov \xreg1, #0x33ff > - msr cptr_el2, \xreg1 /* Disable coprocessor traps to EL2 */ > - msr hstr_el2, xzr /* Disable coprocessor traps to EL2 */ > - mov \xreg1, #3 << 20 > - msr cpacr_el1, \xreg1 /* Enable FP/SIMD at EL1 */ > - > - /* Initialize HCR_EL2 */ > - mov \xreg1, #(1 << 31) /* 64bit EL1 */ > - orr \xreg1, \xreg1, #(1 << 29) /* Disable HVC */ > - msr hcr_el2, \xreg1 > - > - /* SCTLR_EL1 initialization > - * > - * setting RES1 bits (29,28,23,22,20,11) to 1 > - * and RES0 bits (31,30,27,21,17,13,10,6) + > - * UCI,EE,EOE,WXN,nTWE,nTWI,UCT,DZE,I,UMA,SED,ITD, > - * CP15BEN,SA0,SA,C,A,M to 0 > - */ > - mov \xreg1, #0x0800 > - movk \xreg1, #0x30d0, lsl #16 > - msr sctlr_el1, \xreg1 > - > - /* Return to the EL1_SP1 mode from EL2 */ > - mov \xreg1, sp > - msr sp_el1, \xreg1 /* Migrate SP */ > - mrs \xreg1, vbar_el2 > - msr vbar_el1, \xreg1 /* Migrate VBAR */ > - mov \xreg1, #0x3c5 > - msr spsr_el2, \xreg1 /* EL1_SP1 | D | A | I | F */ > - msr elr_el2, lr > - eret > -.endm > - > #if defined(CONFIG_GICV3) > .macro gic_wait_for_interrupt_m xreg1 > 0 : wfi > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > index 2bdc0be..3a22661 100644 > --- a/arch/arm/include/asm/system.h > +++ b/arch/arm/include/asm/system.h > @@ -101,7 +101,6 @@ int __asm_flush_l3_cache(void); > void __asm_switch_ttbr(u64 new_ttbr); > > void armv8_switch_to_el2(void); > -void armv8_switch_to_el1(void); > void gic_init(void); > void gic_send_sgi(unsigned long sgino); > void wait_for_wakeup(void); > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > index 0838d89..e3c9832 100644 > --- a/arch/arm/lib/bootm.c > +++ b/arch/arm/lib/bootm.c > @@ -194,9 +194,6 @@ static void do_nonsec_virt_switch(void) > smp_kick_all_cpus(); > dcache_disable(); /* flush cache before swtiching to EL2 */ > armv8_switch_to_el2(); > -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > - armv8_switch_to_el1(); > -#endif > } > #endif > > diff --git a/arch/arm/mach-exynos/soc.c b/arch/arm/mach-exynos/soc.c > index f9c7468..c27a389 100644 > --- a/arch/arm/mach-exynos/soc.c > +++ b/arch/arm/mach-exynos/soc.c > @@ -28,6 +28,5 @@ void enable_caches(void) > void lowlevel_init(void) > { > armv8_switch_to_el2(); > - armv8_switch_to_el1(); > } > #endif > diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h > index 46cf83b..05bae44 100644 > --- a/include/configs/vexpress_aemv8a.h > +++ b/include/configs/vexpress_aemv8a.h > @@ -12,7 +12,6 @@ > #ifndef CONFIG_SEMIHOSTING > #error CONFIG_TARGET_VEXPRESS64_BASE_FVP requires CONFIG_SEMIHOSTING > #endif > -#define CONFIG_ARMV8_SWITCH_TO_EL1 > #endif > > #define CONFIG_REMAKE_ELF > diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h > index e776e32..c858491 100644 > --- a/include/configs/xilinx_zynqmp.h > +++ b/include/configs/xilinx_zynqmp.h > @@ -13,8 +13,6 @@ > > #define CONFIG_REMAKE_ELF > > -/* #define CONFIG_ARMV8_SWITCH_TO_EL1 */ > - > #define CONFIG_SYS_NO_FLASH > > /* Generic Interrupt Controller Definitions */ > -- > 2.1.0.27.g96db324 >
On 07/07/2016 08:25 AM, Alison Wang wrote: > As CONFIG_ARMV8_SWITCH_TO_EL1 is not used now, this patch is to remove > CONFIG_ARMV8_SWITCH_TO_EL1 and the corresponding functions. > > Signed-off-by: Alison Wang <alison.wang@nxp.com> I'll CC the maintainers for ZynqMP and Exynos as well, but from my side this patch is a great step forward. Reviewed-by: Alexander Graf <agraf@suse.de> > --- > arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 13 -------- > arch/arm/cpu/armv8/start.S | 5 +-- > arch/arm/cpu/armv8/transition.S | 6 ---- > arch/arm/include/asm/macro.h | 47 ---------------------------- > arch/arm/include/asm/system.h | 1 - > arch/arm/lib/bootm.c | 3 -- > arch/arm/mach-exynos/soc.c | 1 - > include/configs/vexpress_aemv8a.h | 1 - > include/configs/xilinx_zynqmp.h | 2 -- > 9 files changed, 1 insertion(+), 78 deletions(-) > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S > index 5af6b73..d3a0117 100644 > --- a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S > +++ b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S > @@ -325,19 +325,12 @@ ENTRY(secondary_boot_func) > #endif > > bl secondary_switch_to_el2 > -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > - bl secondary_switch_to_el1 > -#endif > > slave_cpu: > wfe > ldr x0, [x11] > cbz x0, slave_cpu > -#ifndef CONFIG_ARMV8_SWITCH_TO_EL1 > mrs x1, sctlr_el2 > -#else > - mrs x1, sctlr_el1 > -#endif > tbz x1, #25, cpu_is_le > rev x0, x0 /* BE to LE conversion */ > cpu_is_le: > @@ -350,12 +343,6 @@ ENTRY(secondary_switch_to_el2) > 1: armv8_switch_to_el2_m x0 > ENDPROC(secondary_switch_to_el2) > > -ENTRY(secondary_switch_to_el1) > - switch_el x0, 0f, 1f, 0f > -0: ret > -1: armv8_switch_to_el1_m x0, x1 > -ENDPROC(secondary_switch_to_el1) > - > /* Ensure that the literals used by the secondary boot code are > * assembled within it (this is required so that we can protect > * this area with a single memreserve region > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S > index 670e323..ab434a6 100644 > --- a/arch/arm/cpu/armv8/start.S > +++ b/arch/arm/cpu/armv8/start.S > @@ -242,12 +242,9 @@ WEAK(lowlevel_init) > #endif > > /* > - * All slaves will enter EL2 and optionally EL1. > + * All slaves will enter EL2. > */ > bl armv8_switch_to_el2 > -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > - bl armv8_switch_to_el1 > -#endif > > #endif /* CONFIG_ARMV8_MULTIENTRY */ > > diff --git a/arch/arm/cpu/armv8/transition.S b/arch/arm/cpu/armv8/transition.S > index 253a39b..209d5c8 100644 > --- a/arch/arm/cpu/armv8/transition.S > +++ b/arch/arm/cpu/armv8/transition.S > @@ -15,9 +15,3 @@ ENTRY(armv8_switch_to_el2) > 0: ret > 1: armv8_switch_to_el2_m x0 > ENDPROC(armv8_switch_to_el2) > - > -ENTRY(armv8_switch_to_el1) > - switch_el x0, 0f, 1f, 0f > -0: ret > -1: armv8_switch_to_el1_m x0, x1 > -ENDPROC(armv8_switch_to_el1) > diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h > index 9bb0efa..c1b3452 100644 > --- a/arch/arm/include/asm/macro.h > +++ b/arch/arm/include/asm/macro.h > @@ -167,53 +167,6 @@ lr .req x30 > eret > .endm > > -.macro armv8_switch_to_el1_m, xreg1, xreg2 > - /* Initialize Generic Timers */ > - mrs \xreg1, cnthctl_el2 > - orr \xreg1, \xreg1, #0x3 /* Enable EL1 access to timers */ > - msr cnthctl_el2, \xreg1 > - msr cntvoff_el2, xzr > - > - /* Initilize MPID/MPIDR registers */ > - mrs \xreg1, midr_el1 > - mrs \xreg2, mpidr_el1 > - msr vpidr_el2, \xreg1 > - msr vmpidr_el2, \xreg2 > - > - /* Disable coprocessor traps */ > - mov \xreg1, #0x33ff > - msr cptr_el2, \xreg1 /* Disable coprocessor traps to EL2 */ > - msr hstr_el2, xzr /* Disable coprocessor traps to EL2 */ > - mov \xreg1, #3 << 20 > - msr cpacr_el1, \xreg1 /* Enable FP/SIMD at EL1 */ > - > - /* Initialize HCR_EL2 */ > - mov \xreg1, #(1 << 31) /* 64bit EL1 */ > - orr \xreg1, \xreg1, #(1 << 29) /* Disable HVC */ > - msr hcr_el2, \xreg1 > - > - /* SCTLR_EL1 initialization > - * > - * setting RES1 bits (29,28,23,22,20,11) to 1 > - * and RES0 bits (31,30,27,21,17,13,10,6) + > - * UCI,EE,EOE,WXN,nTWE,nTWI,UCT,DZE,I,UMA,SED,ITD, > - * CP15BEN,SA0,SA,C,A,M to 0 > - */ > - mov \xreg1, #0x0800 > - movk \xreg1, #0x30d0, lsl #16 > - msr sctlr_el1, \xreg1 > - > - /* Return to the EL1_SP1 mode from EL2 */ > - mov \xreg1, sp > - msr sp_el1, \xreg1 /* Migrate SP */ > - mrs \xreg1, vbar_el2 > - msr vbar_el1, \xreg1 /* Migrate VBAR */ > - mov \xreg1, #0x3c5 > - msr spsr_el2, \xreg1 /* EL1_SP1 | D | A | I | F */ > - msr elr_el2, lr > - eret > -.endm > - > #if defined(CONFIG_GICV3) > .macro gic_wait_for_interrupt_m xreg1 > 0 : wfi > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > index 2bdc0be..3a22661 100644 > --- a/arch/arm/include/asm/system.h > +++ b/arch/arm/include/asm/system.h > @@ -101,7 +101,6 @@ int __asm_flush_l3_cache(void); > void __asm_switch_ttbr(u64 new_ttbr); > > void armv8_switch_to_el2(void); > -void armv8_switch_to_el1(void); > void gic_init(void); > void gic_send_sgi(unsigned long sgino); > void wait_for_wakeup(void); > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > index 0838d89..e3c9832 100644 > --- a/arch/arm/lib/bootm.c > +++ b/arch/arm/lib/bootm.c > @@ -194,9 +194,6 @@ static void do_nonsec_virt_switch(void) > smp_kick_all_cpus(); > dcache_disable(); /* flush cache before swtiching to EL2 */ > armv8_switch_to_el2(); > -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > - armv8_switch_to_el1(); > -#endif > } > #endif > > diff --git a/arch/arm/mach-exynos/soc.c b/arch/arm/mach-exynos/soc.c > index f9c7468..c27a389 100644 > --- a/arch/arm/mach-exynos/soc.c > +++ b/arch/arm/mach-exynos/soc.c > @@ -28,6 +28,5 @@ void enable_caches(void) > void lowlevel_init(void) > { > armv8_switch_to_el2(); > - armv8_switch_to_el1(); > } > #endif > diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h > index 46cf83b..05bae44 100644 > --- a/include/configs/vexpress_aemv8a.h > +++ b/include/configs/vexpress_aemv8a.h > @@ -12,7 +12,6 @@ > #ifndef CONFIG_SEMIHOSTING > #error CONFIG_TARGET_VEXPRESS64_BASE_FVP requires CONFIG_SEMIHOSTING > #endif > -#define CONFIG_ARMV8_SWITCH_TO_EL1 > #endif > > #define CONFIG_REMAKE_ELF > diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h > index e776e32..c858491 100644 > --- a/include/configs/xilinx_zynqmp.h > +++ b/include/configs/xilinx_zynqmp.h > @@ -13,8 +13,6 @@ > > #define CONFIG_REMAKE_ELF > > -/* #define CONFIG_ARMV8_SWITCH_TO_EL1 */ > - > #define CONFIG_SYS_NO_FLASH > > /* Generic Interrupt Controller Definitions */
Hi, 2016-07-07 13:44 GMT+02:00 Alexander Graf <agraf@suse.de>: > On 07/07/2016 08:25 AM, Alison Wang wrote: > >> As CONFIG_ARMV8_SWITCH_TO_EL1 is not used now, this patch is to remove >> CONFIG_ARMV8_SWITCH_TO_EL1 and the corresponding functions. >> >> Signed-off-by: Alison Wang <alison.wang@nxp.com> >> > > I'll CC the maintainers for ZynqMP and Exynos as well, but from my side > this patch is a great step forward. > > Reviewed-by: Alexander Graf <agraf@suse.de> I am against this patch. The reason is simple. We are using this option for testing to make sure that hyperviser and OS behaves correctly when they start from different level. Thanks, Michal
Hi, Alex,
As there is strong objection to remove the codes about switching to EL1, I think we have to remain it, do you agree?
If it is remained, I think your suggestion about *always* jumping to ep for both switching to AArch64 and AArch32 modes will make the code hard to realize and very complicated. So I prefer to keep the process in v4 patches. What is your opinion?
Best Regards,
Alison Wang
From: Michal Simek [mailto:monstr@monstr.eu]
Sent: Friday, July 08, 2016 1:31 AM
To: Alexander Graf
Cc: Alison Wang; york sun; Scott Wood; Stuart Yoder; Yang-Leo Li; David Feng; Linus Walleij; ryan.harkin@linaro.org; U-Boot; Zhengxiong Jin
Subject: Re: [U-Boot] [PATCH] armv8: Remove the codes about switching to EL1 before jumping to kernel
Hi,
2016-07-07 13:44 GMT+02:00 Alexander Graf <agraf@suse.de<mailto:agraf@suse.de>>:
On 07/07/2016 08:25 AM, Alison Wang wrote:
As CONFIG_ARMV8_SWITCH_TO_EL1 is not used now, this patch is to remove
CONFIG_ARMV8_SWITCH_TO_EL1 and the corresponding functions.
Signed-off-by: Alison Wang <alison.wang@nxp.com<mailto:alison.wang@nxp.com>>
I'll CC the maintainers for ZynqMP and Exynos as well, but from my side this patch is a great step forward.
Reviewed-by: Alexander Graf <agraf@suse.de<mailto:agraf@suse.de>>
I am against this patch. The reason is simple. We are using this option for testing to make sure that hyperviser and OS
behaves correctly when they start from different level.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu<http://www.monstr.eu> p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On 18.07.16 05:24, Huan Wang wrote: > Hi, Alex, > > > > As there is strong objection to remove the codes about > switching to EL1, I think we have to remain it, do you agree? I agree, yes. > If it is remained, I think your suggestion about > **always** jumping to ep for both switching to AArch64 and AArch32 modes > will make the code hard to realize and very complicated. So I prefer to > keep the process in v4 patches. What is your opinion? I think we should still convert it to a function call based approach. You can either just convert the current flow to functions: static void enter_in_el1(...) { call_in_el1(payload_pc, payload_bits, ...); } #ifdef ENTER_PAYLOAD_IN_EL1 call_in_el2(enter_in_el1, 64bit, ...); #else call_in_el2(payload_pc, payload_bits, ...); #endif Or you could add a check in the EL1 caller if you are in EL3 that you want to go to EL2 first: long call_in_el1(...) { if (current_el() == 3) return call_in_el2(call_in_el1, ...); asm_call_in_el1(...); } Alex
> On 18.07.16 05:24, Huan Wang wrote: > > Hi, Alex, > > > > > > > > As there is strong objection to remove the codes about > > switching to EL1, I think we have to remain it, do you agree? > > I agree, yes. > > > If it is remained, I think your suggestion about > > **always** jumping to ep for both switching to AArch64 and AArch32 > > modes will make the code hard to realize and very complicated. So I > > prefer to keep the process in v4 patches. What is your opinion? > > I think we should still convert it to a function call based approach. > You can either just convert the current flow to functions: > > static void enter_in_el1(...) > { > call_in_el1(payload_pc, payload_bits, ...); } > > #ifdef ENTER_PAYLOAD_IN_EL1 > call_in_el2(enter_in_el1, 64bit, ...); > #else > call_in_el2(payload_pc, payload_bits, ...); #endif > > Or you could add a check in the EL1 caller if you are in EL3 that you > want to go to EL2 first: > > long call_in_el1(...) > { > if (current_el() == 3) > return call_in_el2(call_in_el1, ...); > > asm_call_in_el1(...); > } > > [Alison Wang] Yes, it can work for primary core and secondary cores for LayerScape. For other ARMv8 platforms, such as arch/arm/mach-exynos/soc.c, void lowlevel_init(void) { armv8_switch_to_el2(); armv8_switch_to_el1(); } Is there any appropriate ep we can transfer for these functions? Even for the common arch/arm/cpu/armv8/start.S, bl armv8_switch_to_el2 #ifdef CONFIG_ARMV8_SWITCH_TO_EL1 bl armv8_switch_to_el1 #endif The ep is hard to define. "msr elr_el3, lr" really make sense. Best Regards, Alison Wang
On 08/29/2016 11:29 AM, Huan Wang wrote: >> On 18.07.16 05:24, Huan Wang wrote: >>> Hi, Alex, >>> >>> >>> >>> As there is strong objection to remove the codes about >>> switching to EL1, I think we have to remain it, do you agree? >> I agree, yes. >> >>> If it is remained, I think your suggestion about >>> **always** jumping to ep for both switching to AArch64 and AArch32 >>> modes will make the code hard to realize and very complicated. So I >>> prefer to keep the process in v4 patches. What is your opinion? >> I think we should still convert it to a function call based approach. >> You can either just convert the current flow to functions: >> >> static void enter_in_el1(...) >> { >> call_in_el1(payload_pc, payload_bits, ...); } >> >> #ifdef ENTER_PAYLOAD_IN_EL1 >> call_in_el2(enter_in_el1, 64bit, ...); >> #else >> call_in_el2(payload_pc, payload_bits, ...); #endif >> >> Or you could add a check in the EL1 caller if you are in EL3 that you >> want to go to EL2 first: >> >> long call_in_el1(...) >> { >> if (current_el() == 3) >> return call_in_el2(call_in_el1, ...); >> >> asm_call_in_el1(...); >> } >> >> > [Alison Wang] Yes, it can work for primary core and secondary cores for LayerScape. > > For other ARMv8 platforms, such as > arch/arm/mach-exynos/soc.c, > void lowlevel_init(void) > { > armv8_switch_to_el2(); > armv8_switch_to_el1(); > } > Is there any appropriate ep we can transfer for these functions? First off, I'd be surprised if the sequence above even works at all, as you also need to set up your page tables for el2/el1 if you want to get into those. IMHO the best path for this case is to remove the function :). But double-check with the Samsung folks first. I don't see why they can't use the generic one. > Even for the common arch/arm/cpu/armv8/start.S, > bl armv8_switch_to_el2 > #ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > bl armv8_switch_to_el1 > #endif > The ep is hard to define. It's pretty simple. Just pass a pointer to the instruction after bl into the switch function: /* * All slaves will enter EL2 and optionally EL1. */ adr x0, lowlevel_in_el2 bl asm_call_in_el2 b panic lowlevel_in_el2: #ifdef CONFIG_ARMV8_SWITCH_TO_EL1 adr x0, lowlevel_in_el1 bl asm_call_in_el1 b panic lowlevel_in_el1: #endif Because you already save the real lr in a non-volatile register, all this does is waste a few bytes of stack compared to the previous code. Alex
> On 08/29/2016 11:29 AM, Huan Wang wrote: > >> On 18.07.16 05:24, Huan Wang wrote: > >>> Hi, Alex, > >>> > >>> > >>> > >>> As there is strong objection to remove the codes > >>> about switching to EL1, I think we have to remain it, do you agree? > >> I agree, yes. > >> > >>> If it is remained, I think your suggestion about > >>> **always** jumping to ep for both switching to AArch64 and AArch32 > >>> modes will make the code hard to realize and very complicated. So I > >>> prefer to keep the process in v4 patches. What is your opinion? > >> I think we should still convert it to a function call based approach. > >> You can either just convert the current flow to functions: > >> > >> static void enter_in_el1(...) > >> { > >> call_in_el1(payload_pc, payload_bits, ...); } > >> > >> #ifdef ENTER_PAYLOAD_IN_EL1 > >> call_in_el2(enter_in_el1, 64bit, ...); #else call_in_el2(payload_pc, > >> payload_bits, ...); #endif > >> > >> Or you could add a check in the EL1 caller if you are in EL3 that you > >> want to go to EL2 first: > >> > >> long call_in_el1(...) > >> { > >> if (current_el() == 3) > >> return call_in_el2(call_in_el1, ...); > >> > >> asm_call_in_el1(...); > >> } > >> > >> > > [Alison Wang] Yes, it can work for primary core and secondary cores > for LayerScape. > > > > For other ARMv8 platforms, such as > > arch/arm/mach-exynos/soc.c, > > void lowlevel_init(void) > > { > > armv8_switch_to_el2(); > > armv8_switch_to_el1(); > > } > > Is there any appropriate ep we can transfer for these functions? > > First off, I'd be surprised if the sequence above even works at all, as > you also need to set up your page tables for el2/el1 if you want to get > into those. > > IMHO the best path for this case is to remove the function :). But > double-check with the Samsung folks first. I don't see why they can't > use the generic one. [Alison Wang] Yes, the best way is to remove the function. Let me ask them if we can remove it. > > > Even for the common arch/arm/cpu/armv8/start.S, > > bl armv8_switch_to_el2 > > #ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > > bl armv8_switch_to_el1 > > #endif > > The ep is hard to define. > > It's pretty simple. Just pass a pointer to the instruction after bl into > the switch function: > > /* > * All slaves will enter EL2 and optionally EL1. > */ > adr x0, lowlevel_in_el2 > bl asm_call_in_el2 > b panic > lowlevel_in_el2: > > #ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > adr x0, lowlevel_in_el1 > bl asm_call_in_el1 > b panic > lowlevel_in_el1: > #endif > > Because you already save the real lr in a non-volatile register, all > this does is waste a few bytes of stack compared to the previous code. [Alison Wang] Yes, thanks for your advice. Best Regards, Alison Wang
Hi, > On 08/29/2016 11:29 AM, Huan Wang wrote: > >> On 18.07.16 05:24, Huan Wang wrote: > >>> Hi, Alex, > >>> > >>> > >>> > >>> As there is strong objection to remove the codes > >>> about switching to EL1, I think we have to remain it, do you agree? > >> I agree, yes. > >> > >>> If it is remained, I think your suggestion about > >>> **always** jumping to ep for both switching to AArch64 and AArch32 > >>> modes will make the code hard to realize and very complicated. So I > >>> prefer to keep the process in v4 patches. What is your opinion? > >> I think we should still convert it to a function call based approach. > >> You can either just convert the current flow to functions: > >> > >> static void enter_in_el1(...) > >> { > >> call_in_el1(payload_pc, payload_bits, ...); } > >> > >> #ifdef ENTER_PAYLOAD_IN_EL1 > >> call_in_el2(enter_in_el1, 64bit, ...); #else call_in_el2(payload_pc, > >> payload_bits, ...); #endif > >> > >> Or you could add a check in the EL1 caller if you are in EL3 that you > >> want to go to EL2 first: > >> > >> long call_in_el1(...) > >> { > >> if (current_el() == 3) > >> return call_in_el2(call_in_el1, ...); > >> > >> asm_call_in_el1(...); > >> } > >> > >> > > [Alison Wang] Yes, it can work for primary core and secondary cores > for LayerScape. > > > > For other ARMv8 platforms, such as > > arch/arm/mach-exynos/soc.c, > > void lowlevel_init(void) > > { > > armv8_switch_to_el2(); > > armv8_switch_to_el1(); > > } > > Is there any appropriate ep we can transfer for these functions? > > First off, I'd be surprised if the sequence above even works at all, as > you also need to set up your page tables for el2/el1 if you want to get > into those. > > IMHO the best path for this case is to remove the function :). But > double-check with the Samsung folks first. I don't see why they can't > use the generic one. [Alison Wang] I have some questions about the following function. Could you help to answer? arch/arm/mach-exynos/soc.c, void lowlevel_init(void) { armv8_switch_to_el2(); armv8_switch_to_el1(); } Is this function necessary for exynos platforms? Why not to use the generic lowlevel_init function? Thanks. Best Regards, Alison Wang
> > On 08/29/2016 11:29 AM, Huan Wang wrote: > > >> On 18.07.16 05:24, Huan Wang wrote: > > >>> Hi, Alex, > > >>> > > >>> > > >>> > > >>> As there is strong objection to remove the codes > > >>> about switching to EL1, I think we have to remain it, do you agree? > > >> I agree, yes. > > >> > > >>> If it is remained, I think your suggestion about > > >>> **always** jumping to ep for both switching to AArch64 and AArch32 > > >>> modes will make the code hard to realize and very complicated. So > > >>> I prefer to keep the process in v4 patches. What is your opinion? > > >> I think we should still convert it to a function call based > approach. > > >> You can either just convert the current flow to functions: > > >> > > >> static void enter_in_el1(...) > > >> { > > >> call_in_el1(payload_pc, payload_bits, ...); } > > >> > > >> #ifdef ENTER_PAYLOAD_IN_EL1 > > >> call_in_el2(enter_in_el1, 64bit, ...); #else > > >> call_in_el2(payload_pc, payload_bits, ...); #endif > > >> > > >> Or you could add a check in the EL1 caller if you are in EL3 that > > >> you want to go to EL2 first: > > >> > > >> long call_in_el1(...) > > >> { > > >> if (current_el() == 3) > > >> return call_in_el2(call_in_el1, ...); > > >> > > >> asm_call_in_el1(...); > > >> } > > >> > > >> > > > [Alison Wang] Yes, it can work for primary core and secondary cores > > for LayerScape. > > > > > > For other ARMv8 platforms, such as > > > arch/arm/mach-exynos/soc.c, > > > void lowlevel_init(void) > > > { > > > armv8_switch_to_el2(); > > > armv8_switch_to_el1(); > > > } > > > Is there any appropriate ep we can transfer for these functions? > > > > First off, I'd be surprised if the sequence above even works at all, > > as you also need to set up your page tables for el2/el1 if you want to > > get into those. > > > > IMHO the best path for this case is to remove the function :). But > > double-check with the Samsung folks first. I don't see why they can't > > use the generic one. > [Alison Wang] > > I have some questions about the following function. Could you help to > answer? > > arch/arm/mach-exynos/soc.c, > void lowlevel_init(void) > { > armv8_switch_to_el2(); > armv8_switch_to_el1(); > } > > Is this function necessary for exynos platforms? Why not to use the > generic lowlevel_init function? > Could you help to answer the question? Thanks. Best Regards, Alison Wang
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S index 5af6b73..d3a0117 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S +++ b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S @@ -325,19 +325,12 @@ ENTRY(secondary_boot_func) #endif bl secondary_switch_to_el2 -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 - bl secondary_switch_to_el1 -#endif slave_cpu: wfe ldr x0, [x11] cbz x0, slave_cpu -#ifndef CONFIG_ARMV8_SWITCH_TO_EL1 mrs x1, sctlr_el2 -#else - mrs x1, sctlr_el1 -#endif tbz x1, #25, cpu_is_le rev x0, x0 /* BE to LE conversion */ cpu_is_le: @@ -350,12 +343,6 @@ ENTRY(secondary_switch_to_el2) 1: armv8_switch_to_el2_m x0 ENDPROC(secondary_switch_to_el2) -ENTRY(secondary_switch_to_el1) - switch_el x0, 0f, 1f, 0f -0: ret -1: armv8_switch_to_el1_m x0, x1 -ENDPROC(secondary_switch_to_el1) - /* Ensure that the literals used by the secondary boot code are * assembled within it (this is required so that we can protect * this area with a single memreserve region diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 670e323..ab434a6 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -242,12 +242,9 @@ WEAK(lowlevel_init) #endif /* - * All slaves will enter EL2 and optionally EL1. + * All slaves will enter EL2. */ bl armv8_switch_to_el2 -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 - bl armv8_switch_to_el1 -#endif #endif /* CONFIG_ARMV8_MULTIENTRY */ diff --git a/arch/arm/cpu/armv8/transition.S b/arch/arm/cpu/armv8/transition.S index 253a39b..209d5c8 100644 --- a/arch/arm/cpu/armv8/transition.S +++ b/arch/arm/cpu/armv8/transition.S @@ -15,9 +15,3 @@ ENTRY(armv8_switch_to_el2) 0: ret 1: armv8_switch_to_el2_m x0 ENDPROC(armv8_switch_to_el2) - -ENTRY(armv8_switch_to_el1) - switch_el x0, 0f, 1f, 0f -0: ret -1: armv8_switch_to_el1_m x0, x1 -ENDPROC(armv8_switch_to_el1) diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h index 9bb0efa..c1b3452 100644 --- a/arch/arm/include/asm/macro.h +++ b/arch/arm/include/asm/macro.h @@ -167,53 +167,6 @@ lr .req x30 eret .endm -.macro armv8_switch_to_el1_m, xreg1, xreg2 - /* Initialize Generic Timers */ - mrs \xreg1, cnthctl_el2 - orr \xreg1, \xreg1, #0x3 /* Enable EL1 access to timers */ - msr cnthctl_el2, \xreg1 - msr cntvoff_el2, xzr - - /* Initilize MPID/MPIDR registers */ - mrs \xreg1, midr_el1 - mrs \xreg2, mpidr_el1 - msr vpidr_el2, \xreg1 - msr vmpidr_el2, \xreg2 - - /* Disable coprocessor traps */ - mov \xreg1, #0x33ff - msr cptr_el2, \xreg1 /* Disable coprocessor traps to EL2 */ - msr hstr_el2, xzr /* Disable coprocessor traps to EL2 */ - mov \xreg1, #3 << 20 - msr cpacr_el1, \xreg1 /* Enable FP/SIMD at EL1 */ - - /* Initialize HCR_EL2 */ - mov \xreg1, #(1 << 31) /* 64bit EL1 */ - orr \xreg1, \xreg1, #(1 << 29) /* Disable HVC */ - msr hcr_el2, \xreg1 - - /* SCTLR_EL1 initialization - * - * setting RES1 bits (29,28,23,22,20,11) to 1 - * and RES0 bits (31,30,27,21,17,13,10,6) + - * UCI,EE,EOE,WXN,nTWE,nTWI,UCT,DZE,I,UMA,SED,ITD, - * CP15BEN,SA0,SA,C,A,M to 0 - */ - mov \xreg1, #0x0800 - movk \xreg1, #0x30d0, lsl #16 - msr sctlr_el1, \xreg1 - - /* Return to the EL1_SP1 mode from EL2 */ - mov \xreg1, sp - msr sp_el1, \xreg1 /* Migrate SP */ - mrs \xreg1, vbar_el2 - msr vbar_el1, \xreg1 /* Migrate VBAR */ - mov \xreg1, #0x3c5 - msr spsr_el2, \xreg1 /* EL1_SP1 | D | A | I | F */ - msr elr_el2, lr - eret -.endm - #if defined(CONFIG_GICV3) .macro gic_wait_for_interrupt_m xreg1 0 : wfi diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 2bdc0be..3a22661 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -101,7 +101,6 @@ int __asm_flush_l3_cache(void); void __asm_switch_ttbr(u64 new_ttbr); void armv8_switch_to_el2(void); -void armv8_switch_to_el1(void); void gic_init(void); void gic_send_sgi(unsigned long sgino); void wait_for_wakeup(void); diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 0838d89..e3c9832 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -194,9 +194,6 @@ static void do_nonsec_virt_switch(void) smp_kick_all_cpus(); dcache_disable(); /* flush cache before swtiching to EL2 */ armv8_switch_to_el2(); -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 - armv8_switch_to_el1(); -#endif } #endif diff --git a/arch/arm/mach-exynos/soc.c b/arch/arm/mach-exynos/soc.c index f9c7468..c27a389 100644 --- a/arch/arm/mach-exynos/soc.c +++ b/arch/arm/mach-exynos/soc.c @@ -28,6 +28,5 @@ void enable_caches(void) void lowlevel_init(void) { armv8_switch_to_el2(); - armv8_switch_to_el1(); } #endif diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h index 46cf83b..05bae44 100644 --- a/include/configs/vexpress_aemv8a.h +++ b/include/configs/vexpress_aemv8a.h @@ -12,7 +12,6 @@ #ifndef CONFIG_SEMIHOSTING #error CONFIG_TARGET_VEXPRESS64_BASE_FVP requires CONFIG_SEMIHOSTING #endif -#define CONFIG_ARMV8_SWITCH_TO_EL1 #endif #define CONFIG_REMAKE_ELF diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h index e776e32..c858491 100644 --- a/include/configs/xilinx_zynqmp.h +++ b/include/configs/xilinx_zynqmp.h @@ -13,8 +13,6 @@ #define CONFIG_REMAKE_ELF -/* #define CONFIG_ARMV8_SWITCH_TO_EL1 */ - #define CONFIG_SYS_NO_FLASH /* Generic Interrupt Controller Definitions */
As CONFIG_ARMV8_SWITCH_TO_EL1 is not used now, this patch is to remove CONFIG_ARMV8_SWITCH_TO_EL1 and the corresponding functions. Signed-off-by: Alison Wang <alison.wang@nxp.com> --- arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 13 -------- arch/arm/cpu/armv8/start.S | 5 +-- arch/arm/cpu/armv8/transition.S | 6 ---- arch/arm/include/asm/macro.h | 47 ---------------------------- arch/arm/include/asm/system.h | 1 - arch/arm/lib/bootm.c | 3 -- arch/arm/mach-exynos/soc.c | 1 - include/configs/vexpress_aemv8a.h | 1 - include/configs/xilinx_zynqmp.h | 2 -- 9 files changed, 1 insertion(+), 78 deletions(-)