linux-kernel - Re: [PATCH v3] LoongArch: Remove vcsr in loongarch

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 7 Jul 2022 15:13:34 +0800
From:   WANG Xuerui <kernel@...0n.name>
To:     Huacai Chen <chenhuacai@...nel.org>, huqi <huqi@...ngson.cn>
Cc:     WANG Xuerui <kernel@...0n.name>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        loongarch@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu

On 2022/7/7 14:45, Huacai Chen wrote:
> Hi, all,
>
> I have rewritten the commit message. If no problem, this patch will be
> queued for loongarch-fixes.
> --
> The `vcsr` only exists in the old hardware design, it isn't used in any
> shipped hardware from Loongson-3A5000 on. FPU and LSX/LASX both use the
> `fcsr` as their control and status registers now. For example, the RM
> control bit in fcsr0 is shared by FPU, LSX and LASX.

FPU is a function unit while LSX/LASX are ISA extensions, they don't 
belong to the same category.

"Scalar FP and LSX/LASX instructions all use ...", and "... is shared by 
FP, LSX and LASX instructions." sound better.

> Particularly, fcsr16 to fcsr31 are reserved for LSX/LASX now, access to
> these registers has no visible effect if LSX/LASX is enabled, and will
> cause SXD/ASXD exceptions if LSX/LASX is not enabled.
>
> So, mentions of vcsr are obsolete in the first place, let's remove them.

Works for me and the wording is objective enough.

> --
> Huacai
>
> On Wed, Jul 6, 2022 at 7:29 PM huqi <huqi@...ngson.cn> wrote:
>> From: Qi Hu <huqi@...ngson.cn>
>>
>> The `vcsr` is not used anymore. Remove this member from `loongarch_fpu`.
>>
>>  From 3A5000(LoongArch), `vcsr` is removed in hardware. FP and LSX/LASX
>> both use `fcsr` as their csr.
>>
>> Particularly, fcsr from $r16 to $r31 are reserved for LSX/LASX, an
>> using the registers in this area will cause SXD/ASXD if LSX/LASX is
>> not enabled.
>>
>> Signed-off-by: Qi Hu <huqi@...ngson.cn>
>> ---
>> V3:
>> - Modify commit message to conform to the format.
>> V2:
>> - Add more details in the commit message.
>> ---
>>   arch/loongarch/include/asm/fpregdef.h  |  1 -
>>   arch/loongarch/include/asm/processor.h |  2 --
>>   arch/loongarch/kernel/asm-offsets.c    |  1 -
>>   arch/loongarch/kernel/fpu.S            | 10 ----------
>>   4 files changed, 14 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/fpregdef.h b/arch/loongarch/include/asm/fpregdef.h
>> index adb16e4b43b0..b6be527831dd 100644
>> --- a/arch/loongarch/include/asm/fpregdef.h
>> +++ b/arch/loongarch/include/asm/fpregdef.h
>> @@ -48,6 +48,5 @@
>>   #define fcsr1  $r1
>>   #define fcsr2  $r2
>>   #define fcsr3  $r3
>> -#define vcsr16 $r16
>>
>>   #endif /* _ASM_FPREGDEF_H */
>> diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
>> index 1d63c934b289..57ec45aa078e 100644
>> --- a/arch/loongarch/include/asm/processor.h
>> +++ b/arch/loongarch/include/asm/processor.h
>> @@ -80,7 +80,6 @@ BUILD_FPR_ACCESS(64)
>>
>>   struct loongarch_fpu {
>>          unsigned int    fcsr;
>> -       unsigned int    vcsr;
>>          uint64_t        fcc;    /* 8x8 */
>>          union fpureg    fpr[NUM_FPU_REGS];
>>   };
>> @@ -161,7 +160,6 @@ struct thread_struct {
>>           */                                                     \
>>          .fpu                    = {                             \
>>                  .fcsr           = 0,                            \
>> -               .vcsr           = 0,                            \
>>                  .fcc            = 0,                            \
>>                  .fpr            = {{{0,},},},                   \
>>          },                                                      \
>> diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c
>> index bfb65eb2844f..20cd9e16a95a 100644
>> --- a/arch/loongarch/kernel/asm-offsets.c
>> +++ b/arch/loongarch/kernel/asm-offsets.c
>> @@ -166,7 +166,6 @@ void output_thread_fpu_defines(void)
>>
>>          OFFSET(THREAD_FCSR, loongarch_fpu, fcsr);
>>          OFFSET(THREAD_FCC,  loongarch_fpu, fcc);
>> -       OFFSET(THREAD_VCSR, loongarch_fpu, vcsr);
>>          BLANK();
>>   }
>>
>> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
>> index 75c6ce0682a2..a631a7137667 100644
>> --- a/arch/loongarch/kernel/fpu.S
>> +++ b/arch/loongarch/kernel/fpu.S
>> @@ -146,16 +146,6 @@
>>          movgr2fcsr      fcsr0, \tmp0
>>          .endm
>>
>> -       .macro sc_save_vcsr base, tmp0
>> -       movfcsr2gr      \tmp0, vcsr16
>> -       EX      st.w \tmp0, \base, 0
>> -       .endm
>> -
>> -       .macro sc_restore_vcsr base, tmp0
>> -       EX      ld.w \tmp0, \base, 0
>> -       movgr2fcsr      vcsr16, \tmp0
>> -       .endm
>> -
>>   /*
>>    * Save a thread's fp context.
>>    */
>> --
>> 2.36.1
>>
>>
Apart from the syntactical nit:

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