[PATCH v1 0/2] LoongArch: Fix some build issues with randconfig

[PATCH v1 0/2] LoongArch: Fix some build issues with randconfig

[Tiezhu Yang]

Tiezhu Yang (2):
  LoongArch: Define barrier_before_unreachable() as empty
  LoongArch: Add ifdefs to fix LSX and LASX related issues

 arch/loongarch/Kconfig                |  1 +
 arch/loongarch/include/asm/compiler.h | 13 +++++++++++++
 arch/loongarch/kernel/fpu.S           |  4 ++++
 arch/loongarch/kvm/switch.S           |  4 ++++
 4 files changed, 22 insertions(+)
 create mode 100644 arch/loongarch/include/asm/compiler.h

-- 
2.42.0

[PATCH v1 1/2] LoongArch: Define barrier_before_unreachable() as empty

[Tiezhu Yang]

When building kernel with -Os (CONFIG_CC_OPTIMIZE_FOR_SIZE=y) rather
than the default -O2 (CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set),
there exist many objtool warnings "unreachable instruction", this is
because objtool couldn't find a code path to reach the instruction,
objdump shows that the unreachable instruction is related with BUG()
or unreachable().

In include/linux/compiler-gcc.h, there is an empty inline assembly

  #define barrier_before_unreachable() asm volatile("")

in the definition of unreachable() to work around GCC 7.0 after the
commit 173a3efd3edb ("bug.h: work around GCC PR82365 in BUG()") in
the year of 2018, the unreachable instruction is generated by it.

As far as I can see, this workaround is to fix warnings of frame sizes,
the default CONFIG_FRAME_WARN is 2048 on 64 bit system now, it seems
that there are no -Wframe-larger-than warnings in the current kernel
without this workaround.

I am not sure whether the GCC bug has been fixed, I can not find the
fixup in the link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
and in the GCC repo. So I am not sure whether it is time and proper
to remove this workaround in the common header totally, just remove
it in the arch specified header when compiling kernel with a newer
GCC version (for example GCC 12.1 or higher on LoongArch) at least.

For example:
arch/loongarch/kernel/cpu-probe.o: warning: objtool: cpu_probe+0x664: unreachable instruction

objdump -M no-aliases -D arch/loongarch/kernel/cpu-probe.o

Without this patch:

0000000000000000 <cpu_probe>:
 ...
 65c:   44000d80        bnez            $t0, 12 # 668 <.L345^B1+0x4>

0000000000000660 <.L10001^B2>:
 660:   002a0001        break           0x1

0000000000000664 <.L345^B1>:
 664:   53ff63ff        b               -160    # 5c4 <.L344^B1>
 668:   0280040c        addi.w          $t0, $zero, 1
 66c:   00006d8d        cpucfg          $t1, $t0

With this patch:

0000000000000000 <cpu_probe>:
 ...
 65c:   44000980        bnez            $t0, 8  # 664 <.L345^B1>

0000000000000660 <.L10001^B2>:
 660:   002a0001        break           0x1

0000000000000664 <.L345^B1>:
 664:   0280040c        addi.w          $t0, $zero, 1
 668:   00006d8d        cpucfg          $t1, $t0

Cc: stable@vger.kernel.org # 6.9+
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202407221208.6SSBeN9H-lkp@intel.com/
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/Kconfig                |  1 +
 arch/loongarch/include/asm/compiler.h | 13 +++++++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 arch/loongarch/include/asm/compiler.h

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 70f169210b52..3af0da76d103 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -102,6 +102,7 @@ config LOONGARCH
 	select GPIOLIB
 	select HAS_IOPORT
 	select HAVE_ARCH_AUDITSYSCALL
+	select HAVE_ARCH_COMPILER_H
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN
diff --git a/arch/loongarch/include/asm/compiler.h b/arch/loongarch/include/asm/compiler.h
new file mode 100644
index 000000000000..4f6adb8d533e
--- /dev/null
+++ b/arch/loongarch/include/asm/compiler.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2024 Loongson Technology Corporation Limited
+ */
+#ifndef _ASM_COMPILER_H
+#define _ASM_COMPILER_H
+
+#ifdef barrier_before_unreachable
+#undef barrier_before_unreachable
+#define barrier_before_unreachable() do { } while (0)
+#endif
+
+#endif /* _ASM_COMPILER_H */
-- 
2.42.0

Re: [PATCH v1 1/2] LoongArch: Define barrier_before_unreachable() as empty

[Huacai Chen]

Hi, Arnd and Jinyang,

On Tue, Aug 20, 2024 at 8:37 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> When building kernel with -Os (CONFIG_CC_OPTIMIZE_FOR_SIZE=y) rather
> than the default -O2 (CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set),
> there exist many objtool warnings "unreachable instruction", this is
> because objtool couldn't find a code path to reach the instruction,
> objdump shows that the unreachable instruction is related with BUG()
> or unreachable().
>
> In include/linux/compiler-gcc.h, there is an empty inline assembly
>
>   #define barrier_before_unreachable() asm volatile("")
>
> in the definition of unreachable() to work around GCC 7.0 after the
> commit 173a3efd3edb ("bug.h: work around GCC PR82365 in BUG()") in
> the year of 2018, the unreachable instruction is generated by it.
>
> As far as I can see, this workaround is to fix warnings of frame sizes,
> the default CONFIG_FRAME_WARN is 2048 on 64 bit system now, it seems
> that there are no -Wframe-larger-than warnings in the current kernel
> without this workaround.
>
> I am not sure whether the GCC bug has been fixed, I can not find the
> fixup in the link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
> and in the GCC repo. So I am not sure whether it is time and proper
> to remove this workaround in the common header totally, just remove
> it in the arch specified header when compiling kernel with a newer
> GCC version (for example GCC 12.1 or higher on LoongArch) at least.
What's your opinion? From my point of view, this GCC bug hasn't been
fixed. So there may still be potential problems.

Huacai

>
> For example:
> arch/loongarch/kernel/cpu-probe.o: warning: objtool: cpu_probe+0x664: unreachable instruction
>
> objdump -M no-aliases -D arch/loongarch/kernel/cpu-probe.o
>
> Without this patch:
>
> 0000000000000000 <cpu_probe>:
>  ...
>  65c:   44000d80        bnez            $t0, 12 # 668 <.L345^B1+0x4>
>
> 0000000000000660 <.L10001^B2>:
>  660:   002a0001        break           0x1
>
> 0000000000000664 <.L345^B1>:
>  664:   53ff63ff        b               -160    # 5c4 <.L344^B1>
>  668:   0280040c        addi.w          $t0, $zero, 1
>  66c:   00006d8d        cpucfg          $t1, $t0
>
> With this patch:
>
> 0000000000000000 <cpu_probe>:
>  ...
>  65c:   44000980        bnez            $t0, 8  # 664 <.L345^B1>
>
> 0000000000000660 <.L10001^B2>:
>  660:   002a0001        break           0x1
>
> 0000000000000664 <.L345^B1>:
>  664:   0280040c        addi.w          $t0, $zero, 1
>  668:   00006d8d        cpucfg          $t1, $t0
>
> Cc: stable@vger.kernel.org # 6.9+
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202407221208.6SSBeN9H-lkp@intel.com/
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/Kconfig                |  1 +
>  arch/loongarch/include/asm/compiler.h | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>  create mode 100644 arch/loongarch/include/asm/compiler.h
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index 70f169210b52..3af0da76d103 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -102,6 +102,7 @@ config LOONGARCH
>         select GPIOLIB
>         select HAS_IOPORT
>         select HAVE_ARCH_AUDITSYSCALL
> +       select HAVE_ARCH_COMPILER_H
>         select HAVE_ARCH_JUMP_LABEL
>         select HAVE_ARCH_JUMP_LABEL_RELATIVE
>         select HAVE_ARCH_KASAN
> diff --git a/arch/loongarch/include/asm/compiler.h b/arch/loongarch/include/asm/compiler.h
> new file mode 100644
> index 000000000000..4f6adb8d533e
> --- /dev/null
> +++ b/arch/loongarch/include/asm/compiler.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2024 Loongson Technology Corporation Limited
> + */
> +#ifndef _ASM_COMPILER_H
> +#define _ASM_COMPILER_H
> +
> +#ifdef barrier_before_unreachable
> +#undef barrier_before_unreachable
> +#define barrier_before_unreachable() do { } while (0)
> +#endif
> +
> +#endif /* _ASM_COMPILER_H */
> --
> 2.42.0
>
>

Re: [PATCH v1 1/2] LoongArch: Define barrier_before_unreachable() as empty

[Xi Ruoyao]

On Wed, 2024-08-21 at 15:37 +0800, Huacai Chen wrote:
> > I am not sure whether the GCC bug has been fixed, I can not find the
> > fixup in the link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
> > and in the GCC repo. So I am not sure whether it is time and proper
> > to remove this workaround in the common header totally, just remove
> > it in the arch specified header when compiling kernel with a newer
> > GCC version (for example GCC 12.1 or higher on LoongArch) at least.

> What's your opinion? From my point of view, this GCC bug hasn't been
> fixed. So there may still be potential problems.

I'm pretty sure it isn't fixed.  Using the test case from the bug
report:

struct i2c_board_info {
        char type[20];
        char pad[100];
};

#ifdef NONORETURN
void fortify_panic();
#else
void fortify_panic() __attribute__((noreturn));
#endif


int f(int a)
{
  if (a)
    fortify_panic();
}


void i2c_new_device(struct i2c_board_info *);
int em28xx_dvb_init(int model, int a, int b, int c, int d)
{
        switch (model) {
        case 1:{
                        struct i2c_board_info info = {};
                        f(a);
                        i2c_new_device(&info);
                        break;
                }
        case 2:{
                        struct i2c_board_info info = {};
                        f(b);
                        i2c_new_device(&info);
                        break;
                }
        case 3:{
                        struct i2c_board_info info = { };
                        f(c);
                        i2c_new_device(&info);
                        break;
                }
        case 4:{
                        struct i2c_board_info info = { };
                        f(d);
                        i2c_new_device(&info);
                        break;
                }
        }
        return 0;
}

$ cc -v
Using built-in specs.
COLLECT_GCC=cc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/loongarch64-unknown-linux-gnu/14.2.0/lto-wrapper
Target: loongarch64-unknown-linux-gnu
Configured with: ../configure --prefix=/usr LD=ld --enable-languages=c,c++ --enable-default-pie --enable-default-ssp --disable-multilib --with-build-config=bootstrap-lto --disable-fixincludes --with-system-zlib --enable-host-pie
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.2.0 (GCC) 
$ cc t.c -S -Wframe-larger-than=1 -DNONORETURN -O2
t.c: In function 'em28xx_dvb_init':
t.c:50:1: warning: the frame size of 144 bytes is larger than 1 bytes [-Wframe-larger-than=]
   50 | }
      | ^
$ cc t.c -S -Wframe-larger-than=1 -O2            
t.c: In function 'em28xx_dvb_init':
t.c:50:1: warning: the frame size of 512 bytes is larger than 1 bytes [-Wframe-larger-than=]
   50 | }
      | ^

And I'm puzzled why "unreachable instruction" is not a problem on x86?

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v1 1/2] LoongArch: Define barrier_before_unreachable() as empty

[Jinyang He]

On 2024-08-21 15:52, Xi Ruoyao wrote:

> On Wed, 2024-08-21 at 15:37 +0800, Huacai Chen wrote:
>>> I am not sure whether the GCC bug has been fixed, I can not find the
>>> fixup in the link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
>>> and in the GCC repo. So I am not sure whether it is time and proper
>>> to remove this workaround in the common header totally, just remove
>>> it in the arch specified header when compiling kernel with a newer
>>> GCC version (for example GCC 12.1 or higher on LoongArch) at least.
>> What's your opinion? From my point of view, this GCC bug hasn't been
>> fixed. So there may still be potential problems.
> I'm pretty sure it isn't fixed.  Using the test case from the bug
> report:
>
> struct i2c_board_info {
>          char type[20];
>          char pad[100];
> };
>
> #ifdef NONORETURN
> void fortify_panic();
> #else
> void fortify_panic() __attribute__((noreturn));
> #endif
>
>
> int f(int a)
> {
>    if (a)
>      fortify_panic();
> }
>
>
> void i2c_new_device(struct i2c_board_info *);
> int em28xx_dvb_init(int model, int a, int b, int c, int d)
> {
>          switch (model) {
>          case 1:{
>                          struct i2c_board_info info = {};
>                          f(a);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 2:{
>                          struct i2c_board_info info = {};
>                          f(b);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 3:{
>                          struct i2c_board_info info = { };
>                          f(c);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 4:{
>                          struct i2c_board_info info = { };
>                          f(d);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          }
>          return 0;
> }
>
> $ cc -v
> Using built-in specs.
> COLLECT_GCC=cc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/loongarch64-unknown-linux-gnu/14.2.0/lto-wrapper
> Target: loongarch64-unknown-linux-gnu
> Configured with: ../configure --prefix=/usr LD=ld --enable-languages=c,c++ --enable-default-pie --enable-default-ssp --disable-multilib --with-build-config=bootstrap-lto --disable-fixincludes --with-system-zlib --enable-host-pie
> Thread model: posix
> Supported LTO compression algorithms: zlib zstd
> gcc version 14.2.0 (GCC)
> $ cc t.c -S -Wframe-larger-than=1 -DNONORETURN -O2
> t.c: In function 'em28xx_dvb_init':
> t.c:50:1: warning: the frame size of 144 bytes is larger than 1 bytes [-Wframe-larger-than=]
>     50 | }
>        | ^
> $ cc t.c -S -Wframe-larger-than=1 -O2
> t.c: In function 'em28xx_dvb_init':
> t.c:50:1: warning: the frame size of 512 bytes is larger than 1 bytes [-Wframe-larger-than=]
>     50 | }
>        | ^
>
> And I'm puzzled why "unreachable instruction" is not a problem on x86?
>
Hi, Ruoyao,

I looked the gcc:machine_kexec.c:rtl:jump2+:kexec_reboot, it seems
the gcc thought the block `asm volatile ("")` is same. So for -Os,
if 2+ these blocks appears, it create "b" to the same block.

I tried "-fno-crossjumping" to disable the jump2 pass, it works.
Could we change this block different to solve this? (e.g. by __COUNTER__).
But it seems break the original trick. :-(

Jinyang

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

[Tiezhu Yang]

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.

  AS      arch/loongarch/kernel/fpu.o
arch/loongarch/kernel/fpu.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
arch/loongarch/kernel/fpu.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0

  AS [M]  arch/loongarch/kvm/switch.o
arch/loongarch/kvm/switch.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
arch/loongarch/kvm/switch.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0

  MODPOST Module.symvers
ERROR: modpost: "kvm_restore_lsx" [arch/loongarch/kvm/kvm.ko] undefined!
ERROR: modpost: "kvm_restore_lasx" [arch/loongarch/kvm/kvm.ko] undefined!

Cc: stable@vger.kernel.org # 6.9+
Fixes: cb8a2ef0848c ("LoongArch: Add ORC stack unwinder support")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202408120955.qls5oNQY-lkp@intel.com/
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/kernel/fpu.S | 4 ++++
 arch/loongarch/kvm/switch.S | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
index 69a85f2479fb..6ab640101457 100644
--- a/arch/loongarch/kernel/fpu.S
+++ b/arch/loongarch/kernel/fpu.S
@@ -530,6 +530,10 @@ SYM_FUNC_END(_restore_lasx_context)
 
 #ifdef CONFIG_CPU_HAS_LBT
 STACK_FRAME_NON_STANDARD _restore_fp
+#ifdef CONFIG_CPU_HAS_LSX
 STACK_FRAME_NON_STANDARD _restore_lsx
+#endif
+#ifdef CONFIG_CPU_HAS_LASX
 STACK_FRAME_NON_STANDARD _restore_lasx
 #endif
+#endif
diff --git a/arch/loongarch/kvm/switch.S b/arch/loongarch/kvm/switch.S
index 80e988985a6a..0c292f818492 100644
--- a/arch/loongarch/kvm/switch.S
+++ b/arch/loongarch/kvm/switch.S
@@ -277,6 +277,10 @@ SYM_DATA(kvm_enter_guest_size, .quad kvm_enter_guest_end - kvm_enter_guest)
 
 #ifdef CONFIG_CPU_HAS_LBT
 STACK_FRAME_NON_STANDARD kvm_restore_fpu
+#ifdef CONFIG_CPU_HAS_LSX
 STACK_FRAME_NON_STANDARD kvm_restore_lsx
+#endif
+#ifdef CONFIG_CPU_HAS_LASX
 STACK_FRAME_NON_STANDARD kvm_restore_lasx
 #endif
+#endif
-- 
2.42.0

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

[maobibo]

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.
> 
>    AS      arch/loongarch/kernel/fpu.o
> arch/loongarch/kernel/fpu.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
> arch/loongarch/kernel/fpu.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
> 
>    AS [M]  arch/loongarch/kvm/switch.o
> arch/loongarch/kvm/switch.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
> arch/loongarch/kvm/switch.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
> 
>    MODPOST Module.symvers
> ERROR: modpost: "kvm_restore_lsx" [arch/loongarch/kvm/kvm.ko] undefined!
> ERROR: modpost: "kvm_restore_lasx" [arch/loongarch/kvm/kvm.ko] undefined!
> 
> Cc: stable@vger.kernel.org # 6.9+
> Fixes: cb8a2ef0848c ("LoongArch: Add ORC stack unwinder support")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202408120955.qls5oNQY-lkp@intel.com/
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>   arch/loongarch/kernel/fpu.S | 4 ++++
>   arch/loongarch/kvm/switch.S | 4 ++++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
> index 69a85f2479fb..6ab640101457 100644
> --- a/arch/loongarch/kernel/fpu.S
> +++ b/arch/loongarch/kernel/fpu.S
> @@ -530,6 +530,10 @@ SYM_FUNC_END(_restore_lasx_context)
>   
>   #ifdef CONFIG_CPU_HAS_LBT
>   STACK_FRAME_NON_STANDARD _restore_fp
> +#ifdef CONFIG_CPU_HAS_LSX
>   STACK_FRAME_NON_STANDARD _restore_lsx
> +#endif
> +#ifdef CONFIG_CPU_HAS_LASX
>   STACK_FRAME_NON_STANDARD _restore_lasx
>   #endif
> +#endif
> diff --git a/arch/loongarch/kvm/switch.S b/arch/loongarch/kvm/switch.S
> index 80e988985a6a..0c292f818492 100644
> --- a/arch/loongarch/kvm/switch.S
> +++ b/arch/loongarch/kvm/switch.S
> @@ -277,6 +277,10 @@ SYM_DATA(kvm_enter_guest_size, .quad kvm_enter_guest_end - kvm_enter_guest)
>   
>   #ifdef CONFIG_CPU_HAS_LBT
>   STACK_FRAME_NON_STANDARD kvm_restore_fpu
> +#ifdef CONFIG_CPU_HAS_LSX
>   STACK_FRAME_NON_STANDARD kvm_restore_lsx
> +#endif
> +#ifdef CONFIG_CPU_HAS_LASX
>   STACK_FRAME_NON_STANDARD kvm_restore_lasx
>   #endif
> +#endif
> 
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

Regards
Bibo Mao

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

[Tiezhu Yang]

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.

Thanks,
Tiezhu

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

[maobibo]

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?

Regards
Bibo Mao
> 
> Thanks,
> Tiezhu
> 

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

[Tiezhu Yang]

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.

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

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

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

[Huacai Chen]

Applied, thanks.

Huacai

On Tue, Aug 20, 2024 at 8:37 PM Tiezhu Yang <yangtiezhu@loongson.cn> 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.
>
>   AS      arch/loongarch/kernel/fpu.o
> arch/loongarch/kernel/fpu.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
> arch/loongarch/kernel/fpu.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
>
>   AS [M]  arch/loongarch/kvm/switch.o
> arch/loongarch/kvm/switch.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
> arch/loongarch/kvm/switch.o: warning: objtool: unexpected relocation symbol type in .rela.discard.func_stack_frame_non_standard: 0
>
>   MODPOST Module.symvers
> ERROR: modpost: "kvm_restore_lsx" [arch/loongarch/kvm/kvm.ko] undefined!
> ERROR: modpost: "kvm_restore_lasx" [arch/loongarch/kvm/kvm.ko] undefined!
>
> Cc: stable@vger.kernel.org # 6.9+
> Fixes: cb8a2ef0848c ("LoongArch: Add ORC stack unwinder support")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202408120955.qls5oNQY-lkp@intel.com/
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/kernel/fpu.S | 4 ++++
>  arch/loongarch/kvm/switch.S | 4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
> index 69a85f2479fb..6ab640101457 100644
> --- a/arch/loongarch/kernel/fpu.S
> +++ b/arch/loongarch/kernel/fpu.S
> @@ -530,6 +530,10 @@ SYM_FUNC_END(_restore_lasx_context)
>
>  #ifdef CONFIG_CPU_HAS_LBT
>  STACK_FRAME_NON_STANDARD _restore_fp
> +#ifdef CONFIG_CPU_HAS_LSX
>  STACK_FRAME_NON_STANDARD _restore_lsx
> +#endif
> +#ifdef CONFIG_CPU_HAS_LASX
>  STACK_FRAME_NON_STANDARD _restore_lasx
>  #endif
> +#endif
> diff --git a/arch/loongarch/kvm/switch.S b/arch/loongarch/kvm/switch.S
> index 80e988985a6a..0c292f818492 100644
> --- a/arch/loongarch/kvm/switch.S
> +++ b/arch/loongarch/kvm/switch.S
> @@ -277,6 +277,10 @@ SYM_DATA(kvm_enter_guest_size, .quad kvm_enter_guest_end - kvm_enter_guest)
>
>  #ifdef CONFIG_CPU_HAS_LBT
>  STACK_FRAME_NON_STANDARD kvm_restore_fpu
> +#ifdef CONFIG_CPU_HAS_LSX
>  STACK_FRAME_NON_STANDARD kvm_restore_lsx
> +#endif
> +#ifdef CONFIG_CPU_HAS_LASX
>  STACK_FRAME_NON_STANDARD kvm_restore_lasx
>  #endif
> +#endif
> --
> 2.42.0
>