* [PATCH bpf-next v2] s390/bpf: Emit a barrier for BPF_FETCH instructions
@ 2024-05-07 0:02 Ilya Leoshkevich
2024-05-07 11:10 ` Puranjay Mohan
2024-05-13 0:00 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Ilya Leoshkevich @ 2024-05-07 0:02 UTC (permalink / raw
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Ilya Leoshkevich, Puranjay Mohan
BPF_ATOMIC_OP() macro documentation states that "BPF_ADD | BPF_FETCH"
should be the same as atomic_fetch_add(), which is currently not the
case on s390x: the serialization instruction "bcr 14,0" is missing.
This applies to "and", "or" and "xor" variants too.
s390x is allowed to reorder stores with subsequent fetches from
different addresses, so code relying on BPF_FETCH acting as a barrier,
for example:
stw [%r0], 1
afadd [%r1], %r2
ldxw %r3, [%r4]
may be broken. Fix it by emitting "bcr 14,0".
Note that a separate serialization instruction is not needed for
BPF_XCHG and BPF_CMPXCHG, because COMPARE AND SWAP performs
serialization itself.
Fixes: ba3b86b9cef0 ("s390/bpf: Implement new atomic ops")
Reported-by: Puranjay Mohan <puranjay12@gmail.com>
Closes: https://lore.kernel.org/bpf/mb61p34qvq3wf.fsf@kernel.org/
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
v1: https://lore.kernel.org/bpf/20240506141649.50845-1-iii@linux.ibm.com/
v1 -> v2: Emit a barrier only for BPF_FETCH variants;
Add an example of the code that may break to the commit
message (Puranjay).
arch/s390/net/bpf_jit_comp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index fa2f824e3b06..4be8f5cadd02 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -1427,8 +1427,12 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
EMIT6_DISP_LH(0xeb000000, is32 ? (op32) : (op64), \
(insn->imm & BPF_FETCH) ? src_reg : REG_W0, \
src_reg, dst_reg, off); \
- if (is32 && (insn->imm & BPF_FETCH)) \
- EMIT_ZERO(src_reg); \
+ if (insn->imm & BPF_FETCH) { \
+ /* bcr 14,0 - see atomic_fetch_{add,and,or,xor}() */ \
+ _EMIT2(0x07e0); \
+ if (is32) \
+ EMIT_ZERO(src_reg); \
+ } \
} while (0)
case BPF_ADD:
case BPF_ADD | BPF_FETCH:
--
2.45.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v2] s390/bpf: Emit a barrier for BPF_FETCH instructions
2024-05-07 0:02 [PATCH bpf-next v2] s390/bpf: Emit a barrier for BPF_FETCH instructions Ilya Leoshkevich
@ 2024-05-07 11:10 ` Puranjay Mohan
2024-05-13 0:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Puranjay Mohan @ 2024-05-07 11:10 UTC (permalink / raw
To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko
Cc: bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Ilya Leoshkevich
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> BPF_ATOMIC_OP() macro documentation states that "BPF_ADD | BPF_FETCH"
> should be the same as atomic_fetch_add(), which is currently not the
> case on s390x: the serialization instruction "bcr 14,0" is missing.
> This applies to "and", "or" and "xor" variants too.
>
> s390x is allowed to reorder stores with subsequent fetches from
> different addresses, so code relying on BPF_FETCH acting as a barrier,
> for example:
>
> stw [%r0], 1
> afadd [%r1], %r2
> ldxw %r3, [%r4]
>
> may be broken. Fix it by emitting "bcr 14,0".
>
> Note that a separate serialization instruction is not needed for
> BPF_XCHG and BPF_CMPXCHG, because COMPARE AND SWAP performs
> serialization itself.
>
> Fixes: ba3b86b9cef0 ("s390/bpf: Implement new atomic ops")
> Reported-by: Puranjay Mohan <puranjay12@gmail.com>
> Closes: https://lore.kernel.org/bpf/mb61p34qvq3wf.fsf@kernel.org/
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Puranjay Mohan <puranjay@kernel.org>
Thanks,
Puranjay
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v2] s390/bpf: Emit a barrier for BPF_FETCH instructions
2024-05-07 0:02 [PATCH bpf-next v2] s390/bpf: Emit a barrier for BPF_FETCH instructions Ilya Leoshkevich
2024-05-07 11:10 ` Puranjay Mohan
@ 2024-05-13 0:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-05-13 0:00 UTC (permalink / raw
To: Ilya Leoshkevich; +Cc: ast, daniel, andrii, bpf, hca, gor, agordeev, puranjay12
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Tue, 7 May 2024 02:02:49 +0200 you wrote:
> BPF_ATOMIC_OP() macro documentation states that "BPF_ADD | BPF_FETCH"
> should be the same as atomic_fetch_add(), which is currently not the
> case on s390x: the serialization instruction "bcr 14,0" is missing.
> This applies to "and", "or" and "xor" variants too.
>
> s390x is allowed to reorder stores with subsequent fetches from
> different addresses, so code relying on BPF_FETCH acting as a barrier,
> for example:
>
> [...]
Here is the summary with links:
- [bpf-next,v2] s390/bpf: Emit a barrier for BPF_FETCH instructions
https://git.kernel.org/bpf/bpf-next/c/68378982f0b2
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-13 0:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-07 0:02 [PATCH bpf-next v2] s390/bpf: Emit a barrier for BPF_FETCH instructions Ilya Leoshkevich
2024-05-07 11:10 ` Puranjay Mohan
2024-05-13 0:00 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).