Re: [PATCH v2 1/4] LoongArch: bpf: Use amswap_db for BPF atomic xchg

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch incompletely fixes memory ordering for value-returning BPF atomic RMW instructions. While it adds full barriers for 32/64-bit BPF_XCHG by emitting `amswap_db`, it misses other BPF_FETCH operations, sub-word BPF_XCHG, and BPF_CMPXCHG, leaving them weakly ordered.
--

commit f10ece83a8a616fba153a535a64e9160f3060f64
Author: Chenguang Zhao <zhaochenguang@kylinos.cn>
LoongArch: bpf: Use amswap_db for BPF atomic xchg

The patch updates the LoongArch BPF JIT to use amswap_db.{w,d} instead of
plain amswap.{w,d} for 32-bit and 64-bit BPF_XCHG instructions. This ensures
that value-returning atomic exchanges provide the full memory barrier required
by the Linux Kernel Memory Model.

> diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h
> index 76b723590023d..636cfc524b020 100644
> --- a/arch/loongarch/include/asm/inst.h
> +++ b/arch/loongarch/include/asm/inst.h
[ ... ]
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index 24913dc7f4e83..f071d913e0545 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -482,11 +482,11 @@ static int emit_atomic_rmw(const struct bpf_insn *insn, struct jit_ctx *ctx)
>  			emit_zext_32(ctx, src, true);
>  			break;
>  		case BPF_W:
> -			emit_insn(ctx, amswapw, src, t1, t3);
> +			emit_insn(ctx, amswapdbw, src, t1, t3);
>  			emit_zext_32(ctx, src, true);
>  			break;
>  		case BPF_DW:
> -			emit_insn(ctx, amswapd, src, t1, t3);
> +			emit_insn(ctx, amswapdbd, src, t1, t3);
>  			break;
>  		}
>  		break;

[Severity: High]
This is a pre-existing issue, but does this patch fully resolve the memory
ordering requirements for BPF atomics?

Per the LKMM and BPF documentation, all atomic RMW operations that return a
value must have sequentially consistent memory ordering (a full memory
barrier). While this patch addresses 32-bit and 64-bit BPF_XCHG, several other
value-returning atomics in emit_atomic_rmw() appear to still use weakly
ordered instructions.

For example, BPF_FETCH operations use plain amadd.w and amadd.d:

	case BPF_ADD | BPF_FETCH:
		switch (BPF_SIZE(insn->code)) {
		...
		case BPF_W:
			emit_insn(ctx, amaddw, src, t1, t3);
			emit_zext_32(ctx, src, true);
			break;
		case BPF_DW:
			emit_insn(ctx, amaddd, src, t1, t3);
			break;

Sub-word BPF_XCHG operations still emit weakly ordered amswap.b and amswap.h
without explicit barriers:

	case BPF_XCHG:
		switch (BPF_SIZE(insn->code)) {
		case BPF_B:
			...
			emit_insn(ctx, amswapb, src, t1, t3);
			...
		case BPF_H:
			...
			emit_insn(ctx, amswaph, src, t1, t3);

Additionally, BPF_CMPXCHG emits a bare ll and sc loop without the required
dbar (e.g., dbar 0x700) before and after the loop:

	case BPF_CMPXCHG:
		...
		if (isdw) {
			emit_insn(ctx, lld, r0, t1, 0);
			emit_insn(ctx, bne, t2, r0, 4);
			move_reg(ctx, t3, src);
			emit_insn(ctx, scd, t3, t1, 0);
			emit_insn(ctx, beq, t3, LOONGARCH_GPR_ZERO, -4);

Could we also update these other value-returning atomic instructions in
emit_atomic_rmw() so that all lockless BPF algorithms execute safely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603100438.2177817-1-zhaochenguang@kylinos.cn?part=1