Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1268296 > unrolled thread

[PATCH V2 0/2] arm64: bpf: correct JIT stack setup and make it align with ARM64 AAPCS

Started byYang Shi <yang.shi@linaro.org>
First post2015-11-12 23:20 +0100
Last post2015-11-13 04:30 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH V2 0/2] arm64: bpf: correct JIT stack setup and make it align with ARM64 AAPCS Yang Shi <yang.shi@linaro.org> - 2015-11-12 23:20 +0100
    [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS Yang Shi <yang.shi@linaro.org> - 2015-11-12 23:20 +0100
      Re: [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with  ARM64 AAPCS Z Lim <zlim.lnx@gmail.com> - 2015-11-13 04:30 +0100
        Re: [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align  with ARM64 AAPCS "Shi, Yang" <yang.shi@linaro.org> - 2015-11-13 18:30 +0100
    [PATCH 1/2] arm64: bpf: fix JIT frame pointer setup Yang Shi <yang.shi@linaro.org> - 2015-11-12 23:20 +0100
      Re: [PATCH 1/2] arm64: bpf: fix JIT frame pointer setup Z Lim <zlim.lnx@gmail.com> - 2015-11-13 04:30 +0100

#1268296 — [PATCH V2 0/2] arm64: bpf: correct JIT stack setup and make it align with ARM64 AAPCS

FromYang Shi <yang.shi@linaro.org>
Date2015-11-12 23:20 +0100
Subject[PATCH V2 0/2] arm64: bpf: correct JIT stack setup and make it align with ARM64 AAPCS
Message-ID<qu8nL-6oo-1@gated-at.bofh.it>
Changelog in V2:
Split to two patches according to the suggestion from Zi Shen Lim
Show A64_FP in stack layout diagram
Correct "+64" to "-64"

Yang Shi (2):
      arm64: bpf: fix JIT frame pointer setup
      arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS

 arch/arm64/net/bpf_jit_comp.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1268301 — [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS

FromYang Shi <yang.shi@linaro.org>
Date2015-11-12 23:20 +0100
Subject[PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS
Message-ID<qu8nM-6oo-29@gated-at.bofh.it>
In reply to#1268296
Save and restore FP/LR in BPF prog prologue and epilogue, save SP to FP
in prologue in order to get the correct stack backtrace.

However, ARM64 JIT used FP (x29) as eBPF fp register, FP is subjected to
change during function call so it may cause the BPF prog stack base address
change too.

Use x25 to replace FP as BPF stack base register (fp). Since x25 is callee
saved register, so it will keep intact during function call.
It is initialized in BPF prog prologue when BPF prog is started to run
everytime. When BPF prog exits, it could be just tossed.

So, the BPF stack layout looks like:

                                 high
         original A64_SP =>   0:+-----+ BPF prologue
                                |     | FP/LR and callee saved registers
         BPF fp register => -64:+-----+
                                |     |
                                | ... | BPF prog stack
                                |     |
                                |     |
         current A64_SP/FP =>   +-----+
                                |     |
                                | ... | Function call stack
                                |     |
                                +-----+
                                  low

CC: Zi Shen Lim <zlim.lnx@gmail.com>
CC: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Yang Shi <yang.shi@linaro.org>
---
 arch/arm64/net/bpf_jit_comp.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index ac8b548..8753bb7 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -50,7 +50,7 @@ static const int bpf2a64[] = {
 	[BPF_REG_8] = A64_R(21),
 	[BPF_REG_9] = A64_R(22),
 	/* read-only frame pointer to access stack */
-	[BPF_REG_FP] = A64_FP,
+	[BPF_REG_FP] = A64_R(25),
 	/* temporary register for internal BPF JIT */
 	[TMP_REG_1] = A64_R(23),
 	[TMP_REG_2] = A64_R(24),
@@ -155,17 +155,41 @@ static void build_prologue(struct jit_ctx *ctx)
 	stack_size += 4; /* extra for skb_copy_bits buffer */
 	stack_size = STACK_ALIGN(stack_size);
 
+	/*
+	 * BPF prog stack layout
+	 *
+	 *                         high
+	 * original A64_SP =>   0:+-----+ BPF prologue
+	 *                        |     | FP/LR and callee saved registers
+	 * BPF fp register => -64:+-----+
+	 *                        |     |
+	 *                        | ... | BPF prog stack
+	 *                        |     |
+	 *                        |     |
+	 * current A64_SP/FP =>   +-----+
+	 *                        |     |
+	 *                        | ... | Function call stack
+	 *                        |     |
+	 *                        +-----+
+	 *                          low
+	 *
+	 */
+
+	/* Save FP and LR registers to stay align with ARM64 AAPCS */
+	emit(A64_PUSH(A64_FP, A64_LR, A64_SP), ctx);
+
 	/* Save callee-saved register */
 	emit(A64_PUSH(r6, r7, A64_SP), ctx);
 	emit(A64_PUSH(r8, r9, A64_SP), ctx);
 	if (ctx->tmp_used)
 		emit(A64_PUSH(tmp1, tmp2, A64_SP), ctx);
 
-	/* Set up frame pointer */
+	/* Set up BPF prog stack base register (x25) */
 	emit(A64_MOV(1, fp, A64_SP), ctx);
 
-	/* Set up BPF stack */
+	/* Set up function call stack */
 	emit(A64_SUB_I(1, A64_SP, A64_SP, stack_size), ctx);
+	emit(A64_MOV(1, A64_FP, A64_SP), ctx);
 
 	/* Clear registers A and X */
 	emit_a64_mov_i64(ra, 0, ctx);
@@ -196,8 +220,8 @@ static void build_epilogue(struct jit_ctx *ctx)
 	emit(A64_POP(r8, r9, A64_SP), ctx);
 	emit(A64_POP(r6, r7, A64_SP), ctx);
 
-	/* Restore frame pointer */
-	emit(A64_MOV(1, fp, A64_SP), ctx);
+	/* Restore FP/LR registers */
+	emit(A64_POP(A64_FP, A64_LR, A64_SP), ctx);
 
 	/* Set return value */
 	emit(A64_MOV(1, A64_R(0), r0), ctx);
-- 
2.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1268562 — Re: [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS

FromZ Lim <zlim.lnx@gmail.com>
Date2015-11-13 04:30 +0100
SubjectRe: [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS
Message-ID<quddL-11M-3@gated-at.bofh.it>
In reply to#1268301
On Thu, Nov 12, 2015 at 1:57 PM, Yang Shi <yang.shi@linaro.org> wrote:
>
> Save and restore FP/LR in BPF prog prologue and epilogue, save SP to FP
> in prologue in order to get the correct stack backtrace.
>
> However, ARM64 JIT used FP (x29) as eBPF fp register, FP is subjected to
> change during function call so it may cause the BPF prog stack base address
> change too.
>
> Use x25 to replace FP as BPF stack base register (fp). Since x25 is callee
> saved register, so it will keep intact during function call.
> It is initialized in BPF prog prologue when BPF prog is started to run
> everytime. When BPF prog exits, it could be just tossed.
>
> So, the BPF stack layout looks like:
>
>                                  high
>          original A64_SP =>   0:+-----+ BPF prologue
>                                 |     | FP/LR and callee saved registers
>          BPF fp register => -64:+-----+
>                                 |     |
>                                 | ... | BPF prog stack
>                                 |     |
>                                 |     |
>          current A64_SP/FP =>   +-----+
>                                 |     |
>                                 | ... | Function call stack
>                                 |     |
>                                 +-----+
>                                   low
>

Yang, for stack unwinding to work, shouldn't it be something like the following?

          | LR |
A64_FP => | FP |
          | .. |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1269113 — Re: [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS

From"Shi, Yang" <yang.shi@linaro.org>
Date2015-11-13 18:30 +0100
SubjectRe: [PATCH 2/2] arm64: bpf: make BPF prologue and epilogue align with ARM64 AAPCS
Message-ID<quqkG-UL-17@gated-at.bofh.it>
In reply to#1268562
On 11/12/2015 7:28 PM, Z Lim wrote:
> On Thu, Nov 12, 2015 at 1:57 PM, Yang Shi <yang.shi@linaro.org> wrote:
>>
>> Save and restore FP/LR in BPF prog prologue and epilogue, save SP to FP
>> in prologue in order to get the correct stack backtrace.
>>
>> However, ARM64 JIT used FP (x29) as eBPF fp register, FP is subjected to
>> change during function call so it may cause the BPF prog stack base address
>> change too.
>>
>> Use x25 to replace FP as BPF stack base register (fp). Since x25 is callee
>> saved register, so it will keep intact during function call.
>> It is initialized in BPF prog prologue when BPF prog is started to run
>> everytime. When BPF prog exits, it could be just tossed.
>>
>> So, the BPF stack layout looks like:
>>
>>                                   high
>>           original A64_SP =>   0:+-----+ BPF prologue
>>                                  |     | FP/LR and callee saved registers
>>           BPF fp register => -64:+-----+
>>                                  |     |
>>                                  | ... | BPF prog stack
>>                                  |     |
>>                                  |     |
>>           current A64_SP/FP =>   +-----+
>>                                  |     |
>>                                  | ... | Function call stack
>>                                  |     |
>>                                  +-----+
>>                                    low
>>
>
> Yang, for stack unwinding to work, shouldn't it be something like the following?

Yes, thanks for catching this. v3 will be post soon.

Yang

>
>            | LR |
> A64_FP => | FP |
>            | .. |
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1268304 — [PATCH 1/2] arm64: bpf: fix JIT frame pointer setup

FromYang Shi <yang.shi@linaro.org>
Date2015-11-12 23:20 +0100
Subject[PATCH 1/2] arm64: bpf: fix JIT frame pointer setup
Message-ID<qu8nM-6oo-37@gated-at.bofh.it>
In reply to#1268296
BPF fp should point to the top of the BPF prog stack. The original
implementation made it point to the bottom incorrectly.
Move A64_SP to fp before reserve BPF prog stack space.

CC: Zi Shen Lim <zlim.lnx@gmail.com>
CC: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Yang Shi <yang.shi@linaro.org>
---
 arch/arm64/net/bpf_jit_comp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index a44e529..ac8b548 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -161,12 +161,12 @@ static void build_prologue(struct jit_ctx *ctx)
 	if (ctx->tmp_used)
 		emit(A64_PUSH(tmp1, tmp2, A64_SP), ctx);
 
-	/* Set up BPF stack */
-	emit(A64_SUB_I(1, A64_SP, A64_SP, stack_size), ctx);
-
 	/* Set up frame pointer */
 	emit(A64_MOV(1, fp, A64_SP), ctx);
 
+	/* Set up BPF stack */
+	emit(A64_SUB_I(1, A64_SP, A64_SP, stack_size), ctx);
+
 	/* Clear registers A and X */
 	emit_a64_mov_i64(ra, 0, ctx);
 	emit_a64_mov_i64(rx, 0, ctx);
-- 
2.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1268565 — Re: [PATCH 1/2] arm64: bpf: fix JIT frame pointer setup

FromZ Lim <zlim.lnx@gmail.com>
Date2015-11-13 04:30 +0100
SubjectRe: [PATCH 1/2] arm64: bpf: fix JIT frame pointer setup
Message-ID<quddM-11M-13@gated-at.bofh.it>
In reply to#1268304
On Thu, Nov 12, 2015 at 1:57 PM, Yang Shi <yang.shi@linaro.org> wrote:
> BPF fp should point to the top of the BPF prog stack. The original
> implementation made it point to the bottom incorrectly.
> Move A64_SP to fp before reserve BPF prog stack space.
>
> CC: Zi Shen Lim <zlim.lnx@gmail.com>
> CC: Xi Wang <xi.wang@gmail.com>
> Signed-off-by: Yang Shi <yang.shi@linaro.org>
> ---

Reviewed-by: Zi Shen Lim <zlim.lnx@gmail.com>

Also,

Fixes: e54bcde3d69d ("arm64: eBPF JIT compiler")
Cc: <stable@vger.kernel.org> # 3.18+
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web