From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Johan Almbladh <johan.almbladh@anyfinetworks.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Nicholas Piggin <npiggin@gmail.com>
Cc: bpf@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 4/9] powerpc/bpf: Handle large branch ranges with BPF_EXIT
Date: Tue, 5 Oct 2021 07:46:38 +0200 [thread overview]
Message-ID: <a63a7b97-08cd-b93e-bc12-d17cd6e94229@csgroup.eu> (raw)
In-Reply-To: <1633371632.j9hqy0kjhu.naveen@linux.ibm.com>
Le 04/10/2021 à 20:24, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>>
>>
>> Le 01/10/2021 à 23:14, Naveen N. Rao a écrit :
>>> In some scenarios, it is possible that the program epilogue is outside
>>> the branch range for a BPF_EXIT instruction. Instead of rejecting such
>>> programs, emit an indirect branch. We track the size of the bpf program
>>> emitted after the initial run and do a second pass since BPF_EXIT can
>>> end up emitting different number of instructions depending on the
>>> program size.
>>>
>>> Suggested-by: Jordan Niethe <jniethe5@gmail.com>
>>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/net/bpf_jit.h | 3 +++
>>> arch/powerpc/net/bpf_jit_comp.c | 22 +++++++++++++++++++++-
>>> arch/powerpc/net/bpf_jit_comp32.c | 2 +-
>>> arch/powerpc/net/bpf_jit_comp64.c | 2 +-
>>> 4 files changed, 26 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
>>> index 89bd744c2bffd4..4023de1698b9f5 100644
>>> --- a/arch/powerpc/net/bpf_jit.h
>>> +++ b/arch/powerpc/net/bpf_jit.h
>>> @@ -126,6 +126,7 @@
>>> #define SEEN_FUNC 0x20000000 /* might call external helpers */
>>> #define SEEN_TAILCALL 0x40000000 /* uses tail calls */
>>> +#define SEEN_BIG_PROG 0x80000000 /* large prog, >32MB */
>>> #define SEEN_VREG_MASK 0x1ff80000 /* Volatile registers r3-r12 */
>>> #define SEEN_NVREG_MASK 0x0003ffff /* Non volatile registers
>>> r14-r31 */
>>> @@ -179,6 +180,8 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32
>>> *image, struct codegen_context *
>>> void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx);
>>> void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx);
>>> void bpf_jit_realloc_regs(struct codegen_context *ctx);
>>> +int bpf_jit_emit_exit_insn(u32 *image, struct codegen_context *ctx,
>>> + int tmp_reg, unsigned long exit_addr);
>>> #endif
>>> diff --git a/arch/powerpc/net/bpf_jit_comp.c
>>> b/arch/powerpc/net/bpf_jit_comp.c
>>> index fcbf7a917c566e..3204872fbf2738 100644
>>> --- a/arch/powerpc/net/bpf_jit_comp.c
>>> +++ b/arch/powerpc/net/bpf_jit_comp.c
>>> @@ -72,6 +72,21 @@ static int bpf_jit_fixup_subprog_calls(struct
>>> bpf_prog *fp, u32 *image,
>>> return 0;
>>> }
>>> +int bpf_jit_emit_exit_insn(u32 *image, struct codegen_context *ctx,
>>> + int tmp_reg, unsigned long exit_addr)
>>> +{
>>> + if (!(ctx->seen & SEEN_BIG_PROG) &&
>>> is_offset_in_branch_range(exit_addr)) {
>>> + PPC_JMP(exit_addr);
>>> + } else {
>>> + ctx->seen |= SEEN_BIG_PROG;
>>> + PPC_FUNC_ADDR(tmp_reg, (unsigned long)image + exit_addr);
>>> + EMIT(PPC_RAW_MTCTR(tmp_reg));
>>> + EMIT(PPC_RAW_BCTR());
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> struct powerpc64_jit_data {
>>> struct bpf_binary_header *header;
>>> u32 *addrs;
>>> @@ -155,12 +170,17 @@ struct bpf_prog *bpf_int_jit_compile(struct
>>> bpf_prog *fp)
>>> goto out_addrs;
>>> }
>>> + if (!is_offset_in_branch_range((long)cgctx.idx * 4))
>>> + cgctx.seen |= SEEN_BIG_PROG;
>>> +
>>> /*
>>> * If we have seen a tail call, we need a second pass.
>>> * This is because bpf_jit_emit_common_epilogue() is called
>>> * from bpf_jit_emit_tail_call() with a not yet stable ctx->seen.
>>> + * We also need a second pass if we ended up with too large
>>> + * a program so as to fix branches.
>>> */
>>> - if (cgctx.seen & SEEN_TAILCALL) {
>>> + if (cgctx.seen & (SEEN_TAILCALL | SEEN_BIG_PROG)) {
>>> cgctx.idx = 0;
>>> if (bpf_jit_build_body(fp, 0, &cgctx, addrs, false)) {
>>> fp = org_fp;
>>> diff --git a/arch/powerpc/net/bpf_jit_comp32.c
>>> b/arch/powerpc/net/bpf_jit_comp32.c
>>> index a74d52204f8da2..d2a67574a23066 100644
>>> --- a/arch/powerpc/net/bpf_jit_comp32.c
>>> +++ b/arch/powerpc/net/bpf_jit_comp32.c
>>> @@ -852,7 +852,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32
>>> *image, struct codegen_context *
>>> * we'll just fall through to the epilogue.
>>> */
>>> if (i != flen - 1)
>>> - PPC_JMP(exit_addr);
>>> + bpf_jit_emit_exit_insn(image, ctx, tmp_reg, exit_addr);
>>
>> On ppc32, if you use tmp_reg you must flag it. But I think you could
>> use r0 instead.
>
> Indeed. Can we drop tracking of the temp registers and using them while
> remapping registers? Are you seeing significant benefits with re-use of
> those temp registers?
>
I'm not sure to follow you.
On ppc32, all volatile registers are used for function arguments, so
temp registers are necessarily non-volatile so we track them as all
non-volatile registers we use.
I think saving on stack only the non-volatile registers we use provides
real benefit, otherwise you wouldn't have implemented it would you ?
Anyway here you should use _R0 instead of tmp_reg.
Christophe
next prev parent reply other threads:[~2021-10-05 5:46 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-01 21:14 [PATCH 0/9] powerpc/bpf: Various fixes Naveen N. Rao
2021-10-01 21:14 ` [PATCH 1/9] powerpc/lib: Add helper to check if offset is within conditional branch range Naveen N. Rao
2021-10-01 21:37 ` Song Liu
2021-10-04 18:02 ` Naveen N. Rao
2021-10-03 7:50 ` Christophe Leroy
2021-10-04 18:03 ` Naveen N. Rao
2021-10-01 21:14 ` [PATCH 2/9] powerpc/bpf: Validate branch ranges Naveen N. Rao
2021-10-01 21:45 ` Song Liu
2021-10-02 17:29 ` Johan Almbladh
2021-10-03 7:54 ` Christophe Leroy
2021-10-04 18:11 ` Naveen N. Rao
2021-10-01 21:14 ` [PATCH 3/9] powerpc/bpf: Remove unused SEEN_STACK Naveen N. Rao
2021-10-01 21:47 ` Song Liu
2021-10-02 17:30 ` Johan Almbladh
2021-10-03 7:55 ` Christophe Leroy
2021-10-04 18:11 ` Naveen N. Rao
2021-10-05 5:50 ` Christophe Leroy
2021-10-05 20:22 ` Naveen N. Rao
2021-10-01 21:14 ` [PATCH 4/9] powerpc/bpf: Handle large branch ranges with BPF_EXIT Naveen N. Rao
2021-10-01 21:53 ` Song Liu
2021-10-02 17:31 ` Johan Almbladh
2021-10-03 7:59 ` Christophe Leroy
2021-10-04 18:24 ` Naveen N. Rao
2021-10-05 5:46 ` Christophe Leroy [this message]
2022-01-07 11:46 ` Naveen N. Rao
2021-10-01 21:14 ` [PATCH 5/9] powerpc/bpf: Fix BPF_MOD when imm == 1 Naveen N. Rao
2021-10-01 21:55 ` Song Liu
2021-10-02 17:32 ` Johan Almbladh
2021-10-01 21:14 ` [PATCH 6/9] powerpc/bpf: Fix BPF_SUB when imm == 0x80000000 Naveen N. Rao
2021-10-01 22:01 ` Song Liu
2021-10-02 17:33 ` Johan Almbladh
2021-10-03 8:07 ` Christophe Leroy
2021-10-04 18:18 ` Naveen N. Rao
2021-10-05 5:40 ` Christophe Leroy
2021-10-01 21:14 ` [PATCH 7/9] powerpc/bpf: Limit 'ldbrx' to processors compliant with ISA v2.06 Naveen N. Rao
2021-10-02 17:35 ` Johan Almbladh
2021-10-01 21:14 ` [PATCH 8/9] powerpc/security: Add a helper to query stf_barrier type Naveen N. Rao
2021-10-01 21:14 ` [PATCH 9/9] powerpc/bpf: Emit stf barrier instruction sequences for BPF_NOSPEC Naveen N. Rao
2021-10-02 17:41 ` [PATCH 0/9] powerpc/bpf: Various fixes Johan Almbladh
2021-10-04 18:19 ` Naveen N. Rao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a63a7b97-08cd-b93e-bc12-d17cd6e94229@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=alexei.starovoitov@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=johan.almbladh@anyfinetworks.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=naveen.n.rao@linux.vnet.ibm.com \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).