[RFC PATCH] arm64: Fix EL1/EL2 early init inconsistencies with VHE

[RFC PATCH] arm64: Fix EL1/EL2 early init inconsistencies with VHE

[Dave Martin]

When using the Virtualisation Host Extensions, EL1 is not used in
the host and requires no separate configuration.

In addition, with VHE enabled, non-hyp-specific EL2 configuration
that does not need to be done early will be done anyway in
__cpu_setup via the _EL1 system register aliases.  In particular,
the layout and definition of CPTR_EL2 are changed by enabling VHE
so that they resemble CPACR_EL1, so existing code to initialise
CPTR_EL2 becomes architecturally wrong in this case.

This patch simply skips the affected initialisation code in the
non-VHE case.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---

Note -- not tested yet, and I'm still unclear on whether this is the
correct architectural approach...

 arch/arm64/kernel/head.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 4203d5f..67132c1 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -588,6 +588,15 @@ set_hcr:
 	msr	vpidr_el2, x0
 	msr	vmpidr_el2, x1
 
+	/*
+	 * When VHE is not in use, early init of EL2 and EL1 needs to be
+	 * done here.
+	 * When VHE _is_ in use, EL1 will not be used in the host and
+	 * requires no configuration, and all non-hyp-specific EL2 setup
+	 * will be done via the _EL1 system register aliases in __cpu_setup.
+	 */
+	cbnz	x2, 1f
+
 	/* sctlr_el1 */
 	mov	x0, #0x0800			// Set/clear RES{1,0} bits
 CPU_BE(	movk	x0, #0x33d0, lsl #16	)	// Set EE and E0E on BE systems
@@ -597,6 +606,7 @@ CPU_LE(	movk	x0, #0x30d0, lsl #16	)	// Clear EE and E0E on LE systems
 	/* Coprocessor traps. */
 	mov	x0, #0x33ff
 	msr	cptr_el2, x0			// Disable copro. traps to EL2
+1:
 
 #ifdef CONFIG_COMPAT
 	msr	hstr_el2, xzr			// Disable CP15 traps to EL2
-- 
2.1.4

[RFC PATCH] arm64: Fix EL1/EL2 early init inconsistencies with VHE

[Dave Martin]

When using the Virtualisation Host Extensions, EL1 is not used in
the host and requires no separate configuration.

In addition, with VHE enabled, non-hyp-specific EL2 configuration
that does not need to be done early will be done anyway in
__cpu_setup via the _EL1 system register aliases.  In particular,
the layout and definition of CPTR_EL2 are changed by enabling VHE
so that they resemble CPACR_EL1, so existing code to initialise
CPTR_EL2 becomes architecturally wrong in this case.

This patch simply skips the affected initialisation code in the
non-VHE case.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---

Note -- not tested yet, and I'm still unclear on whether this is the
correct architectural approach...

 arch/arm64/kernel/head.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 4203d5f..67132c1 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -588,6 +588,15 @@ set_hcr:
 	msr	vpidr_el2, x0
 	msr	vmpidr_el2, x1
 
+	/*
+	 * When VHE is not in use, early init of EL2 and EL1 needs to be
+	 * done here.
+	 * When VHE _is_ in use, EL1 will not be used in the host and
+	 * requires no configuration, and all non-hyp-specific EL2 setup
+	 * will be done via the _EL1 system register aliases in __cpu_setup.
+	 */
+	cbnz	x2, 1f
+
 	/* sctlr_el1 */
 	mov	x0, #0x0800			// Set/clear RES{1,0} bits
 CPU_BE(	movk	x0, #0x33d0, lsl #16	)	// Set EE and E0E on BE systems
@@ -597,6 +606,7 @@ CPU_LE(	movk	x0, #0x30d0, lsl #16	)	// Clear EE and E0E on LE systems
 	/* Coprocessor traps. */
 	mov	x0, #0x33ff
 	msr	cptr_el2, x0			// Disable copro. traps to EL2
+1:
 
 #ifdef CONFIG_COMPAT
 	msr	hstr_el2, xzr			// Disable CP15 traps to EL2
-- 
2.1.4

Re: [RFC PATCH] arm64: Fix EL1/EL2 early init inconsistencies with VHE

[Marc Zyngier]

Hi Dave,

On 18/04/16 18:57, Dave Martin wrote:
> When using the Virtualisation Host Extensions, EL1 is not used in
> the host and requires no separate configuration.
> 
> In addition, with VHE enabled, non-hyp-specific EL2 configuration
> that does not need to be done early will be done anyway in
> __cpu_setup via the _EL1 system register aliases.  In particular,
> the layout and definition of CPTR_EL2 are changed by enabling VHE
> so that they resemble CPACR_EL1, so existing code to initialise
> CPTR_EL2 becomes architecturally wrong in this case.
> 
> This patch simply skips the affected initialisation code in the
> non-VHE case.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
> 
> Note -- not tested yet, and I'm still unclear on whether this is the
> correct architectural approach...

This looks correct to me. Given that we're not leaving EL2, there is no
reason to do things earlier than they are done at EL1, and this fixes an
obvious bug on CPTR_EL2 access.

I've given it a go on a VHE model, and it ran just fine, so:

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

[RFC PATCH] arm64: Fix EL1/EL2 early init inconsistencies with VHE

[Marc Zyngier]

Hi Dave,

On 18/04/16 18:57, Dave Martin wrote:
> When using the Virtualisation Host Extensions, EL1 is not used in
> the host and requires no separate configuration.
> 
> In addition, with VHE enabled, non-hyp-specific EL2 configuration
> that does not need to be done early will be done anyway in
> __cpu_setup via the _EL1 system register aliases.  In particular,
> the layout and definition of CPTR_EL2 are changed by enabling VHE
> so that they resemble CPACR_EL1, so existing code to initialise
> CPTR_EL2 becomes architecturally wrong in this case.
> 
> This patch simply skips the affected initialisation code in the
> non-VHE case.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
> 
> Note -- not tested yet, and I'm still unclear on whether this is the
> correct architectural approach...

This looks correct to me. Given that we're not leaving EL2, there is no
reason to do things earlier than they are done at EL1, and this fixes an
obvious bug on CPTR_EL2 access.

I've given it a go on a VHE model, and it ran just fine, so:

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [RFC PATCH] arm64: Fix EL1/EL2 early init inconsistencies with VHE

[Catalin Marinas]

On Mon, Apr 18, 2016 at 06:57:26PM +0100, Dave P Martin wrote:
> When using the Virtualisation Host Extensions, EL1 is not used in
> the host and requires no separate configuration.
> 
> In addition, with VHE enabled, non-hyp-specific EL2 configuration
> that does not need to be done early will be done anyway in
> __cpu_setup via the _EL1 system register aliases.  In particular,
> the layout and definition of CPTR_EL2 are changed by enabling VHE
> so that they resemble CPACR_EL1, so existing code to initialise
> CPTR_EL2 becomes architecturally wrong in this case.
> 
> This patch simply skips the affected initialisation code in the
> non-VHE case.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>

I queued this for 4.6 (with Marc's ack). Thanks.

-- 
Catalin

[RFC PATCH] arm64: Fix EL1/EL2 early init inconsistencies with VHE

[Catalin Marinas]

On Mon, Apr 18, 2016 at 06:57:26PM +0100, Dave P Martin wrote:
> When using the Virtualisation Host Extensions, EL1 is not used in
> the host and requires no separate configuration.
> 
> In addition, with VHE enabled, non-hyp-specific EL2 configuration
> that does not need to be done early will be done anyway in
> __cpu_setup via the _EL1 system register aliases.  In particular,
> the layout and definition of CPTR_EL2 are changed by enabling VHE
> so that they resemble CPACR_EL1, so existing code to initialise
> CPTR_EL2 becomes architecturally wrong in this case.
> 
> This patch simply skips the affected initialisation code in the
> non-VHE case.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>

I queued this for 4.6 (with Marc's ack). Thanks.

-- 
Catalin