On Tue, Jul 18, 2023 at 7:31 PM Yonghong Song <yhs@xxxxxxxx> wrote:
>
>
>
> 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.
Heh. I've been using the same.
Most of the comments in the x86 JIT code are from there :)
and it actually returns 99 -> cdq, 4899 -> cqo
cltd/cqto must be aliases ?
