[PATCH 0/2] Allow mixing bpf2bpf calls with tailcalls on LoongArch

[PATCH 0/2] Allow mixing bpf2bpf calls with tailcalls on LoongArch

[Hengqi Chen]

This patchset enables mixing bpf2bpf calls with tailcalls on LoongArch.

The first patch fixes JIT for function calls, like:

  [   29.346981] multi-func JIT bug 105 != 103

This is because we are emiting variable instructions for 64-bit immediate moves.
During the first pass of JIT, the placeholder address is just zero, emiting two
instructions for it. In the extra pass, the function address is in XKVRANGE,
emiting four instructions for it. This change the instruction index in JIT context.
Fix it by using a fixed 4-instruction sequence.

The second patch enables mixing bpf2bpf calls with tailcalls on LoongArch.

Hengqi Chen (2):
  LoongArch: BPF: Treat function address as 64-bit value
  LoongArch: BPF: Support mixing bpf2bpf and tailcalls

 arch/loongarch/net/bpf_jit.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

--
2.31.1

[PATCH 1/2] LoongArch: BPF: Treat function address as 64-bit value

[Hengqi Chen]

Let's always use 4 instructions for function address in JIT.
So that the instruction sequences don't change between the first
pass and the extra pass for function calls.

Fixes: 5dc615520c4d ("LoongArch: Add BPF JIT support")
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 arch/loongarch/net/bpf_jit.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index c4b1947ebf76..2d952110be72 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -446,6 +446,27 @@ static int add_exception_handler(const struct bpf_insn *insn,
 	return 0;
 }
 
+static inline void emit_addr_move(struct jit_ctx *ctx, enum loongarch_gpr rd, u64 addr)
+{
+	u64 imm_11_0, imm_31_12, imm_51_32, imm_63_52;
+
+	/* lu12iw rd, imm_31_12 */
+	imm_31_12 = (addr >> 12) & 0xfffff;
+	emit_insn(ctx, lu12iw, rd, imm_31_12);
+
+	/* ori rd, rd, imm_11_0 */
+	imm_11_0 = addr & 0xfff;
+	emit_insn(ctx, ori, rd, rd, imm_11_0);
+
+	/* lu32id rd, imm_51_32 */
+	imm_51_32 = (addr >> 32) & 0xfffff;
+	emit_insn(ctx, lu32id, rd, imm_51_32);
+
+	/* lu52id rd, rd, imm_63_52 */
+	imm_63_52 = (addr >> 52) & 0xfff;
+	emit_insn(ctx, lu52id, rd, rd, imm_63_52);
+}
+
 static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool extra_pass)
 {
 	u8 tm = -1;
@@ -841,7 +862,7 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext
 		if (ret < 0)
 			return ret;
 
-		move_imm(ctx, t1, func_addr, is32);
+		emit_addr_move(ctx, t1, func_addr);
 		emit_insn(ctx, jirl, t1, LOONGARCH_GPR_RA, 0);
 		move_reg(ctx, regmap[BPF_REG_0], LOONGARCH_GPR_A0);
 		break;
-- 
2.31.1

[PATCH 2/2] LoongArch: BPF: Support mixing bpf2bpf and tailcalls

[Hengqi Chen]

The current implementation already allow such mixing.
Let's enable it in JIT.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 arch/loongarch/net/bpf_jit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index 2d952110be72..01892dfa637e 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -1269,3 +1269,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 
 	return prog;
 }
+
+/* Indicate the JIT backend supports mixing bpf2bpf and tailcalls. */
+bool bpf_jit_supports_subprog_tailcalls(void)
+{
+	return true;
+}
-- 
2.31.1

Re: [PATCH 1/2] LoongArch: BPF: Treat function address as 64-bit value

[Tiezhu Yang]

Hi Hengqi,

On 02/12/2023 11:52 AM, Hengqi Chen wrote:
> Let's always use 4 instructions for function address in JIT.
> So that the instruction sequences don't change between the first
> pass and the extra pass for function calls.
>
> Fixes: 5dc615520c4d ("LoongArch: Add BPF JIT support")
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>  arch/loongarch/net/bpf_jit.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index c4b1947ebf76..2d952110be72 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -446,6 +446,27 @@ static int add_exception_handler(const struct bpf_insn *insn,
>  	return 0;
>  }
>
> +static inline void emit_addr_move(struct jit_ctx *ctx, enum loongarch_gpr rd, u64 addr)
> +{
> +	u64 imm_11_0, imm_31_12, imm_51_32, imm_63_52;
> +
> +	/* lu12iw rd, imm_31_12 */
> +	imm_31_12 = (addr >> 12) & 0xfffff;
> +	emit_insn(ctx, lu12iw, rd, imm_31_12);
> +
> +	/* ori rd, rd, imm_11_0 */
> +	imm_11_0 = addr & 0xfff;
> +	emit_insn(ctx, ori, rd, rd, imm_11_0);
> +
> +	/* lu32id rd, imm_51_32 */
> +	imm_51_32 = (addr >> 32) & 0xfffff;
> +	emit_insn(ctx, lu32id, rd, imm_51_32);
> +
> +	/* lu52id rd, rd, imm_63_52 */
> +	imm_63_52 = (addr >> 52) & 0xfff;
> +	emit_insn(ctx, lu52id, rd, rd, imm_63_52);
> +}
> +
>  static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool extra_pass)
>  {
>  	u8 tm = -1;
> @@ -841,7 +862,7 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext
>  		if (ret < 0)
>  			return ret;
>
> -		move_imm(ctx, t1, func_addr, is32);
> +		emit_addr_move(ctx, t1, func_addr);
>  		emit_insn(ctx, jirl, t1, LOONGARCH_GPR_RA, 0);
>  		move_reg(ctx, regmap[BPF_REG_0], LOONGARCH_GPR_A0);
>  		break;
>

The code itself looks good to me.

Could you please give more detailed info in the commit message?
For example, description of problem, steps to reproduce, ...
I think the descriptions in the cover letter are useful, it is
better to record them in the commit message.

Additionally, emit_addr_move() is similar with move_imm(), it is
better to define emit_addr_move() before move_imm() in bpf_jit.h.

Thanks,
Tiezhu

Re: [PATCH 1/2] LoongArch: BPF: Treat function address as 64-bit value

[Tiezhu Yang]

On 02/13/2023 11:01 AM, Tiezhu Yang wrote:
> Hi Hengqi,
>
> On 02/12/2023 11:52 AM, Hengqi Chen wrote:
>> Let's always use 4 instructions for function address in JIT.
>> So that the instruction sequences don't change between the first
>> pass and the extra pass for function calls.
>>
>> Fixes: 5dc615520c4d ("LoongArch: Add BPF JIT support")
>> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
>> ---
>>  arch/loongarch/net/bpf_jit.c | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
>> index c4b1947ebf76..2d952110be72 100644
>> --- a/arch/loongarch/net/bpf_jit.c
>> +++ b/arch/loongarch/net/bpf_jit.c
>> @@ -446,6 +446,27 @@ static int add_exception_handler(const struct
>> bpf_insn *insn,
>>      return 0;
>>  }
>>
>> +static inline void emit_addr_move(struct jit_ctx *ctx, enum
>> loongarch_gpr rd, u64 addr)

Small nit:

Maybe use move_addr() ( like move_imm() ) is better than
emit_addr_move()?

Thanks,
Tiezhu

Re: [PATCH 1/2] LoongArch: BPF: Treat function address as 64-bit value

[Hengqi Chen]

Hi Tiezhu,

On Mon, Feb 13, 2023 at 11:02 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> Hi Hengqi,
>
> On 02/12/2023 11:52 AM, Hengqi Chen wrote:
> > Let's always use 4 instructions for function address in JIT.
> > So that the instruction sequences don't change between the first
> > pass and the extra pass for function calls.
> >
> > Fixes: 5dc615520c4d ("LoongArch: Add BPF JIT support")
> > Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> > ---
> >  arch/loongarch/net/bpf_jit.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> > index c4b1947ebf76..2d952110be72 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -446,6 +446,27 @@ static int add_exception_handler(const struct bpf_insn *insn,
> >       return 0;
> >  }
> >
> > +static inline void emit_addr_move(struct jit_ctx *ctx, enum loongarch_gpr rd, u64 addr)
> > +{
> > +     u64 imm_11_0, imm_31_12, imm_51_32, imm_63_52;
> > +
> > +     /* lu12iw rd, imm_31_12 */
> > +     imm_31_12 = (addr >> 12) & 0xfffff;
> > +     emit_insn(ctx, lu12iw, rd, imm_31_12);
> > +
> > +     /* ori rd, rd, imm_11_0 */
> > +     imm_11_0 = addr & 0xfff;
> > +     emit_insn(ctx, ori, rd, rd, imm_11_0);
> > +
> > +     /* lu32id rd, imm_51_32 */
> > +     imm_51_32 = (addr >> 32) & 0xfffff;
> > +     emit_insn(ctx, lu32id, rd, imm_51_32);
> > +
> > +     /* lu52id rd, rd, imm_63_52 */
> > +     imm_63_52 = (addr >> 52) & 0xfff;
> > +     emit_insn(ctx, lu52id, rd, rd, imm_63_52);
> > +}
> > +
> >  static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool extra_pass)
> >  {
> >       u8 tm = -1;
> > @@ -841,7 +862,7 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext
> >               if (ret < 0)
> >                       return ret;
> >
> > -             move_imm(ctx, t1, func_addr, is32);
> > +             emit_addr_move(ctx, t1, func_addr);
> >               emit_insn(ctx, jirl, t1, LOONGARCH_GPR_RA, 0);
> >               move_reg(ctx, regmap[BPF_REG_0], LOONGARCH_GPR_A0);
> >               break;
> >
>
> The code itself looks good to me.
>
> Could you please give more detailed info in the commit message?
> For example, description of problem, steps to reproduce, ...
> I think the descriptions in the cover letter are useful, it is
> better to record them in the commit message.
>
> Additionally, emit_addr_move() is similar with move_imm(), it is
> better to define emit_addr_move() before move_imm() in bpf_jit.h.
>
> Thanks,
> Tiezhu
>

I've addressed your comments and send a v2 for review.
The second patch is dropped, as it is incomplete.

Cheers,
---
Hengqi