Re: [PATCH bpf-next v4 04/10] bpf: Introduce load-acquire and store-release instructions

[Peilin Ye <yepeilin@xxxxxxxxxx>]

On Mon, Mar 03, 2025 at 10:43:07AM -0800, Alexei Starovoitov wrote:
> > compiler: arc-elf-gcc (GCC) 13.2.0
> 
> ...
> 
> >    kernel/bpf/core.c:2228:25: note: in expansion of macro 'LOAD_ACQUIRE'
> >     2228 |                         LOAD_ACQUIRE(DW, u64)
> >          |                         ^~~~~~~~~~~~

*facepalm*

> Peilin,
> 
> how do you plan on fixing this?
> So far I could only think of making compilation conditional for CONFIG_64BIT
> and let the verifier reject these insns on 32-bit arches.

Agreed - after searching for "Need native word sized stores/loads for
atomicity." on lore, it appears that we can't have 64-bit
smp_{load_acquire,store_release}() on 32-bit arches.  My draft fix is:

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 323af18d7d49..6df1d3e379a4 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2225,7 +2225,9 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
                        LOAD_ACQUIRE(B,   u8)
                        LOAD_ACQUIRE(H,  u16)
                        LOAD_ACQUIRE(W,  u32)
+#ifdef CONFIG_64BIT
                        LOAD_ACQUIRE(DW, u64)
+#endif
 #undef LOAD_ACQUIRE
                        default:
                                goto default_label;
@@ -2241,7 +2243,9 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
                        STORE_RELEASE(B,   u8)
                        STORE_RELEASE(H,  u16)
                        STORE_RELEASE(W,  u32)
+#ifdef CONFIG_64BIT
                        STORE_RELEASE(DW, u64)
+#endif
 #undef STORE_RELEASE
                        default:
                                goto default_label;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index dac7af73ac8a..d0e4c6bab37d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7814,8 +7814,16 @@ static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn)
        case BPF_CMPXCHG:
                return check_atomic_rmw(env, insn);
        case BPF_LOAD_ACQ:
+               if (BPF_SIZE(insn->code) == BPF_DW && BITS_PER_LONG != 64) {
+                       verbose(env, "64-bit load-acquire is only supported on 64-bit arches\n");
+                       return -ENOTSUPP;
+               }
                return check_atomic_load(env, insn);
        case BPF_STORE_REL:
+               if (BPF_SIZE(insn->code) == BPF_DW && BITS_PER_LONG != 64) {
+                       verbose(env, "64-bit store-release is only supported on 64-bit arches\n");
+                       return -ENOTSUPP;
+               }
                return check_atomic_store(env, insn);
        default:
                verbose(env, "BPF_ATOMIC uses invalid atomic opcode %02x\n",

Thanks,
Peilin Ye