KVM: arm64: Move FP state ownership from flag to a tristate

The KVM FP code uses a pair of flags to denote three states:

- FP_ENABLED set: the guest owns the FP state
- FP_HOST set: the host owns the FP state
- FP_ENABLED and FP_HOST clear: nobody owns the FP state at all

and both flags set is an illegal state, which nothing ever checks
for...

As it turns out, this isn't really a good match for flags, and
we'd be better off if this was a simpler tristate, each state
having a name that actually reflect the state:

- FP_STATE_FREE
- FP_STATE_HOST_OWNED
- FP_STATE_GUEST_OWNED

Kill the two flags, and move over to an enum encoding these
three states. This results in less confusing code, and less risk of
ending up in the uncharted territory of a 4th state if we forget
to clear one of the two flags.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Reiji Watanabe <reijiw@google.com>

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 63103cc1bdc4e54843a2ce676e99e0a2ca55fbaa..372c5642cfabaa11df3822ab0596a341f03ae241 100644 (file)
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -325,6 +325,13 @@ struct kvm_vcpu_arch {
        /* Exception Information */
        struct kvm_vcpu_fault_info fault;
 
+       /* Ownership of the FP regs */
+       enum {
+               FP_STATE_FREE,
+               FP_STATE_HOST_OWNED,
+               FP_STATE_GUEST_OWNED,
+       } fp_state;
+
        /* Miscellaneous vcpu state flags */
        u64 flags;
 
@@ -430,8 +437,6 @@ 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_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 */

diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index edbc0183c89b0e1adaaed1d4c16816e245de64f0..d397efe1a378b3ef6deb7c2678a2b70efc5e07a5 100644 (file)
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -77,8 +77,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
        BUG_ON(!current->mm);
        BUG_ON(test_thread_flag(TIF_SVE));
 
-       vcpu->arch.flags &= ~KVM_ARM64_FP_ENABLED;
-       vcpu->arch.flags |= KVM_ARM64_FP_HOST;
+       vcpu->arch.fp_state = FP_STATE_HOST_OWNED;
 
        vcpu->arch.flags &= ~KVM_ARM64_HOST_SVE_ENABLED;
        if (read_sysreg(cpacr_el1) & CPACR_EL1_ZEN_EL0EN)
@@ -98,9 +97,8 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
                if (read_sysreg(cpacr_el1) & CPACR_EL1_SMEN_EL0EN)
                        vcpu->arch.flags |= KVM_ARM64_HOST_SME_ENABLED;
 
-               if (read_sysreg_s(SYS_SVCR) &
-                   (SVCR_SM_MASK | SVCR_ZA_MASK)) {
-                       vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
+               if (read_sysreg_s(SYS_SVCR) & (SVCR_SM_MASK | SVCR_ZA_MASK)) {
+                       vcpu->arch.fp_state = FP_STATE_FREE;
                        fpsimd_save_and_flush_cpu_state();
                }
        }
@@ -119,7 +117,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 void kvm_arch_vcpu_ctxflush_fp(struct kvm_vcpu *vcpu)
 {
        if (!system_supports_fpsimd() || test_thread_flag(TIF_FOREIGN_FPSTATE))
-               vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_FP_HOST);
+               vcpu->arch.fp_state = FP_STATE_FREE;
 }
 
 /*
@@ -133,7 +131,7 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
 {
        WARN_ON_ONCE(!irqs_disabled());
 
-       if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
+       if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) {
                /*
                 * Currently we do not support SME guests so SVCR is
                 * always 0 and we just need a variable to point to.
@@ -176,7 +174,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
                                         CPACR_EL1_SMEN_EL1EN);
        }
 
-       if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
+       if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) {
                if (vcpu_has_sve(vcpu)) {
                        __vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
 

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index e543203849436a81d9995e3222878c53297e9446..6cbbb6c02f663e73c7fd643022b2b4de295b8e2c 100644 (file)
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -40,7 +40,7 @@ extern struct kvm_exception_table_entry __stop___kvm_ex_table;
 /* Check whether the FP regs are owned by the guest */
 static inline bool guest_owns_fp_regs(struct kvm_vcpu *vcpu)
 {
-       return !!(vcpu->arch.flags & KVM_ARM64_FP_ENABLED);
+       return vcpu->arch.fp_state == FP_STATE_GUEST_OWNED;
 }
 
 /* Save the 32-bit only FPSIMD system register state */
@@ -179,10 +179,8 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
        isb();
 
        /* Write out the host state if it's in the registers */
-       if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
+       if (vcpu->arch.fp_state == FP_STATE_HOST_OWNED)
                __fpsimd_save_state(vcpu->arch.host_fpsimd_state);
-               vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
-       }
 
        /* Restore the guest state */
        if (sve_guest)
@@ -194,7 +192,7 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
        if (!(read_sysreg(hcr_el2) & HCR_RW))
                write_sysreg(__vcpu_sys_reg(vcpu, FPEXC32_EL2), fpexc32_el2);
 
-       vcpu->arch.flags |= KVM_ARM64_FP_ENABLED;
+       vcpu->arch.fp_state = FP_STATE_GUEST_OWNED;
 
        return true;
 }

diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index a6b9f1186577a382ae6f9e873a6e46c5fc65f2e8..764bdc423cb82a3cbc888f19d9ef2c07a498b496 100644 (file)
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -123,7 +123,7 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
        }
 
        cptr = CPTR_EL2_DEFAULT;
-       if (vcpu_has_sve(vcpu) && (vcpu->arch.flags & KVM_ARM64_FP_ENABLED))
+       if (vcpu_has_sve(vcpu) && (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED))
                cptr |= CPTR_EL2_TZ;
        if (cpus_have_final_cap(ARM64_SME))
                cptr &= ~CPTR_EL2_TSM;
@@ -335,7 +335,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 
        __sysreg_restore_state_nvhe(host_ctxt);
 
-       if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
+       if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)
                __fpsimd_save_fpexc32(vcpu);
 
        __debug_switch_to_host(vcpu);

diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 46f365254e9f12262b3d97bff20405836e6a5ab8..bce7fc51f9a172900f357d7a0906647bac8f0159 100644 (file)
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -175,7 +175,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
        sysreg_restore_host_state_vhe(host_ctxt);
 
-       if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
+       if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)
                __fpsimd_save_fpexc32(vcpu);
 
        __debug_switch_to_host(vcpu);