Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu

[WANG Xuerui]

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@xxxxxxxxxxx> wrote:
> > From: Qi Hu <huqi@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>