| Message ID | 20250404174435.3288106-6-mark.rutland@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: Preparatory FPSIMD/SVE/SME fixes | expand |
On Fri, Apr 04, 2025 at 06:44:26PM +0100, Mark Rutland wrote: > When a task's SVE vector length (NSVL) is changed, and the task happens > to have SVCR.{SM,ZA}=={0,0}, vec_set_vector_length() opportunistically > frees the task's sme_state and clears TIF_SME. > > The opportunistic freeing was added with no rationale in commit: > > d4d5be94a8787242 ("arm64/fpsimd: Ensure SME storage is allocated after SVE VL changes") > > That commit fixed an unrelated problem where the task's sve_state was > freed while it could be used to store streaming mode register state, > where the fix was to re-allocate the task's sve_state. IIRC the concern was ending up in a state where we have SME enabled but do not have SVE storage allocated, that could lead to us trying to save the streaming SVE registers to a buffer that isn't there. However in what ended up getting posted we always do a sve_alloc() (and we'd need to have done more anyway) so that situation can't arise and this is as you say just redundant. > Remove the unnecessary opportunistic freeing of the task's sme_state. ...and only do this when changing the SME vector length. Reviewed-by: Mark Brown <broonie@kernel.org>
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 757445d72e5b7..128774015772a 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -868,19 +868,10 @@ int vec_set_vector_length(struct task_struct *task, enum vec_type type, task->thread.fp_type = FP_STATE_FPSIMD; } - if (system_supports_sme()) { - if (type == ARM64_VEC_SME || - !(task->thread.svcr & (SVCR_SM_MASK | SVCR_ZA_MASK))) { - /* - * We are changing the SME VL or weren't using - * SME anyway, discard the state and force a - * reallocation. - */ - task->thread.svcr &= ~(SVCR_SM_MASK | - SVCR_ZA_MASK); - clear_tsk_thread_flag(task, TIF_SME); - free_sme = true; - } + if (system_supports_sme() && type == ARM64_VEC_SME) { + task->thread.svcr &= ~(SVCR_SM_MASK | SVCR_ZA_MASK); + clear_tsk_thread_flag(task, TIF_SME); + free_sme = true; } if (task == current)
When a task's SVE vector length (NSVL) is changed, and the task happens to have SVCR.{SM,ZA}=={0,0}, vec_set_vector_length() opportunistically frees the task's sme_state and clears TIF_SME. The opportunistic freeing was added with no rationale in commit: d4d5be94a8787242 ("arm64/fpsimd: Ensure SME storage is allocated after SVE VL changes") That commit fixed an unrelated problem where the task's sve_state was freed while it could be used to store streaming mode register state, where the fix was to re-allocate the task's sve_state. There is no need to free and/or reallocate the task's sme_state when the SVE vector length changes, and there is no need to clear TIF_SME. Given the SME vector length (SVL) doesn't change, the task's sme_state remains correctly sized. Remove the unnecessary opportunistic freeing of the task's sme_state. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Marc Zyngier <maz@kernel.org> Cc: Mark Brown <broonie@kernel.org> Cc: Will Deacon <will@kernel.org> --- arch/arm64/kernel/fpsimd.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)