Skip to content

bpf, riscv64: Support load-acquire and store-release instructions #8868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: bpf-next_base
Choose a base branch
from
Open

bpf, riscv64: Support load-acquire and store-release instructions #8868

wants to merge 8 commits into from
+314 −146

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf, riscv64: Support load-acquire and store-release instructions
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=958319

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 38d976c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=958319
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 38d976c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=958319
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 358b1c0
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=958319
version: 1

peilin-ye and others added 7 commits April 30, 2025 16:03
@peilin-ye
bpf/verifier: Handle BPF_LOAD_ACQ instructions in insn_def_regno()
7e93882 
In preparation for supporting BPF load-acquire and store-release
instructions for architectures where bpf_jit_needs_zext() returns true
(e.g. riscv64), make insn_def_regno() handle load-acquires properly.

Signed-off-by: Peilin Ye <[email protected]>
bpf, riscv64: Introduce emit_load_*() and emit_store_*()
17a7161 
We're planning to add support for the load-acquire and store-release
BPF instructions.  Define emit_load_<size>() and emit_store_<size>()
to enable/facilitate the (re)use of their code.

Tested-by: Peilin Ye <[email protected]>
Signed-off-by: Andrea Parri <[email protected]>
[[email protected]: cosmetic change to commit title]
Signed-off-by: Peilin Ye <[email protected]>
Reviewed-by: Pu Lehui <[email protected]>
bpf, riscv64: Support load-acquire and store-release instructions
9d97335 
Support BPF load-acquire (BPF_LOAD_ACQ) and store-release
(BPF_STORE_REL) instructions in the riscv64 JIT compiler.  For example,
consider the following 64-bit load-acquire (assuming little-endian):

  db 10 00 00 00 01 00 00  r1 = 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 will emit an LD instruction followed by a FENCE R,RW
instruction for the above, e.g.:

  ld x7,0(x6)
  fence r,rw

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

A FENCE RW,W instruction followed by an SH instruction will be emitted,
e.g.:

  fence rw,w
  sh x2,0(x4)

8-bit and 16-bit load-acquires are zero-extending (cf., LBU, LHU).  The
verifier always rejects misaligned load-acquires/store-releases (even if
BPF_F_ANY_ALIGNMENT is set), so the emitted load and store instructions
are guaranteed to be single-copy atomic.

Introduce primitives to emit the relevant (and the most common/used in
the kernel) fences, i.e. fences with R -> RW, RW -> W and RW -> RW.

Rename emit_atomic() to emit_atomic_rmw() to make it clear that it only
handles RMW atomics, and replace its is64 parameter to allow to perform
the required checks on the opsize (BPF_SIZE(code)).

Tested-by: Peilin Ye <[email protected]>
Signed-off-by: Andrea Parri <[email protected]>
[[email protected]: whitespace changes; cosmetic changes to commit
                      title and message]
Signed-off-by: Peilin Ye <[email protected]>
Reviewed-by: Pu Lehui <[email protected]>
@peilin-ye
bpf, riscv64: Skip redundant zext instruction after load-acquire
6bfc618 
Currently, the verifier inserts a zext instruction right after every 8-,
16- or 32-bit load-acquire, which is already zero-extending.  Skip such
redundant zext instructions.

While we are here, update that already-obsolete comment about "skip the
next instruction" in build_body().  Also change emit_atomic_rmw()'s
parameters to keep it consistent with emit_atomic_ld_st().

Note that checking 'insn[1]' relies on 'insn' not being the last
instruction, which should have been guaranteed by the verifier; we
already use 'insn[1]' elsewhere in the file for similar purposes.
Additionally, we don't check if 'insn[1]' is actually a zext for our
load-acquire's dst_reg, or some other registers - in other words, here
we are relying on the verifier to always insert a redundant zext right
after a 8/16/32-bit load-acquire, for its dst_reg.

Signed-off-by: Peilin Ye <[email protected]>
Reviewed-by: Pu Lehui <[email protected]>
@peilin-ye
selftests/bpf: Use CAN_USE_LOAD_ACQ_STORE_REL when appropriate
8954dc4 
Instead of open-coding the conditions, use
'#ifdef CAN_USE_LOAD_ACQ_STORE_REL' to guard the following tests:

  verifier_precision/bpf_load_acquire
  verifier_precision/bpf_store_release
  verifier_store_release/*

Note that, for the first two tests in verifier_precision.c, switching to
'#ifdef CAN_USE_LOAD_ACQ_STORE_REL' means also checking if
'__clang_major__ >= 18', which has already been guaranteed by the outer
'#if' check.

Signed-off-by: Peilin Ye <[email protected]>
@peilin-ye
selftests/bpf: Avoid passing out-of-range values to __retval()
cf426ad 
Currently, we pass 0x1234567890abcdef to __retval() for the following
two tests:

  verifier_load_acquire/load_acquire_64
  verifier_store_release/store_release_64

However, the upper 32 bits of that value are being ignored, since
__retval() expects an int.  Actually, the tests would still pass even if
I change '__retval(0x1234567890abcdef)' to e.g. '__retval(0x90abcdef)'.

Restructure the tests a bit to test the entire 64-bit values properly.
Do the same to their 8-, 16- and 32-bit variants as well to keep the
style consistent.

Fixes: ff3afe5 ("selftests/bpf: Add selftests for load-acquire and store-release instructions")
Signed-off-by: Peilin Ye <[email protected]>
@peilin-ye
selftests/bpf: Verify zero-extension behavior in load-acquire tests
c8195e7 
Verify that 8-, 16- and 32-bit load-acquires are zero-extending by using
immediate values with their highest bit set.  Do the same for the 64-bit
variant to keep the style consistent.

Signed-off-by: Peilin Ye <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 358b1c0
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=958319
version: 1

@peilin-ye
selftests/bpf: Enable non-arena load-acquire/store-release selftests …
4552c2b 
…for riscv64

For riscv64, enable all BPF_{LOAD_ACQ,STORE_REL} selftests except the
arena_atomics/* ones (not guarded behind CAN_USE_LOAD_ACQ_STORE_REL),
since arena access is not yet supported.

Signed-off-by: Peilin Ye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next new V1-ci-pass V1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@peilin-ye