[PATCH v7 06/16] arm64/sve: Refactor user SVE trap maintenance for external use

[marc.zyngier@arm.com (Marc Zyngier)]

On 09/05/18 17:41, Dave Martin wrote:
> On Wed, May 09, 2018 at 05:36:30PM +0100, Marc Zyngier wrote:
>> On 09/05/18 17:12, Dave Martin wrote:
>>> In preparation for optimising the way KVM manages switching the
>>> guest and host FPSIMD state, it is necessary to provide a means for
>>> code outside arch/arm64/kernel/fpsimd.c to restore the user trap
>>> configuration for SVE correctly for the current task.
>>>
>>> Rather than requiring external code to duplicate the maintenance
>>> explicitly, this patch wraps moves the trap maintenenace to
>>> fpsimd_bind_to_cpu(), since it is logically part of the work of
>>> associating the current task with the cpu.
>>>
>>> Because fpsimd_bind_to_cpu() is rather a cryptic name to publish
>>> alongside fpsimd_bind_state_to_cpu(), the former function is
>>> renamed to fpsimd_bind_task_to_cpu() to make its purpose more
>>> explicit.
>>>
>>> This patch makes appropriate changes to ensure that
>>> fpsimd_bind_task_to_cpu() is always called alongside
>>> task_fpsimd_load(), so that the trap maintenance continues to be
>>> done in every situation where it was done prior to this patch.
>>>
>>> As a side-effect, the metadata updates done by
>>> fpsimd_bind_task_to_cpu() now change from conditional to
>>> unconditional in the "already bound" case of sigreturn.  This is
>>> harmless, and a couple of extra stores on this slow path will not
>>> impact performance.  I consider this a reasonable price to pay for
>>> a slightly cleaner interface.
>>>
>>> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>>> ---
>>>  arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++--------------
>>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> 
> [...]
> 
>>> @@ -1006,6 +996,16 @@ static void fpsimd_bind_to_cpu(void)
>>>  	last->st = &current->thread.uw.fpsimd_state;
>>>  	last->sve_in_use = test_thread_flag(TIF_SVE);
>>>  	current->thread.fpsimd_cpu = smp_processor_id();
>>> +
>>> +	if (system_supports_sve()) {
>>
>> Is it worth moving the last->sve_in_use assignment here? If the system
>> doesn't have SVE, the assignment is always 0, and we probably don't need
>> to do it at all. It could avoid the double call to test_thread_flag.
> 
> Could do now that you point it out ... but patch 13 gets rid of it
> entirely anyway.  This is (was) part of the (now legacy) mechanism for
> working round KVM's inability to lazily save the host SVE state.
> 
> If I respin I could tweak it here, but it would be purely cosmetic from
> the whole-series point of view.
Nah, don't even bother.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...