* [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info
@ 2022-02-08 1:25 Hou Tao
2022-02-08 1:25 ` [PATCH bpf-next v3 1/2] bpf, arm64: call build_prologue() first in first JIT pass Hou Tao
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Hou Tao @ 2022-02-08 1:25 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau
Cc: Ard Biesheuvel, Yonghong Song, Andrii Nakryiko, Zi Shen Lim,
Will Deacon, Catalin Marinas, bpf, linux-arm-kernel, houtao1
Hi,
The patchset addresses two issues in bpf line info for arm64:
(1) insn_to_jit_off only considers the body itself and ignores
prologue before the body. Fixed in patch #1.
(2) insn_to_jit_off passed to bpf_prog_fill_jited_linfo() is
calculated in instruction granularity instead of bytes
granularity. Fixed in patch #2.
Comments are always welcome.
Regards,
Tao
Change Log:
v3:
* patch #2: explain why bpf2a64_offset() needs update
* add Fixes tags in both patches
v2: https://lore.kernel.org/bpf/20220125105707.292449-1-houtao1@huawei.com
* split into two independent patches (from Daniel)
* use AARCH64_INSN_SIZE instead of defining INSN_SIZE
v1: https://lore.kernel.org/bpf/20220104014236.1512639-1-houtao1@huawei.com
Hou Tao (2):
bpf, arm64: call build_prologue() first in first JIT pass
bpf, arm64: calculate offset as byte-offset for bpf line info
arch/arm64/net/bpf_jit_comp.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
--
2.27.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v3 1/2] bpf, arm64: call build_prologue() first in first JIT pass
2022-02-08 1:25 [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info Hou Tao
@ 2022-02-08 1:25 ` Hou Tao
2022-02-08 1:25 ` [PATCH bpf-next v3 2/2] bpf, arm64: calculate offset as byte-offset for bpf line info Hou Tao
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Hou Tao @ 2022-02-08 1:25 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau
Cc: Ard Biesheuvel, Yonghong Song, Andrii Nakryiko, Zi Shen Lim,
Will Deacon, Catalin Marinas, bpf, linux-arm-kernel, houtao1
BPF line info needs ctx->offset to be the instruction offset
in the whole jited image instead of the body itself, so also
call build_prologue() first in first JIT pass.
Fixes: 37ab566c178d ("bpf: arm64: Enable arm64 jit to provide bpf_line_info")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
arch/arm64/net/bpf_jit_comp.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 2375ed3e4c8a..68b35c83e637 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1190,15 +1190,18 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
goto out_off;
}
- /* 1. Initial fake pass to compute ctx->idx. */
-
- /* Fake pass to fill in ctx->offset. */
- if (build_body(&ctx, extra_pass)) {
+ /*
+ * 1. Initial fake pass to compute ctx->idx and ctx->offset.
+ *
+ * BPF line info needs ctx->offset[i] to be the offset of
+ * instruction[i] in jited image, so build prologue first.
+ */
+ if (build_prologue(&ctx, was_classic)) {
prog = orig_prog;
goto out_off;
}
- if (build_prologue(&ctx, was_classic)) {
+ if (build_body(&ctx, extra_pass)) {
prog = orig_prog;
goto out_off;
}
--
2.27.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v3 2/2] bpf, arm64: calculate offset as byte-offset for bpf line info
2022-02-08 1:25 [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info Hou Tao
2022-02-08 1:25 ` [PATCH bpf-next v3 1/2] bpf, arm64: call build_prologue() first in first JIT pass Hou Tao
@ 2022-02-08 1:25 ` Hou Tao
2022-02-18 23:20 ` Daniel Borkmann
2022-02-16 11:10 ` [PATCH bpf-next v3 0/2] bpf, arm64: fix " Hou Tao
2022-02-16 18:29 ` Martin KaFai Lau
3 siblings, 1 reply; 7+ messages in thread
From: Hou Tao @ 2022-02-08 1:25 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau
Cc: Ard Biesheuvel, Yonghong Song, Andrii Nakryiko, Zi Shen Lim,
Will Deacon, Catalin Marinas, bpf, linux-arm-kernel, houtao1
insn_to_jit_off passed to bpf_prog_fill_jited_linfo() is calculated
in instruction granularity instead of bytes granularity, but bpf
line info requires byte offset, so fixing it by calculating ctx->offset
as byte-offset. bpf2a64_offset() needs to return relative instruction
offset by using ctx->offfset, so update it accordingly.
Fixes: 37ab566c178d ("bpf: arm64: Enable arm64 jit to provide bpf_line_info")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
arch/arm64/net/bpf_jit_comp.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 68b35c83e637..aed07cba78ec 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -164,9 +164,14 @@ static inline int bpf2a64_offset(int bpf_insn, int off,
/*
* Whereas arm64 branch instructions encode the offset
* from the branch itself, so we must subtract 1 from the
- * instruction offset.
+ * instruction offset. The unit of ctx->offset is byte, so
+ * subtract AARCH64_INSN_SIZE from it. bpf2a64_offset()
+ * returns instruction offset, so divide by AARCH64_INSN_SIZE
+ * at the end.
*/
- return ctx->offset[bpf_insn + off] - (ctx->offset[bpf_insn] - 1);
+ return (ctx->offset[bpf_insn + off] -
+ (ctx->offset[bpf_insn] - AARCH64_INSN_SIZE)) /
+ AARCH64_INSN_SIZE;
}
static void jit_fill_hole(void *area, unsigned int size)
@@ -1087,13 +1092,14 @@ static int build_body(struct jit_ctx *ctx, bool extra_pass)
const struct bpf_insn *insn = &prog->insnsi[i];
int ret;
+ /* BPF line info needs byte-offset instead of insn-offset */
if (ctx->image == NULL)
- ctx->offset[i] = ctx->idx;
+ ctx->offset[i] = ctx->idx * AARCH64_INSN_SIZE;
ret = build_insn(insn, ctx, extra_pass);
if (ret > 0) {
i++;
if (ctx->image == NULL)
- ctx->offset[i] = ctx->idx;
+ ctx->offset[i] = ctx->idx * AARCH64_INSN_SIZE;
continue;
}
if (ret)
@@ -1105,7 +1111,7 @@ static int build_body(struct jit_ctx *ctx, bool extra_pass)
* instruction (end of program)
*/
if (ctx->image == NULL)
- ctx->offset[i] = ctx->idx;
+ ctx->offset[i] = ctx->idx * AARCH64_INSN_SIZE;
return 0;
}
--
2.27.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info
2022-02-08 1:25 [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info Hou Tao
2022-02-08 1:25 ` [PATCH bpf-next v3 1/2] bpf, arm64: call build_prologue() first in first JIT pass Hou Tao
2022-02-08 1:25 ` [PATCH bpf-next v3 2/2] bpf, arm64: calculate offset as byte-offset for bpf line info Hou Tao
@ 2022-02-16 11:10 ` Hou Tao
2022-02-16 18:29 ` Martin KaFai Lau
3 siblings, 0 replies; 7+ messages in thread
From: Hou Tao @ 2022-02-16 11:10 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Will Deacon
Cc: Ard Biesheuvel, Yonghong Song, Andrii Nakryiko, Zi Shen Lim,
Catalin Marinas, bpf, linux-arm-kernel
ping ?
On 2/8/2022 9:25 AM, Hou Tao wrote:
> Hi,
>
> The patchset addresses two issues in bpf line info for arm64:
>
> (1) insn_to_jit_off only considers the body itself and ignores
> prologue before the body. Fixed in patch #1.
>
> (2) insn_to_jit_off passed to bpf_prog_fill_jited_linfo() is
> calculated in instruction granularity instead of bytes
> granularity. Fixed in patch #2.
>
> Comments are always welcome.
>
> Regards,
> Tao
>
> Change Log:
> v3:
> * patch #2: explain why bpf2a64_offset() needs update
> * add Fixes tags in both patches
>
> v2: https://lore.kernel.org/bpf/20220125105707.292449-1-houtao1@huawei.com
> * split into two independent patches (from Daniel)
> * use AARCH64_INSN_SIZE instead of defining INSN_SIZE
>
> v1: https://lore.kernel.org/bpf/20220104014236.1512639-1-houtao1@huawei.com
>
> Hou Tao (2):
> bpf, arm64: call build_prologue() first in first JIT pass
> bpf, arm64: calculate offset as byte-offset for bpf line info
>
> arch/arm64/net/bpf_jit_comp.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info
2022-02-08 1:25 [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info Hou Tao
` (2 preceding siblings ...)
2022-02-16 11:10 ` [PATCH bpf-next v3 0/2] bpf, arm64: fix " Hou Tao
@ 2022-02-16 18:29 ` Martin KaFai Lau
3 siblings, 0 replies; 7+ messages in thread
From: Martin KaFai Lau @ 2022-02-16 18:29 UTC (permalink / raw)
To: Hou Tao
Cc: Alexei Starovoitov, Daniel Borkmann, Ard Biesheuvel,
Yonghong Song, Andrii Nakryiko, Zi Shen Lim, Will Deacon,
Catalin Marinas, bpf, linux-arm-kernel
On Tue, Feb 08, 2022 at 09:25:37AM +0800, Hou Tao wrote:
> Hi,
>
> The patchset addresses two issues in bpf line info for arm64:
>
> (1) insn_to_jit_off only considers the body itself and ignores
> prologue before the body. Fixed in patch #1.
>
> (2) insn_to_jit_off passed to bpf_prog_fill_jited_linfo() is
> calculated in instruction granularity instead of bytes
> granularity. Fixed in patch #2.
>
> Comments are always welcome.
>
> Regards,
> Tao
>
> Change Log:
> v3:
> * patch #2: explain why bpf2a64_offset() needs update
> * add Fixes tags in both patches
It makes sense to me on the bpf linfo expectation. It will be
useful to also get some eyes from the arm experts.
Acked-by: Martin KaFai Lau <kafai@fb.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v3 2/2] bpf, arm64: calculate offset as byte-offset for bpf line info
2022-02-08 1:25 ` [PATCH bpf-next v3 2/2] bpf, arm64: calculate offset as byte-offset for bpf line info Hou Tao
@ 2022-02-18 23:20 ` Daniel Borkmann
2022-02-22 3:58 ` Hou Tao
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2022-02-18 23:20 UTC (permalink / raw)
To: Hou Tao, Alexei Starovoitov, Martin KaFai Lau
Cc: Ard Biesheuvel, Yonghong Song, Andrii Nakryiko, Zi Shen Lim,
Will Deacon, Catalin Marinas, bpf, linux-arm-kernel
On 2/8/22 2:25 AM, Hou Tao wrote:
> insn_to_jit_off passed to bpf_prog_fill_jited_linfo() is calculated
> in instruction granularity instead of bytes granularity, but bpf
> line info requires byte offset, so fixing it by calculating ctx->offset
> as byte-offset. bpf2a64_offset() needs to return relative instruction
> offset by using ctx->offfset, so update it accordingly.
>
> Fixes: 37ab566c178d ("bpf: arm64: Enable arm64 jit to provide bpf_line_info")
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> arch/arm64/net/bpf_jit_comp.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 68b35c83e637..aed07cba78ec 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -164,9 +164,14 @@ static inline int bpf2a64_offset(int bpf_insn, int off,
> /*
> * Whereas arm64 branch instructions encode the offset
> * from the branch itself, so we must subtract 1 from the
> - * instruction offset.
> + * instruction offset. The unit of ctx->offset is byte, so
> + * subtract AARCH64_INSN_SIZE from it. bpf2a64_offset()
> + * returns instruction offset, so divide by AARCH64_INSN_SIZE
> + * at the end.
> */
> - return ctx->offset[bpf_insn + off] - (ctx->offset[bpf_insn] - 1);
> + return (ctx->offset[bpf_insn + off] -
> + (ctx->offset[bpf_insn] - AARCH64_INSN_SIZE)) /
> + AARCH64_INSN_SIZE;
> }
>
> static void jit_fill_hole(void *area, unsigned int size)
> @@ -1087,13 +1092,14 @@ static int build_body(struct jit_ctx *ctx, bool extra_pass)
> const struct bpf_insn *insn = &prog->insnsi[i];
> int ret;
>
> + /* BPF line info needs byte-offset instead of insn-offset */
> if (ctx->image == NULL)
> - ctx->offset[i] = ctx->idx;
> + ctx->offset[i] = ctx->idx * AARCH64_INSN_SIZE;
> ret = build_insn(insn, ctx, extra_pass);
> if (ret > 0) {
> i++;
> if (ctx->image == NULL)
> - ctx->offset[i] = ctx->idx;
> + ctx->offset[i] = ctx->idx * AARCH64_INSN_SIZE;
> continue;
> }
> if (ret)
> @@ -1105,7 +1111,7 @@ static int build_body(struct jit_ctx *ctx, bool extra_pass)
> * instruction (end of program)
> */
> if (ctx->image == NULL)
> - ctx->offset[i] = ctx->idx;
> + ctx->offset[i] = ctx->idx * AARCH64_INSN_SIZE;
Both patches look good to me. For this one specifically, given bpf2a64_offset()
needs to return relative instruction offset via ctx->offfset, can't we just
simplify it like this w/o the AARCH64_INSN_SIZE back/forth dance?
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 74f9a9b6a053..72f4702a9d01 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -999,7 +999,7 @@ struct arm64_jit_data {
struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
{
- int image_size, prog_size, extable_size;
+ int image_size, prog_size, extable_size, i;
struct bpf_prog *tmp, *orig_prog = prog;
struct bpf_binary_header *header;
struct arm64_jit_data *jit_data;
@@ -1130,6 +1130,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
prog->jited_len = prog_size;
if (!prog->is_func || extra_pass) {
+ /* BPF line info needs byte-offset instead of insn-offset. */
+ for (i = 0; i < prog->len + 1; i++)
+ ctx.offset[i] *= AARCH64_INSN_SIZE;
bpf_prog_fill_jited_linfo(prog, ctx.offset + 1);
out_off:
kfree(ctx.offset);
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v3 2/2] bpf, arm64: calculate offset as byte-offset for bpf line info
2022-02-18 23:20 ` Daniel Borkmann
@ 2022-02-22 3:58 ` Hou Tao
0 siblings, 0 replies; 7+ messages in thread
From: Hou Tao @ 2022-02-22 3:58 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau
Cc: Ard Biesheuvel, Yonghong Song, Andrii Nakryiko, Zi Shen Lim,
Will Deacon, Catalin Marinas, bpf, linux-arm-kernel
Hi,
On 2/19/2022 7:20 AM, Daniel Borkmann wrote:
> On 2/8/22 2:25 AM, Hou Tao wrote:
>> insn_to_jit_off passed to bpf_prog_fill_jited_linfo() is calculated
>> in instruction granularity instead of bytes granularity, but bpf
>> line info requires byte offset, so fixing it by calculating ctx->offset
>> as byte-offset. bpf2a64_offset() needs to return relative instruction
>> offset by using ctx->offfset, so update it accordingly.
>>
>> Fixes: 37ab566c178d ("bpf: arm64: Enable arm64 jit to provide bpf_line_info")
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> arch/arm64/net/bpf_jit_comp.c | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
[snip]
>> if (ret)
>> @@ -1105,7 +1111,7 @@ static int build_body(struct jit_ctx *ctx, bool
>> extra_pass)
>> * instruction (end of program)
>> */
>> if (ctx->image == NULL)
>> - ctx->offset[i] = ctx->idx;
>> + ctx->offset[i] = ctx->idx * AARCH64_INSN_SIZE;
>
> Both patches look good to me. For this one specifically, given bpf2a64_offset()
> needs to return relative instruction offset via ctx->offfset, can't we just
> simplify it like this w/o the AARCH64_INSN_SIZE back/forth dance?
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 74f9a9b6a053..72f4702a9d01 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -999,7 +999,7 @@ struct arm64_jit_data {
>
> struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> {
> - int image_size, prog_size, extable_size;
> + int image_size, prog_size, extable_size, i;
> struct bpf_prog *tmp, *orig_prog = prog;
> struct bpf_binary_header *header;
> struct arm64_jit_data *jit_data;
> @@ -1130,6 +1130,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> prog->jited_len = prog_size;
>
> if (!prog->is_func || extra_pass) {
> + /* BPF line info needs byte-offset instead of insn-offset. */
> + for (i = 0; i < prog->len + 1; i++)
> + ctx.offset[i] *= AARCH64_INSN_SIZE;
> bpf_prog_fill_jited_linfo(prog, ctx.offset + 1);
> out_off:
> kfree(ctx.offset);
The fix is much simpler. I will check whether or not it works.
Thanks,
Tao
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-22 4:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 1:25 [PATCH bpf-next v3 0/2] bpf, arm64: fix bpf line info Hou Tao
2022-02-08 1:25 ` [PATCH bpf-next v3 1/2] bpf, arm64: call build_prologue() first in first JIT pass Hou Tao
2022-02-08 1:25 ` [PATCH bpf-next v3 2/2] bpf, arm64: calculate offset as byte-offset for bpf line info Hou Tao
2022-02-18 23:20 ` Daniel Borkmann
2022-02-22 3:58 ` Hou Tao
2022-02-16 11:10 ` [PATCH bpf-next v3 0/2] bpf, arm64: fix " Hou Tao
2022-02-16 18:29 ` Martin KaFai Lau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).