Re: [PATCH 03/18] KVM: arm64: Drop FP_FOREIGN_STATE from the hypervisor code
From: Reiji Watanabe <hidden>
Date: 2022-06-03 05:24:53
Also in:
kvm, kvmarm
Hi Marc, On Sat, May 28, 2022 at 4:38 AM Marc Zyngier [off-list ref] wrote:
The vcpu KVM_ARM64_FP_FOREIGN_FPSTATE flag tracks the thread's own TIF_FOREIGN_FPSTATE so that we can evaluate just before running the vcpu whether it the FP regs contain something that is owned by the vcpu or not by updating the rest of the FP flags. We do this in the hypervisor code in order to make sure we're in a context where we are not interruptible. But we already have a hook in the run loop to generate this flag. We may as well update the FP flags directly and save the pointless flag tracking. Whilst we're at it, rename update_fp_enabled() to guest_owns_fp_regs() to indicate what the leftover of this helper actually do. Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Reiji Watanabe <redacted>
quoted hunk ↗ jump to hunk
--- a/arch/arm64/kvm/fpsimd.c +++ b/arch/arm64/kvm/fpsimd.c@@ -107,16 +107,19 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) } /* - * Called just before entering the guest once we are no longer - * preemptable. Syncs the host's TIF_FOREIGN_FPSTATE with the KVM - * mirror of the flag used by the hypervisor. + * Called just before entering the guest once we are no longer preemptable + * and interrupts are disabled. If we have managed to run anything using + * FP while we were preemptible (such as off the back of an interrupt), + * then neither the host nor the guest own the FP hardware (and it was the + * responsibility of the code that used FP to save the existing state). + * + * Note that not supporting FP is basically the same thing as far as the + * hypervisor is concerned (nothing to save). */ void kvm_arch_vcpu_ctxflush_fp(struct kvm_vcpu *vcpu) { - if (test_thread_flag(TIF_FOREIGN_FPSTATE)) - vcpu->arch.flags |= KVM_ARM64_FP_FOREIGN_FPSTATE; - else - vcpu->arch.flags &= ~KVM_ARM64_FP_FOREIGN_FPSTATE; + if (!system_supports_fpsimd() || test_thread_flag(TIF_FOREIGN_FPSTATE)) + vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_FP_HOST); }
Although kvm_arch_vcpu_load_fp() unconditionally sets KVM_ARM64_FP_HOST, perhaps having kvm_arch_vcpu_load_fp() set KVM_ARM64_FP_HOST only when FP is supported might be more consistent? Then, checking system_supports_fpsimd() is unnecessary here. (KVM_ARM64_FP_ENABLED is not set when FP is not supported) Thanks, Reiji _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel