Introduce load-acquire and store-release BPF instructions
v5 cover Introduce load-acquire and store-release BPF instructions
Hi all!
This patchset adds kernel support for BPF load-acquire and store-release instructions (for background, please see [1]), including core/verifier and arm64/x86-64 JIT compiler changes, as well as selftests. riscv64 is also planned to be supported. The corresponding LLVM changes can be found at:
https://github.com/llvm/llvm-project/pull/108636
The first 3 patches from v4 have already been applied:
- [bpf-next,v4,01/10] bpf/verifier: Factor out atomic_ptr_type_ok()
https://git.kernel.org/bpf/bpf-next/c/b2d9ef71d4c9 - [bpf-next,v4,02/10] bpf/verifier: Factor out check_atomic_rmw() https://git.kernel.org/bpf/bpf-next/c/d430c46c7580 - [bpf-next,v4,03/10] bpf/verifier: Factor out check_load_mem() and check_store_reg() https://git.kernel.org/bpf/bpf-next/c/d38ad248fb7a
Please refer to the LLVM PR and individual kernel patches for details. Thanks!
v4: https://lore.kernel.org/bpf/[email protected]/
v4..v5 notable changes:
o (kernel test robot) for 32-bit arches: make the verifier reject 64-bit load-acquires/store-releases, and fix build error in interpreter changes
* tested ARCH=arc build following instructions from kernel test robot
o (Alexei) drop Documentation/ patch (v4 10/10) for now
v3: https://lore.kernel.org/bpf/[email protected]/
v3..v4 notable changes:
o (Alexei) add x86-64 JIT support (including arena)
o add Acked-by: tags from Xu
v2: https://lore.kernel.org/bpf/[email protected]/
v2..v3 notable changes:
o (Alexei) change encoding to BPF_LOAD_ACQ=0x100, BPF_STORE_REL=0x110
o add Acked-by: tags from Ilya and Eduard
o make new selftests depend on:
* __clang_major__ >= 18, and
* ENABLE_ATOMICS_TESTS is defined (currently this means -mcpu=v3 or v4), and
* JIT supports load_acq/store_rel (currenty only arm64)
o work around llvm-17 CI job failure by conditionally define __arena_global variables as 64-bit if __clang_major__ < 18, to make sure .addr_space.1 has no holes
o add Google copyright notice in new files
v1: https://lore.kernel.org/all/[email protected]/
v1..v2 notable changes:
o (Eduard) for x86 and s390, make
bpf_jit_supports_insn(..., /*in_arena=*/true) return false
for load_acq/store_rel
o add Eduard's Acked-by: tag
o (Eduard) extract LDX and non-ATOMIC STX handling into helpers, see
PATCH v2 3/9 o allow unpriv programs to store-release pointers to stack
o (Alexei) make it clearer in the interpreter code (PATCH v2 4/9) that only W and DW are supported for atomic RMW
o test misaligned load_acq/store_rel
o (Eduard) other selftests/ changes:
* test load_acq/store_rel with !atomic_ptr_type_ok() pointers:
- PTR_TO_CTX, for is_ctx_reg()
- PTR_TO_PACKET, for is_pkt_reg()
- PTR_TO_FLOW_KEYS, for is_flow_key_reg()
- PTR_TO_SOCKET, for is_sk_reg()
* drop atomics/ tests
* delete unnecessary 'pid' checks from arena_atomics/ tests
* avoid depending on __BPF_FEATURE_LOAD_ACQ_STORE_REL, use __imm_insn() and inline asm macros instead
RFC v1: https://lore.kernel.org/all/[email protected]
RFC v1..v1 notable changes:
o 1-2/8: minor verifier.c refactoring patches
o 3/8: core/verifier changes
* (Eduard) handle load-acquire properly in backtrack_insn()
* (Eduard) avoid skipping checks (e.g., bpf_jit_supports_insn()) for load-acquires
* track the value stored by store-releases, just like how non-atomic STX instructions are handled
* (Eduard) add missing link in commit message
* (Eduard) always print 'r' for disasm.c changes
o 4/8: arm64/insn: avoid treating load_acq/store_rel as
load_ex/store_ex
o 5/8: arm64/insn: add load_acq/store_rel
* (Xu) include Should-Be-One (SBO) bits in "mask" and "value", to avoid setting fixed bits during runtime (JIT-compile time)
o 6/8: arm64 JIT compiler changes
* (Xu) use emit_a64_add_i() for "pointer + offset" to optimize code emission
o 7/8: selftests
* (Eduard) avoid adding new tests to the 'test_verifier' runner
* add more tests, e.g., checking mark_precise logic
o 8/8: instruction-set.rst changes
[1] https://lore.kernel.org/all/[email protected]/
Thanks,
Peilin Ye (6):
bpf: Introduce load-acquire and store-release instructions
arm64: insn: Add BIT(23) to {load,store}_ex's mask
arm64: insn: Add load-acquire and store-release instructions bpf, arm64: Support load-acquire and store-release instructions bpf, x86: Support load-acquire and store-release instructions selftests/bpf: Add selftests for load-acquire and store-release
instructions
arch/arm64/include/asm/insn.h | 12 +- arch/arm64/lib/insn.c | 29 ++ arch/arm64/net/bpf_jit.h | 20 ++ arch/arm64/net/bpf_jit_comp.c | 86 +++++- arch/s390/net/bpf_jit_comp.c | 14 +- arch/x86/net/bpf_jit_comp.c | 95 ++++++- include/linux/bpf.h | 15 + include/linux/filter.h | 2 + include/uapi/linux/bpf.h | 3 + kernel/bpf/core.c | 67 ++++- kernel/bpf/disasm.c | 12 + kernel/bpf/verifier.c | 59 +++- tools/include/uapi/linux/bpf.h | 3 + .../selftests/bpf/prog_tests/arena_atomics.c | 66 ++++- .../selftests/bpf/prog_tests/verifier.c | 4 + .../selftests/bpf/progs/arena_atomics.c | 121 +++++++- .../bpf/progs/verifier_load_acquire.c | 197 +++++++++++++ .../selftests/bpf/progs/verifier_precision.c | 49 ++++ .../bpf/progs/verifier_store_release.c | 264 ++++++++++++++++++ 19 files changed, 1085 insertions(+), 33 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/verifier_load_acquire.c create mode 100644 tools/testing/selftests/bpf/progs/verifier_store_release.c
Signature · 1 line
2.48.1.711.g2feabab25a-goog
PATCH v5 1/6 bpf: Introduce load-acquire and store-release instructions
Introduce BPF instructions with load-acquire and store-release semantics, as discussed in [1]. Define 2 new flags:
#define BPF_LOAD_ACQ 0x100 #define BPF_STORE_REL 0x110
A "load-acquire" is a BPF_STX | BPF_ATOMIC instruction with the 'imm' field set to BPF_LOAD_ACQ (0x100).
Similarly, a "store-release" is a BPF_STX | BPF_ATOMIC instruction with the 'imm' field set to BPF_STORE_REL (0x110).
Unlike existing atomic read-modify-write operations that only support BPF_W (32-bit) and BPF_DW (64-bit) size modifiers, load-acquires and store-releases also support BPF_B (8-bit) and BPF_H (16-bit). As an exception, however, 64-bit load-acquires/store-releases are not supported on 32-bit architectures (to fix a build error reported by the kernel test robot).
An 8- or 16-bit load-acquire zero-extends the value before writing it to a 32-bit register, just like ARM64 instruction LDARH and friends.
Similar to existing atomic read-modify-write operations, misaligned load-acquires/store-releases are not allowed (even if BPF_F_ANY_ALIGNMENT is set).
As an example, consider the following 64-bit load-acquire BPF instruction (assuming little-endian):
db 10 00 00 00 01 00 00 r0 = load_acquire((u64 *)(r1 + 0x0))
opcode (0xdb): BPF_ATOMIC | BPF_DW | BPF_STX
imm (0x00000100): BPF_LOAD_ACQ
Similarly, a 16-bit BPF store-release:
cb 21 00 00 10 01 00 00 store_release((u16 *)(r1 + 0x0), w2)
opcode (0xcb): BPF_ATOMIC | BPF_H | BPF_STX
imm (0x00000110): BPF_STORE_REL
In arch/{arm64,s390,x86}/net/bpf_jit_comp.c, have bpf_jit_supports_insn(..., /*in_arena=*/true) return false for the new instructions, until the corresponding JIT compiler supports them in arena.
[1] https://lore.kernel.org/all/[email protected]/
--- arch/arm64/net/bpf_jit_comp.c | 4 ++ arch/s390/net/bpf_jit_comp.c | 14 +++++-- arch/x86/net/bpf_jit_comp.c | 4 ++ include/linux/bpf.h | 15 ++++++++ include/linux/filter.h | 2 + include/uapi/linux/bpf.h | 3 ++ kernel/bpf/core.c | 67 +++++++++++++++++++++++++++++++--- kernel/bpf/disasm.c | 12 ++++++ kernel/bpf/verifier.c | 59 ++++++++++++++++++++++++++++-- tools/include/uapi/linux/bpf.h | 3 ++ 10 files changed, 170 insertions(+), 13 deletions(-)
arch/arm64/net/bpf_jit_comp.c +4 -0
@@ -2667,8 +2667,12 @@ bool bpf_jit_supports_insn(struct bpf_insn *insn, bool in_arena)if (!in_arena)return true;switch (insn->code) { +case BPF_STX |BPF_ATOMIC |BPF_B : +case BPF_STX |BPF_ATOMIC |BPF_H :case BPF_STX |BPF_ATOMIC |BPF_W :case BPF_STX |BPF_ATOMIC |BPF_DW : +if (bpf_atomic_is_load_store (insn)) +return false;if (!cpus_have_cap (ARM64_HAS_LSE_ATOMICS ))return false; }
arch/s390/net/bpf_jit_comp.c +10 -4
@@ -2919,10 +2919,16 @@ bool bpf_jit_supports_arena(void)bool bpf_jit_supports_insn (struct bpf_insn *insn,bool in_arena) { -/* -* Currently the verifier uses this function only to check which -* atomic stores to arena are supported, and they all are. -*/ +if (!in_arena) +return true; +switch (insn->code) { +case BPF_STX |BPF_ATOMIC |BPF_B : +case BPF_STX |BPF_ATOMIC |BPF_H : +case BPF_STX |BPF_ATOMIC |BPF_W : +case BPF_STX |BPF_ATOMIC |BPF_DW : +if (bpf_atomic_is_load_store (insn)) +return false; + }return true; }
arch/x86/net/bpf_jit_comp.c +4 -0
@@ -3771,8 +3771,12 @@ bool bpf_jit_supports_insn(struct bpf_insn *insn, bool in_arena)if (!in_arena)return true;switch (insn->code) { +case BPF_STX |BPF_ATOMIC |BPF_B : +case BPF_STX |BPF_ATOMIC |BPF_H :case BPF_STX |BPF_ATOMIC |BPF_W :case BPF_STX |BPF_ATOMIC |BPF_DW : +if (bpf_atomic_is_load_store (insn)) +return false;if (insn->imm == (BPF_AND |BPF_FETCH ) || insn->imm == (BPF_OR |BPF_FETCH ) || insn->imm == (BPF_XOR |BPF_FETCH ))
include/linux/bpf.h +15 -0
@@ -991,6 +991,21 @@ static inline bool bpf_pseudo_func(const struct bpf_insn *insn)return bpf_is_ldimm64 (insn) && insn->src_reg ==BPF_PSEUDO_FUNC ; } +/* Given a BPF_ATOMIC instruction @atomic_insn, return true if it is an +* atomic load or store, and false if it is a read-modify-write instruction. +*/ +static inline bool +bpf_atomic_is_load_store (const struct bpf_insn *atomic_insn) +{ +switch (atomic_insn->imm) { +case BPF_LOAD_ACQ : +case BPF_STORE_REL : +return true; +default : +return false; + } +} +struct bpf_prog_ops {int (*test_run)(struct bpf_prog *prog,const union bpf_attr *kattr,union bpf_attr __user *uattr);
include/linux/filter.h +2 -0
@@ -364,6 +364,8 @@ static inline bool insn_is_cast_user(const struct bpf_insn *insn)* BPF_XOR | BPF_FETCH src_reg = atomic_fetch_xor(dst_reg + off16, src_reg); * BPF_XCHG src_reg = atomic_xchg(dst_reg + off16, src_reg) * BPF_CMPXCHG r0 = atomic_cmpxchg(dst_reg + off16, r0, src_reg) +* BPF_LOAD_ACQ dst_reg = smp_load_acquire(src_reg + off16) +* BPF_STORE_REL smp_store_release(dst_reg + off16, src_reg) */ #define BPF_ATOMIC_OP (SIZE ,OP ,DST ,SRC ,OFF ) \
include/uapi/linux/bpf.h +3 -0
@@ -51,6 +51,9 @@#define BPF_XCHG (0xe0 | BPF_FETCH)/* atomic exchange */ #define BPF_CMPXCHG (0xf0 | BPF_FETCH)/* atomic compare-and-write */ +#define BPF_LOAD_ACQ 0x100/* load-acquire */ +#define BPF_STORE_REL 0x110/* store-release */ +enum bpf_cond_pseudo_jmp {BPF_MAY_GOTO =0 , };
kernel/bpf/core.c +61 -6
119 lines not on this page · raw message
kernel/bpf/disasm.c +12 -0
@@ -267,6 +267,18 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,BPF_SIZE (insn->code) ==BPF_DW ?"64" :"" , bpf_ldst_string[BPF_SIZE (insn->code) >>3 ], insn->dst_reg, insn->off, insn->src_reg); + }else if (BPF_MODE (insn->code) ==BPF_ATOMIC && + insn->imm ==BPF_LOAD_ACQ ) { +verbose (cbs->private_data,"(%02x) r%d = load_acquire((%s *)(r%d %+d))\n" , + insn->code, insn->dst_reg, + bpf_ldst_string[BPF_SIZE (insn->code) >>3 ], + insn->src_reg, insn->off); + }else if (BPF_MODE (insn->code) ==BPF_ATOMIC && + insn->imm ==BPF_STORE_REL ) { +verbose (cbs->private_data,"(%02x) store_release((%s *)(r%d %+d), r%d)\n" , + insn->code, + bpf_ldst_string[BPF_SIZE (insn->code) >>3 ], + insn->dst_reg, insn->off, insn->src_reg); }else {verbose (cbs->private_data,"BUG_%02x\n" , insn->code); }
kernel/bpf/verifier.c +56 -3
95 lines not on this page · raw message
tools/include/uapi/linux/bpf.h +3 -0
@@ -51,6 +51,9 @@#define BPF_XCHG (0xe0 | BPF_FETCH)/* atomic exchange */ #define BPF_CMPXCHG (0xf0 | BPF_FETCH)/* atomic compare-and-write */ +#define BPF_LOAD_ACQ 0x100/* load-acquire */ +#define BPF_STORE_REL 0x110/* store-release */ +enum bpf_cond_pseudo_jmp {BPF_MAY_GOTO =0 , };
Signature · 1 line
2.48.1.711.g2feabab25a-goog
Re: [PATCH bpf-next v5 1/6] bpf: Introduce load-acquire and store-release instructions
Quoted 4 lines · Peilin Ye
switch (insn->imm) {kernel/bpf/verifier.c +0
-0@@ -7780,6 +7813,24 @@ static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn)case BPF_XCHG :
kernel/bpf/verifier.c static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn) · line 7823
Quoted 10 lines
kernel/bpf/verifier.c +8
-0case BPF_CMPXCHG :return check_atomic_rmw (env, insn); +case BPF_LOAD_ACQ : +#ifndef CONFIG_64BIT +if (BPF_SIZE (insn->code) ==BPF_DW ) { +verbose (env, +"64-bit load-acquires are only supported on 64-bit arches\n" ); +return -EOPNOTSUPP ; + } +#endif
Your earlier proposal of:
if (BPF_SIZE(insn->code) == BPF_DW && BITS_PER_LONG != 64) {
was cleaner.
Why did you pick ifndef ?
Quoted 9 lines
kernel/bpf/verifier.c +9
-0+return check_atomic_load (env, insn); +case BPF_STORE_REL : +#ifndef CONFIG_64BIT +if (BPF_SIZE (insn->code) ==BPF_DW ) { +verbose (env, +"64-bit store-releases are only supported on 64-bit arches\n" ); +return -EOPNOTSUPP ; + } +#endif
Re: [PATCH bpf-next v5 1/6] bpf: Introduce load-acquire and store-release instructions
Hi Alexei,
Quoted 17 lines · Alexei Starovoitov
kernel/bpf/verifier.c static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn) · line 7823
Quoted 13 lines
switch (insn->imm) {kernel/bpf/verifier.c +8
-0@@ -7780,6 +7813,24 @@ static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn)case BPF_XCHG :case BPF_CMPXCHG :return check_atomic_rmw (env, insn); +case BPF_LOAD_ACQ : +#ifndef CONFIG_64BIT +if (BPF_SIZE (insn->code) ==BPF_DW ) { +verbose (env, +"64-bit load-acquires are only supported on 64-bit arches\n" ); +return -EOPNOTSUPP ; + } +#endif Your earlier proposal of:
if (BPF_SIZE(insn->code)==BPF_DW&&BITS_PER_LONG!= 64) {
Quoted 2 lines
was cleaner.
Why did you pick ifndef ?
Likely overthinking, but I wanted to avoid this check at all for 64-bit arches, so it's just a little bit faster. Should I change it back to checking BITS_PER_LONG ?
Thanks,
Peilin Ye
Re: [PATCH bpf-next v5 1/6] bpf: Introduce load-acquire and store-release instructions
Quoted 21 lines · Peilin Ye
Hi Alexei,
Quoted 17 lines · Alexei Starovoitov
kernel/bpf/verifier.c static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn) · line 7823
Quoted 13 lines
switch (insn->imm) {kernel/bpf/verifier.c +8
-0@@ -7780,6 +7813,24 @@ static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn)case BPF_XCHG :case BPF_CMPXCHG :return check_atomic_rmw (env, insn); +case BPF_LOAD_ACQ : +#ifndef CONFIG_64BIT +if (BPF_SIZE (insn->code) ==BPF_DW ) { +verbose (env, +"64-bit load-acquires are only supported on 64-bit arches\n" ); +return -EOPNOTSUPP ; + } +#endif Your earlier proposal of:
if (BPF_SIZE(insn->code)==BPF_DW&&BITS_PER_LONG!= 64) {
Quoted 6 lines
Quoted 2 lines
was cleaner.
Why did you pick ifndef ?Likely overthinking, but I wanted to avoid this check at all for 64-bit arches, so it's just a little bit faster. Should I change it back to checking
BITS_PER_LONG?
In general #ifdef in .c is the last resort. We avoid it when possible.
In core.c we probably cannot, but here we can. So yes. please respin.
I bet the compiler will produce the exact same code.
Re: [PATCH bpf-next v5 1/6] bpf: Introduce load-acquire and store-release instructions
Quoted 20 lines · Alexei Starovoitov
Quoted 20 lines
Quoted 19 lines
kernel/bpf/verifier.c static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn) · line 7823
Quoted 13 lines
switch (insn->imm) {kernel/bpf/verifier.c +8
-0@@ -7780,6 +7813,24 @@ static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn)case BPF_XCHG :case BPF_CMPXCHG :return check_atomic_rmw (env, insn); +case BPF_LOAD_ACQ : +#ifndef CONFIG_64BIT +if (BPF_SIZE (insn->code) ==BPF_DW ) { +verbose (env, +"64-bit load-acquires are only supported on 64-bit arches\n" ); +return -EOPNOTSUPP ; + } +#endif Your earlier proposal of:
if (BPF_SIZE(insn->code)==BPF_DW&&BITS_PER_LONG!= 64) {was cleaner.
Why did you pick ifndef ?
Quoted 6 lines
Quoted 3 lines
Likely overthinking, but I wanted to avoid this check at all for 64-bit arches, so it's just a little bit faster. Should I change it back to checking
BITS_PER_LONG?In general #ifdef in .c is the last resort.
We avoid it when possible.
Got it.
Quoted 2 lines
In
core.cwe probably cannot, but here we can.
So yes. please respin.
Sure!
Quoted 1 line
I bet the compiler will produce the exact same code.
Thanks,
Peilin Ye
PATCH v5 2/6 arm64: insn: Add BIT(23) to {load,store}_ex's mask
In reply to Peilin YeWe are planning to add load-acquire (LDAR{,B,H}) and store-release (STLR{,B,H}) instructions to insn.{c,h}; add BIT(23) to mask of load_ex and store_ex to prevent aarch64_insn_is_{load,store}_ex() from returning false-positives for load-acquire and store-release instructions.
Reference: Arm Architecture Reference Manual (ARM DDI 0487K.a,
ID032224),
* C6.2.228 LDXR
* C6.2.165 LDAXR
* C6.2.161 LDAR
* C6.2.393 STXR
* C6.2.360 STLXR
* C6.2.353 STLR
--- arch/arm64/include/asm/insn.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/insn.h +2 -2
@@ -351,8 +351,8 @@ __AARCH64_INSN_FUNCS(ldr_imm, 0x3FC00000, 0x39400000) __AARCH64_INSN_FUNCS(ldr_lit, 0xBF000000, 0x18000000) __AARCH64_INSN_FUNCS(ldrsw_lit, 0xFF000000, 0x98000000) __AARCH64_INSN_FUNCS(exclusive, 0x3F800000, 0x08000000) -__AARCH64_INSN_FUNCS(load_ex, 0x3F400000, 0x08400000) -__AARCH64_INSN_FUNCS(store_ex, 0x3F400000, 0x08000000) +__AARCH64_INSN_FUNCS(load_ex, 0x3FC00000, 0x08400000) +__AARCH64_INSN_FUNCS(store_ex, 0x3FC00000, 0x08000000) __AARCH64_INSN_FUNCS(mops, 0x3B200C00, 0x19000400) __AARCH64_INSN_FUNCS(stp, 0x7FC00000, 0x29000000)__AARCH64_INSN_FUNCS (ldp,0x7FC00000 ,0x29400000 )
Signature · 1 line
2.48.1.711.g2feabab25a-goog
PATCH v5 3/6 arm64: insn: Add load-acquire and store-release instructions
In reply to Peilin YeAdd load-acquire ("load_acq", LDAR{,B,H}) and store-release ("store_rel", STLR{,B,H}) instructions. Breakdown of encoding:
size L (Rs) o0 (Rt2) Rn Rt
mask (0x3fdffc00): 00 111111 1 1 0 11111 1 11111 00000 00000
value, load_acq (0x08dffc00): 00 001000 1 1 0 11111 1 11111 00000 00000
value, store_rel (0x089ffc00): 00 001000 1 0 0 11111 1 11111 00000 00000
As suggested by Xu [1], include all Should-Be-One (SBO) bits ("Rs" and "Rt2" fields) in the "mask" and "value" numbers.
It is worth noting that we are adding the "no offset" variant of STLR instead of the "pre-index" variant, which has a different encoding.
Reference: Arm Architecture Reference Manual (ARM DDI 0487K.a,
ID032224),
* C6.2.161 LDAR
* C6.2.353 STLR
[1] https://lore.kernel.org/bpf/[email protected]/
--- arch/arm64/include/asm/insn.h | 8 ++++++++ arch/arm64/lib/insn.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+)
arch/arm64/include/asm/insn.h +8 -0
@@ -188,8 +188,10 @@ enum aarch64_insn_ldst_type {AARCH64_INSN_LDST_STORE_PAIR_PRE_INDEX ,AARCH64_INSN_LDST_LOAD_PAIR_POST_INDEX ,AARCH64_INSN_LDST_STORE_PAIR_POST_INDEX , +AARCH64_INSN_LDST_LOAD_ACQ ,AARCH64_INSN_LDST_LOAD_EX ,AARCH64_INSN_LDST_LOAD_ACQ_EX , +AARCH64_INSN_LDST_STORE_REL ,AARCH64_INSN_LDST_STORE_EX ,AARCH64_INSN_LDST_STORE_REL_EX , AARCH64_INSN_LDST_SIGNED_LOAD_IMM_OFFSET,
@@ -351,6 +353,8 @@ __AARCH64_INSN_FUNCS(ldr_imm, 0x3FC00000, 0x39400000) __AARCH64_INSN_FUNCS(ldr_lit, 0xBF000000, 0x18000000) __AARCH64_INSN_FUNCS(ldrsw_lit, 0xFF000000, 0x98000000) __AARCH64_INSN_FUNCS(exclusive, 0x3F800000, 0x08000000) +__AARCH64_INSN_FUNCS(load_acq, 0x3FDFFC00, 0x08DFFC00) +__AARCH64_INSN_FUNCS(store_rel, 0x3FDFFC00, 0x089FFC00) __AARCH64_INSN_FUNCS(load_ex, 0x3FC00000, 0x08400000) __AARCH64_INSN_FUNCS(store_ex, 0x3FC00000, 0x08000000)__AARCH64_INSN_FUNCS (mops,0x3B200C00 ,0x19000400 )
@@ -602,6 +606,10 @@ u32 aarch64_insn_gen_load_store_pair(enum aarch64_insn_register reg1,int offset, enum aarch64_insn_variant variant, enum aarch64_insn_ldst_type type); +u32 aarch64_insn_gen_load_acq_store_rel (enum aarch64_insn_register reg, +enum aarch64_insn_register base, +enum aarch64_insn_size_type size, +enum aarch64_insn_ldst_type type); u32 aarch64_insn_gen_load_store_ex(enum aarch64_insn_register reg, enum aarch64_insn_register base, enum aarch64_insn_register state,
arch/arm64/lib/insn.c +29 -0
@@ -540,6 +540,35 @@ u32 aarch64_insn_gen_load_store_pair(enum aarch64_insn_register reg1, offset >> shift); } +u32 aarch64_insn_gen_load_acq_store_rel (enum aarch64_insn_register reg, +enum aarch64_insn_register base, +enum aarch64_insn_size_type size, +enum aarch64_insn_ldst_type type) +{ +u32 insn; + +switch (type) { +case AARCH64_INSN_LDST_LOAD_ACQ : + insn =aarch64_insn_get_load_acq_value (); +break ; +case AARCH64_INSN_LDST_STORE_REL : + insn =aarch64_insn_get_store_rel_value (); +break ; +default : +pr_err ("%s: unknown load-acquire/store-release encoding %d\n" , + __func__, type); +return AARCH64_BREAK_FAULT ; + } + + insn =aarch64_insn_encode_ldst_size (size, insn); + + insn =aarch64_insn_encode_register (AARCH64_INSN_REGTYPE_RT , insn, + reg); + +return aarch64_insn_encode_register (AARCH64_INSN_REGTYPE_RN , insn, + base); +} + u32 aarch64_insn_gen_load_store_ex(enum aarch64_insn_register reg, enum aarch64_insn_register base, enum aarch64_insn_register state,
Signature · 1 line
2.48.1.711.g2feabab25a-goog
PATCH v5 4/6 bpf, arm64: Support load-acquire and store-release instructions
In reply to Peilin YeSupport BPF load-acquire (BPF_LOAD_ACQ) and store-release (BPF_STORE_REL) instructions in the arm64 JIT compiler. For example (assuming little-endian):
db 10 00 00 00 01 00 00 r0 = load_acquire((u64 *)(r1 + 0x0)) 95 00 00 00 00 00 00 00 exit
opcode (0xdb): BPF_ATOMIC | BPF_DW | BPF_STX
imm (0x00000100): BPF_LOAD_ACQ
The JIT compiler would emit an LDAR instruction for the above, e.g.:
ldar x7, [x0]
Similarly, consider the following 16-bit store-release:
cb 21 00 00 10 01 00 00 store_release((u16 *)(r1 + 0x0), w2) 95 00 00 00 00 00 00 00 exit
opcode (0xcb): BPF_ATOMIC | BPF_H | BPF_STX
imm (0x00000110): BPF_STORE_REL
An STLRH instruction would be emitted, e.g.:
stlrh w1, [x0]
For a complete mapping:
load-acquire 8-bit LDARB
(BPF_LOAD_ACQ) 16-bit LDARH
32-bit LDAR (32-bit)
64-bit LDAR (64-bit)
store-release 8-bit STLRB
(BPF_STORE_REL) 16-bit STLRH
32-bit STLR (32-bit)
64-bit STLR (64-bit)Arena accesses are supported.bpf_jit_supports_insn(..., /*in_arena=*/true) always returns true for BPF_LOAD_ACQ and BPF_STORE_REL instructions, as they don't depend on ARM64_HAS_LSE_ATOMICS.
--- arch/arm64/net/bpf_jit.h | 20 ++++++++ arch/arm64/net/bpf_jit_comp.c | 90 ++++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 6 deletions(-)
arch/arm64/net/bpf_jit.h +20 -0
@@ -119,6 +119,26 @@aarch64_insn_gen_load_store_ex (Rt, Rn, Rs,A64_SIZE (sf), \AARCH64_INSN_LDST_STORE_REL_EX ) +/* Load-acquire & store-release */ +#define A64_LDAR (Rt, Rn, size) \ + aarch64_insn_gen_load_acq_store_rel(Rt, Rn, AARCH64_INSN_SIZE_##size, \ + AARCH64_INSN_LDST_LOAD_ACQ) +#define A64_STLR (Rt, Rn, size) \ + aarch64_insn_gen_load_acq_store_rel(Rt, Rn, AARCH64_INSN_SIZE_##size, \ + AARCH64_INSN_LDST_STORE_REL) + +/* Rt = [Rn] (load acquire) */ +#define A64_LDARB (Wt, Xn) A64_LDAR(Wt, Xn, 8) +#define A64_LDARH (Wt, Xn) A64_LDAR(Wt, Xn, 16) +#define A64_LDAR32 (Wt, Xn) A64_LDAR(Wt, Xn, 32) +#define A64_LDAR64 (Xt, Xn) A64_LDAR(Xt, Xn, 64) + +/* [Rn] = Rt (store release) */ +#define A64_STLRB (Wt, Xn) A64_STLR(Wt, Xn, 8) +#define A64_STLRH (Wt, Xn) A64_STLR(Wt, Xn, 16) +#define A64_STLR32 (Wt, Xn) A64_STLR(Wt, Xn, 32) +#define A64_STLR64 (Xt, Xn) A64_STLR(Xt, Xn, 64) +/* * LSE atomics *
arch/arm64/net/bpf_jit_comp.c +84 -6
114 lines not on this page · raw message
Signature · 1 line
2.48.1.711.g2feabab25a-goog
PATCH v5 5/6 bpf, x86: Support load-acquire and store-release instructions
In reply to Peilin YeRecently we introduced BPF load-acquire (BPF_LOAD_ACQ) and store-release (BPF_STORE_REL) instructions. For x86-64, simply implement them as regular BPF_LDX/BPF_STX loads and stores. The verifier always rejects misaligned load-acquires/store-releases (even if BPF_F_ANY_ALIGNMENT is set), so emitted MOV* instructions are guaranteed to be atomic.
Arena accesses are supported. 8- and 16-bit load-acquires are zero-extending (i.e., MOVZBQ, MOVZWQ).
Rename emit_atomic{,_index}() to emit_atomic_rmw{,_index}() to make it clear that they only handle read-modify-write atomics, and extend their @atomic_op parameter from u8 to u32, since we are starting to use more than the lowest 8 bits of the 'imm' field.
--- arch/x86/net/bpf_jit_comp.c | 99 ++++++++++++++++++++++++++++++------- 1 file changed, 82 insertions(+), 17 deletions(-)
arch/x86/net/bpf_jit_comp.c +82 -17
@@ -1242,8 +1242,8 @@ static void emit_st_r12(u8 **pprog, u32 size, u32 dst_reg, int off, int imm)emit_st_index (pprog, size, dst_reg,X86_REG_R12 , off, imm); } -static int emit_atomic (u8 **pprog,u8 atomic_op, -u32 dst_reg,u32 src_reg,s16 off,u8 bpf_size) +static int emit_atomic_rmw (u8 **pprog,u32 atomic_op, +u32 dst_reg,u32 src_reg,s16 off,u8 bpf_size) {u8 *prog = *pprog;
@@ -1283,8 +1283,9 @@ static int emit_atomic(u8 **pprog, u8 atomic_op,return 0 ; } -static int emit_atomic_index (u8 **pprog,u8 atomic_op,u32 size, -u32 dst_reg,u32 src_reg,u32 index_reg,int off) +static int emit_atomic_rmw_index (u8 **pprog,u32 atomic_op,u32 size, +u32 dst_reg,u32 src_reg,u32 index_reg, +int off) {u8 *prog = *pprog;
@@ -1297,7 +1298,7 @@ static int emit_atomic_index(u8 **pprog, u8 atomic_op, u32 size,EMIT1 (add_3mod (0x48 , dst_reg, src_reg, index_reg));break ;default : -pr_err ("bpf_jit: 1 and 2 byte atomics are not supported\n" ); +pr_err ("bpf_jit: 1- and 2-byte RMW atomics are not supported\n" );return -EFAULT ; }
@@ -1331,6 +1332,49 @@ static int emit_atomic_index(u8 **pprog, u8 atomic_op, u32 size,return 0 ; } +static int emit_atomic_ld_st (u8 **pprog,u32 atomic_op,u32 dst_reg, +u32 src_reg,s16 off,u8 bpf_size) +{ +switch (atomic_op) { +case BPF_LOAD_ACQ : +/* dst_reg = smp_load_acquire(src_reg + off16) */ +emit_ldx (pprog, bpf_size, dst_reg, src_reg, off); +break ; +case BPF_STORE_REL : +/* smp_store_release(dst_reg + off16, src_reg) */ +emit_stx (pprog, bpf_size, dst_reg, src_reg, off); +break ; +default : +pr_err ("bpf_jit: unknown atomic load/store opcode %02x\n" , + atomic_op); +return -EFAULT ; + } + +return 0 ; +} + +static int emit_atomic_ld_st_index (u8 **pprog,u32 atomic_op,u32 size, +u32 dst_reg,u32 src_reg,u32 index_reg, +int off) +{ +switch (atomic_op) { +case BPF_LOAD_ACQ : +/* dst_reg = smp_load_acquire(src_reg + idx_reg + off16) */ +emit_ldx_index (pprog, size, dst_reg, src_reg, index_reg, off); +break ; +case BPF_STORE_REL : +/* smp_store_release(dst_reg + idx_reg + off16, src_reg) */ +emit_stx_index (pprog, size, dst_reg, src_reg, index_reg, off); +break ; +default : +pr_err ("bpf_jit: unknown atomic load/store opcode %02x\n" , + atomic_op); +return -EFAULT ; + } + +return 0 ; +} +#define DONT_CLEAR 1bool ex_handler_bpf (const struct exception_table_entry *x,struct pt_regs *regs)
@@ -2113,6 +2157,13 @@ st: if (is_imm8(insn->off)) }break ; +case BPF_STX |BPF_ATOMIC |BPF_B : +case BPF_STX |BPF_ATOMIC |BPF_H : +if (!bpf_atomic_is_load_store (insn)) { +pr_err ("bpf_jit: 1- and 2-byte RMW atomics are not supported\n" ); +return -EFAULT ; + } + fallthrough;case BPF_STX |BPF_ATOMIC |BPF_W :case BPF_STX |BPF_ATOMIC |BPF_DW : if (insn->imm == (BPF_AND | BPF_FETCH) ||
@@ -2148,10 +2199,10 @@ st: if (is_imm8(insn->off))EMIT2 (simple_alu_opcodes[BPF_OP (insn->imm)],add_2reg (0xC0 ,AUX_REG , real_src_reg));/* Attempt to swap in new value */ - err =emit_atomic (&prog,BPF_CMPXCHG , - real_dst_reg,AUX_REG , - insn->off, -BPF_SIZE (insn->code)); + err =emit_atomic_rmw (&prog,BPF_CMPXCHG , + real_dst_reg,AUX_REG , + insn->off, +BPF_SIZE (insn->code));if (WARN_ON (err))return err;/*
@@ -2166,17 +2217,35 @@ st: if (is_imm8(insn->off))break ; } - err =emit_atomic (&prog, insn->imm, dst_reg, src_reg, - insn->off,BPF_SIZE (insn->code)); +if (bpf_atomic_is_load_store (insn)) + err =emit_atomic_ld_st (&prog, insn->imm, dst_reg, src_reg, + insn->off,BPF_SIZE (insn->code)); +else + err =emit_atomic_rmw (&prog, insn->imm, dst_reg, src_reg, + insn->off,BPF_SIZE (insn->code));if (err)return err;break ; +case BPF_STX |BPF_PROBE_ATOMIC |BPF_B : +case BPF_STX |BPF_PROBE_ATOMIC |BPF_H : +if (!bpf_atomic_is_load_store (insn)) { +pr_err ("bpf_jit: 1- and 2-byte RMW atomics are not supported\n" ); +return -EFAULT ; + } + fallthrough;case BPF_STX |BPF_PROBE_ATOMIC |BPF_W :case BPF_STX |BPF_PROBE_ATOMIC |BPF_DW : start_of_ldx = prog; - err =emit_atomic_index (&prog, insn->imm,BPF_SIZE (insn->code), - dst_reg, src_reg,X86_REG_R12 , insn->off); + +if (bpf_atomic_is_load_store (insn)) + err =emit_atomic_ld_st_index (&prog, insn->imm, +BPF_SIZE (insn->code), dst_reg, + src_reg,X86_REG_R12 , insn->off); +else + err =emit_atomic_rmw_index (&prog, insn->imm,BPF_SIZE (insn->code), + dst_reg, src_reg,X86_REG_R12 , + insn->off);if (err)return err; goto populate_extable;
@@ -3771,12 +3840,8 @@ bool bpf_jit_supports_insn(struct bpf_insn *insn, bool in_arena)if (!in_arena)return true;switch (insn->code) { -case BPF_STX |BPF_ATOMIC |BPF_B : -case BPF_STX |BPF_ATOMIC |BPF_H :case BPF_STX |BPF_ATOMIC |BPF_W :case BPF_STX |BPF_ATOMIC |BPF_DW : -if (bpf_atomic_is_load_store (insn)) -return false;if (insn->imm == (BPF_AND |BPF_FETCH ) || insn->imm == (BPF_OR |BPF_FETCH ) || insn->imm == (BPF_XOR |BPF_FETCH ))
Signature · 1 line
2.48.1.711.g2feabab25a-goog
PATCH v5 6/6 selftests/bpf: Add selftests for load-acquire and store-release instructions
In reply to Peilin YeAdd several ./test_progs tests:
- arena_atomics/load_acquire
- arena_atomics/store_release
- verifier_load_acquire/*
- verifier_store_release/*
- verifier_precision/bpf_load_acquire
- verifier_precision/bpf_store_release
The last two tests are added to check if backtrack_insn() handles the new instructions correctly.
Additionally, the last test also makes sure that the verifier "remembers" the value (in src_reg) we store-release into e.g. a stack slot. For example, if we take a look at the test program:
#0: r1 = 8;
/* store_release((u64 *)(r10 - 8), r1); */
#1: .8byte %[store_release];
#2: r1 = *(u64 *)(r10 - 8);
#3: r2 = r10;
#4: r2 += r1;
#5: r0 = 0;
#6: exit;At #1, if the verifier doesn't remember that we wrote 8 to the stack, then later at #4 we would be adding an unbounded scalar value to the stack pointer, which would cause the program to be rejected:
VERIFIER LOG:
=============
...
math between fp pointer and register with unbounded min value is not allowed
For easier CI integration, instead of using built-ins like __atomic_{load,store}_n() which depend on the new __BPF_FEATURE_LOAD_ACQ_STORE_REL pre-defined macro, manually craft load-acquire/store-release instructions using __imm_insn(), as suggested by Eduard.
All new tests depend on:
(1) Clang major version >= 18, and
(2) ENABLE_ATOMICS_TESTS is defined (currently implies -mcpu=v3 or
v4), and
(3) JIT supports load-acquire/store-release (currently arm64 and
x86-64)In .../progs/arena_atomics.c:
/* 8-byte-aligned */ __u8 __arena_global load_acquire8_value = 0x12; /* 1-byte hole */ __u16 __arena_global load_acquire16_value = 0x1234;
That 1-byte hole in the .addr_space.1 ELF section caused clang-17 to crash:
fatal error: error in backend: unable to write nop sequence of 1 bytes
To work around such llvm-17 CI job failures, conditionally define __arena_global variables as 64-bit if __clang_major__ < 18, to make sure .addr_space.1 has no holes. Ideally we should avoid compiling this file using clang-17 at all (arena tests depend on __BPF_FEATURE_ADDR_SPACE_CAST, and are skipped for llvm-17 anyway), but that is a separate topic.
--- .../selftests/bpf/prog_tests/arena_atomics.c | 66 ++++- .../selftests/bpf/prog_tests/verifier.c | 4 + .../selftests/bpf/progs/arena_atomics.c | 121 +++++++- .../bpf/progs/verifier_load_acquire.c | 197 +++++++++++++ .../selftests/bpf/progs/verifier_precision.c | 49 ++++ .../bpf/progs/verifier_store_release.c | 264 ++++++++++++++++++ 6 files changed, 698 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/verifier_load_acquire.c create mode 100644 tools/testing/selftests/bpf/progs/verifier_store_release.c
tools/testing/selftests/bpf/prog_tests/arena_atomics.c +65 -1
@@ -162,6 +162,66 @@ static void test_uaf(struct arena_atomics *skel)ASSERT_EQ (skel->arena->uaf_recovery_fails,0 ,"uaf_recovery_fails" ); } +static void test_load_acquire (struct arena_atomics *skel) +{ +LIBBPF_OPTS (bpf_test_run_opts, topts); +int err, prog_fd; + +if (skel->data->skip_lacq_srel_tests) { +printf ("%s:SKIP: ENABLE_ATOMICS_TESTS not defined, Clang doesn't support addr_space_cast, and/or JIT doesn't support load-acquire\n" , + __func__); +test__skip (); +return ; + } + +/* No need to attach it, just run it directly */ + prog_fd =bpf_program__fd (skel->progs.load_acquire); + err =bpf_prog_test_run_opts (prog_fd, &topts); +if (!ASSERT_OK (err,"test_run_opts err" )) +return ; +if (!ASSERT_OK (topts.retval,"test_run_opts retval" )) +return ; + +ASSERT_EQ (skel->arena->load_acquire8_result,0x12 , +"load_acquire8_result" ); +ASSERT_EQ (skel->arena->load_acquire16_result,0x1234 , +"load_acquire16_result" ); +ASSERT_EQ (skel->arena->load_acquire32_result,0x12345678 , +"load_acquire32_result" ); +ASSERT_EQ (skel->arena->load_acquire64_result,0x1234567890abcdef , +"load_acquire64_result" ); +} + +static void test_store_release (struct arena_atomics *skel) +{ +LIBBPF_OPTS (bpf_test_run_opts, topts); +int err, prog_fd; + +if (skel->data->skip_lacq_srel_tests) { +printf ("%s:SKIP: ENABLE_ATOMICS_TESTS not defined, Clang doesn't support addr_space_cast, and/or JIT doesn't support store-release\n" , + __func__); +test__skip (); +return ; + } + +/* No need to attach it, just run it directly */ + prog_fd =bpf_program__fd (skel->progs.store_release); + err =bpf_prog_test_run_opts (prog_fd, &topts); +if (!ASSERT_OK (err,"test_run_opts err" )) +return ; +if (!ASSERT_OK (topts.retval,"test_run_opts retval" )) +return ; + +ASSERT_EQ (skel->arena->store_release8_result,0x12 , +"store_release8_result" ); +ASSERT_EQ (skel->arena->store_release16_result,0x1234 , +"store_release16_result" ); +ASSERT_EQ (skel->arena->store_release32_result,0x12345678 , +"store_release32_result" ); +ASSERT_EQ (skel->arena->store_release64_result,0x1234567890abcdef , +"store_release64_result" ); +} +void test_arena_atomics (void ) {struct arena_atomics *skel;
@@ -171,7 +231,7 @@ void test_arena_atomics(void)if (!ASSERT_OK_PTR (skel,"arena atomics skeleton open" ))return ; -if (skel->data->skip_tests) { +if (skel->data->skip_all_tests) {printf ("%s:SKIP:no ENABLE_ATOMICS_TESTS or no addr_space_cast support in clang" , __func__);test__skip ();
@@ -198,6 +258,10 @@ void test_arena_atomics(void)test_xchg (skel);if (test__start_subtest ("uaf" ))test_uaf (skel); +if (test__start_subtest ("load_acquire" )) +test_load_acquire (skel); +if (test__start_subtest ("store_release" )) +test_store_release (skel); cleanup:arena_atomics__destroy (skel);
tools/testing/selftests/bpf/prog_tests/verifier.c +4 -0
@@ -45,6 +45,7 @@#include "verifier_ldsx.skel.h" #include "verifier_leak_ptr.skel.h" #include "verifier_linked_scalars.skel.h" +#include "verifier_load_acquire.skel.h" #include "verifier_loops1.skel.h" #include "verifier_lwt.skel.h" #include "verifier_map_in_map.skel.h"
@@ -80,6 +81,7 @@#include "verifier_spill_fill.skel.h" #include "verifier_spin_lock.skel.h" #include "verifier_stack_ptr.skel.h" +#include "verifier_store_release.skel.h" #include "verifier_subprog_precision.skel.h" #include "verifier_subreg.skel.h" #include "verifier_tailcall_jit.skel.h"
@@ -173,6 +175,7 @@ void test_verifier_int_ptr(void) { RUN(verifier_int_ptr); }void test_verifier_iterating_callbacks (void ) {RUN (verifier_iterating_callbacks); }void test_verifier_jeq_infer_not_null (void ) {RUN (verifier_jeq_infer_not_null); }void test_verifier_jit_convergence (void ) {RUN (verifier_jit_convergence); } +void test_verifier_load_acquire (void ) {RUN (verifier_load_acquire); }void test_verifier_ld_ind (void ) {RUN (verifier_ld_ind); }void test_verifier_ldsx (void ) {RUN (verifier_ldsx); }void test_verifier_leak_ptr (void ) {RUN (verifier_leak_ptr); }
@@ -211,6 +214,7 @@ void test_verifier_sockmap_mutate(void) { RUN(verifier_sockmap_mutate); }void test_verifier_spill_fill (void ) {RUN (verifier_spill_fill); }void test_verifier_spin_lock (void ) {RUN (verifier_spin_lock); }void test_verifier_stack_ptr (void ) {RUN (verifier_stack_ptr); } +void test_verifier_store_release (void ) {RUN (verifier_store_release); }void test_verifier_subprog_precision (void ) {RUN (verifier_subprog_precision); }void test_verifier_subreg (void ) {RUN (verifier_subreg); }void test_verifier_tailcall_jit (void ) {RUN (verifier_tailcall_jit); }
tools/testing/selftests/bpf/progs/arena_atomics.c +119 -2
138 lines not on this page · raw message
tools/testing/selftests/bpf/progs/verifier_load_acquire.c new file +197 -0
197 lines not on this page · raw message
tools/testing/selftests/bpf/progs/verifier_precision.c +49 -0
61 lines not on this page · raw message
tools/testing/selftests/bpf/progs/verifier_store_release.c new file +264 -0
264 lines not on this page · raw message
Signature · 1 line
2.48.1.711.g2feabab25a-goog