From 23afc82539cfcce105bf18c5c835c75e463ca349 Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Mon, 24 Jan 2022 15:57:19 +0000 Subject: [PATCH 1/3] KVM: arm64: Add comments for context flush and sync callbacks Add a little bit of information on where _ctxflush_fp() and _ctxsync_fp() are called to help people unfamiliar with the code get up to speed. Signed-off-by: Mark Brown Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20220124155720.3943374-2-broonie@kernel.org --- arch/arm64/kvm/fpsimd.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c index 2f48fd362a8c..397fdac75cb1 100644 --- a/arch/arm64/kvm/fpsimd.c +++ b/arch/arm64/kvm/fpsimd.c @@ -84,6 +84,11 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) vcpu->arch.flags |= KVM_ARM64_HOST_SVE_ENABLED; } +/* + * 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. + */ void kvm_arch_vcpu_ctxflush_fp(struct kvm_vcpu *vcpu) { if (test_thread_flag(TIF_FOREIGN_FPSTATE)) @@ -93,10 +98,11 @@ void kvm_arch_vcpu_ctxflush_fp(struct kvm_vcpu *vcpu) } /* - * If the guest FPSIMD state was loaded, update the host's context - * tracking data mark the CPU FPSIMD regs as dirty and belonging to vcpu - * so that they will be written back if the kernel clobbers them due to - * kernel-mode NEON before re-entry into the guest. + * Called just after exiting the guest. If the guest FPSIMD state + * was loaded, update the host's context tracking data mark the CPU + * FPSIMD regs as dirty and belonging to vcpu so that they will be + * written back if the kernel clobbers them due to kernel-mode NEON + * before re-entry into the guest. */ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) { From 01a244decc760b1ae2caa045647d79ff431bf37b Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Mon, 24 Jan 2022 15:57:20 +0000 Subject: [PATCH 2/3] KVM: arm64: Add some more comments in kvm_hyp_handle_fpsimd() The handling for FPSIMD/SVE traps is multi stage and involves some trap manipulation which isn't quite so immediately obvious as might be desired so add a few more comments. Signed-off-by: Mark Brown Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20220124155720.3943374-3-broonie@kernel.org --- arch/arm64/kvm/hyp/include/hyp/switch.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 701cfb964905..667654bd3734 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -173,6 +173,8 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code) return false; /* Valid trap. Switch the context: */ + + /* First disable enough traps to allow us to update the registers */ if (has_vhe()) { reg = CPACR_EL1_FPEN; if (sve_guest) @@ -188,11 +190,13 @@ 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) { __fpsimd_save_state(vcpu->arch.host_fpsimd_state); vcpu->arch.flags &= ~KVM_ARM64_FP_HOST; } + /* Restore the guest state */ if (sve_guest) __hyp_sve_restore_guest(vcpu); else From 432110cd83caa655c646ec5d8ca6a9e9afb9ccba Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Mon, 24 Jan 2022 16:11:15 +0000 Subject: [PATCH 3/3] arm64/fpsimd: Clarify the purpose of using last in fpsimd_save() When saving the floating point context in fpsimd_save() we always reference the state using last-> rather than using current->. Looking at the FP code in isolation the reason for this is not entirely obvious, it's done because when KVM is running it will bind the guest context and rely on the host writing out the guest state on context switch away from the guest. There's a slight trick here in that KVM still uses TIF_FOREIGN_FPSTATE and TIF_SVE to communicate what needs to be saved, it maintains those flags and restores them when it is done running the guest so that the normal restore paths function when we return back to userspace. Add a comment to explain this to help future readers work out what's going on a bit faster. Signed-off-by: Mark Brown Reviewed-by: Catalin Marinas Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20220124161115.115200-1-broonie@kernel.org --- arch/arm64/kernel/fpsimd.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 5280e098cfb5..47af76e53221 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -348,7 +348,13 @@ static void task_fpsimd_load(void) /* * Ensure FPSIMD/SVE storage in memory for the loaded context is up to - * date with respect to the CPU registers. + * date with respect to the CPU registers. Note carefully that the + * current context is the context last bound to the CPU stored in + * last, if KVM is involved this may be the guest VM context rather + * than the host thread for the VM pointed to by current. This means + * that we must always reference the state storage via last rather + * than via current, other than the TIF_ flags which KVM will + * carefully maintain for us. */ static void fpsimd_save(void) {