On Mon, 15 Aug 2022 23:55:23 +0100,
Mark Brown <[email protected]> wrote:
>
> Since 8383741ab2e773a99 (KVM: arm64: Get rid of host SVE tracking/saving)
> KVM has not tracked the host SVE state, relying on the fact that we
> currently disable SVE whenever we perform a syscall. This may not be true
> in future since performance optimisation may result in us keeping SVE
> enabled in order to avoid needing to take access traps to reenable it.
> Handle this by clearing TIF_SVE and converting the stored task state to
> FPSIMD format when preparing to run the guest. This is done with a new
> call fpsimd_kvm_prepare() to keep the direct state manipulation
> functions internal to fpsimd.c.
>
> Signed-off-by: Mark Brown <[email protected]>
> ---
> arch/arm64/include/asm/fpsimd.h | 1 +
> arch/arm64/kernel/fpsimd.c | 23 +++++++++++++++++++++++
> arch/arm64/kvm/fpsimd.c | 3 ++-
> 3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index 6f86b7ab6c28..c07e4abaca3d 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -56,6 +56,7 @@ extern void fpsimd_signal_preserve_current_state(void);
> extern void fpsimd_preserve_current_state(void);
> extern void fpsimd_restore_current_state(void);
> extern void fpsimd_update_current_state(struct user_fpsimd_state const
> *state);
> +extern void fpsimd_kvm_prepare(void);
>
> extern void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *state,
> void *sve_state, unsigned int sve_vl,
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 23834d96d1e7..549e11645e0f 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -1627,6 +1627,29 @@ void fpsimd_signal_preserve_current_state(void)
> sve_to_fpsimd(current);
> }
>
> +/*
> + * Called by KVM when entering the guest.
> + */
> +void fpsimd_kvm_prepare(void)
> +{
> + if (!system_supports_sve())
> + return;
> +
> + /*
> + * KVM does not save host SVE state since we can only enter
> + * the guest from a syscall so the ABI means that only the
> + * non-saved SVE state needs to be saved. If we have left
> + * SVE enabled for performance reasons then update the task
> + * state to be FPSIMD only.
> + */
> + get_cpu_fpsimd_context();
> +
> + if (test_and_clear_thread_flag(TIF_SVE))
> + sve_to_fpsimd(current);
> +
> + put_cpu_fpsimd_context();
> +}
> +
> /*
> * Associate current's FPSIMD context with this cpu
> * The caller must have ownership of the cpu FPSIMD context before calling
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index ec8e4494873d..1c1b309ef420 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -75,7 +75,8 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
> void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> {
> BUG_ON(!current->mm);
> - BUG_ON(test_thread_flag(TIF_SVE));
> +
> + fpsimd_kvm_prepare();
Why is this *before* the check against system_supports_fpsimd()? I
don't think the architecture allows SVE without FP, for obvious
reasons...
>
> if (!system_supports_fpsimd())
> return;
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
