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

Peilin Ye
2025-03-03T22:17:04Z

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