* [PATCH 0/4] KVM: arm64: Running the EL2 nVHE code with WXN
@ 2021-03-10 15:26 Marc Zyngier
2021-03-10 15:26 ` [PATCH 1/4] arm64: Use INIT_SCTLR_EL1_MMU_OFF to disable the MMU on CPU restart Marc Zyngier
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Marc Zyngier @ 2021-03-10 15:26 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Mark Rutland,
Will Deacon, qperret, kernel-team
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(-)
--
2.29.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] arm64: Use INIT_SCTLR_EL1_MMU_OFF to disable the MMU on CPU restart
2021-03-10 15:26 [PATCH 0/4] KVM: arm64: Running the EL2 nVHE code with WXN Marc Zyngier
@ 2021-03-10 15:26 ` Marc Zyngier
2021-03-10 15:26 ` [PATCH 2/4] KVM: arm64: Use INIT_SCTLR_EL2_MMU_OFF to disable the MMU on KVM teardown Marc Zyngier
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2021-03-10 15:26 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Mark Rutland,
Will Deacon, qperret, kernel-team
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 <maz@kernel.org>
---
arch/arm64/kernel/cpu-reset.S | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
index 37721eb6f9a1..d47ff63a5b66 100644
--- a/arch/arm64/kernel/cpu-reset.S
+++ b/arch/arm64/kernel/cpu-reset.S
@@ -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
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] KVM: arm64: Use INIT_SCTLR_EL2_MMU_OFF to disable the MMU on KVM teardown
2021-03-10 15:26 [PATCH 0/4] KVM: arm64: Running the EL2 nVHE code with WXN Marc Zyngier
2021-03-10 15:26 ` [PATCH 1/4] arm64: Use INIT_SCTLR_EL1_MMU_OFF to disable the MMU on CPU restart Marc Zyngier
@ 2021-03-10 15:26 ` Marc Zyngier
2021-03-10 15:26 ` [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS Marc Zyngier
2021-03-10 15:26 ` [PATCH 4/4] KVM: arm64: Force SCTLR_EL2.WXN when running nVHE Marc Zyngier
3 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2021-03-10 15:26 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Mark Rutland,
Will Deacon, qperret, kernel-team
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 <maz@kernel.org>
---
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
index c631e29fb001..4eb584ae13d9 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
@@ -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
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-10 15:26 [PATCH 0/4] KVM: arm64: Running the EL2 nVHE code with WXN Marc Zyngier
2021-03-10 15:26 ` [PATCH 1/4] arm64: Use INIT_SCTLR_EL1_MMU_OFF to disable the MMU on CPU restart Marc Zyngier
2021-03-10 15:26 ` [PATCH 2/4] KVM: arm64: Use INIT_SCTLR_EL2_MMU_OFF to disable the MMU on KVM teardown Marc Zyngier
@ 2021-03-10 15:26 ` Marc Zyngier
2021-03-10 15:46 ` Will Deacon
2021-03-10 15:26 ` [PATCH 4/4] KVM: arm64: Force SCTLR_EL2.WXN when running nVHE Marc Zyngier
3 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2021-03-10 15:26 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Mark Rutland,
Will Deacon, qperret, kernel-team
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 <maz@kernel.org>
---
arch/arm64/include/asm/sysreg.h | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index dfd4edbfe360..9d1aef631646 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -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. */
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
index 4eb584ae13d9..7423f4d961a4 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
@@ -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 | \
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] KVM: arm64: Force SCTLR_EL2.WXN when running nVHE
2021-03-10 15:26 [PATCH 0/4] KVM: arm64: Running the EL2 nVHE code with WXN Marc Zyngier
` (2 preceding siblings ...)
2021-03-10 15:26 ` [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS Marc Zyngier
@ 2021-03-10 15:26 ` Marc Zyngier
3 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2021-03-10 15:26 UTC (permalink / raw)
To: kvm, kvmarm, linux-arm-kernel
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Mark Rutland,
Will Deacon, qperret, kernel-team
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 <maz@kernel.org>
---
arch/arm64/include/asm/sysreg.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 9d1aef631646..cc17f7d487ed 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -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)) | \
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-10 15:26 ` [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS Marc Zyngier
@ 2021-03-10 15:46 ` Will Deacon
2021-03-10 16:05 ` Marc Zyngier
0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2021-03-10 15:46 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, kvmarm, linux-arm-kernel, James Morse, Julien Thierry,
Suzuki K Poulose, Mark Rutland, qperret, kernel-team
On Wed, Mar 10, 2021 at 03:26:55PM +0000, Marc Zyngier wrote:
> 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 <maz@kernel.org>
> ---
> arch/arm64/include/asm/sysreg.h | 2 +-
> arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index dfd4edbfe360..9d1aef631646 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -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. */
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> index 4eb584ae13d9..7423f4d961a4 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> @@ -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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-10 15:46 ` Will Deacon
@ 2021-03-10 16:05 ` Marc Zyngier
2021-03-10 16:15 ` Will Deacon
0 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2021-03-10 16:05 UTC (permalink / raw)
To: Will Deacon
Cc: kvm, kvmarm, linux-arm-kernel, James Morse, Julien Thierry,
Suzuki K Poulose, Mark Rutland, qperret, kernel-team
On Wed, 10 Mar 2021 15:46:26 +0000,
Will Deacon <will@kernel.org> wrote:
>
> On Wed, Mar 10, 2021 at 03:26:55PM +0000, Marc Zyngier wrote:
> > 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 <maz@kernel.org>
> > ---
> > arch/arm64/include/asm/sysreg.h | 2 +-
> > arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> > index dfd4edbfe360..9d1aef631646 100644
> > --- a/arch/arm64/include/asm/sysreg.h
> > +++ b/arch/arm64/include/asm/sysreg.h
> > @@ -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. */
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > index 4eb584ae13d9..7423f4d961a4 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > @@ -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.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-10 16:05 ` Marc Zyngier
@ 2021-03-10 16:15 ` Will Deacon
2021-03-10 17:49 ` Marc Zyngier
0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2021-03-10 16:15 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, kvmarm, linux-arm-kernel, James Morse, Julien Thierry,
Suzuki K Poulose, Mark Rutland, qperret, kernel-team
On Wed, Mar 10, 2021 at 04:05:17PM +0000, Marc Zyngier wrote:
> On Wed, 10 Mar 2021 15:46:26 +0000,
> Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Mar 10, 2021 at 03:26:55PM +0000, Marc Zyngier wrote:
> > > 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 <maz@kernel.org>
> > > ---
> > > arch/arm64/include/asm/sysreg.h | 2 +-
> > > arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> > > index dfd4edbfe360..9d1aef631646 100644
> > > --- a/arch/arm64/include/asm/sysreg.h
> > > +++ b/arch/arm64/include/asm/sysreg.h
> > > @@ -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. */
> > > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > index 4eb584ae13d9..7423f4d961a4 100644
> > > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > @@ -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.
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-10 16:15 ` Will Deacon
@ 2021-03-10 17:49 ` Marc Zyngier
2021-03-10 18:20 ` Will Deacon
0 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2021-03-10 17:49 UTC (permalink / raw)
To: Will Deacon
Cc: kvm, kvmarm, linux-arm-kernel, James Morse, Julien Thierry,
Suzuki K Poulose, Mark Rutland, qperret, kernel-team
On Wed, 10 Mar 2021 16:15:47 +0000,
Will Deacon <will@kernel.org> wrote:
>
> On Wed, Mar 10, 2021 at 04:05:17PM +0000, Marc Zyngier wrote:
> > On Wed, 10 Mar 2021 15:46:26 +0000,
> > Will Deacon <will@kernel.org> wrote:
> > >
> > > On Wed, Mar 10, 2021 at 03:26:55PM +0000, Marc Zyngier wrote:
> > > > 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 <maz@kernel.org>
> > > > ---
> > > > arch/arm64/include/asm/sysreg.h | 2 +-
> > > > arch/arm64/kvm/hyp/nvhe/hyp-init.S | 2 +-
> > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> > > > index dfd4edbfe360..9d1aef631646 100644
> > > > --- a/arch/arm64/include/asm/sysreg.h
> > > > +++ b/arch/arm64/include/asm/sysreg.h
> > > > @@ -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. */
> > > > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > index 4eb584ae13d9..7423f4d961a4 100644
> > > > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > @@ -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.
>
> 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?
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index dfd4edbfe360..593b9bf91bbd 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -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)
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
index 4eb584ae13d9..2e16b2098bbd 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
@@ -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.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-10 17:49 ` Marc Zyngier
@ 2021-03-10 18:20 ` Will Deacon
2021-03-11 11:35 ` Mark Rutland
0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2021-03-10 18:20 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvm, kvmarm, linux-arm-kernel, James Morse, Julien Thierry,
Suzuki K Poulose, Mark Rutland, qperret, kernel-team
On Wed, Mar 10, 2021 at 05:49:17PM +0000, Marc Zyngier wrote:
> On Wed, 10 Mar 2021 16:15:47 +0000,
> Will Deacon <will@kernel.org> wrote:
> > On Wed, Mar 10, 2021 at 04:05:17PM +0000, Marc Zyngier wrote:
> > > On Wed, 10 Mar 2021 15:46:26 +0000,
> > > Will Deacon <will@kernel.org> wrote:
> > > > On Wed, Mar 10, 2021 at 03:26:55PM +0000, Marc Zyngier wrote:
> > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > > index 4eb584ae13d9..7423f4d961a4 100644
> > > > > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > > @@ -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.
> >
> > 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?
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index dfd4edbfe360..593b9bf91bbd 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -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)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> index 4eb584ae13d9..2e16b2098bbd 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> @@ -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)
Beautiful!
With that, you can have my ack on the whole series:
Acked-by: Will Deacon <will@kernel.org>
Will
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-10 18:20 ` Will Deacon
@ 2021-03-11 11:35 ` Mark Rutland
2021-03-11 12:00 ` Mark Rutland
0 siblings, 1 reply; 12+ messages in thread
From: Mark Rutland @ 2021-03-11 11:35 UTC (permalink / raw)
To: Will Deacon
Cc: Marc Zyngier, kvm, kvmarm, linux-arm-kernel, James Morse,
Julien Thierry, Suzuki K Poulose, qperret, kernel-team
On Wed, Mar 10, 2021 at 06:20:22PM +0000, Will Deacon wrote:
> On Wed, Mar 10, 2021 at 05:49:17PM +0000, Marc Zyngier wrote:
> > On Wed, 10 Mar 2021 16:15:47 +0000,
> > Will Deacon <will@kernel.org> wrote:
> > > On Wed, Mar 10, 2021 at 04:05:17PM +0000, Marc Zyngier wrote:
> > > > On Wed, 10 Mar 2021 15:46:26 +0000,
> > > > Will Deacon <will@kernel.org> wrote:
> > > > > On Wed, Mar 10, 2021 at 03:26:55PM +0000, Marc Zyngier wrote:
> > > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > > > index 4eb584ae13d9..7423f4d961a4 100644
> > > > > > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > > > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > > > > > @@ -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.
> > >
> > > 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?
> >
> > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> > index dfd4edbfe360..593b9bf91bbd 100644
> > --- a/arch/arm64/include/asm/sysreg.h
> > +++ b/arch/arm64/include/asm/sysreg.h
> > @@ -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)
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > index 4eb584ae13d9..2e16b2098bbd 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > @@ -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)
>
> Beautiful!
>
> With that, you can have my ack on the whole series:
>
> Acked-by: Will Deacon <will@kernel.org>
FWIW, likewise:
Acked-by: Mark Rutland <nark.rutland@arm.com>
This is really nice!
Thanks,
Mark.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS
2021-03-11 11:35 ` Mark Rutland
@ 2021-03-11 12:00 ` Mark Rutland
0 siblings, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2021-03-11 12:00 UTC (permalink / raw)
To: Will Deacon; +Cc: kvm, Marc Zyngier, kernel-team, kvmarm, linux-arm-kernel
On Thu, Mar 11, 2021 at 11:35:29AM +0000, Mark Rutland wrote:
> Acked-by: Mark Rutland <nark.rutland@arm.com>
Upon reflection, maybe I should spell my own name correctly:
Acked-by: Mark Rutland <mark.rutland@arm.com>
... lest you decide to add a Mocked-by tag instead ;)
Mark.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-03-11 12:01 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 15:26 [PATCH 0/4] KVM: arm64: Running the EL2 nVHE code with WXN Marc Zyngier
2021-03-10 15:26 ` [PATCH 1/4] arm64: Use INIT_SCTLR_EL1_MMU_OFF to disable the MMU on CPU restart Marc Zyngier
2021-03-10 15:26 ` [PATCH 2/4] KVM: arm64: Use INIT_SCTLR_EL2_MMU_OFF to disable the MMU on KVM teardown Marc Zyngier
2021-03-10 15:26 ` [PATCH 3/4] KVM: arm64: Rename SCTLR_ELx_FLAGS to SCTLR_EL2_FLAGS Marc Zyngier
2021-03-10 15:46 ` Will Deacon
2021-03-10 16:05 ` Marc Zyngier
2021-03-10 16:15 ` Will Deacon
2021-03-10 17:49 ` Marc Zyngier
2021-03-10 18:20 ` Will Deacon
2021-03-11 11:35 ` Mark Rutland
2021-03-11 12:00 ` Mark Rutland
2021-03-10 15:26 ` [PATCH 4/4] KVM: arm64: Force SCTLR_EL2.WXN when running nVHE Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).