[PATCH 06/18] arm64: fpsimd: Remove sve_set_vq() and sme_set_vq()

[Vladimir Murzin]

On 5/21/26 14:25, Mark Rutland wrote:
> The sve_set_vq() and sme_set_vq() assembly functions (and the
> sve_load_vq and sme_load_vq macros they use) are open-coded forms of
> sysreg_clear_set*(). There's no need for these to be implemented
> out-of-line in assembly, and the 'vq_minus_1' argument is unusual and
> confusing.
> 
> Use sysreg_clear_set_s() directly, where the necessary 'vq - 1' encoding
> is more obviously part of encoding the register value.
> 
> For now, sve_flush_live() is left with the unusual vq_minus_1 argument.
> This will be addressed in subsequent patches.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Fuad Tabba <tabba at google.com>
> Cc: James Morse <james.morse at arm.com>
> Cc: Marc Zyngier <maz at kernel.org>
> Cc: Mark Brown <broonie at kernel.org>
> Cc: Oliver Upton <oupton at kernel.org>
> Cc: Will Deacon <will at kernel.org>
> ---
>  arch/arm64/include/asm/fpsimd.h       |  2 --
>  arch/arm64/include/asm/fpsimdmacros.h | 22 ----------------------
>  arch/arm64/kernel/entry-fpsimd.S      | 10 ----------
>  arch/arm64/kernel/fpsimd.c            | 24 +++++++++++++-----------
>  4 files changed, 13 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index d9d00b45ab115..8efa3c0402a7a 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -146,8 +146,6 @@ extern void sve_load_state(void const *state, u32 const *pfpsr,
>  			   int restore_ffr);
>  extern void sve_flush_live(bool flush_ffr, unsigned long vq_minus_1);
>  extern unsigned int sve_get_vl(void);
> -extern void sve_set_vq(unsigned long vq_minus_1);
> -extern void sme_set_vq(unsigned long vq_minus_1);
>  extern void sme_save_state(void *state, int zt);
>  extern void sme_load_state(void const *state, int zt);
>  
> diff --git a/arch/arm64/include/asm/fpsimdmacros.h b/arch/arm64/include/asm/fpsimdmacros.h
> index cda81d009c9bd..adf33d2da40c3 100644
> --- a/arch/arm64/include/asm/fpsimdmacros.h
> +++ b/arch/arm64/include/asm/fpsimdmacros.h
> @@ -265,28 +265,6 @@
>  	.purgem _for__body
>  .endm
>  
> -/* Update ZCR_EL1.LEN with the new VQ */
> -.macro sve_load_vq xvqminus1, xtmp, xtmp2
> -		mrs_s		\xtmp, SYS_ZCR_EL1
> -		bic		\xtmp2, \xtmp, ZCR_ELx_LEN_MASK
> -		orr		\xtmp2, \xtmp2, \xvqminus1
> -		cmp		\xtmp2, \xtmp
> -		b.eq		921f
> -		msr_s		SYS_ZCR_EL1, \xtmp2	//self-synchronising
> -921:
> -.endm
> -
> -/* Update SMCR_EL1.LEN with the new VQ */
> -.macro sme_load_vq xvqminus1, xtmp, xtmp2
> -		mrs_s		\xtmp, SYS_SMCR_EL1
> -		bic		\xtmp2, \xtmp, SMCR_ELx_LEN_MASK
> -		orr		\xtmp2, \xtmp2, \xvqminus1
> -		cmp		\xtmp2, \xtmp
> -		b.eq		921f
> -		msr_s		SYS_SMCR_EL1, \xtmp2	//self-synchronising
> -921:
> -.endm
> -
>  /* Preserve the first 128-bits of Znz and zero the rest. */
>  .macro _sve_flush_z nz
>  	_sve_check_zreg \nz
> diff --git a/arch/arm64/kernel/entry-fpsimd.S b/arch/arm64/kernel/entry-fpsimd.S
> index 6325db1a2179c..88c555745b584 100644
> --- a/arch/arm64/kernel/entry-fpsimd.S
> +++ b/arch/arm64/kernel/entry-fpsimd.S
> @@ -62,11 +62,6 @@ SYM_FUNC_START(sve_get_vl)
>  	ret
>  SYM_FUNC_END(sve_get_vl)
>  
> -SYM_FUNC_START(sve_set_vq)
> -	sve_load_vq x0, x1, x2
> -	ret
> -SYM_FUNC_END(sve_set_vq)
> -
>  /*
>   * Zero all SVE registers but the first 128-bits of each vector
>   *
> @@ -94,11 +89,6 @@ SYM_FUNC_START(sme_get_vl)
>  	ret
>  SYM_FUNC_END(sme_get_vl)
>  
> -SYM_FUNC_START(sme_set_vq)
> -	sme_load_vq x0, x1, x2
> -	ret
> -SYM_FUNC_END(sme_set_vq)
> -
>  /*
>   * Save the ZA and ZT state
>   *
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index a8395cb303344..2578c2372c89e 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -377,8 +377,10 @@ static void task_fpsimd_load(void)
>  			if (!thread_sm_enabled(&current->thread))
>  				WARN_ON_ONCE(!test_and_set_thread_flag(TIF_SVE));
>  
> -			if (test_thread_flag(TIF_SVE))
> -				sve_set_vq(sve_vq_from_vl(task_get_sve_vl(current)) - 1);
> +			if (test_thread_flag(TIF_SVE)) {
> +				unsigned long vq = sve_vq_from_vl(task_get_sve_vl(current));
> +				sysreg_clear_set_s(SYS_ZCR_EL1, ZCR_ELx_LEN, vq - 1);
> +			}
>  
>  			restore_sve_regs = true;
>  			restore_ffr = true;
> @@ -403,8 +405,10 @@ static void task_fpsimd_load(void)
>  		unsigned long sme_vl = task_get_sme_vl(current);
>  
>  		/* Ensure VL is set up for restoring data */
> -		if (test_thread_flag(TIF_SME))
> -			sme_set_vq(sve_vq_from_vl(sme_vl) - 1);
> +		if (test_thread_flag(TIF_SME)) {
> +			unsigned long vq = sve_vq_from_vl(sme_vl);
> +			sysreg_clear_set_s(SYS_SMCR_EL1, SMCR_ELx_LEN, vq - 1);
> +		}
>  
>  		write_sysreg_s(current->thread.svcr, SYS_SVCR);
>  
> @@ -1332,10 +1336,9 @@ void do_sve_acc(unsigned long esr, struct pt_regs *regs)
>  	 * any effective streaming mode SVE state.
>  	 */
>  	if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
> -		unsigned long vq_minus_one =
> -			sve_vq_from_vl(task_get_sve_vl(current)) - 1;
> -		sve_set_vq(vq_minus_one);
> -		sve_flush_live(true, vq_minus_one);
> +		unsigned long vq = sve_vq_from_vl(task_get_sve_vl(current));
> +		sysreg_clear_set_s(SYS_ZCR_EL1, ZCR_ELx_LEN, vq - 1);
> +		sve_flush_live(true, vq - 1);
>  		fpsimd_bind_task_to_cpu();
>  	} else {
>  		fpsimd_to_sve(current);
> @@ -1465,9 +1468,8 @@ void do_sme_acc(unsigned long esr, struct pt_regs *regs)
>  		WARN_ON(1);
>  
>  	if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
> -		unsigned long vq_minus_one =
> -			sve_vq_from_vl(task_get_sme_vl(current)) - 1;
> -		sme_set_vq(vq_minus_one);
> +		unsigned long vq = sve_vq_from_vl(task_get_sme_vl(current));
> +		sysreg_clear_set_s(SYS_SMCR_EL1, SMCR_ELx_LEN, vq - 1);
>  
>  		fpsimd_bind_task_to_cpu();
>  	} else {
> -- 2.30.2
> 

I was slightly confused by not seeing _MASK in the new code, yet both are actually the same thing.

FWIW,

Reviewed-by: Vladimir Murzin <vladimir.murzin at arm.com>