[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ae8baaa9-663f-4fba-8b40-57eba1c682d2@linux.ibm.com>
Date: Thu, 24 Jul 2025 14:20:40 +0530
From: Hari Bathini <hbathini@...ux.ibm.com>
To: Puranjay Mohan <puranjay@...nel.org>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau
<martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
Naveen N Rao <naveen@...nel.org>, Mykola Lysenko <mykolal@...com>,
Peilin Ye <yepeilin@...gle.com>,
Kumar Kartikeya Dwivedi <memxor@...il.com>
Cc: linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
bpf@...r.kernel.org, "Paul E . McKenney" <paulmck@...nel.org>,
lkmm@...ts.linux.dev
Subject: Re: [PATCH RESEND bpf-next 1/1] powerpc64/bpf: Add jit support for
load_acquire and store_release
On 18/07/25 1:59 am, Puranjay Mohan wrote:
> Add JIT support for the load_acquire and store_release instructions. The
> implementation is similar to the kernel where:
>
> load_acquire => plain load -> lwsync
> store_release => lwsync -> plain store
>
> To test the correctness of the implementation, following selftests were
> run:
>
> [fedora@...ux-kernel bpf]$ sudo ./test_progs -a \
> verifier_load_acquire,verifier_store_release,atomics
> #11/1 atomics/add:OK
> #11/2 atomics/sub:OK
> #11/3 atomics/and:OK
> #11/4 atomics/or:OK
> #11/5 atomics/xor:OK
> #11/6 atomics/cmpxchg:OK
> #11/7 atomics/xchg:OK
> #11 atomics:OK
> #519/1 verifier_load_acquire/load-acquire, 8-bit:OK
> #519/2 verifier_load_acquire/load-acquire, 8-bit @unpriv:OK
> #519/3 verifier_load_acquire/load-acquire, 16-bit:OK
> #519/4 verifier_load_acquire/load-acquire, 16-bit @unpriv:OK
> #519/5 verifier_load_acquire/load-acquire, 32-bit:OK
> #519/6 verifier_load_acquire/load-acquire, 32-bit @unpriv:OK
> #519/7 verifier_load_acquire/load-acquire, 64-bit:OK
> #519/8 verifier_load_acquire/load-acquire, 64-bit @unpriv:OK
> #519/9 verifier_load_acquire/load-acquire with uninitialized
> src_reg:OK
> #519/10 verifier_load_acquire/load-acquire with uninitialized src_reg
> @unpriv:OK
> #519/11 verifier_load_acquire/load-acquire with non-pointer src_reg:OK
> #519/12 verifier_load_acquire/load-acquire with non-pointer src_reg
> @unpriv:OK
> #519/13 verifier_load_acquire/misaligned load-acquire:OK
> #519/14 verifier_load_acquire/misaligned load-acquire @unpriv:OK
> #519/15 verifier_load_acquire/load-acquire from ctx pointer:OK
> #519/16 verifier_load_acquire/load-acquire from ctx pointer @unpriv:OK
> #519/17 verifier_load_acquire/load-acquire with invalid register R15:OK
> #519/18 verifier_load_acquire/load-acquire with invalid register R15
> @unpriv:OK
> #519/19 verifier_load_acquire/load-acquire from pkt pointer:OK
> #519/20 verifier_load_acquire/load-acquire from flow_keys pointer:OK
> #519/21 verifier_load_acquire/load-acquire from sock pointer:OK
> #519 verifier_load_acquire:OK
> #556/1 verifier_store_release/store-release, 8-bit:OK
> #556/2 verifier_store_release/store-release, 8-bit @unpriv:OK
> #556/3 verifier_store_release/store-release, 16-bit:OK
> #556/4 verifier_store_release/store-release, 16-bit @unpriv:OK
> #556/5 verifier_store_release/store-release, 32-bit:OK
> #556/6 verifier_store_release/store-release, 32-bit @unpriv:OK
> #556/7 verifier_store_release/store-release, 64-bit:OK
> #556/8 verifier_store_release/store-release, 64-bit @unpriv:OK
> #556/9 verifier_store_release/store-release with uninitialized
> src_reg:OK
> #556/10 verifier_store_release/store-release with uninitialized src_reg
> @unpriv:OK
> #556/11 verifier_store_release/store-release with uninitialized
> dst_reg:OK
> #556/12 verifier_store_release/store-release with uninitialized dst_reg
> @unpriv:OK
> #556/13 verifier_store_release/store-release with non-pointer
> dst_reg:OK
> #556/14 verifier_store_release/store-release with non-pointer dst_reg
> @unpriv:OK
> #556/15 verifier_store_release/misaligned store-release:OK
> #556/16 verifier_store_release/misaligned store-release @unpriv:OK
> #556/17 verifier_store_release/store-release to ctx pointer:OK
> #556/18 verifier_store_release/store-release to ctx pointer @unpriv:OK
> #556/19 verifier_store_release/store-release, leak pointer to stack:OK
> #556/20 verifier_store_release/store-release, leak pointer to stack
> @unpriv:OK
> #556/21 verifier_store_release/store-release, leak pointer to map:OK
> #556/22 verifier_store_release/store-release, leak pointer to map
> @unpriv:OK
> #556/23 verifier_store_release/store-release with invalid register
> R15:OK
> #556/24 verifier_store_release/store-release with invalid register R15
> @unpriv:OK
> #556/25 verifier_store_release/store-release to pkt pointer:OK
> #556/26 verifier_store_release/store-release to flow_keys pointer:OK
> #556/27 verifier_store_release/store-release to sock pointer:OK
> #556 verifier_store_release:OK
> Summary: 3/55 PASSED, 0 SKIPPED, 0 FAILED
>
Thanks for the patch. Looks good.
Reviewed-by: Hari Bathini <hbathini@...ux.ibm.com>
> Signed-off-by: Puranjay Mohan <puranjay@...nel.org>
> ---
> arch/powerpc/include/asm/ppc-opcode.h | 1 +
> arch/powerpc/net/bpf_jit_comp64.c | 82 ++++++++++++++++++++
> tools/testing/selftests/bpf/progs/bpf_misc.h | 3 +-
> 3 files changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 4312bcb913a42..8053b24afc395 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -425,6 +425,7 @@
> #define PPC_RAW_SC() (0x44000002)
> #define PPC_RAW_SYNC() (0x7c0004ac)
> #define PPC_RAW_ISYNC() (0x4c00012c)
> +#define PPC_RAW_LWSYNC() (0x7c2004ac)
>
> /*
> * Define what the VSX XX1 form instructions will look like, then add
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index a25a6ffe7d7cc..025524378443e 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -409,6 +409,71 @@ asm (
> " blr ;"
> );
>
> +static int emit_atomic_ld_st(const struct bpf_insn insn, struct codegen_context *ctx, u32 *image)
> +{
> + u32 code = insn.code;
> + u32 dst_reg = bpf_to_ppc(insn.dst_reg);
> + u32 src_reg = bpf_to_ppc(insn.src_reg);
> + u32 size = BPF_SIZE(code);
> + u32 tmp1_reg = bpf_to_ppc(TMP_REG_1);
> + u32 tmp2_reg = bpf_to_ppc(TMP_REG_2);
> + s16 off = insn.off;
> + s32 imm = insn.imm;
> +
> + switch (imm) {
> + case BPF_LOAD_ACQ:
> + switch (size) {
> + case BPF_B:
> + EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> + break;
> + case BPF_H:
> + EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> + break;
> + case BPF_W:
> + EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> + break;
> + case BPF_DW:
> + if (off % 4) {
> + EMIT(PPC_RAW_LI(tmp1_reg, off));
> + EMIT(PPC_RAW_LDX(dst_reg, src_reg, tmp1_reg));
> + } else {
> + EMIT(PPC_RAW_LD(dst_reg, src_reg, off));
> + }
> + break;
> + }
> + EMIT(PPC_RAW_LWSYNC());
> + break;
> + case BPF_STORE_REL:
> + EMIT(PPC_RAW_LWSYNC());
> + switch (size) {
> + case BPF_B:
> + EMIT(PPC_RAW_STB(src_reg, dst_reg, off));
> + break;
> + case BPF_H:
> + EMIT(PPC_RAW_STH(src_reg, dst_reg, off));
> + break;
> + case BPF_W:
> + EMIT(PPC_RAW_STW(src_reg, dst_reg, off));
> + break;
> + case BPF_DW:
> + if (off % 4) {
> + EMIT(PPC_RAW_LI(tmp2_reg, off));
> + EMIT(PPC_RAW_STDX(src_reg, dst_reg, tmp2_reg));
> + } else {
> + EMIT(PPC_RAW_STD(src_reg, dst_reg, off));
> + }
> + break;
> + }
> + break;
> + default:
> + pr_err_ratelimited("unexpected atomic load/store op code %02x\n",
> + imm);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> /* Assemble the body code between the prologue & epilogue */
> int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct codegen_context *ctx,
> u32 *addrs, int pass, bool extra_pass)
> @@ -898,8 +963,25 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct code
> /*
> * BPF_STX ATOMIC (atomic ops)
> */
> + 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[i])) {
> + ret = emit_atomic_ld_st(insn[i], ctx, image);
> + if (ret)
> + return ret;
> +
> + if (size != BPF_DW && insn_is_zext(&insn[i + 1]))
> + addrs[++i] = ctx->idx * 4;
> + break;
> + } else if (size == BPF_B || size == BPF_H) {
> + pr_err_ratelimited(
> + "eBPF filter atomic op code %02x (@%d) unsupported\n",
> + code, i);
> + return -EOPNOTSUPP;
> + }
> +
> save_reg = tmp2_reg;
> ret_reg = src_reg;
>
> diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
> index 530752ddde8e4..c1cfd297aabf1 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_misc.h
> +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
> @@ -229,7 +229,8 @@
>
> #if __clang_major__ >= 18 && defined(ENABLE_ATOMICS_TESTS) && \
> (defined(__TARGET_ARCH_arm64) || defined(__TARGET_ARCH_x86) || \
> - (defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64))
> + (defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64)) || \
> + (defined(__TARGET_ARCH_powerpc))
> #define CAN_USE_LOAD_ACQ_STORE_REL
> #endif
>
Powered by blists - more mailing lists