linux-kernel - Re: [PATCH] LoongArch: Allow usage of LSX/LASX in the kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <94bd0300-a920-a0d7-244b-6caeba5fff26@xen0n.name>
Date:   Tue, 1 Aug 2023 00:22:30 +0800
From:   WANG Xuerui <kernel@...0n.name>
To:     Huacai Chen <chenhuacai@...ngson.cn>,
        Huacai Chen <chenhuacai@...nel.org>
Cc:     loongarch@...ts.linux.dev, Xuefeng Li <lixuefeng@...ngson.cn>,
        Guo Ren <guoren@...nel.org>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        linux-kernel@...r.kernel.org, loongson-kernel@...ts.loongnix.cn
Subject: Re: [PATCH] LoongArch: Allow usage of LSX/LASX in the kernel

On 7/22/23 15:22, Huacai Chen wrote:
> Allow usage of LSX/LASX in the kernel by extending kernel_fpu_begin()
> and kernel_fpu_end().
>
> Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> ---
>   arch/loongarch/kernel/kfpu.c | 55 +++++++++++++++++++++++++++++++++---
>   1 file changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/arch/loongarch/kernel/kfpu.c b/arch/loongarch/kernel/kfpu.c
> index 5c46ae8c6cac..ec5b28e570c9 100644
> --- a/arch/loongarch/kernel/kfpu.c
> +++ b/arch/loongarch/kernel/kfpu.c
> @@ -8,19 +8,40 @@
>   #include <asm/fpu.h>
>   #include <asm/smp.h>
>   
> +static unsigned int euen_mask = CSR_EUEN_FPEN;
> +
> +/*
> + * The critical section between kernel_fpu_begin() and kernel_fpu_end()
> + * is non-reentrant. It is the caller's responsibility to avoid reentrance.
> + * See drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c as an example.
> + */
>   static DEFINE_PER_CPU(bool, in_kernel_fpu);
> +static DEFINE_PER_CPU(unsigned int, euen_current);
>   
>   void kernel_fpu_begin(void)
>   {
> +	unsigned int *euen_curr;
> +
>   	preempt_disable();
>   
>   	WARN_ON(this_cpu_read(in_kernel_fpu));
>   
>   	this_cpu_write(in_kernel_fpu, true);
> +	euen_curr = this_cpu_ptr(&euen_current);
>   
> -	if (!is_fpu_owner())
> -		enable_fpu();
> +	*euen_curr = csr_xchg32(euen_mask, euen_mask, LOONGARCH_CSR_EUEN);
> +
> +#ifdef CONFIG_CPU_HAS_LASX
> +	if (*euen_curr & CSR_EUEN_LASXEN)
> +		_save_lasx(&current->thread.fpu);
> +	else
> +#endif
> +#ifdef CONFIG_CPU_HAS_LSX
> +	if (*euen_curr & CSR_EUEN_LSXEN)
> +		_save_lsx(&current->thread.fpu);
>   	else
> +#endif
> +	if (*euen_curr & CSR_EUEN_FPEN)
>   		_save_fp(&current->thread.fpu);
>   
>   	write_fcsr(LOONGARCH_FCSR0, 0);
> @@ -29,15 +50,41 @@ EXPORT_SYMBOL_GPL(kernel_fpu_begin);
>   
>   void kernel_fpu_end(void)
>   {
> +	unsigned int *euen_curr;
> +
>   	WARN_ON(!this_cpu_read(in_kernel_fpu));
>   
> -	if (!is_fpu_owner())
> -		disable_fpu();
> +	euen_curr = this_cpu_ptr(&euen_current);
> +
> +#ifdef CONFIG_CPU_HAS_LASX
> +	if (*euen_curr & CSR_EUEN_LASXEN)
> +		_restore_lasx(&current->thread.fpu);
>   	else
> +#endif
> +#ifdef CONFIG_CPU_HAS_LSX
> +	if (*euen_curr & CSR_EUEN_LSXEN)
> +		_restore_lsx(&current->thread.fpu);
> +	else
> +#endif
> +	if (*euen_curr & CSR_EUEN_FPEN)
>   		_restore_fp(&current->thread.fpu);
>   
> +	*euen_curr = csr_xchg32(*euen_curr, euen_mask, LOONGARCH_CSR_EUEN);
> +
>   	this_cpu_write(in_kernel_fpu, false);
>   
>   	preempt_enable();
>   }
>   EXPORT_SYMBOL_GPL(kernel_fpu_end);
> +
> +static int __init init_euen_mask(void)
> +{
> +	if (cpu_has_lsx)
> +		euen_mask |= CSR_EUEN_LSXEN;
> +
> +	if (cpu_has_lasx)
> +		euen_mask |= CSR_EUEN_LASXEN;
> +
> +	return 0;
> +}
> +arch_initcall(init_euen_mask);

I've stressed this code a bit with my RAID5/6 LASX patch applied, then 
running $(nproc) copies of vector workload while letting the kernel 
repeatedly scrub a RAID6 array created from a bunch of loop devices, and 
there wasn't signs of context corruption either side. Although I only 
tested for a few minutes (so a Tested-by isn't appropriate yet), the 
code LGTM nevertheless, so...

Reviewed-by: WANG Xuerui <git@...0n.name>

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/