diff mbox series

[bpf-next,v4,08/10] bpf, x86: Create two helpers for some arith operations

Message ID 20241010175633.1898994-1-yonghong.song@linux.dev (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpf: Support private stack for bpf progs | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 fail Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 fail Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-11 fail Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-15 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-16 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 fail Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 16 maintainers not CCed: dave.hansen@linux.intel.com john.fastabend@gmail.com dsahern@kernel.org jolsa@kernel.org x86@kernel.org kpsingh@kernel.org song@kernel.org haoluo@google.com bp@alien8.de netdev@vger.kernel.org sdf@fomichev.me martin.lau@linux.dev hpa@zytor.com tglx@linutronix.de eddyz87@gmail.com mingo@redhat.com
netdev/build_clang success Errors and warnings before: 6 this patch: 6
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 7 this patch: 7
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yonghong Song Oct. 10, 2024, 5:56 p.m. UTC 
Two helpers are extracted from bpf/x86 jit:
  - a helper to handle 'reg1 <op>= reg2' where <op> is add/sub/and/or/xor
  - a helper to handle 'reg *= imm'

Both helpers will be used in the subsequent patch.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 arch/x86/net/bpf_jit_comp.c | 51 ++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 17 deletions(-)

Comments

Alexei Starovoitov Oct. 10, 2024, 8:21 p.m. UTC | #1 
On Thu, Oct 10, 2024 at 10:56 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> Two helpers are extracted from bpf/x86 jit:
>   - a helper to handle 'reg1 <op>= reg2' where <op> is add/sub/and/or/xor
>   - a helper to handle 'reg *= imm'
>
> Both helpers will be used in the subsequent patch.
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  arch/x86/net/bpf_jit_comp.c | 51 ++++++++++++++++++++++++-------------
>  1 file changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index a6ba85cec49a..297dd64f4b6a 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -1475,6 +1475,37 @@ static void emit_alu_helper_1(u8 **pprog, u8 insn_code, u32 dst_reg, s32 imm32)
>         *pprog = prog;
>  }
>
> +/* emit ADD/SUB/AND/OR/XOR 'reg1 <op>= reg2' operations */
> +static void emit_alu_helper_2(u8 **pprog, u8 insn_code, u32 dst_reg, u32 src_reg)
> +{
> +       u8 b2 = 0;
> +       u8 *prog = *pprog;
> +
> +       maybe_emit_mod(&prog, dst_reg, src_reg,
> +                      BPF_CLASS(insn_code) == BPF_ALU64);
> +       b2 = simple_alu_opcodes[BPF_OP(insn_code)];
> +       EMIT2(b2, add_2reg(0xC0, dst_reg, src_reg));
> +
> +       *pprog = prog;
> +}
> +
> +/* emit 'reg *= imm' operations */
> +static void emit_alu_helper_3(u8 **pprog, u8 insn_code, u32 dst_reg, s32 imm32)

_1, _2, _3 ?!

There must be a better way to name the helpers. Like:

_1 -> emit_alu_imm
_2 -> emit_alu_reg
_3 -> emit_mul_imm
Yonghong Song Oct. 11, 2024, 4:16 a.m. UTC | #2 
On 10/10/24 1:21 PM, Alexei Starovoitov wrote:
> On Thu, Oct 10, 2024 at 10:56 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Two helpers are extracted from bpf/x86 jit:
>>    - a helper to handle 'reg1 <op>= reg2' where <op> is add/sub/and/or/xor
>>    - a helper to handle 'reg *= imm'
>>
>> Both helpers will be used in the subsequent patch.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   arch/x86/net/bpf_jit_comp.c | 51 ++++++++++++++++++++++++-------------
>>   1 file changed, 34 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index a6ba85cec49a..297dd64f4b6a 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -1475,6 +1475,37 @@ static void emit_alu_helper_1(u8 **pprog, u8 insn_code, u32 dst_reg, s32 imm32)
>>          *pprog = prog;
>>   }
>>
>> +/* emit ADD/SUB/AND/OR/XOR 'reg1 <op>= reg2' operations */
>> +static void emit_alu_helper_2(u8 **pprog, u8 insn_code, u32 dst_reg, u32 src_reg)
>> +{
>> +       u8 b2 = 0;
>> +       u8 *prog = *pprog;
>> +
>> +       maybe_emit_mod(&prog, dst_reg, src_reg,
>> +                      BPF_CLASS(insn_code) == BPF_ALU64);
>> +       b2 = simple_alu_opcodes[BPF_OP(insn_code)];
>> +       EMIT2(b2, add_2reg(0xC0, dst_reg, src_reg));
>> +
>> +       *pprog = prog;
>> +}
>> +
>> +/* emit 'reg *= imm' operations */
>> +static void emit_alu_helper_3(u8 **pprog, u8 insn_code, u32 dst_reg, s32 imm32)
> _1, _2, _3 ?!
>
> There must be a better way to name the helpers. Like:
>
> _1 -> emit_alu_imm
> _2 -> emit_alu_reg
> _3 -> emit_mul_imm

I struggle to get a proper name here. I originally thought about to use
emit_alu_reg_imm, emit_alu_reg_reg, but in my case, even emit_alu_reg_imm
only supports add/sub/and/or/xor and it does not support mul/div/mod, so
emit_alu_reg_imm does not really cover all alu operations so I chose
another name which is also not good.

I guess I can use the above you suggested in the above which actually
covers most alu operations.
diff mbox series

Patch

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index a6ba85cec49a..297dd64f4b6a 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1475,6 +1475,37 @@  static void emit_alu_helper_1(u8 **pprog, u8 insn_code, u32 dst_reg, s32 imm32)
 	*pprog = prog;
 }
 
+/* emit ADD/SUB/AND/OR/XOR 'reg1 <op>= reg2' operations */
+static void emit_alu_helper_2(u8 **pprog, u8 insn_code, u32 dst_reg, u32 src_reg)
+{
+	u8 b2 = 0;
+	u8 *prog = *pprog;
+
+	maybe_emit_mod(&prog, dst_reg, src_reg,
+		       BPF_CLASS(insn_code) == BPF_ALU64);
+	b2 = simple_alu_opcodes[BPF_OP(insn_code)];
+	EMIT2(b2, add_2reg(0xC0, dst_reg, src_reg));
+
+	*pprog = prog;
+}
+
+/* emit 'reg *= imm' operations */
+static void emit_alu_helper_3(u8 **pprog, u8 insn_code, u32 dst_reg, s32 imm32)
+{
+	u8 *prog = *pprog;
+
+	maybe_emit_mod(&prog, dst_reg, dst_reg, BPF_CLASS(insn_code) == BPF_ALU64);
+
+	if (is_imm8(imm32))
+		/* imul dst_reg, dst_reg, imm8 */
+		EMIT3(0x6B, add_2reg(0xC0, dst_reg, dst_reg), imm32);
+	else
+		/* imul dst_reg, dst_reg, imm32 */
+		EMIT2_off32(0x69, add_2reg(0xC0, dst_reg, dst_reg), imm32);
+
+	*pprog = prog;
+}
+
 static void emit_root_priv_frame_ptr(u8 **pprog, struct bpf_prog *bpf_prog,
 				     u32 orig_stack_depth)
 {
@@ -1578,7 +1609,7 @@  static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
 		const s32 imm32 = insn->imm;
 		u32 dst_reg = insn->dst_reg;
 		u32 src_reg = insn->src_reg;
-		u8 b2 = 0, b3 = 0;
+		u8 b3 = 0;
 		u8 *start_of_ldx;
 		s64 jmp_offset;
 		s16 insn_off;
@@ -1606,10 +1637,7 @@  static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
 		case BPF_ALU64 | BPF_AND | BPF_X:
 		case BPF_ALU64 | BPF_OR | BPF_X:
 		case BPF_ALU64 | BPF_XOR | BPF_X:
-			maybe_emit_mod(&prog, dst_reg, src_reg,
-				       BPF_CLASS(insn->code) == BPF_ALU64);
-			b2 = simple_alu_opcodes[BPF_OP(insn->code)];
-			EMIT2(b2, add_2reg(0xC0, dst_reg, src_reg));
+			emit_alu_helper_2(&prog, insn->code, dst_reg, src_reg);
 			break;
 
 		case BPF_ALU64 | BPF_MOV | BPF_X:
@@ -1772,18 +1800,7 @@  static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
 
 		case BPF_ALU | BPF_MUL | BPF_K:
 		case BPF_ALU64 | BPF_MUL | BPF_K:
-			maybe_emit_mod(&prog, dst_reg, dst_reg,
-				       BPF_CLASS(insn->code) == BPF_ALU64);
-
-			if (is_imm8(imm32))
-				/* imul dst_reg, dst_reg, imm8 */
-				EMIT3(0x6B, add_2reg(0xC0, dst_reg, dst_reg),
-				      imm32);
-			else
-				/* imul dst_reg, dst_reg, imm32 */
-				EMIT2_off32(0x69,
-					    add_2reg(0xC0, dst_reg, dst_reg),
-					    imm32);
+			emit_alu_helper_3(&prog, insn->code, dst_reg, imm32);
 			break;
 
 		case BPF_ALU | BPF_MUL | BPF_X: