[PATCH] arm64: kvm: Remove redundant KVM_ARM64_FP_HOST flag

[Andrew Scull <ascull@google.com>]

The FPSIMD registers can be in one of three states:
 (a) loaded with the user task's state
 (b) loaded with the vcpu's state
 (c) dirty with transient state

KVM_ARM64_FP_HOST identifies the case (a). When loading the vcpu state,
this is used to decide whether to save the current FPSIMD registers to
the user task.

However, at the point of loading the vcpu's FPSIMD state, it is known
that we are not in state (b). States (a) and (c) can be distinguished by
by checking the TIF_FOREIGN_FPSTATE bit, as was previously being done to
prepare the KVM_ARM64_FP_HOST flag but without the need for mirroring
the state.

Signed-off-by: Andrew Scull <ascull@google.com>
---
This is the result of trying to get my head around the FPSIMD handling.
If I've misunderstood something I'll be very happy to have it explained
to me :)
---
 arch/arm64/include/asm/kvm_host.h       | 11 +++++----
 arch/arm64/kvm/fpsimd.c                 |  1 -
 arch/arm64/kvm/hyp/include/hyp/switch.h | 30 +++++++++++++++++--------
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index e0920df1d0c1..d3652745282d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -370,12 +370,11 @@ struct kvm_vcpu_arch {
 /* vcpu_arch flags field values: */
 #define KVM_ARM64_DEBUG_DIRTY		(1 << 0)
 #define KVM_ARM64_FP_ENABLED		(1 << 1) /* guest FP regs loaded */
-#define KVM_ARM64_FP_HOST		(1 << 2) /* host FP regs loaded */
-#define KVM_ARM64_HOST_SVE_IN_USE	(1 << 3) /* backup for host TIF_SVE */
-#define KVM_ARM64_HOST_SVE_ENABLED	(1 << 4) /* SVE enabled for EL0 */
-#define KVM_ARM64_GUEST_HAS_SVE		(1 << 5) /* SVE exposed to guest */
-#define KVM_ARM64_VCPU_SVE_FINALIZED	(1 << 6) /* SVE config completed */
-#define KVM_ARM64_GUEST_HAS_PTRAUTH	(1 << 7) /* PTRAUTH exposed to guest */
+#define KVM_ARM64_HOST_SVE_IN_USE	(1 << 2) /* backup for host TIF_SVE */
+#define KVM_ARM64_HOST_SVE_ENABLED	(1 << 3) /* SVE enabled for EL0 */
+#define KVM_ARM64_GUEST_HAS_SVE		(1 << 4) /* SVE exposed to guest */
+#define KVM_ARM64_VCPU_SVE_FINALIZED	(1 << 5) /* SVE config completed */
+#define KVM_ARM64_GUEST_HAS_PTRAUTH	(1 << 6) /* PTRAUTH exposed to guest */
 
 #define vcpu_has_sve(vcpu) (system_supports_sve() && \
 			    ((vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_SVE))
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index e329a36b2bee..4e9afeb31989 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -65,7 +65,6 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 	vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED |
 			      KVM_ARM64_HOST_SVE_IN_USE |
 			      KVM_ARM64_HOST_SVE_ENABLED);
-	vcpu->arch.flags |= KVM_ARM64_FP_HOST;
 
 	if (test_thread_flag(TIF_SVE))
 		vcpu->arch.flags |= KVM_ARM64_HOST_SVE_IN_USE;
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 8f622688fa64..beadf17f12a6 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -33,16 +33,24 @@ extern const char __hyp_panic_string[];
 static inline bool update_fp_enabled(struct kvm_vcpu *vcpu)
 {
 	/*
-	 * When the system doesn't support FP/SIMD, we cannot rely on
-	 * the _TIF_FOREIGN_FPSTATE flag. However, we always inject an
-	 * abort on the very first access to FP and thus we should never
-	 * see KVM_ARM64_FP_ENABLED. For added safety, make sure we always
+	 * When entering the vcpu during a KVM_VCPU_RUN call before the vcpu
+	 * has used FPSIMD, FPSIMD is disabled for the vcpu and will trap when
+	 * it is first used. The FPSIMD state currently bound to the cpu is
+	 * that of the user task.
+	 *
+	 * After the vcpu has used FPSIMD, on subsequent entries into the vcpu
+	 * for the same KVM_VCPU_RUN call, the vcpu's FPSIMD state is bound to
+	 * the cpu. Therefore, if _TIF_FOREIGN_FPSTATE is set, we know the
+	 * FPSIMD registers no longer contain the vcpu's state. In this case we
+	 * must, once again, disable FPSIMD.
+	 *
+	 * When the system doesn't support FPSIMD, we cannot rely on the
+	 * _TIF_FOREIGN_FPSTATE flag. For added safety, make sure we always
 	 * trap the accesses.
 	 */
 	if (!system_supports_fpsimd() ||
 	    vcpu->arch.host_thread_info->flags & _TIF_FOREIGN_FPSTATE)
-		vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED |
-				      KVM_ARM64_FP_HOST);
+		vcpu->arch.flags &= ~KVM_ARM64_FP_ENABLED;
 
 	return !!(vcpu->arch.flags & KVM_ARM64_FP_ENABLED);
 }
@@ -245,7 +253,13 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 
 	isb();
 
-	if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
+	/*
+	 * The trap means that the vcpu's FPSIMD state is not loaded. If
+	 * _TIF_FOREIGN_FPSTATE is set, the current state does not need to be
+	 * saved. Otherwise, the user task's state is currently loaded and
+	 * needs to be saved.
+	 */
+	if (!(vcpu->arch.host_thread_info->flags & _TIF_FOREIGN_FPSTATE)) {
 		/*
 		 * In the SVE case, VHE is assumed: it is enforced by
 		 * Kconfig and kvm_arch_init().
@@ -260,8 +274,6 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 		} else {
 			__fpsimd_save_state(vcpu->arch.host_fpsimd_state);
 		}
-
-		vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
 	}
 
 	if (sve_guest) {
-- 
2.27.0.383.g050319c2ae-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm