Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

arm64/fpsimd: Have KVM explicitly say which FP registers to save

In order to avoid needlessly saving and restoring the guest registers KVM
relies on the host FPSMID code to save the guest registers when we context
switch away from the guest. This is done by binding the KVM guest state to
the CPU on top of the task state that was originally there, then carefully
managing the TIF_SVE flag for the task to cause the host to save the full
SVE state when needed regardless of the needs of the host task. This works
well enough but isn't terribly direct about what is going on and makes it
much more complicated to try to optimise what we're doing with the SVE
register state.

Let's instead have KVM pass in the register state it wants saving when it
binds to the CPU. We introduce a new FP_STATE_CURRENT for use
during normal task binding to indicate that we should base our
decisions on the current task. This should not be used when
actually saving. Ideally we might want to use a separate enum for
the type to save but this enum and the enum values would then
need to be named which has problems with clarity and ambiguity.

In order to ease any future debugging that might be required this patch
does not actually update any of the decision making about what to save,
it merely starts tracking the new information and warns if the requested
state is not what we would otherwise have decided to save.

Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20221115094640.112848-4-broonie@kernel.org
Signed-off-by: Will Deacon <will@kernel.org>

authored by

Mark Brown and committed by
Will Deacon
deeb8f9a baa85152

+35 -5
+2 -1
arch/arm64/include/asm/fpsimd.h
··· 61 61 extern void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *state, 62 62 void *sve_state, unsigned int sve_vl, 63 63 void *za_state, unsigned int sme_vl, 64 - u64 *svcr, enum fp_type *type); 64 + u64 *svcr, enum fp_type *type, 65 + enum fp_type to_save); 65 66 66 67 extern void fpsimd_flush_task_state(struct task_struct *target); 67 68 extern void fpsimd_save_and_flush_cpu_state(void);
+1
arch/arm64/include/asm/processor.h
··· 123 123 }; 124 124 125 125 enum fp_type { 126 + FP_STATE_CURRENT, /* Save based on current task state. */ 126 127 FP_STATE_FPSIMD, 127 128 FP_STATE_SVE, 128 129 };
+24 -3
arch/arm64/kernel/fpsimd.c
··· 126 126 unsigned int sve_vl; 127 127 unsigned int sme_vl; 128 128 enum fp_type *fp_type; 129 + enum fp_type to_save; 129 130 }; 130 131 131 132 static DEFINE_PER_CPU(struct fpsimd_last_state_struct, fpsimd_last_state); ··· 357 356 * but userspace is discouraged from relying on this. 358 357 * 359 358 * task->thread.sve_state does not need to be non-NULL, valid or any 360 - * particular size: it must not be dereferenced. 359 + * particular size: it must not be dereferenced and any data stored 360 + * there should be considered stale and not referenced. 361 361 * 362 362 * * SVE state - FP_STATE_SVE: 363 363 * ··· 371 369 * task->thread.uw.fpsimd_state should be ignored. 372 370 * 373 371 * task->thread.sve_state must point to a valid buffer at least 374 - * sve_state_size(task) bytes in size. 372 + * sve_state_size(task) bytes in size. The data stored in 373 + * task->thread.uw.fpsimd_state.vregs should be considered stale 374 + * and not referenced. 375 375 * 376 376 * * FPSR and FPCR are always stored in task->thread.uw.fpsimd_state 377 377 * irrespective of whether TIF_SVE is clear or set, since these are ··· 461 457 save_sve_regs = true; 462 458 save_ffr = true; 463 459 vl = last->sve_vl; 460 + } 461 + 462 + /* 463 + * Validate that an explicitly specified state to save is 464 + * consistent with the task state. 465 + */ 466 + switch (last->to_save) { 467 + case FP_STATE_CURRENT: 468 + break; 469 + case FP_STATE_FPSIMD: 470 + WARN_ON_ONCE(save_sve_regs); 471 + break; 472 + case FP_STATE_SVE: 473 + WARN_ON_ONCE(!save_sve_regs); 474 + break; 464 475 } 465 476 466 477 if (system_supports_sme()) { ··· 1712 1693 last->sme_vl = task_get_sme_vl(current); 1713 1694 last->svcr = &current->thread.svcr; 1714 1695 last->fp_type = &current->thread.fp_type; 1696 + last->to_save = FP_STATE_CURRENT; 1715 1697 current->thread.fpsimd_cpu = smp_processor_id(); 1716 1698 1717 1699 /* ··· 1737 1717 void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st, void *sve_state, 1738 1718 unsigned int sve_vl, void *za_state, 1739 1719 unsigned int sme_vl, u64 *svcr, 1740 - enum fp_type *type) 1720 + enum fp_type *type, enum fp_type to_save) 1741 1721 { 1742 1722 struct fpsimd_last_state_struct *last = 1743 1723 this_cpu_ptr(&fpsimd_last_state); ··· 1752 1732 last->sve_vl = sve_vl; 1753 1733 last->sme_vl = sme_vl; 1754 1734 last->fp_type = type; 1735 + last->to_save = to_save; 1755 1736 } 1756 1737 1757 1738 /*
+8 -1
arch/arm64/kvm/fpsimd.c
··· 130 130 */ 131 131 void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) 132 132 { 133 + enum fp_type fp_type; 134 + 133 135 WARN_ON_ONCE(!irqs_disabled()); 134 136 135 137 if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) { 138 + if (vcpu_has_sve(vcpu)) 139 + fp_type = FP_STATE_SVE; 140 + else 141 + fp_type = FP_STATE_FPSIMD; 142 + 136 143 /* 137 144 * Currently we do not support SME guests so SVCR is 138 145 * always 0 and we just need a variable to point to. ··· 148 141 vcpu->arch.sve_state, 149 142 vcpu->arch.sve_max_vl, 150 143 NULL, 0, &vcpu->arch.svcr, 151 - &vcpu->arch.fp_type); 144 + &vcpu->arch.fp_type, fp_type); 152 145 153 146 clear_thread_flag(TIF_FOREIGN_FPSTATE); 154 147 update_thread_flag(TIF_SVE, vcpu_has_sve(vcpu));