Skip to content

Commit 981f94c

Browse files
bjackmanAlexei Starovoitov
bjackman
authored and
Alexei Starovoitov
committed
bpf: Add bitwise atomic instructions
This adds instructions for atomic[64]_[fetch_]and atomic[64]_[fetch_]or atomic[64]_[fetch_]xor All these operations are isomorphic enough to implement with the same verifier, interpreter, and x86 JIT code, hence being a single commit. The main interesting thing here is that x86 doesn't directly support the fetch_ version these operations, so we need to generate a CMPXCHG loop in the JIT. This requires the use of two temporary registers, IIUC it's safe to use BPF_REG_AX and x86's AUX_REG for this purpose. Signed-off-by: Brendan Jackman <jackmanb@google.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Acked-by: Yonghong Song <yhs@fb.com> Link: https://lore.kernel.org/bpf/20210114181751.768687-10-jackmanb@google.com
1 parent 4629106 commit 981f94c

File tree

6 files changed

+87
-5
lines changed

6 files changed

+87
-5
lines changed

‎arch/x86/net/bpf_jit_comp.c‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,10 @@ static int emit_atomic(u8 **pprog, u8 atomic_op,
808808
/* emit opcode */
809809
switch (atomic_op) {
810810
case BPF_ADD:
811+
case BPF_SUB:
812+
case BPF_AND:
813+
case BPF_OR:
814+
case BPF_XOR:
811815
/* lock *(u32/u64*)(dst_reg + off) <op>= src_reg */
812816
EMIT1(simple_alu_opcodes[atomic_op]);
813817
break;
@@ -1292,8 +1296,52 @@ st: if (is_imm8(insn->off))
12921296

12931297
case BPF_STX | BPF_ATOMIC | BPF_W:
12941298
case BPF_STX | BPF_ATOMIC | BPF_DW:
1299+
if (insn->imm == (BPF_AND | BPF_FETCH) ||
1300+
insn->imm == (BPF_OR | BPF_FETCH) ||
1301+
insn->imm == (BPF_XOR | BPF_FETCH)) {
1302+
u8 *branch_target;
1303+
bool is64 = BPF_SIZE(insn->code) == BPF_DW;
1304+
1305+
/*
1306+
* Can't be implemented with a single x86 insn.
1307+
* Need to do a CMPXCHG loop.
1308+
*/
1309+
1310+
/* Will need RAX as a CMPXCHG operand so save R0 */
1311+
emit_mov_reg(&prog, true, BPF_REG_AX, BPF_REG_0);
1312+
branch_target = prog;
1313+
/* Load old value */
1314+
emit_ldx(&prog, BPF_SIZE(insn->code),
1315+
BPF_REG_0, dst_reg, insn->off);
1316+
/*
1317+
* Perform the (commutative) operation locally,
1318+
* put the result in the AUX_REG.
1319+
*/
1320+
emit_mov_reg(&prog, is64, AUX_REG, BPF_REG_0);
1321+
maybe_emit_mod(&prog, AUX_REG, src_reg, is64);
1322+
EMIT2(simple_alu_opcodes[BPF_OP(insn->imm)],
1323+
add_2reg(0xC0, AUX_REG, src_reg));
1324+
/* Attempt to swap in new value */
1325+
err = emit_atomic(&prog, BPF_CMPXCHG,
1326+
dst_reg, AUX_REG, insn->off,
1327+
BPF_SIZE(insn->code));
1328+
if (WARN_ON(err))
1329+
return err;
1330+
/*
1331+
* ZF tells us whether we won the race. If it's
1332+
* cleared we need to try again.
1333+
*/
1334+
EMIT2(X86_JNE, -(prog - branch_target) - 2);
1335+
/* Return the pre-modification value */
1336+
emit_mov_reg(&prog, is64, src_reg, BPF_REG_0);
1337+
/* Restore R0 after clobbering RAX */
1338+
emit_mov_reg(&prog, true, BPF_REG_0, BPF_REG_AX);
1339+
break;
1340+
1341+
}
1342+
12951343
err = emit_atomic(&prog, insn->imm, dst_reg, src_reg,
1296-
insn->off, BPF_SIZE(insn->code));
1344+
insn->off, BPF_SIZE(insn->code));
12971345
if (err)
12981346
return err;
12991347
break;

‎include/linux/filter.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,13 @@ static inline bool insn_is_zext(const struct bpf_insn *insn)
264264
* Atomic operations:
265265
*
266266
* BPF_ADD *(uint *) (dst_reg + off16) += src_reg
267+
* BPF_AND *(uint *) (dst_reg + off16) &= src_reg
268+
* BPF_OR *(uint *) (dst_reg + off16) |= src_reg
269+
* BPF_XOR *(uint *) (dst_reg + off16) ^= src_reg
267270
* BPF_ADD | BPF_FETCH src_reg = atomic_fetch_add(dst_reg + off16, src_reg);
271+
* BPF_AND | BPF_FETCH src_reg = atomic_fetch_and(dst_reg + off16, src_reg);
272+
* BPF_OR | BPF_FETCH src_reg = atomic_fetch_or(dst_reg + off16, src_reg);
273+
* BPF_XOR | BPF_FETCH src_reg = atomic_fetch_xor(dst_reg + off16, src_reg);
268274
* BPF_XCHG src_reg = atomic_xchg(dst_reg + off16, src_reg)
269275
* BPF_CMPXCHG r0 = atomic_cmpxchg(dst_reg + off16, r0, src_reg)
270276
*/

‎kernel/bpf/core.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,6 +1642,9 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
16421642
STX_ATOMIC_W:
16431643
switch (IMM) {
16441644
ATOMIC_ALU_OP(BPF_ADD, add)
1645+
ATOMIC_ALU_OP(BPF_AND, and)
1646+
ATOMIC_ALU_OP(BPF_OR, or)
1647+
ATOMIC_ALU_OP(BPF_XOR, xor)
16451648
#undef ATOMIC_ALU_OP
16461649

16471650
case BPF_XCHG:

‎kernel/bpf/disasm.c‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ const char *const bpf_alu_string[16] = {
8080
[BPF_END >> 4] = "endian",
8181
};
8282

83+
static const char *const bpf_atomic_alu_string[16] = {
84+
[BPF_ADD >> 4] = "add",
85+
[BPF_AND >> 4] = "and",
86+
[BPF_OR >> 4] = "or",
87+
[BPF_XOR >> 4] = "or",
88+
};
89+
8390
static const char *const bpf_ldst_string[] = {
8491
[BPF_W >> 3] = "u32",
8592
[BPF_H >> 3] = "u16",
@@ -154,17 +161,23 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
154161
insn->dst_reg,
155162
insn->off, insn->src_reg);
156163
else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
157-
insn->imm == BPF_ADD) {
158-
verbose(cbs->private_data, "(%02x) lock *(%s *)(r%d %+d) += r%d\n",
164+
(insn->imm == BPF_ADD || insn->imm == BPF_ADD ||
165+
insn->imm == BPF_OR || insn->imm == BPF_XOR)) {
166+
verbose(cbs->private_data, "(%02x) lock *(%s *)(r%d %+d) %s r%d\n",
159167
insn->code,
160168
bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
161169
insn->dst_reg, insn->off,
170+
bpf_alu_string[BPF_OP(insn->imm) >> 4],
162171
insn->src_reg);
163172
} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
164-
insn->imm == (BPF_ADD | BPF_FETCH)) {
165-
verbose(cbs->private_data, "(%02x) r%d = atomic%s_fetch_add((%s *)(r%d %+d), r%d)\n",
173+
(insn->imm == (BPF_ADD | BPF_FETCH) ||
174+
insn->imm == (BPF_AND | BPF_FETCH) ||
175+
insn->imm == (BPF_OR | BPF_FETCH) ||
176+
insn->imm == (BPF_XOR | BPF_FETCH))) {
177+
verbose(cbs->private_data, "(%02x) r%d = atomic%s_fetch_%s((%s *)(r%d %+d), r%d)\n",
166178
insn->code, insn->src_reg,
167179
BPF_SIZE(insn->code) == BPF_DW ? "64" : "",
180+
bpf_atomic_alu_string[BPF_OP(insn->imm) >> 4],
168181
bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
169182
insn->dst_reg, insn->off, insn->src_reg);
170183
} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&

‎kernel/bpf/verifier.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3612,6 +3612,12 @@ static int check_atomic(struct bpf_verifier_env *env, int insn_idx, struct bpf_i
36123612
switch (insn->imm) {
36133613
case BPF_ADD:
36143614
case BPF_ADD | BPF_FETCH:
3615+
case BPF_AND:
3616+
case BPF_AND | BPF_FETCH:
3617+
case BPF_OR:
3618+
case BPF_OR | BPF_FETCH:
3619+
case BPF_XOR:
3620+
case BPF_XOR | BPF_FETCH:
36153621
case BPF_XCHG:
36163622
case BPF_CMPXCHG:
36173623
break;

‎tools/include/linux/filter.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,13 @@
173173
* Atomic operations:
174174
*
175175
* BPF_ADD *(uint *) (dst_reg + off16) += src_reg
176+
* BPF_AND *(uint *) (dst_reg + off16) &= src_reg
177+
* BPF_OR *(uint *) (dst_reg + off16) |= src_reg
178+
* BPF_XOR *(uint *) (dst_reg + off16) ^= src_reg
176179
* BPF_ADD | BPF_FETCH src_reg = atomic_fetch_add(dst_reg + off16, src_reg);
180+
* BPF_AND | BPF_FETCH src_reg = atomic_fetch_and(dst_reg + off16, src_reg);
181+
* BPF_OR | BPF_FETCH src_reg = atomic_fetch_or(dst_reg + off16, src_reg);
182+
* BPF_XOR | BPF_FETCH src_reg = atomic_fetch_xor(dst_reg + off16, src_reg);
177183
* BPF_XCHG src_reg = atomic_xchg(dst_reg + off16, src_reg)
178184
* BPF_CMPXCHG r0 = atomic_cmpxchg(dst_reg + off16, r0, src_reg)
179185
*/

0 commit comments

Comments
 (0)