Re: [PATCH v1 1/2] LoongArch: BPF: Make arena support depend on SCQ extension

[Huacai Chen]

On Wed, Nov 4, 2026 at 2:58 PM Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> wrote:
>
> On 11/4/26 12:29 PM, Huacai Chen wrote:
> > On Tue, Nov 3, 2026 at 10:24 AM Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> wrote:
>
> ...
>
> >> So, the current patch has no problem IMO.
> > OK, then I think we can use:
> >
> > return __is_defined(system_has_cmpxchg128) && system_has_cmpxchg128();
>
> If CONFIG_AS_HAS_SCQ_EXTENSION is not enabled, system_has_cmpxchg128()
> is not defined at all. If we change the code to follow your suggestion,
> something like this:
>
> ```
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index b5ed4881d3c0..6f572d24d2c3 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -336,7 +336,7 @@ config AS_HAS_LVZ_EXTENSION
>          def_bool $(as-instr,hvcl 0)
>
>   config AS_HAS_SCQ_EXTENSION
> -       def_bool $(as-instr,sc.q \$t0$(comma)\$t1$(comma)\$t2)
> +       def_bool n
>
>   config CC_HAS_ANNOTATE_TABLEJUMP
>          def_bool $(cc-option,-mannotate-tablejump)
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index d7ea515045ec..d124632b0fa9 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -2495,11 +2495,7 @@ bool bpf_jit_supports_arena(void)
>           * atomic cmpxchg; this is provided by not only hardware SCQ
> extension
>           * but also assembler support on LoongArch.
>           */
> -#ifdef system_has_cmpxchg128
> -       return system_has_cmpxchg128();
> -#else
> -       return false;
> -#endif
> +       return __is_defined(system_has_cmpxchg128) &&
> system_has_cmpxchg128();
>   }
>
>   bool bpf_jit_supports_fsession(void)
> ```
>
> We will encounter a fatal build error:
>
>    CC      arch/loongarch/net/bpf_jit.o
> arch/loongarch/net/bpf_jit.c: In function ‘bpf_jit_supports_arena’:
> arch/loongarch/net/bpf_jit.c:2498:55: error: implicit declaration of
> function ‘system_has_cmpxchg128’ [-Wimplicit-function-declaration]
>   2498 |         return __is_defined(system_has_cmpxchg128) &&
> system_has_cmpxchg128();
>        |
> ^~~~~~~~~~~~~~~~~~~~~
> make[5]: *** [scripts/Makefile.build:290: arch/loongarch/net/bpf_jit.o]
> Error 1
>
> Even if we change it to use IS_ENABLED(CONFIG_AS_HAS_SCQ_EXTENSION),
> something like this:
>
> ```
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index b5ed4881d3c0..6f572d24d2c3 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -336,7 +336,7 @@ config AS_HAS_LVZ_EXTENSION
>          def_bool $(as-instr,hvcl 0)
>
>   config AS_HAS_SCQ_EXTENSION
> -       def_bool $(as-instr,sc.q \$t0$(comma)\$t1$(comma)\$t2)
> +       def_bool n
>
>   config CC_HAS_ANNOTATE_TABLEJUMP
>          def_bool $(cc-option,-mannotate-tablejump)
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index d7ea515045ec..1b87b1e4262e 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -2495,11 +2495,7 @@ bool bpf_jit_supports_arena(void)
>           * atomic cmpxchg; this is provided by not only hardware SCQ
> extension
>           * but also assembler support on LoongArch.
>           */
> -#ifdef system_has_cmpxchg128
> -       return system_has_cmpxchg128();
> -#else
> -       return false;
> -#endif
> +       return IS_ENABLED(CONFIG_AS_HAS_SCQ_EXTENSION) &&
> system_has_cmpxchg128();
>   }
>
>   bool bpf_jit_supports_fsession(void)
> ```
>
> It still fails with the same error due to the macro visibility:
>
>    CC      arch/loongarch/net/bpf_jit.o
> arch/loongarch/net/bpf_jit.c: In function ‘bpf_jit_supports_arena’:
> arch/loongarch/net/bpf_jit.c:2498:59: error: implicit declaration of
> function ‘system_has_cmpxchg128’ [-Wimplicit-function-declaration]
>   2498 |         return IS_ENABLED(CONFIG_AS_HAS_SCQ_EXTENSION) &&
> system_has_cmpxchg128();
>        |
> ^~~~~~~~~~~~~~~~~~~~~
> make[5]: *** [scripts/Makefile.build:290: arch/loongarch/net/bpf_jit.o]
> Error 1
>
> Therefore, using the explicit #ifdef block in the original patch is the
> only way to ensure toolchain compatibility and compile-time robustness
> here IMO.
This is OK:

return cpu_has_scq && __is_defined(system_has_cmpxchg128);

It means two things:
1. CPU supports SC.Q;
2. Kernel defines system_has_cmpxchg128.

Very clear.

Huacai

>
> Anyway, I will modify the patch subject, update the commit message
> and code comment to make it more clear.
>
> Thanks,
> Tiezhu
>