[PATCH v2 0/2] arm64/sme: Fix SMPRI_EL1 traps for KVM guests

[PATCH v2 0/2] arm64/sme: Fix SMPRI_EL1 traps for KVM guests

[Mark Brown]

When adding support for SME I missed management of traps for SMPRI_EL1
in KVM guests, and also the need to manage TPIDR2_EL0 fine grained traps
for EL1.  This series adds that management, and also updates the
documentation to help avoid such issues in future.

This was noticed due to Vincent Donnefort noticing that qemu has
implemented support for SME virtualisation but not fine grained traps
which was causing problems for qemu in nVHE mode since that already used
fine grained traps with SME, the FGT registers are not implemented and
UNDEF.  As a result of this change VHE will also be affected so a
separate series "KVM: arm: Refuse to enable KVM on systems with SME but
not FGT" has been sent which will ensure we don't run on such systems
though initial review suggests that this will likely be treated as a
qemu documentation issue.

v2:
 - Rebase onto v6.1-rc2
 - Also manage TPIDR2 fine grained traps for VHE mode.

Mark Brown (2):
  arm64: booting: Document our requirements for fine grained traps with
    SME
  KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

 Documentation/arm64/booting.rst |  8 ++++++++
 arch/arm64/kvm/hyp/vhe/switch.c | 26 ++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)


base-commit: 30a0b95b1335e12efef89dd78518ed3e4a71a763
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 0/2] arm64/sme: Fix SMPRI_EL1 traps for KVM guests

[Mark Brown]

When adding support for SME I missed management of traps for SMPRI_EL1
in KVM guests, and also the need to manage TPIDR2_EL0 fine grained traps
for EL1.  This series adds that management, and also updates the
documentation to help avoid such issues in future.

This was noticed due to Vincent Donnefort noticing that qemu has
implemented support for SME virtualisation but not fine grained traps
which was causing problems for qemu in nVHE mode since that already used
fine grained traps with SME, the FGT registers are not implemented and
UNDEF.  As a result of this change VHE will also be affected so a
separate series "KVM: arm: Refuse to enable KVM on systems with SME but
not FGT" has been sent which will ensure we don't run on such systems
though initial review suggests that this will likely be treated as a
qemu documentation issue.

v2:
 - Rebase onto v6.1-rc2
 - Also manage TPIDR2 fine grained traps for VHE mode.

Mark Brown (2):
  arm64: booting: Document our requirements for fine grained traps with
    SME
  KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

 Documentation/arm64/booting.rst |  8 ++++++++
 arch/arm64/kvm/hyp/vhe/switch.c | 26 ++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)


base-commit: 30a0b95b1335e12efef89dd78518ed3e4a71a763
-- 
2.30.2

[PATCH v2 1/2] arm64: booting: Document our requirements for fine grained traps with SME

[Mark Brown]

With SME we require that fine grained traps on access to TPIDR2_EL0 and
SMPRI_EL1 are disabled but did not document that fact. Add the relevant
register bits.

Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 Documentation/arm64/booting.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/arm64/booting.rst b/Documentation/arm64/booting.rst
index 8aefa1001ae5..8c324ad638de 100644
--- a/Documentation/arm64/booting.rst
+++ b/Documentation/arm64/booting.rst
@@ -340,6 +340,14 @@ Before jumping into the kernel, the following conditions must be met:
     - SMCR_EL2.LEN must be initialised to the same value for all CPUs the
       kernel will execute on.
 
+    - HWFGRTR_EL2.nTPIDR2_EL0 (bit 55) must be initialised to 0b01.
+
+    - HWFGWTR_EL2.nTPIDR2_EL0 (bit 55) must be initialised to 0b01.
+
+    - HWFGRTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
+
+    - HWFGWTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
+
   For CPUs with the Scalable Matrix Extension FA64 feature (FEAT_SME_FA64)
 
   - If EL3 is present:
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/2] arm64: booting: Document our requirements for fine grained traps with SME

[Mark Brown]

With SME we require that fine grained traps on access to TPIDR2_EL0 and
SMPRI_EL1 are disabled but did not document that fact. Add the relevant
register bits.

Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 Documentation/arm64/booting.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/arm64/booting.rst b/Documentation/arm64/booting.rst
index 8aefa1001ae5..8c324ad638de 100644
--- a/Documentation/arm64/booting.rst
+++ b/Documentation/arm64/booting.rst
@@ -340,6 +340,14 @@ Before jumping into the kernel, the following conditions must be met:
     - SMCR_EL2.LEN must be initialised to the same value for all CPUs the
       kernel will execute on.
 
+    - HWFGRTR_EL2.nTPIDR2_EL0 (bit 55) must be initialised to 0b01.
+
+    - HWFGWTR_EL2.nTPIDR2_EL0 (bit 55) must be initialised to 0b01.
+
+    - HWFGRTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
+
+    - HWFGWTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
+
   For CPUs with the Scalable Matrix Extension FA64 feature (FEAT_SME_FA64)
 
   - If EL3 is present:
-- 
2.30.2

[PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Mark Brown]

On systems with SME access to the SMPRI_EL1 priority management register is
controlled by the nSMPRI_EL1 fine grained trap and TPIDR2_EL0 is controlled
by nTPIDR2_EL0. We manage these traps in nVHE mode but do not do so when in
VHE mode, add the required management.

Without this these registers could be used as side channels where implemented.

Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/hyp/vhe/switch.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 7acb87eaa092..9dac3a1a85f7 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -63,10 +63,20 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 		__activate_traps_fpsimd32(vcpu);
 	}
 
-	if (cpus_have_final_cap(ARM64_SME))
+	if (cpus_have_final_cap(ARM64_SME)) {
 		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
 			     sctlr_el2);
 
+		sysreg_clear_set_s(SYS_HFGRTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+	}
+
 	write_sysreg(val, cpacr_el1);
 
 	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el1);
@@ -88,9 +98,21 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 	 */
 	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_SPECULATIVE_AT));
 
-	if (cpus_have_final_cap(ARM64_SME))
+	if (cpus_have_final_cap(ARM64_SME)) {
+		/*
+		 * Enable access to SMPRI_EL1 - we don't need to
+		 * control nTPIDR2_EL0 in VHE mode.
+		 */
+		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+
 		write_sysreg(read_sysreg(sctlr_el2) | SCTLR_ELx_ENTP2,
 			     sctlr_el2);
+	}
 
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 
-- 
2.30.2

[PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Mark Brown]

On systems with SME access to the SMPRI_EL1 priority management register is
controlled by the nSMPRI_EL1 fine grained trap and TPIDR2_EL0 is controlled
by nTPIDR2_EL0. We manage these traps in nVHE mode but do not do so when in
VHE mode, add the required management.

Without this these registers could be used as side channels where implemented.

Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/hyp/vhe/switch.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 7acb87eaa092..9dac3a1a85f7 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -63,10 +63,20 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 		__activate_traps_fpsimd32(vcpu);
 	}
 
-	if (cpus_have_final_cap(ARM64_SME))
+	if (cpus_have_final_cap(ARM64_SME)) {
 		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
 			     sctlr_el2);
 
+		sysreg_clear_set_s(SYS_HFGRTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+	}
+
 	write_sysreg(val, cpacr_el1);
 
 	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el1);
@@ -88,9 +98,21 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 	 */
 	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_SPECULATIVE_AT));
 
-	if (cpus_have_final_cap(ARM64_SME))
+	if (cpus_have_final_cap(ARM64_SME)) {
+		/*
+		 * Enable access to SMPRI_EL1 - we don't need to
+		 * control nTPIDR2_EL0 in VHE mode.
+		 */
+		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+
 		write_sysreg(read_sysreg(sctlr_el2) | SCTLR_ELx_ENTP2,
 			     sctlr_el2);
+	}
 
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Marc Zyngier]

On Tue, 01 Nov 2022 11:27:15 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> On systems with SME access to the SMPRI_EL1 priority management register is
> controlled by the nSMPRI_EL1 fine grained trap and TPIDR2_EL0 is controlled
> by nTPIDR2_EL0. We manage these traps in nVHE mode but do not do so when in
> VHE mode, add the required management.
> 
> Without this these registers could be used as side channels where implemented.
> 
> Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  arch/arm64/kvm/hyp/vhe/switch.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index 7acb87eaa092..9dac3a1a85f7 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -63,10 +63,20 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
>  		__activate_traps_fpsimd32(vcpu);
>  	}
>  
> -	if (cpus_have_final_cap(ARM64_SME))
> +	if (cpus_have_final_cap(ARM64_SME)) {
>  		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
>  			     sctlr_el2);

I still question this. As far as I can tell, it only affects the host
context (HCR_EL2.{E2H,TGE}={1,1}).

This is outlined in the description of the HFGWTR_EL2.nTPIDR2_EL0 bit:

<quote>
	If EL2 is implemented and enabled in the current Security
	state, HCR_EL2.{E2H, TGE} != {1, 1}, and either EL3 is
	not implemented or SCR_EL3.FGTEn == 1, then MSR writes
	of TPIDR2_EL0 at EL1 and EL0 using AArch64 are
	trapped to EL2 and reported with EC syndrome value 0x18,
	unless the write generates a higher priority exception.
</quote>

Similar language is there for the read counterpart.

So I can only conclude that messing with SCTLR_EL2 is superfluous and
doesn't affect the execution in a guest context.

>  
> +		sysreg_clear_set_s(SYS_HFGRTR_EL2,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> +				   0);
> +		sysreg_clear_set_s(SYS_HFGWTR_EL2,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> +				   0);
> +	}
> +
>  	write_sysreg(val, cpacr_el1);
>  
>  	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el1);
> @@ -88,9 +98,21 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
>  	 */
>  	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_SPECULATIVE_AT));
>  
> -	if (cpus_have_final_cap(ARM64_SME))
> +	if (cpus_have_final_cap(ARM64_SME)) {
> +		/*
> +		 * Enable access to SMPRI_EL1 - we don't need to
> +		 * control nTPIDR2_EL0 in VHE mode.
> +		 */

This comment is factually wrong.

> +		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> +		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> +
>  		write_sysreg(read_sysreg(sctlr_el2) | SCTLR_ELx_ENTP2,
>  			     sctlr_el2);

Same as above. It serves no purpose AFAICT.

> +	}
>  
>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>  

And then you realise that the VHE and nVHE behaviours are strictly
identical and that they should simply be merged. Or rather, that we
should make use of the existing infrastructure to enable/disable the
trapping of the SME ops when it actually matters: at load/put time for
VHE, at enter/exit for nVHE.

I came up with the following patch, compile-tested only.

Thanks,

	M.

From 5c0913a6de47e32b071ea5534319015e0a61db71 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Tue, 1 Nov 2022 12:19:51 +0000
Subject: [PATCH] KVM: arm64: Fix SMPRI_EL1/TPIDR2_EL0 trapping on VHE

The trapping of SMPRI_EL1 and TPIDR2_EL0 currently only really
work on nVHE, as only this mode uses the fine-grained trapping
that controls these two registers.

Move the trapping enable/disable code into
__{de,}activate_traps_common(), allowing it to be called when it
actually matters on VHE, and remove the flipping of EL2 control
for TPIDR2_EL0, which only affects the host access of this
register.

Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 20 +++++++++++++++++++
 arch/arm64/kvm/hyp/nvhe/switch.c        | 26 -------------------------
 arch/arm64/kvm/hyp/vhe/switch.c         |  8 --------
 3 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 6cbbb6c02f66..3330d1b76bdd 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -87,6 +87,17 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
 
 	vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2);
 	write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
+
+	if (cpus_have_final_cap(ARM64_SME)) {
+		sysreg_clear_set_s(SYS_HFGRTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+	}
 }
 
 static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
@@ -96,6 +107,15 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
 	write_sysreg(0, hstr_el2);
 	if (kvm_arm_support_pmu_v3())
 		write_sysreg(0, pmuserenr_el0);
+
+	if (cpus_have_final_cap(ARM64_SME)) {
+		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+	}
 }
 
 static inline void ___activate_traps(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 8e9d49a964be..c2cb46ca4fb6 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -55,18 +55,6 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 	write_sysreg(val, cptr_el2);
 	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el2);
 
-	if (cpus_have_final_cap(ARM64_SME)) {
-		val = read_sysreg_s(SYS_HFGRTR_EL2);
-		val &= ~(HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			 HFGxTR_EL2_nSMPRI_EL1_MASK);
-		write_sysreg_s(val, SYS_HFGRTR_EL2);
-
-		val = read_sysreg_s(SYS_HFGWTR_EL2);
-		val &= ~(HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			 HFGxTR_EL2_nSMPRI_EL1_MASK);
-		write_sysreg_s(val, SYS_HFGWTR_EL2);
-	}
-
 	if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
 		struct kvm_cpu_context *ctxt = &vcpu->arch.ctxt;
 
@@ -110,20 +98,6 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 
 	write_sysreg(this_cpu_ptr(&kvm_init_params)->hcr_el2, hcr_el2);
 
-	if (cpus_have_final_cap(ARM64_SME)) {
-		u64 val;
-
-		val = read_sysreg_s(SYS_HFGRTR_EL2);
-		val |= HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			HFGxTR_EL2_nSMPRI_EL1_MASK;
-		write_sysreg_s(val, SYS_HFGRTR_EL2);
-
-		val = read_sysreg_s(SYS_HFGWTR_EL2);
-		val |= HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			HFGxTR_EL2_nSMPRI_EL1_MASK;
-		write_sysreg_s(val, SYS_HFGWTR_EL2);
-	}
-
 	cptr = CPTR_EL2_DEFAULT;
 	if (vcpu_has_sve(vcpu) && (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED))
 		cptr |= CPTR_EL2_TZ;
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 7acb87eaa092..1a97391fedd2 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -63,10 +63,6 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 		__activate_traps_fpsimd32(vcpu);
 	}
 
-	if (cpus_have_final_cap(ARM64_SME))
-		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
-			     sctlr_el2);
-
 	write_sysreg(val, cpacr_el1);
 
 	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el1);
@@ -88,10 +84,6 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 	 */
 	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_SPECULATIVE_AT));
 
-	if (cpus_have_final_cap(ARM64_SME))
-		write_sysreg(read_sysreg(sctlr_el2) | SCTLR_ELx_ENTP2,
-			     sctlr_el2);
-
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 
 	if (!arm64_kernel_unmapped_at_el0())
-- 
2.34.1


-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Marc Zyngier]

On Tue, 01 Nov 2022 11:27:15 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> On systems with SME access to the SMPRI_EL1 priority management register is
> controlled by the nSMPRI_EL1 fine grained trap and TPIDR2_EL0 is controlled
> by nTPIDR2_EL0. We manage these traps in nVHE mode but do not do so when in
> VHE mode, add the required management.
> 
> Without this these registers could be used as side channels where implemented.
> 
> Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  arch/arm64/kvm/hyp/vhe/switch.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index 7acb87eaa092..9dac3a1a85f7 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -63,10 +63,20 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
>  		__activate_traps_fpsimd32(vcpu);
>  	}
>  
> -	if (cpus_have_final_cap(ARM64_SME))
> +	if (cpus_have_final_cap(ARM64_SME)) {
>  		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
>  			     sctlr_el2);

I still question this. As far as I can tell, it only affects the host
context (HCR_EL2.{E2H,TGE}={1,1}).

This is outlined in the description of the HFGWTR_EL2.nTPIDR2_EL0 bit:

<quote>
	If EL2 is implemented and enabled in the current Security
	state, HCR_EL2.{E2H, TGE} != {1, 1}, and either EL3 is
	not implemented or SCR_EL3.FGTEn == 1, then MSR writes
	of TPIDR2_EL0 at EL1 and EL0 using AArch64 are
	trapped to EL2 and reported with EC syndrome value 0x18,
	unless the write generates a higher priority exception.
</quote>

Similar language is there for the read counterpart.

So I can only conclude that messing with SCTLR_EL2 is superfluous and
doesn't affect the execution in a guest context.

>  
> +		sysreg_clear_set_s(SYS_HFGRTR_EL2,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> +				   0);
> +		sysreg_clear_set_s(SYS_HFGWTR_EL2,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
> +				   0);
> +	}
> +
>  	write_sysreg(val, cpacr_el1);
>  
>  	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el1);
> @@ -88,9 +98,21 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
>  	 */
>  	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_SPECULATIVE_AT));
>  
> -	if (cpus_have_final_cap(ARM64_SME))
> +	if (cpus_have_final_cap(ARM64_SME)) {
> +		/*
> +		 * Enable access to SMPRI_EL1 - we don't need to
> +		 * control nTPIDR2_EL0 in VHE mode.
> +		 */

This comment is factually wrong.

> +		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> +		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
> +				   HFGxTR_EL2_nSMPRI_EL1_MASK |
> +				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
> +
>  		write_sysreg(read_sysreg(sctlr_el2) | SCTLR_ELx_ENTP2,
>  			     sctlr_el2);

Same as above. It serves no purpose AFAICT.

> +	}
>  
>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>  

And then you realise that the VHE and nVHE behaviours are strictly
identical and that they should simply be merged. Or rather, that we
should make use of the existing infrastructure to enable/disable the
trapping of the SME ops when it actually matters: at load/put time for
VHE, at enter/exit for nVHE.

I came up with the following patch, compile-tested only.

Thanks,

	M.

From 5c0913a6de47e32b071ea5534319015e0a61db71 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Tue, 1 Nov 2022 12:19:51 +0000
Subject: [PATCH] KVM: arm64: Fix SMPRI_EL1/TPIDR2_EL0 trapping on VHE

The trapping of SMPRI_EL1 and TPIDR2_EL0 currently only really
work on nVHE, as only this mode uses the fine-grained trapping
that controls these two registers.

Move the trapping enable/disable code into
__{de,}activate_traps_common(), allowing it to be called when it
actually matters on VHE, and remove the flipping of EL2 control
for TPIDR2_EL0, which only affects the host access of this
register.

Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 20 +++++++++++++++++++
 arch/arm64/kvm/hyp/nvhe/switch.c        | 26 -------------------------
 arch/arm64/kvm/hyp/vhe/switch.c         |  8 --------
 3 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 6cbbb6c02f66..3330d1b76bdd 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -87,6 +87,17 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
 
 	vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2);
 	write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
+
+	if (cpus_have_final_cap(ARM64_SME)) {
+		sysreg_clear_set_s(SYS_HFGRTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK,
+				   0);
+	}
 }
 
 static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
@@ -96,6 +107,15 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
 	write_sysreg(0, hstr_el2);
 	if (kvm_arm_support_pmu_v3())
 		write_sysreg(0, pmuserenr_el0);
+
+	if (cpus_have_final_cap(ARM64_SME)) {
+		sysreg_clear_set_s(SYS_HFGRTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+		sysreg_clear_set_s(SYS_HFGWTR_EL2, 0,
+				   HFGxTR_EL2_nSMPRI_EL1_MASK |
+				   HFGxTR_EL2_nTPIDR2_EL0_MASK);
+	}
 }
 
 static inline void ___activate_traps(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 8e9d49a964be..c2cb46ca4fb6 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -55,18 +55,6 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 	write_sysreg(val, cptr_el2);
 	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el2);
 
-	if (cpus_have_final_cap(ARM64_SME)) {
-		val = read_sysreg_s(SYS_HFGRTR_EL2);
-		val &= ~(HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			 HFGxTR_EL2_nSMPRI_EL1_MASK);
-		write_sysreg_s(val, SYS_HFGRTR_EL2);
-
-		val = read_sysreg_s(SYS_HFGWTR_EL2);
-		val &= ~(HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			 HFGxTR_EL2_nSMPRI_EL1_MASK);
-		write_sysreg_s(val, SYS_HFGWTR_EL2);
-	}
-
 	if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
 		struct kvm_cpu_context *ctxt = &vcpu->arch.ctxt;
 
@@ -110,20 +98,6 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 
 	write_sysreg(this_cpu_ptr(&kvm_init_params)->hcr_el2, hcr_el2);
 
-	if (cpus_have_final_cap(ARM64_SME)) {
-		u64 val;
-
-		val = read_sysreg_s(SYS_HFGRTR_EL2);
-		val |= HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			HFGxTR_EL2_nSMPRI_EL1_MASK;
-		write_sysreg_s(val, SYS_HFGRTR_EL2);
-
-		val = read_sysreg_s(SYS_HFGWTR_EL2);
-		val |= HFGxTR_EL2_nTPIDR2_EL0_MASK |
-			HFGxTR_EL2_nSMPRI_EL1_MASK;
-		write_sysreg_s(val, SYS_HFGWTR_EL2);
-	}
-
 	cptr = CPTR_EL2_DEFAULT;
 	if (vcpu_has_sve(vcpu) && (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED))
 		cptr |= CPTR_EL2_TZ;
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 7acb87eaa092..1a97391fedd2 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -63,10 +63,6 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 		__activate_traps_fpsimd32(vcpu);
 	}
 
-	if (cpus_have_final_cap(ARM64_SME))
-		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
-			     sctlr_el2);
-
 	write_sysreg(val, cpacr_el1);
 
 	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el1);
@@ -88,10 +84,6 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 	 */
 	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_SPECULATIVE_AT));
 
-	if (cpus_have_final_cap(ARM64_SME))
-		write_sysreg(read_sysreg(sctlr_el2) | SCTLR_ELx_ENTP2,
-			     sctlr_el2);
-
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 
 	if (!arm64_kernel_unmapped_at_el0())
-- 
2.34.1


-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

On Tue, Nov 01, 2022 at 01:33:16PM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > -	if (cpus_have_final_cap(ARM64_SME))
> > +	if (cpus_have_final_cap(ARM64_SME)) {
> >  		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
> >  			     sctlr_el2);

> I still question this. As far as I can tell, it only affects the host
> context (HCR_EL2.{E2H,TGE}={1,1}).

> This is outlined in the description of the HFGWTR_EL2.nTPIDR2_EL0 bit:

Oh, I see what you meant there - I was purely focusing on the new code
with the fine grained traps, not the existing code.

> So I can only conclude that messing with SCTLR_EL2 is superfluous and
> doesn't affect the execution in a guest context.

Yes, if you look at the pseudocode for TPIDR2_EL0 that's the case.  It's
either EnTP2 for HCR_EL2.<E2H,TGE> == '11' or the fine grained trap
otherwise.

> > +		/*
> > +		 * Enable access to SMPRI_EL1 - we don't need to
> > +		 * control nTPIDR2_EL0 in VHE mode.
> > +		 */

> This comment is factually wrong.

Bah, I fixed one but not the other sorry :/

> Date: Tue, 1 Nov 2022 12:19:51 +0000
> Subject: [PATCH] KVM: arm64: Fix SMPRI_EL1/TPIDR2_EL0 trapping on VHE
> 
> The trapping of SMPRI_EL1 and TPIDR2_EL0 currently only really
> work on nVHE, as only this mode uses the fine-grained trapping
> that controls these two registers.
> 
> Move the trapping enable/disable code into
> __{de,}activate_traps_common(), allowing it to be called when it
> actually matters on VHE, and remove the flipping of EL2 control
> for TPIDR2_EL0, which only affects the host access of this
> register.
> 
> Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org

Reported-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1902 bytes --]

On Tue, Nov 01, 2022 at 01:33:16PM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > -	if (cpus_have_final_cap(ARM64_SME))
> > +	if (cpus_have_final_cap(ARM64_SME)) {
> >  		write_sysreg(read_sysreg(sctlr_el2) & ~SCTLR_ELx_ENTP2,
> >  			     sctlr_el2);

> I still question this. As far as I can tell, it only affects the host
> context (HCR_EL2.{E2H,TGE}={1,1}).

> This is outlined in the description of the HFGWTR_EL2.nTPIDR2_EL0 bit:

Oh, I see what you meant there - I was purely focusing on the new code
with the fine grained traps, not the existing code.

> So I can only conclude that messing with SCTLR_EL2 is superfluous and
> doesn't affect the execution in a guest context.

Yes, if you look at the pseudocode for TPIDR2_EL0 that's the case.  It's
either EnTP2 for HCR_EL2.<E2H,TGE> == '11' or the fine grained trap
otherwise.

> > +		/*
> > +		 * Enable access to SMPRI_EL1 - we don't need to
> > +		 * control nTPIDR2_EL0 in VHE mode.
> > +		 */

> This comment is factually wrong.

Bah, I fixed one but not the other sorry :/

> Date: Tue, 1 Nov 2022 12:19:51 +0000
> Subject: [PATCH] KVM: arm64: Fix SMPRI_EL1/TPIDR2_EL0 trapping on VHE
> 
> The trapping of SMPRI_EL1 and TPIDR2_EL0 currently only really
> work on nVHE, as only this mode uses the fine-grained trapping
> that controls these two registers.
> 
> Move the trapping enable/disable code into
> __{de,}activate_traps_common(), allowing it to be called when it
> actually matters on VHE, and remove the flipping of EL2 control
> for TPIDR2_EL0, which only affects the host access of this
> register.
> 
> Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org

Reported-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: (subset) [PATCH v2 0/2] arm64/sme: Fix SMPRI_EL1 traps for KVM guests

[Marc Zyngier]

On Tue, 1 Nov 2022 11:27:13 +0000, Mark Brown wrote:
> When adding support for SME I missed management of traps for SMPRI_EL1
> in KVM guests, and also the need to manage TPIDR2_EL0 fine grained traps
> for EL1.  This series adds that management, and also updates the
> documentation to help avoid such issues in future.
> 
> This was noticed due to Vincent Donnefort noticing that qemu has
> implemented support for SME virtualisation but not fine grained traps
> which was causing problems for qemu in nVHE mode since that already used
> fine grained traps with SME, the FGT registers are not implemented and
> UNDEF.  As a result of this change VHE will also be affected so a
> separate series "KVM: arm: Refuse to enable KVM on systems with SME but
> not FGT" has been sent which will ensure we don't run on such systems
> though initial review suggests that this will likely be treated as a
> qemu documentation issue.
> 
> [...]

Applied to fixes, thanks!

[1/2] arm64: booting: Document our requirements for fine grained traps with SME
      commit: be0ddf5293a7895a8c9096e1a8560930c6a0ab3f

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: (subset) [PATCH v2 0/2] arm64/sme: Fix SMPRI_EL1 traps for KVM guests

[Marc Zyngier]

On Tue, 1 Nov 2022 11:27:13 +0000, Mark Brown wrote:
> When adding support for SME I missed management of traps for SMPRI_EL1
> in KVM guests, and also the need to manage TPIDR2_EL0 fine grained traps
> for EL1.  This series adds that management, and also updates the
> documentation to help avoid such issues in future.
> 
> This was noticed due to Vincent Donnefort noticing that qemu has
> implemented support for SME virtualisation but not fine grained traps
> which was causing problems for qemu in nVHE mode since that already used
> fine grained traps with SME, the FGT registers are not implemented and
> UNDEF.  As a result of this change VHE will also be affected so a
> separate series "KVM: arm: Refuse to enable KVM on systems with SME but
> not FGT" has been sent which will ensure we don't run on such systems
> though initial review suggests that this will likely be treated as a
> qemu documentation issue.
> 
> [...]

Applied to fixes, thanks!

[1/2] arm64: booting: Document our requirements for fine grained traps with SME
      commit: be0ddf5293a7895a8c9096e1a8560930c6a0ab3f

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Marc Zyngier]

On Tue, 01 Nov 2022 14:30:09 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Tue, Nov 01, 2022 at 01:33:16PM +0000, Marc Zyngier wrote:
> > Date: Tue, 1 Nov 2022 12:19:51 +0000
> > Subject: [PATCH] KVM: arm64: Fix SMPRI_EL1/TPIDR2_EL0 trapping on VHE
> > 
> > The trapping of SMPRI_EL1 and TPIDR2_EL0 currently only really
> > work on nVHE, as only this mode uses the fine-grained trapping
> > that controls these two registers.
> > 
> > Move the trapping enable/disable code into
> > __{de,}activate_traps_common(), allowing it to be called when it
> > actually matters on VHE, and remove the flipping of EL2 control
> > for TPIDR2_EL0, which only affects the host access of this
> > register.
> > 
> > Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Cc: stable@vger.kernel.org
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Thanks. I've now applied the first patch and this one to kvmarm/fixes.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v2 2/2] KVM: arm64: Trap access to SMPRI_EL1 and TPIDR2 in VHE mode

[Marc Zyngier]

On Tue, 01 Nov 2022 14:30:09 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Tue, Nov 01, 2022 at 01:33:16PM +0000, Marc Zyngier wrote:
> > Date: Tue, 1 Nov 2022 12:19:51 +0000
> > Subject: [PATCH] KVM: arm64: Fix SMPRI_EL1/TPIDR2_EL0 trapping on VHE
> > 
> > The trapping of SMPRI_EL1 and TPIDR2_EL0 currently only really
> > work on nVHE, as only this mode uses the fine-grained trapping
> > that controls these two registers.
> > 
> > Move the trapping enable/disable code into
> > __{de,}activate_traps_common(), allowing it to be called when it
> > actually matters on VHE, and remove the flipping of EL2 control
> > for TPIDR2_EL0, which only affects the host access of this
> > register.
> > 
> > Fixes: 861262ab8627 ("KVM: arm64: Handle SME host state when running guests")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Cc: stable@vger.kernel.org
> 
> Reported-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Thanks. I've now applied the first patch and this one to kvmarm/fixes.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel