Re: [PATCH bpf-next v2 05/15] bpf: Support new signed div/mod instructions.

[Yonghong Song <yhs@xxxxxxxx>]

On 7/18/23 4:00 PM, Eduard Zingerman wrote:
> On Wed, 2023-07-12 at 23:07 -0700, Yonghong Song wrote:
> > Add interpreter/jit support for new signed div/mod insns.
> > The new signed div/mod instructions are encoded with
> > unsigned div/mod instructions plus insn->off == 1.
> > Also add basic verifier support to ensure new insns get
> > accepted.
> >
> > Signed-off-by: Yonghong Song <yhs@xxxxxx>
> > ---
> >   arch/x86/net/bpf_jit_comp.c | 27 +++++++----
> >   kernel/bpf/core.c           | 96 ++++++++++++++++++++++++++++++-------
> >   kernel/bpf/verifier.c       |  6 ++-
> >   3 files changed, 103 insertions(+), 26 deletions(-)
> >
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index adda5e7626b4..3176b60d25c7 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -1194,15 +1194,26 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
> >   				/* mov rax, dst_reg */
> >   				emit_mov_reg(&prog, is64, BPF_REG_0, dst_reg);
> >   
> > -			/*
> > -			 * xor edx, edx
> > -			 * equivalent to 'xor rdx, rdx', but one byte less
> > -			 */
> > -			EMIT2(0x31, 0xd2);
> > +			if (insn->off == 0) {
> > +				/*
> > +				 * xor edx, edx
> > +				 * equivalent to 'xor rdx, rdx', but one byte less
> > +				 */
> > +				EMIT2(0x31, 0xd2);
> >   
> > -			/* div src_reg */
> > -			maybe_emit_1mod(&prog, src_reg, is64);
> > -			EMIT2(0xF7, add_1reg(0xF0, src_reg));
> > +				/* div src_reg */
> > +				maybe_emit_1mod(&prog, src_reg, is64);
> > +				EMIT2(0xF7, add_1reg(0xF0, src_reg));
> > +			} else {
> > +				if (BPF_CLASS(insn->code) == BPF_ALU)
> > +					EMIT1(0x99); /* cltd */
> > +				else
> > +					EMIT2(0x48, 0x99); /* cqto */
>
> Nitpick: I can't find names cltd/cqto in the Intel instruction manual,
>           instead it uses names cdq/cqo for these instructions.
>           (See Vol. 2A pages 3-315 and 3-497)

I got these asm names from
  https://defuse.ca/online-x86-assembler.htm
I will check the Intel insn manual and make the change
accordingly.

> > +
> > +				/* idiv src_reg */
> > +				maybe_emit_1mod(&prog, src_reg, is64);
> > +				EMIT2(0xF7, add_1reg(0xF8, src_reg));
> > +			}
> >   
> >   			if (BPF_OP(insn->code) == BPF_MOD &&
> >   			    dst_reg != BPF_REG_3)
[...]