On Tue, May 22, 2018 at 05:05:02PM +0100, Dave P Martin wrote:
> fpsimd_last_state.st is set to NULL as a way of indicating that
> current's FPSIMD registers are no longer loaded in the cpu. In
> particular, this is done when the kernel temporarily uses or
> clobbers the FPSIMD registers for its own purposes, as in CPU PM or
> kernel-mode NEON, resulting in them being populated with garbage
> data not belonging to a task.
>
> Commit 17eed27b02da ("arm64/sve: KVM: Prevent guests from using
> SVE") factors this operation out as a new helper
> fpsimd_flush_cpu_state() to make it clearer what is being done
> here, and on SVE systems this helper is now used, via
> kvm_fpsimd_flush_cpu_state(), to invalidate the registers after KVM
> has run a vcpu. The reason for this is that KVM does not yet
> understand how to restore the full host SVE registers itself after
> loading the guest FPSIMD context into them.
>
> This exposes a particular problem: if fpsimd_last_state.st is set
> to NULL without also setting TIF_FOREIGN_FPSTATE, the kernel may
> continue to think that current's FPSIMD registers are live even
> though they have actually been clobbered.
>
> Prior to the aforementioned commit, the only path where
> fpsimd_last_state.st is set to NULL without setting
> TIF_FOREIGN_FPSTATE is when kernel_neon_begin() is called by a
> kernel thread (where current->mm can be NULL). This does not
> matter, because the only harm is that at context-switch time
> fpsimd_thread_switch() may unnecessarily save the FPSIMD registers
> back to current's thread_struct (even though kernel threads are not
> considered to have any FPSIMD context of their own and the
> registers will never be reloaded).
>
> Note that although CPU_PM_ENTER lacks the TIF_FOREIGN_FPSTATE
> setting, every CPU passing through that path must subsequently pass
> through CPU_PM_EXIT before it can re-enter the kernel proper.
> CPU_PM_EXIT sets the flag.
>
> The sve_flush_cpu_state() function added by commit 17eed27b02da
> also lacks the proper maintenance of TIF_FOREIGN_FPSTATE. This may
> cause the bits of a host task's SVE registers that do not alias the
> FPSIMD register file to spontaneously appear zeroed if a KVM vcpu
> runs in the same task in the meantime. Although this effect is
> hidden by the fact that the non-FPSIMD bits of the SVE registers
> are zeroed by a syscall anyway, it is doubtless a bad idea to rely
> on these different code paths interacting correctly under future
> maintenance.
>
> This patch makes TIF_FOREIGN_FPSTATE an unconditional side-effect
> of fpsimd_flush_cpu_state(), and removes the set_thread_flag()
> calls that become redunant as a result. This ensures that
> TIF_FOREIGN_FPSTATE cannot remain clear if the FPSIMD state in the
> FPSIMD registers is invalid.
>
> Signed-off-by: Dave Martin <Dave.Martin@xxxxxxx>
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Will Deacon <will.deacon@xxxxxxx>
> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
