[PATCH 2/3] arm64: fpsimd: preserve/restore kernel mode NEON at context switch

[Mark Brown]

On Tue, Nov 21, 2023 at 02:52:15PM +0100, Ard Biesheuvel wrote:

> So introduce a thread_info flag TIF_USING_KMODE_NEON, and modify the
> context switch hook for FPSIMD to preserve resp. restore the kernel mode

Can we bikeshed this to TIF_USING_KMODE_FP please?

Also is "context switch hook ... preserve resp. restore" just "context
switch hooks to preserve and restore" or am I missing something there?

> -	/* Save unsaved fpsimd state, if any: */
> -	fpsimd_save();
> +	if (!test_thread_flag(TIF_USING_KMODE_NEON)) {
> +		/* Save unsaved user mode fpsimd state, if any: */
> +		fpsimd_save();
> +	} else {
> +		fpsimd_save_state(&current->thread.kmode_fpsimd_state);
> +	}

This all works but I'm having a hard time loving the fact that it's
not integrated with the cleverness that we're doing with KVM to switch
out where a FP save gets written to, we've open coded a FPSIMD only
register save and we'll have to extend that to support SVE or SME (which
I'm pretty sure we'll want to do eventually).

Unless we're going to totally rework how KVM does things it seems like
it's better to use fpsimd_bind_state_to_cpu() in kernel_neon_begin(),
but that does raise the complexity with checking TIF_FOREIGN_FPSTATE in
there.  I *think* what we want here is to do something like rename the
existing fpsimd_save() to user_fpsimd_save() (or something), pull out
the actual register->memory bit into a new fpsimd_save() that's only
used there, in fpsimd_thread_switch() and in the newly added code.  That
way there's only one place where we have code that knows how to write
data out to memory and we always have whatever memory backs any live
data in the registers bound for use there, that seems less likely to go
wrong in future and easier to add asserts for.

I don't *think* we need to worry about overwriting whatever KVM bound
there since we're about to set TIF_FOREIGN_FPSTATE and start using the
registers so we won't be saving again, I could be missing some case
there though.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20231121/5db7bee9/attachment-0001.sig>