Re: [PATCH bpf-next v2 2/2] bpf, mips: Add support for BPF_SDIV and BPF_SMOD

Johan Almbladh <[email protected]>
Newsgroups org.kernel.vger.bpf,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mips
Message-ID <CAM1=_QS79dDBfaaNQXnWd61AqHd1M4o9aDMMiftnoJveNr=FZg@mail.gmail.com>
On Wed, Jul 29, 2026 at 6:30 PM Nicholas Dudar <[email protected]> wrote:
>
> The MIPS JITs handle BPF_DIV and BPF_MOD without inspecting
> insn->off, which distinguishes BPF_SDIV and BPF_SMOD. Signed
> operations therefore use unsigned instructions, or unsigned helpers
> for ALU64 on 32-bit MIPS, and produce unsigned results for negative
> operands.

Thanks for the patch! In addition to your SDIV/SMOD fixes, I would
like to bring the MIPS JIT to full v4 compliance. All the tests in
test_bpf should be jit'ed and pass. Let me know if you are already
working on the remaining v4 support, otherwise I could look into it.

> arch/mips/include/asm/uasm.h   |  6 ++++
> arch/mips/mm/uasm-mips.c       | 10 ++++++
> arch/mips/mm/uasm.c            | 20 ++++++++----

Consider making the uasm additions a separate patch. It would be more clear IMO.

> +static s64 jit_smod64(s64 a, s64 b)
> +{
> +       u64 quot = div64_s64(a, b);
> +
> +       return a - quot * b;

Should not "quot" be s64? It would be more clear to not rely on
implicit signed/unsigned promotions.

> @@ -650,6 +665,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
>         u8 code = insn->code;
>         s16 off = insn->off;
>         s32 imm = insn->imm;
> +       bool is_signed = off == 1;

The interpretation of the offset field is opcode-specific. The
SDIV/SMOD instructions are encoded as DIV/MOD with off = 1. MOVSX
instead use off to encode the width of the load.

Please propagate the raw offset value and do the interpretation in the
div/mod helpers instead. This also makes it easier to add the
remaining v4 instructions later.

Thanks,
Johan
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.