Subject: Re: [PATH bpf-next 09/13] s390: bpf: implement jitting of JMP32

From: Martin Schwidefsky <[email-masked]>
Date: Thu, 20 Dec 2018 07:47:56 +0100

On Wed, 19 Dec 2018 17:44:16 -0500
Jiong Wang <[email-masked]> wrote:

> This patch implements code-gen for new JMP32 instructions on s390.
> 
> Cc: Martin Schwidefsky <[email-masked]>
> Cc: Heiko Carstens <[email-masked]>
> Signed-off-by: Jiong Wang <[email-masked]>
> ---
>  arch/s390/net/bpf_jit_comp.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index 3ff758e..c7101e5 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -1186,21 +1186,25 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
>  		/* lgfi %w1,imm (load sign extend imm) */
>  		EMIT6_IMM(0xc0010000, REG_W1, imm);
>  		/* cgrj %dst,%w1,mask,off */

Please correct the comment about the instruction as well. Dependent on the
BPF_JMP_SUBOP_32BIT in src_reg the instruction is either cgrj or crj.

> -		EMIT6_PCREL(0xec000000, 0x0064, dst_reg, REG_W1, i, off, mask);
> +		EMIT6_PCREL(0xec000000, src_reg ? 0x0076 : 0x0064,
> +			    dst_reg, REG_W1, i, off, mask);
>  		break;
>  branch_ku:
>  		/* lgfi %w1,imm (load sign extend imm) */
>  		EMIT6_IMM(0xc0010000, REG_W1, imm);
>  		/* clgrj %dst,%w1,mask,off */

Same here, dependent on src_reg either clgrj or clrj.

> -		EMIT6_PCREL(0xec000000, 0x0065, dst_reg, REG_W1, i, off, mask);
> +		EMIT6_PCREL(0xec000000, src_reg ? 0x0077 : 0x0065,
> +			    dst_reg, REG_W1, i, off, mask);
>  		break;
>  branch_xs:
>  		/* cgrj %dst,%src,mask,off */

Same same, dependent on imm either cgrj or crj.

> -		EMIT6_PCREL(0xec000000, 0x0064, dst_reg, src_reg, i, off, mask);
> +		EMIT6_PCREL(0xec000000, imm ? 0x0076 : 0x0064,
> +			    dst_reg, src_reg, i, off, mask);
>  		break;
>  branch_xu:
>  		/* clgrj %dst,%src,mask,off */

And again, dependent on imm either clgrj or clrj.

> -		EMIT6_PCREL(0xec000000, 0x0065, dst_reg, src_reg, i, off, mask);
> +		EMIT6_PCREL(0xec000000, imm ? 0x0077 : 0x0065,
> +			    dst_reg, src_reg, i, off, mask);
>  		break;
>  branch_oc:
>  		/* brc mask,jmp_off (branch instruction needs 4 bytes) */

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.