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

[WANG Xuerui]

On 2022/7/7 16:30, Jiaxun Yang wrote:
> 在2022年7月7日七月 上午8:35，Qi Hu写道：
> > 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.
> > >
> > > 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.
> > Or simply say this?
> >
> > Remove VCSR from "loongarch_fpu" which is used for debugging.
> Then you it will be a cross talk vulnerability if you simply ignore it in kernel.
>
> Processes can create a covert channel for surreptitious data exchange based on VCSR like m1racles[1] for apple m1.

Hmm this seems to not be the case; according to Ruoyao's experiment 
results, writes to FCSR16 are ignored and reads return all-zeros, hence 
no risk of creating a covert channel by this cleanup.

However, the experiment result is counter-intuitive to me after all; are 
you aware that perhaps *some* 3A5000's in the wild actually have 
functional VCSR? If so, the state obviously needs isolation 
(saving/restoring simply becoming no-ops on later steppings), but 
otherwise this cleanup should be correct.

> Thanks
> [1]: https://m1racles.com/
>
> > > --
> > > 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