Skip to contentSkip to conversation

KVM: arm64: Running the EL2 nVHE code with WXN

PATCH In review Marc Zyngier 3 participants 12 messages Mar 10, 2021 – Mar 11, 2021

Subsystems (3)
KVM arm64 KVM arm64
lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

cover KVM: arm64: Running the EL2 nVHE code with WXN

Since we use distinct permissions when mapping things at EL2 depending on whether they are text or data, we are already using a W^X setup with nVHE.

This trivial series aims to enforce it by setting SCTLR_EL2.WXN at all times. It just cleans up a couple of code paths so that SCTLR_ELx_FLAGS is only used by the KVM setup code, and finally sets the WXN flag permanently.

Lightly tested on an A53 system with 4KB and 64KB pages.

Thanks,

	M.

Marc Zyngier (4):
arm64: Use INIT_SCTLR_EL1_MMU_OFF to disable the MMU on CPU restart
KVM: arm64: Use INIT_SCTLR_EL2_MMU_OFF to disable the MMU on KVM
teardown
KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
KVM: arm64: Force SCTLR_EL2.WXN when running nVHE

 arch/arm64/include/asm/sysreg.h    | 5 +++--
 arch/arm64/kernel/cpu-reset.S      | 5 +----
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 6 ++----
 3 files changed, 6 insertions(+), 10 deletions(-)
Signature · 6 lines
2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
author maintainer
#1 lore raw

PATCH 1/4 arm64: Use INIT_SCTLR_EL1_MMU_OFF to disable the MMU on CPU restart

Instead of doing a RMW on SCTLR_EL1 to disable the MMU, use the existing define that loads the right set of bits.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpu-reset.S | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
arch/arm64/kernel/cpu-reset.S +1 -4
index 37721eb6f9a1..d47ff63a5b66 100644
@@ -30,10 +30,7 @@
  * flat identity mapping.
  */
 SYM_CODE_START(__cpu_soft_restart)
-	/* Clear sctlr_el1 flags. */
-	mrs	x12, sctlr_el1
-	mov_q	x13, SCTLR_ELx_FLAGS
-	bic	x12, x12, x13
+	mov_q	x12, INIT_SCTLR_EL1_MMU_OFF
 	pre_disable_mmu_workaround
 	/*
 	 * either disable EL1&0 translation regime or disable EL2&0 translation
Signature · 6 lines
2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
author maintainer
#2 lore raw

PATCH 2/4 KVM: arm64: Use INIT_SCTLR_EL2_MMU_OFF to disable the MMU on KVM teardown

In reply to Marc Zyngier

Instead of doing a RMW on SCTLR_EL2 to disable the MMU, use the existing define that loads the right set of bits.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -3
index c631e29fb001..4eb584ae13d9 100644
@@ -221,9 +221,7 @@ SYM_CODE_START(__kvm_handle_stub_hvc)
 	mov	x0, xzr
 reset:
 	/* Reset kvm back to the hyp stub. */
-	mrs	x5, sctlr_el2
-	mov_q	x6, SCTLR_ELx_FLAGS
-	bic	x5, x5, x6		// Clear SCTL_M and etc
+	mov_q	x5, INIT_SCTLR_EL2_MMU_OFF
 	pre_disable_mmu_workaround
 	msr	sctlr_el2, x5
 	isb
Signature · 6 lines
2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
author maintainer
#3 lore raw

PATCH 3/4 KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

In reply to Marc Zyngier

Only the nVHE EL2 code is using this define, so let's make it plain that it is EL2 only.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/sysreg.h    | 2 +-
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +1 -1
index dfd4edbfe360..9d1aef631646 100644
@@ -579,7 +579,7 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
+#define SCTLR_EL2_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
 			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
 
 /* SCTLR_EL2 specific flags. */
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -1
index 4eb584ae13d9..7423f4d961a4 100644
@@ -122,7 +122,7 @@ alternative_else_nop_endif
 	 * as well as the EE bit on BE. Drop the A flag since the compiler
 	 * is allowed to generate unaligned accesses.
 	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
+	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_EL2_FLAGS & ~SCTLR_ELx_A))
 CPU_BE(	orr	x0, x0, #SCTLR_ELx_EE)
 alternative_if ARM64_HAS_ADDRESS_AUTH
 	mov_q	x1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
Signature · 6 lines
2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
maintainer
#4 lore raw

Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

Quoted 27 lines · Marc Zyngier

Only the nVHE EL2 code is using this define, so let's make it plain that it is EL2 only.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/sysreg.h    | 2 +-
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +1 -1
index dfd4edbfe360..9d1aef631646 100644
@@ -579,7 +579,7 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
+#define SCTLR_EL2_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
 			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
 
 /* SCTLR_EL2 specific flags. */
arch/arm64/kvm/hyp/nvhe/hyp-init.S +0 -0
index 4eb584ae13d9..7423f4d961a4 100644

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 125

Quoted 5 lines
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -1
@@ -122,7 +122,7 @@ alternative_else_nop_endif
 	 * as well as the EE bit on BE. Drop the A flag since the compiler
 	 * is allowed to generate unaligned accesses.
 	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
+	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_EL2_FLAGS & ~SCTLR_ELx_A))

Can we just drop SCTLR_ELx_A from SCTLR_EL2_FLAGS instead of clearing it here?

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

author maintainer
#5 lore raw

Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

Quoted 23 lines · Will Deacon
Quoted 21 lines · Marc Zyngier

Only the nVHE EL2 code is using this define, so let's make it plain that it is EL2 only.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/sysreg.h    | 2 +-
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +1 -1
index dfd4edbfe360..9d1aef631646 100644
@@ -579,7 +579,7 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
+#define SCTLR_EL2_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
 			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
Quoted 14 lines

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 125

Quoted 11 lines
arch/arm64/include/asm/sysreg.h +0 -0
 /* SCTLR_EL2 specific flags. */
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -1
index 4eb584ae13d9..7423f4d961a4 100644
@@ -122,7 +122,7 @@ alternative_else_nop_endif
 	 * as well as the EE bit on BE. Drop the A flag since the compiler
 	 * is allowed to generate unaligned accesses.
 	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
+	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_EL2_FLAGS & ~SCTLR_ELx_A))

Can we just drop SCTLR_ELx_A from SCTLR_EL2_FLAGS instead of clearing it here?

Absolutely. That'd actually be an improvement.

Thanks,

	M.
Signature · 5 lines
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
maintainer
#6 lore raw

Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

Quoted 40 lines · Marc Zyngier
Quoted 23 lines · Will Deacon
Quoted 21 lines · Marc Zyngier

Only the nVHE EL2 code is using this define, so let's make it plain that it is EL2 only.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/sysreg.h    | 2 +-
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +1 -1
index dfd4edbfe360..9d1aef631646 100644
@@ -579,7 +579,7 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
+#define SCTLR_EL2_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
 			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
Quoted 14 lines

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 125

Quoted 11 lines
arch/arm64/include/asm/sysreg.h +0 -0
 /* SCTLR_EL2 specific flags. */
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -1
index 4eb584ae13d9..7423f4d961a4 100644
@@ -122,7 +122,7 @@ alternative_else_nop_endif
 	 * as well as the EE bit on BE. Drop the A flag since the compiler
 	 * is allowed to generate unaligned accesses.
 	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
+	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_EL2_FLAGS & ~SCTLR_ELx_A))

Can we just drop SCTLR_ELx_A from SCTLR_EL2_FLAGS instead of clearing it here?

Quoted 1 line

Absolutely. That'd actually be an improvement.

In fact, maybe just define INIT_SCTLR_EL2_MMU_ON to mirror what we do for EL1 (i.e. including the RES1 bits) and then use that here?

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

author maintainer
#7 lore raw

Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

Quoted 42 lines · Will Deacon
Quoted 40 lines · Marc Zyngier
Quoted 37 lines · Will Deacon
Quoted 27 lines · Marc Zyngier

Only the nVHE EL2 code is using this define, so let's make it plain that it is EL2 only.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/sysreg.h    | 2 +-
 arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +1 -1
index dfd4edbfe360..9d1aef631646 100644
@@ -579,7 +579,7 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
+#define SCTLR_EL2_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
 			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
 
 /* SCTLR_EL2 specific flags. */
arch/arm64/kvm/hyp/nvhe/hyp-init.S +0 -0
index 4eb584ae13d9..7423f4d961a4 100644

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 125

Quoted 5 lines
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -1
@@ -122,7 +122,7 @@ alternative_else_nop_endif
 	 * as well as the EE bit on BE. Drop the A flag since the compiler
 	 * is allowed to generate unaligned accesses.
 	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
+	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_EL2_FLAGS & ~SCTLR_ELx_A))

Can we just drop SCTLR_ELx_A from SCTLR_EL2_FLAGS instead of clearing it here?

Quoted 4 lines
Quoted 1 line

Absolutely. That'd actually be an improvement.

In fact, maybe just define INIT_SCTLR_EL2_MMU_ON to mirror what we do for EL1 (i.e. including the RES1 bits) and then use that here?

Like this?

arch/arm64/include/asm/sysreg.h +4 -3
index dfd4edbfe360..593b9bf91bbd 100644
@@ -579,9 +579,6 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
-			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
-
 /* SCTLR_EL2 specific flags. */
 #define SCTLR_EL2_RES1	((BIT(4))  | (BIT(5))  | (BIT(11)) | (BIT(16)) | \
 			 (BIT(18)) | (BIT(22)) | (BIT(23)) | (BIT(28)) | \

@@ -593,6 +590,10 @@
 #define ENDIAN_SET_EL2		0
 #endif
 
+#define INIT_SCTLR_EL2_MMU_ON						\
+	(SCTLR_ELx_M  | SCTLR_ELx_C | SCTLR_ELx_SA | SCTLR_ELx_I |	\
+	 SCTLR_ELx_IESB | ENDIAN_SET_EL2 | SCTLR_EL2_RES1)
+
 #define INIT_SCTLR_EL2_MMU_OFF \
 	(SCTLR_EL2_RES1 | ENDIAN_SET_EL2)
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -7
index 4eb584ae13d9..2e16b2098bbd 100644
@@ -117,13 +117,7 @@ alternative_else_nop_endif
 	tlbi	alle2
 	dsb	sy
 
-	/*
-	 * Preserve all the RES1 bits while setting the default flags,
-	 * as well as the EE bit on BE. Drop the A flag since the compiler
-	 * is allowed to generate unaligned accesses.
-	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
-CPU_BE(	orr	x0, x0, #SCTLR_ELx_EE)
+	mov_q	x0, INIT_SCTLR_EL2_MMU_ON
 alternative_if ARM64_HAS_ADDRESS_AUTH
 	mov_q	x1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
 		     SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)

	M.
Signature · 5 lines
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
maintainer Acked-by
#8 lore raw

Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

Quoted 61 lines · Marc Zyngier
Quoted 18 lines · Will Deacon
Quoted 17 lines · Marc Zyngier
Quoted 14 lines · Will Deacon

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 125

Quoted 10 lines · Marc Zyngier
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -1
index 4eb584ae13d9..7423f4d961a4 100644
@@ -122,7 +122,7 @@ alternative_else_nop_endif
 	 * as well as the EE bit on BE. Drop the A flag since the compiler
 	 * is allowed to generate unaligned accesses.
 	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
+	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_EL2_FLAGS & ~SCTLR_ELx_A))

Can we just drop SCTLR_ELx_A from SCTLR_EL2_FLAGS instead of clearing it here?

Quoted 4 lines
Quoted 1 line

Absolutely. That'd actually be an improvement.

In fact, maybe just define INIT_SCTLR_EL2_MMU_ON to mirror what we do for EL1 (i.e. including the RES1 bits) and then use that here?

Like this?

arch/arm64/include/asm/sysreg.h +4 -3
index dfd4edbfe360..593b9bf91bbd 100644
@@ -579,9 +579,6 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
-			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
-
 /* SCTLR_EL2 specific flags. */
 #define SCTLR_EL2_RES1	((BIT(4))  | (BIT(5))  | (BIT(11)) | (BIT(16)) | \
 			 (BIT(18)) | (BIT(22)) | (BIT(23)) | (BIT(28)) | \

@@ -593,6 +590,10 @@
 #define ENDIAN_SET_EL2		0
 #endif
 
+#define INIT_SCTLR_EL2_MMU_ON						\
+	(SCTLR_ELx_M  | SCTLR_ELx_C | SCTLR_ELx_SA | SCTLR_ELx_I |	\
+	 SCTLR_ELx_IESB | ENDIAN_SET_EL2 | SCTLR_EL2_RES1)
+
 #define INIT_SCTLR_EL2_MMU_OFF \
 	(SCTLR_EL2_RES1 | ENDIAN_SET_EL2)
arch/arm64/kvm/hyp/nvhe/hyp-init.S +0 -1
index 4eb584ae13d9..2e16b2098bbd 100644
@@ -117,13 +117,7 @@ alternative_else_nop_endif
 	tlbi	alle2
 	dsb	sy
 
-	/*

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 123

Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -6
-	 * Preserve all the RES1 bits while setting the default flags,
-	 * as well as the EE bit on BE. Drop the A flag since the compiler
-	 * is allowed to generate unaligned accesses.
-	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
-CPU_BE(	orr	x0, x0, #SCTLR_ELx_EE)
+	mov_q	x0, INIT_SCTLR_EL2_MMU_ON
 alternative_if ARM64_HAS_ADDRESS_AUTH
 	mov_q	x1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
 		     SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)

Beautiful!

With that, you can have my ack on the whole series:

Acked-by Will Deacon <[email protected]>

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

reviewer Acked-by
#9 lore raw

Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

Quoted 75 lines · Will Deacon
Quoted 61 lines · Marc Zyngier
Quoted 18 lines · Will Deacon
Quoted 17 lines · Marc Zyngier
Quoted 14 lines · Will Deacon

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 125

Quoted 10 lines · Marc Zyngier
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -1
index 4eb584ae13d9..7423f4d961a4 100644
@@ -122,7 +122,7 @@ alternative_else_nop_endif
 	 * as well as the EE bit on BE. Drop the A flag since the compiler
 	 * is allowed to generate unaligned accesses.
 	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
+	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_EL2_FLAGS & ~SCTLR_ELx_A))

Can we just drop SCTLR_ELx_A from SCTLR_EL2_FLAGS instead of clearing it here?

Quoted 4 lines
Quoted 1 line

Absolutely. That'd actually be an improvement.

In fact, maybe just define INIT_SCTLR_EL2_MMU_ON to mirror what we do for EL1 (i.e. including the RES1 bits) and then use that here?

Like this?

arch/arm64/include/asm/sysreg.h +4 -3
index dfd4edbfe360..593b9bf91bbd 100644
@@ -579,9 +579,6 @@
 #define SCTLR_ELx_A	(BIT(1))
 #define SCTLR_ELx_M	(BIT(0))
 
-#define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
-			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
-
 /* SCTLR_EL2 specific flags. */
 #define SCTLR_EL2_RES1	((BIT(4))  | (BIT(5))  | (BIT(11)) | (BIT(16)) | \
 			 (BIT(18)) | (BIT(22)) | (BIT(23)) | (BIT(28)) | \

@@ -593,6 +590,10 @@
 #define ENDIAN_SET_EL2		0
 #endif
 
+#define INIT_SCTLR_EL2_MMU_ON						\
+	(SCTLR_ELx_M  | SCTLR_ELx_C | SCTLR_ELx_SA | SCTLR_ELx_I |	\
+	 SCTLR_ELx_IESB | ENDIAN_SET_EL2 | SCTLR_EL2_RES1)
+
 #define INIT_SCTLR_EL2_MMU_OFF \
 	(SCTLR_EL2_RES1 | ENDIAN_SET_EL2)
arch/arm64/kvm/hyp/nvhe/hyp-init.S +0 -1
index 4eb584ae13d9..2e16b2098bbd 100644
@@ -117,13 +117,7 @@ alternative_else_nop_endif
 	tlbi	alle2
 	dsb	sy
 
-	/*

arch/arm64/kvm/hyp/nvhe/hyp-init.S alternative_else_nop_endif · line 123

Quoted 10 lines
arch/arm64/kvm/hyp/nvhe/hyp-init.S +1 -6
-	 * Preserve all the RES1 bits while setting the default flags,
-	 * as well as the EE bit on BE. Drop the A flag since the compiler
-	 * is allowed to generate unaligned accesses.
-	 */
-	mov_q	x0, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
-CPU_BE(	orr	x0, x0, #SCTLR_ELx_EE)
+	mov_q	x0, INIT_SCTLR_EL2_MMU_ON
 alternative_if ARM64_HAS_ADDRESS_AUTH
 	mov_q	x1, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
 		     SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)

Beautiful!

Quoted 3 lines

With that, you can have my ack on the whole series:

Acked-by Will Deacon <[email protected]>

FWIW, likewise:

Acked-by Mark Rutland <[email protected]>

This is really nice!

Thanks,
Mark.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

reviewer Acked-by
#10 lore raw

Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS

Quoted 1 line · Mark Rutland
Acked-by Mark Rutland <[email protected]>

Upon reflection, maybe I should spell my own name correctly:

Acked-by Mark Rutland <[email protected]>

... lest you decide to add a Mocked-by tag instead ;)

Mark.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

author maintainer
#11 lore raw

PATCH 4/4 KVM: arm64: Force SCTLR_EL2.WXN when running nVHE

In reply to Marc Zyngier

As the EL2 nVHE object is nicely split into sections and that we already differenciating permissions for data and code, we can enable SCTLR_EL2.WXN so that we don't have to worry about misconfiguration of the page tables.

Flip the WXN bit and get the ball running!

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/sysreg.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/include/asm/sysreg.h +2 -1
index 9d1aef631646..cc17f7d487ed 100644
@@ -580,7 +580,8 @@
 #define SCTLR_ELx_M	(BIT(0))
 
 #define SCTLR_EL2_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
-			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
+			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB | \
+			 SCTLR_ELx_WXN)
 
 /* SCTLR_EL2 specific flags. */
 #define SCTLR_EL2_RES1	((BIT(4))  | (BIT(5))  | (BIT(11)) | (BIT(16)) | \
Signature · 6 lines
2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm