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

[Tiezhu Yang]

On 11/4/26 5:54 PM, Huacai Chen wrote:
> On Wed, Nov 4, 2026 at 2:58 PM Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> wrote:

...

> > 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.

I tested your suggestion:
return cpu_has_scq && __is_defined(system_has_cmpxchg128);

There is no build error. However, this introduces a fatal functional
bug because __is_defined(system_has_cmpxchg128) is blindly evaluated
to 0 as a text string token. As a result, the expression is always
translated into "return cpu_has_scq && 0;", which disables BPF Arena
support permanently even on the Loongson-3A6000 system with hardware
SCQ extension and assembler sc.q support.

I did the following changes:

```
diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index 3ab039d0afb6..b61cca6a8947 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -2489,7 +2489,13 @@ bool bpf_jit_bypass_spec_v4(void)

 bool bpf_jit_supports_arena(void)
 {
-       return true;
+       /*
+        * The arena range tree uses kmalloc_nolock(), which requires 
128-bit
+        * atomic cmpxchg; this is provided by not only hardware SCQ 
extension
+        * but also assembler support on LoongArch.
+        */
+       pr_info("__is_defined(system_has_cmpxchg128)=%d\n", 
__is_defined(system_has_cmpxchg128));
+       return cpu_has_scq && __is_defined(system_has_cmpxchg128);
 }

 bool bpf_jit_supports_fsession(void)
```

Here is the test result on the Loongson-3A6000 system with hardware
SCQ extension and assembler sc.q support:

  $ sudo ./test_progs -a jit_insns/cast_to_user
  ...
  libbpf: map 'arena': failed to create: -EOPNOTSUPP
  run_subtest:FAIL:unexpected_load_failure unexpected error: -95 (errno 95)
  ...
  #197/2   jit_insns/cast_to_user:FAIL
  #197     jit_insns:FAIL
  Summary: 0/0 PASSED, 0 SKIPPED, 1/1 FAILED

  $ dmesg -t | grep system_has_cmpxchg128
  __is_defined(system_has_cmpxchg128)=0

This is not expected.

AFAICT, there are two viable ways to handle both assembler and runtime
constraints:

(1)Use system_has_cmpxchg128 at compile-time and run-time. This is
more aligned with semantic intent, as BPF relies on the generic
128-bit cmpxchg interface.

```
bool bpf_jit_supports_arena(void)
{
        /*
         * The arena range tree uses kmalloc_nolock(), which requires 
128-bit
         * 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
}
```

(2) Use CONFIG_AS_HAS_SCQ_EXTENSION at compile-time and cpu_has_scq
at run-time. This is more aligned with kernel coding aesthetics by
avoiding #ifdef blocks.

```
bool bpf_jit_supports_arena(void)
{
        /*
         * The arena range tree uses kmalloc_nolock(), which requires 
128-bit
         * atomic cmpxchg; this is provided by not only hardware SCQ 
extension
         * but also assembler support on LoongArch.
         */
        return IS_ENABLED(CONFIG_AS_HAS_SCQ_EXTENSION) && cpu_has_scq;
}
```

Since cpu_has_scq and system_has_cmpxchg128() share the exact same
runtime expansion (cpu_opt(LOONGARCH_CPU_SCQ)), both approaches are
functionally equivalent and produce identical machine code.

Personally, I feel Approach 1 represents the pure semantic intent
better, whereas Approach 2 looks more aesthetically pleasing.

Which one do you prefer for the v2 patch?

Thanks,
Tiezhu