[RFC PATCH] KVM: arm64: Treat FPEXC32_EL2 as a regular guest system register

[Dave.Martin@arm.com (Dave Martin)]

Currently FPEXC32_EL2 is handled specially when context-switching.
This made sense for arm, where FPEXC affects host execution
(including VFP/SIMD register save/restore at Hyp).

However, FPEXC has no architectural effect when running in AArch64.
The only case where an arm64 host may execute in AArch32 is when
running compat user code at EL0: the architecture explicitly
documents FPEXC as having no effect in this case either when the
kernel (EL2 in the VHE case; otherwise EL1) is AArch64.

So, we can just handle FPEXC32_EL2 (which is the AArch64 alias for
this register) as a regular AArch32-only system register and switch
it without any special handling or synchronisation.

This allows some gnarly special-case code to be eliminated from
hyp.  The extra isb() in __activate_traps_fpsimd32() is dropped
too: for the reasons explained above arm64 never required this
anyway.

Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Christoffer Dall <christoffer.dall@arm.com>
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---

I could do with some confirmation from someone that my interpretation of
the architecture is in fact correct here.  The CheckFPAdvSIMDEnabled64()
and CheckAdvSIMDEnabled() pseudocode functions may be a reasonable
starting point (this was my reference where the wordy text is unclear).

If Alexander could try this out, that would be good.

This cleanup applies on top of the following previously pubished fix:
[PULL 2/4] arm64: KVM: Only force FPEXC32_EL2.EN if trapping FPSIMD
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-September/599537.html


 arch/arm64/kvm/hyp/switch.c    | 45 ++----------------------------------------
 arch/arm64/kvm/hyp/sysreg-sr.c |  2 ++
 2 files changed, 4 insertions(+), 43 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index ca46153..0450b85 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -43,32 +43,6 @@ static bool __hyp_text update_fp_enabled(struct kvm_vcpu *vcpu)
 	return !!(vcpu->arch.flags & KVM_ARM64_FP_ENABLED);
 }
 
-/* Save the 32-bit only FPSIMD system register state */
-static void __hyp_text __fpsimd_save_fpexc32(struct kvm_vcpu *vcpu)
-{
-	if (!vcpu_el1_is_32bit(vcpu))
-		return;
-
-	vcpu->arch.ctxt.sys_regs[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
-}
-
-static void __hyp_text __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
-{
-	/*
-	 * We are about to set CPTR_EL2.TFP to trap all floating point
-	 * register accesses to EL2, however, the ARM ARM clearly states that
-	 * traps are only taken to EL2 if the operation would not otherwise
-	 * trap to EL1.  Therefore, always make sure that for 32-bit guests,
-	 * we set FPEXC.EN to prevent traps to EL1, when setting the TFP bit.
-	 * If FP/ASIMD is not implemented, FPEXC is UNDEFINED and any access to
-	 * it will cause an exception.
-	 */
-	if (vcpu_el1_is_32bit(vcpu) && system_supports_fpsimd()) {
-		write_sysreg(1 << 30, fpexc32_el2);
-		isb();
-	}
-}
-
 static void __hyp_text __activate_traps_common(struct kvm_vcpu *vcpu)
 {
 	/* Trap on AArch32 cp15 c15 (impdef sysregs) accesses (EL1 or EL0) */
@@ -98,10 +72,8 @@ static void activate_traps_vhe(struct kvm_vcpu *vcpu)
 	val = read_sysreg(cpacr_el1);
 	val |= CPACR_EL1_TTA;
 	val &= ~CPACR_EL1_ZEN;
-	if (!update_fp_enabled(vcpu)) {
+	if (!update_fp_enabled(vcpu))
 		val &= ~CPACR_EL1_FPEN;
-		__activate_traps_fpsimd32(vcpu);
-	}
 
 	write_sysreg(val, cpacr_el1);
 
@@ -116,10 +88,8 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu)
 
 	val = CPTR_EL2_DEFAULT;
 	val |= CPTR_EL2_TTA | CPTR_EL2_TZ;
-	if (!update_fp_enabled(vcpu)) {
+	if (!update_fp_enabled(vcpu))
 		val |= CPTR_EL2_TFP;
-		__activate_traps_fpsimd32(vcpu);
-	}
 
 	write_sysreg(val, cptr_el2);
 }
@@ -366,11 +336,6 @@ static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
 
 	__fpsimd_restore_state(&vcpu->arch.ctxt.gp_regs.fp_regs);
 
-	/* Skip restoring fpexc32 for AArch64 guests */
-	if (!(read_sysreg(hcr_el2) & HCR_RW))
-		write_sysreg(vcpu->arch.ctxt.sys_regs[FPEXC32_EL2],
-			     fpexc32_el2);
-
 	vcpu->arch.flags |= KVM_ARM64_FP_ENABLED;
 
 	return true;
@@ -522,9 +487,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_restore_host_state_vhe(host_ctxt);
 
-	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
-		__fpsimd_save_fpexc32(vcpu);
-
 	__debug_switch_to_host(vcpu);
 
 	return exit_code;
@@ -580,9 +542,6 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 
 	__sysreg_restore_state_nvhe(host_ctxt);
 
-	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
-		__fpsimd_save_fpexc32(vcpu);
-
 	/*
 	 * This must come after restoring the host sysregs, since a non-VHE
 	 * system may enable SPE here and make use of the TTBRs.
diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9ce2239..12994a9 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -195,6 +195,7 @@ void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
 
 	sysreg[DACR32_EL2] = read_sysreg(dacr32_el2);
 	sysreg[IFSR32_EL2] = read_sysreg(ifsr32_el2);
+	sysreg[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
 
 	if (has_vhe() || vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY)
 		sysreg[DBGVCR32_EL2] = read_sysreg(dbgvcr32_el2);
@@ -217,6 +218,7 @@ void __hyp_text __sysreg32_restore_state(struct kvm_vcpu *vcpu)
 
 	write_sysreg(sysreg[DACR32_EL2], dacr32_el2);
 	write_sysreg(sysreg[IFSR32_EL2], ifsr32_el2);
+	write_sysreg(sysreg[FPEXC32_EL2], fpexc32_el2);
 
 	if (has_vhe() || vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY)
 		write_sysreg(sysreg[DBGVCR32_EL2], dbgvcr32_el2);
-- 
2.1.4