Re: [PATCH v1 2/2] LoongArch: Add ifdefs to fix LSX and LASX related issues

[maobibo]

On 2024/8/22 上午10:51, Tiezhu Yang wrote:
> On 08/22/2024 09:09 AM, maobibo wrote:
> > On 2024/8/21 下午6:07, Tiezhu Yang wrote:
> > > On 08/21/2024 09:12 AM, maobibo wrote:
> > > > On 2024/8/20 下午8:37, Tiezhu Yang wrote:
> > > > > There exist some issues when building kernel if CONFIG_CPU_HAS_LBT
> > > > > is set
> > > > > but CONFIG_CPU_HAS_LSX and CONFIG_CPU_HAS_LASX are not set. In this
> > > > > case,
> > > > > there are no definitions of _restore_lsx and _restore_lasx and there
> > > > > are
> > > > > also no definitions of kvm_restore_lsx and kvm_restore_lasx in fpu.S
> > > > > and
> > > > > switch.S respectively, just add some ifdefs to fix the issues.
> > >
> > > ...
> > >
> > > > It is hard to understand to put CONFIG_CPU_HAS_LASX inside
> > > > CONFIG_CPU_HAS_LBT, in general option CONFIG_CPU_HAS_LBT has nothing
> > > > with CONFIG_CPU_HAS_LASX.
> > > >
> > > > Would you like to add parameter func in macro fpu_restore_csr like 
> > > > this?
> > > >
> > > > --- a/arch/loongarch/include/asm/asmmacro.h
> > > > +++ b/arch/loongarch/include/asm/asmmacro.h
> > > > @@ -55,10 +55,11 @@
> > > >  #endif
> > > >         .endm
> > > >
> > > > -       .macro fpu_restore_csr thread tmp0 tmp1
> > > > +       .macro fpu_restore_csr thread tmp0 tmp1 func
> > > >         ldptr.w         \tmp0, \thread, THREAD_FCSR
> > > >         movgr2fcsr      fcsr0, \tmp0
> > > >  #ifdef CONFIG_CPU_HAS_LBT
> > > > +       STACK_FRAME_NON_STANDARD        \func
> > > >         /* TM bit is always 0 if LBT not supported */
> > > >         andi            \tmp0, \tmp0, FPU_CSR_TM
> > > >         beqz            \tmp0, 2f
> > > > @@ -282,10 +283,10 @@
> > > >         lsx_save_data           \thread, \tmp0
> > > >         .endm
> > >
> > > For the record, I modified the related code as you suggested and
> > > tested with various configs, it works well.
> > >
> > > But as we discussed offline, these current changes are small relatively
> > > and the STACK_FRAME_NON_STANDARD related code may be removed if there
> > > is a proper possible way, so just leave it as is in my opinion.
> > Just be curious, what is the proper possible way to remove this, is it
> > solved with new gcc/binutil version?  Do we plan to drop latest
> > gcc/binutils support in near future only in order to remove these
> > STACK_FRAME_NON_STANDARD code?
>
> These STACK_FRAME_NON_STANDARD related assembler code are irrelevant
> with compiler toolchains, the proper possible way is not yet clear
> for now but it is only related with kernel code itself.
OK, I see. Thanks for the explanation, and I am ok for this patch.

Regards
Bibo Mao
> The support of compiler toolchains is to solve the issues such as
> the relocation type of label diff and jumptable info of swith case.
>
> The above two things are independent.
>
> Thanks,
> Tiezhu