From: Christophe Leroy <christophe.leroy@csgroup.eu> To: Hari Bathini <hbathini@linux.ibm.com>, naveen.n.rao@linux.ibm.com, mpe@ellerman.id.au, ast@kernel.org, daniel@iogearbox.net Cc: paulus@samba.org, andrii@kernel.org, kafai@fb.com, songliubraving@fb.com, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v3 7/8] bpf ppc32: Add BPF_PROBE_MEM support for JIT Date: Tue, 21 Sep 2021 16:22:27 +0200 [thread overview] Message-ID: <a278a8a7-8389-bba3-4b40-90a7b9765b17@csgroup.eu> (raw) In-Reply-To: <20210921132943.489732-8-hbathini@linux.ibm.com> Le 21/09/2021 à 15:29, Hari Bathini a écrit : > BPF load instruction with BPF_PROBE_MEM mode can cause a fault > inside kernel. Append exception table for such instructions > within BPF program. > > Unlike other archs which uses extable 'fixup' field to pass dest_reg > and nip, BPF exception table on PowerPC follows the generic PowerPC > exception table design, where it populates both fixup and extable > sections within BPF program. fixup section contains 3 instructions, > first 2 instructions clear dest_reg (lower & higher 32-bit registers) > and last instruction jumps to next instruction in the BPF code. > extable 'insn' field contains relative offset of the instruction and > 'fixup' field contains relative offset of the fixup entry. Example > layout of BPF program with extable present: > > +------------------+ > | | > | | > 0x4020 -->| lwz r28,4(r4) | > | | > | | > 0x40ac -->| lwz r3,0(r24) | > | lwz r4,4(r24) | > | | > | | > |------------------| > 0x4278 -->| li r28,0 | \ > | li r27,0 | | fixup entry > | b 0x4024 | / > 0x4284 -->| li r4,0 | > | li r3,0 | > | b 0x40b4 | > |------------------| > 0x4290 -->| insn=0xfffffd90 | \ extable entry > | fixup=0xffffffe4 | / > 0x4298 -->| insn=0xfffffe14 | > | fixup=0xffffffe8 | > +------------------+ > > (Addresses shown here are chosen random, not real) > > Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> > --- > > Changes in v3: > * Changed how BPF_FIXUP_LEN is defined based on Chris' suggestion. > > > arch/powerpc/net/bpf_jit.h | 4 ++++ > arch/powerpc/net/bpf_jit_comp.c | 2 ++ > arch/powerpc/net/bpf_jit_comp32.c | 34 +++++++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+) > > diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h > index 561689a2abdf..800734056200 100644 > --- a/arch/powerpc/net/bpf_jit.h > +++ b/arch/powerpc/net/bpf_jit.h > @@ -144,7 +144,11 @@ struct codegen_context { > unsigned int exentry_idx; > }; > > +#ifdef CONFIG_PPC32 > +#define BPF_FIXUP_LEN 3 /* Three instructions => 12 bytes */ > +#else > #define BPF_FIXUP_LEN 2 /* Two instructions => 8 bytes */ > +#endif > > static inline void bpf_flush_icache(void *start, void *end) > { > diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c > index f02457c6b54f..1a0041997050 100644 > --- a/arch/powerpc/net/bpf_jit_comp.c > +++ b/arch/powerpc/net/bpf_jit_comp.c > @@ -297,6 +297,8 @@ int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct code > (ctx->exentry_idx * BPF_FIXUP_LEN * 4); > > fixup[0] = PPC_RAW_LI(dst_reg, 0); > + if (IS_ENABLED(CONFIG_PPC32)) > + fixup[1] = PPC_RAW_LI(dst_reg - 1, 0); /* clear higher 32-bit register too */ > > fixup[BPF_FIXUP_LEN - 1] = > PPC_RAW_BRANCH((long)(pc + jmp_off) - (long)&fixup[BPF_FIXUP_LEN - 1]); > diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c > index 820c7848434e..1239643f532c 100644 > --- a/arch/powerpc/net/bpf_jit_comp32.c > +++ b/arch/powerpc/net/bpf_jit_comp32.c > @@ -812,11 +812,19 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > */ > case BPF_LDX | BPF_MEM | BPF_B: /* dst = *(u8 *)(ul) (src + off) */ > fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_B: > + fallthrough; Same comment about the fallthroughs > case BPF_LDX | BPF_MEM | BPF_H: /* dst = *(u16 *)(ul) (src + off) */ > fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_H: > + fallthrough; > case BPF_LDX | BPF_MEM | BPF_W: /* dst = *(u32 *)(ul) (src + off) */ > fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_W: > + fallthrough; > case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */ > + fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_DW: > switch (size) { > case BPF_B: > EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off)); > @@ -841,6 +849,32 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > > if (size != BPF_DW && !fp->aux->verifier_zext) > EMIT(PPC_RAW_LI(dst_reg_h, 0)); > + > + if (BPF_MODE(code) == BPF_PROBE_MEM) { > + int insn_idx = ctx->idx - 1; > + int jmp_off = 4; > + > + /* > + * In case of BPF_DW, two lwz instructions are emitted, one > + * for higher 32-bit and another for lower 32-bit. So, set > + * ex->insn to the first of the two and jump over both > + * instructions in fixup. > + * > + * Similarly, with !verifier_zext, two instructions are > + * emitted for BPF_B/H/W case. So, set ex->insn to the > + * instruction that could fault and skip over both > + * instructions. > + */ > + if (size == BPF_DW || !fp->aux->verifier_zext) { > + insn_idx -= 1; > + jmp_off += 4; > + } > + > + ret = bpf_add_extable_entry(fp, image, pass, ctx, insn_idx, > + jmp_off, dst_reg); > + if (ret) > + return ret; > + } > break; > > /* >
WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu> To: Hari Bathini <hbathini@linux.ibm.com>, naveen.n.rao@linux.ibm.com, mpe@ellerman.id.au, ast@kernel.org, daniel@iogearbox.net Cc: songliubraving@fb.com, netdev@vger.kernel.org, john.fastabend@gmail.com, andrii@kernel.org, kpsingh@kernel.org, paulus@samba.org, yhs@fb.com, bpf@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kafai@fb.com Subject: Re: [PATCH v3 7/8] bpf ppc32: Add BPF_PROBE_MEM support for JIT Date: Tue, 21 Sep 2021 16:22:27 +0200 [thread overview] Message-ID: <a278a8a7-8389-bba3-4b40-90a7b9765b17@csgroup.eu> (raw) In-Reply-To: <20210921132943.489732-8-hbathini@linux.ibm.com> Le 21/09/2021 à 15:29, Hari Bathini a écrit : > BPF load instruction with BPF_PROBE_MEM mode can cause a fault > inside kernel. Append exception table for such instructions > within BPF program. > > Unlike other archs which uses extable 'fixup' field to pass dest_reg > and nip, BPF exception table on PowerPC follows the generic PowerPC > exception table design, where it populates both fixup and extable > sections within BPF program. fixup section contains 3 instructions, > first 2 instructions clear dest_reg (lower & higher 32-bit registers) > and last instruction jumps to next instruction in the BPF code. > extable 'insn' field contains relative offset of the instruction and > 'fixup' field contains relative offset of the fixup entry. Example > layout of BPF program with extable present: > > +------------------+ > | | > | | > 0x4020 -->| lwz r28,4(r4) | > | | > | | > 0x40ac -->| lwz r3,0(r24) | > | lwz r4,4(r24) | > | | > | | > |------------------| > 0x4278 -->| li r28,0 | \ > | li r27,0 | | fixup entry > | b 0x4024 | / > 0x4284 -->| li r4,0 | > | li r3,0 | > | b 0x40b4 | > |------------------| > 0x4290 -->| insn=0xfffffd90 | \ extable entry > | fixup=0xffffffe4 | / > 0x4298 -->| insn=0xfffffe14 | > | fixup=0xffffffe8 | > +------------------+ > > (Addresses shown here are chosen random, not real) > > Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> > --- > > Changes in v3: > * Changed how BPF_FIXUP_LEN is defined based on Chris' suggestion. > > > arch/powerpc/net/bpf_jit.h | 4 ++++ > arch/powerpc/net/bpf_jit_comp.c | 2 ++ > arch/powerpc/net/bpf_jit_comp32.c | 34 +++++++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+) > > diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h > index 561689a2abdf..800734056200 100644 > --- a/arch/powerpc/net/bpf_jit.h > +++ b/arch/powerpc/net/bpf_jit.h > @@ -144,7 +144,11 @@ struct codegen_context { > unsigned int exentry_idx; > }; > > +#ifdef CONFIG_PPC32 > +#define BPF_FIXUP_LEN 3 /* Three instructions => 12 bytes */ > +#else > #define BPF_FIXUP_LEN 2 /* Two instructions => 8 bytes */ > +#endif > > static inline void bpf_flush_icache(void *start, void *end) > { > diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c > index f02457c6b54f..1a0041997050 100644 > --- a/arch/powerpc/net/bpf_jit_comp.c > +++ b/arch/powerpc/net/bpf_jit_comp.c > @@ -297,6 +297,8 @@ int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct code > (ctx->exentry_idx * BPF_FIXUP_LEN * 4); > > fixup[0] = PPC_RAW_LI(dst_reg, 0); > + if (IS_ENABLED(CONFIG_PPC32)) > + fixup[1] = PPC_RAW_LI(dst_reg - 1, 0); /* clear higher 32-bit register too */ > > fixup[BPF_FIXUP_LEN - 1] = > PPC_RAW_BRANCH((long)(pc + jmp_off) - (long)&fixup[BPF_FIXUP_LEN - 1]); > diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c > index 820c7848434e..1239643f532c 100644 > --- a/arch/powerpc/net/bpf_jit_comp32.c > +++ b/arch/powerpc/net/bpf_jit_comp32.c > @@ -812,11 +812,19 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > */ > case BPF_LDX | BPF_MEM | BPF_B: /* dst = *(u8 *)(ul) (src + off) */ > fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_B: > + fallthrough; Same comment about the fallthroughs > case BPF_LDX | BPF_MEM | BPF_H: /* dst = *(u16 *)(ul) (src + off) */ > fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_H: > + fallthrough; > case BPF_LDX | BPF_MEM | BPF_W: /* dst = *(u32 *)(ul) (src + off) */ > fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_W: > + fallthrough; > case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */ > + fallthrough; > + case BPF_LDX | BPF_PROBE_MEM | BPF_DW: > switch (size) { > case BPF_B: > EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off)); > @@ -841,6 +849,32 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > > if (size != BPF_DW && !fp->aux->verifier_zext) > EMIT(PPC_RAW_LI(dst_reg_h, 0)); > + > + if (BPF_MODE(code) == BPF_PROBE_MEM) { > + int insn_idx = ctx->idx - 1; > + int jmp_off = 4; > + > + /* > + * In case of BPF_DW, two lwz instructions are emitted, one > + * for higher 32-bit and another for lower 32-bit. So, set > + * ex->insn to the first of the two and jump over both > + * instructions in fixup. > + * > + * Similarly, with !verifier_zext, two instructions are > + * emitted for BPF_B/H/W case. So, set ex->insn to the > + * instruction that could fault and skip over both > + * instructions. > + */ > + if (size == BPF_DW || !fp->aux->verifier_zext) { > + insn_idx -= 1; > + jmp_off += 4; > + } > + > + ret = bpf_add_extable_entry(fp, image, pass, ctx, insn_idx, > + jmp_off, dst_reg); > + if (ret) > + return ret; > + } > break; > > /* >
next prev parent reply other threads:[~2021-09-21 14:22 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-21 13:29 [PATCH v3 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 13:29 ` [PATCH v3 1/8] bpf powerpc: Remove unused SEEN_STACK Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 13:29 ` [PATCH v3 2/8] bpf powerpc: Remove extra_pass from bpf_jit_build_body() Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 13:29 ` [PATCH v3 3/8] bpf powerpc: refactor JIT compiler code Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 14:02 ` Christophe Leroy 2021-09-21 14:02 ` Christophe Leroy 2021-09-21 14:16 ` Christophe Leroy 2021-09-21 14:16 ` Christophe Leroy 2021-09-21 13:29 ` [PATCH v3 4/8] powerpc/ppc-opcode: introduce PPC_RAW_BRANCH() macro Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 13:29 ` [PATCH v3 5/8] bpf ppc64: Add BPF_PROBE_MEM support for JIT Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 14:18 ` Christophe Leroy 2021-09-21 14:18 ` Christophe Leroy 2021-09-21 13:29 ` [PATCH v3 6/8] bpf ppc64: Access only if addr is kernel address Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 14:21 ` Christophe Leroy 2021-09-21 14:21 ` Christophe Leroy 2021-09-21 13:29 ` [PATCH v3 7/8] bpf ppc32: Add BPF_PROBE_MEM support for JIT Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 14:22 ` Christophe Leroy [this message] 2021-09-21 14:22 ` Christophe Leroy 2021-09-21 13:29 ` [PATCH v3 8/8] bpf ppc32: Access only if addr is kernel address Hari Bathini 2021-09-21 13:29 ` Hari Bathini 2021-09-21 14:27 ` Christophe Leroy 2021-09-21 14:27 ` Christophe Leroy
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=a278a8a7-8389-bba3-4b40-90a7b9765b17@csgroup.eu \ --to=christophe.leroy@csgroup.eu \ --cc=andrii@kernel.org \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=daniel@iogearbox.net \ --cc=hbathini@linux.ibm.com \ --cc=john.fastabend@gmail.com \ --cc=kafai@fb.com \ --cc=kpsingh@kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=naveen.n.rao@linux.ibm.com \ --cc=netdev@vger.kernel.org \ --cc=paulus@samba.org \ --cc=songliubraving@fb.com \ --cc=yhs@fb.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.