From: Jean-Philippe Brucker <jean-philippe@linaro.org> To: Xu Kuohai <xukuohai@huawei.com> Cc: bpf@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Alexei Starovoitov <ast@kernel.org>, Zi Shen Lim <zlim.lnx@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, "David S . Miller" <davem@davemloft.net>, Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>, David Ahern <dsahern@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>, Jakub Kicinski <kuba@kernel.org>, Jesper Dangaard Brouer <hawk@kernel.org>, Russell King <rmk+kernel@armlinux.org.uk>, James Morse <james.morse@arm.com>, Hou Tao <houtao1@huawei.com>, Jason Wang <wangborong@cdjrlc.com> Subject: Re: [PATCH bpf-next v6 4/4] bpf, arm64: bpf trampoline for arm64 Date: Fri, 8 Jul 2022 09:24:22 +0100 [thread overview] Message-ID: <YsfptiexC0wFABFL@myrica> (raw) In-Reply-To: <a24109d5-b79a-99de-0fd5-66b0ec34e5ed@huawei.com> On Fri, Jul 08, 2022 at 12:35:33PM +0800, Xu Kuohai wrote: > >> + > >> + emit(A64_ADD_I(1, A64_R(0), A64_SP, args_off), ctx); > >> + if (!p->jited) > >> + emit_addr_mov_i64(A64_R(1), (const u64)p->insnsi, ctx); > >> + > >> + emit_call((const u64)p->bpf_func, ctx); > >> + > >> + /* store return value */ > >> + if (save_ret) > >> + emit(A64_STR64I(r0, A64_SP, retval_off), ctx); > > > > Here too I think it should be x0. I'm guessing r0 may work for jitted > > functions but not interpreted ones > > > > Yes, r0 is only correct for jitted code, will fix it to: > > if (save_ret) > emit(A64_STR64I(p->jited ? r0 : A64_R(0), A64_SP, retval_off), > ctx); I don't think we need this test because x0 should be correct in all cases. x7 happens to equal x0 when jitted due to the way build_epilogue() builds the function at the moment, but we shouldn't rely on that. > >> + if (flags & BPF_TRAMP_F_CALL_ORIG) { > >> + restore_args(ctx, args_off, nargs); > >> + /* call original func */ > >> + emit(A64_LDR64I(A64_R(10), A64_SP, retaddr_off), ctx); > >> + emit(A64_BLR(A64_R(10)), ctx); > > > > I don't think we can do this when BTI is enabled because we're not jumping > > to a BTI instruction. We could introduce one in a patched BPF function > > (there currently is one if CONFIG_ARM64_PTR_AUTH_KERNEL), but probably not > > in a kernel function. > > > > We could fo like FUNCTION_GRAPH_TRACER does and return to the patched > > function after modifying its LR. Not sure whether that works with pointer > > auth though. > > > > Yes, the blr instruction should be replaced with ret instruction, thanks! > > The layout for bpf prog and regular kernel function is as follows, with > bti always coming first and paciasp immediately after patchsite, so the > ret instruction should work in all cases. > > bpf prog or kernel function: > bti c // if BTI > mov x9, lr > bl <trampoline> ------> trampoline: > ... > mov lr, <return_entry> > mov x10, <ORIG_CALL_entry> > ORIG_CALL_entry: <------- ret x10 > return_entry: > ... > paciasp // if PA > ... Actually I just noticed that CONFIG_ARM64_BTI_KERNEL depends on CONFIG_ARM64_PTR_AUTH_KERNEL, so we should be able to rely on there always being a PACIASP at ORIG_CALL_entry, and since it's a landing pad for BLR we don't need to make this a RET 92e2294d870b ("arm64: bti: Support building kernel C code using BTI") Thanks, Jean
WARNING: multiple messages have this Message-ID (diff)
From: Jean-Philippe Brucker <jean-philippe@linaro.org> To: Xu Kuohai <xukuohai@huawei.com> Cc: bpf@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Alexei Starovoitov <ast@kernel.org>, Zi Shen Lim <zlim.lnx@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, "David S . Miller" <davem@davemloft.net>, Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>, David Ahern <dsahern@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>, Jakub Kicinski <kuba@kernel.org>, Jesper Dangaard Brouer <hawk@kernel.org>, Russell King <rmk+kernel@armlinux.org.uk>, James Morse <james.morse@arm.com>, Hou Tao <houtao1@huawei.com>, Jason Wang <wangborong@cdjrlc.com> Subject: Re: [PATCH bpf-next v6 4/4] bpf, arm64: bpf trampoline for arm64 Date: Fri, 8 Jul 2022 09:24:22 +0100 [thread overview] Message-ID: <YsfptiexC0wFABFL@myrica> (raw) In-Reply-To: <a24109d5-b79a-99de-0fd5-66b0ec34e5ed@huawei.com> On Fri, Jul 08, 2022 at 12:35:33PM +0800, Xu Kuohai wrote: > >> + > >> + emit(A64_ADD_I(1, A64_R(0), A64_SP, args_off), ctx); > >> + if (!p->jited) > >> + emit_addr_mov_i64(A64_R(1), (const u64)p->insnsi, ctx); > >> + > >> + emit_call((const u64)p->bpf_func, ctx); > >> + > >> + /* store return value */ > >> + if (save_ret) > >> + emit(A64_STR64I(r0, A64_SP, retval_off), ctx); > > > > Here too I think it should be x0. I'm guessing r0 may work for jitted > > functions but not interpreted ones > > > > Yes, r0 is only correct for jitted code, will fix it to: > > if (save_ret) > emit(A64_STR64I(p->jited ? r0 : A64_R(0), A64_SP, retval_off), > ctx); I don't think we need this test because x0 should be correct in all cases. x7 happens to equal x0 when jitted due to the way build_epilogue() builds the function at the moment, but we shouldn't rely on that. > >> + if (flags & BPF_TRAMP_F_CALL_ORIG) { > >> + restore_args(ctx, args_off, nargs); > >> + /* call original func */ > >> + emit(A64_LDR64I(A64_R(10), A64_SP, retaddr_off), ctx); > >> + emit(A64_BLR(A64_R(10)), ctx); > > > > I don't think we can do this when BTI is enabled because we're not jumping > > to a BTI instruction. We could introduce one in a patched BPF function > > (there currently is one if CONFIG_ARM64_PTR_AUTH_KERNEL), but probably not > > in a kernel function. > > > > We could fo like FUNCTION_GRAPH_TRACER does and return to the patched > > function after modifying its LR. Not sure whether that works with pointer > > auth though. > > > > Yes, the blr instruction should be replaced with ret instruction, thanks! > > The layout for bpf prog and regular kernel function is as follows, with > bti always coming first and paciasp immediately after patchsite, so the > ret instruction should work in all cases. > > bpf prog or kernel function: > bti c // if BTI > mov x9, lr > bl <trampoline> ------> trampoline: > ... > mov lr, <return_entry> > mov x10, <ORIG_CALL_entry> > ORIG_CALL_entry: <------- ret x10 > return_entry: > ... > paciasp // if PA > ... Actually I just noticed that CONFIG_ARM64_BTI_KERNEL depends on CONFIG_ARM64_PTR_AUTH_KERNEL, so we should be able to rely on there always being a PACIASP at ORIG_CALL_entry, and since it's a landing pad for BLR we don't need to make this a RET 92e2294d870b ("arm64: bti: Support building kernel C code using BTI") Thanks, Jean _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-07-08 8:24 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-06-25 16:12 [PATCH bpf-next v6 0/4] bpf trampoline for arm64 Xu Kuohai 2022-06-25 16:12 ` Xu Kuohai 2022-06-25 16:12 ` [PATCH bpf-next v6 1/4] bpf: Remove is_valid_bpf_tramp_flags() Xu Kuohai 2022-06-25 16:12 ` Xu Kuohai 2022-06-25 16:12 ` [PATCH bpf-next v6 2/4] arm64: Add LDR (literal) instruction Xu Kuohai 2022-06-25 16:12 ` Xu Kuohai 2022-07-05 16:39 ` Will Deacon 2022-07-05 16:39 ` Will Deacon 2022-07-06 1:43 ` Xu Kuohai 2022-07-06 1:43 ` Xu Kuohai 2022-06-25 16:12 ` [PATCH bpf-next v6 3/4] bpf, arm64: Impelment bpf_arch_text_poke() for arm64 Xu Kuohai 2022-06-25 16:12 ` Xu Kuohai 2022-07-07 16:41 ` Jean-Philippe Brucker 2022-07-07 16:41 ` Jean-Philippe Brucker 2022-07-08 2:41 ` Xu Kuohai 2022-07-08 2:41 ` Xu Kuohai 2022-07-08 8:25 ` Jean-Philippe Brucker 2022-07-08 8:25 ` Jean-Philippe Brucker 2022-06-25 16:12 ` [PATCH bpf-next v6 4/4] bpf, arm64: bpf trampoline " Xu Kuohai 2022-06-25 16:12 ` Xu Kuohai 2022-07-07 16:37 ` Jean-Philippe Brucker 2022-07-07 16:37 ` Jean-Philippe Brucker 2022-07-08 4:35 ` Xu Kuohai 2022-07-08 4:35 ` Xu Kuohai 2022-07-08 8:24 ` Jean-Philippe Brucker [this message] 2022-07-08 8:24 ` Jean-Philippe Brucker 2022-07-08 9:08 ` Xu Kuohai 2022-07-08 9:08 ` Xu Kuohai 2022-06-30 21:12 ` [PATCH bpf-next v6 0/4] " Daniel Borkmann 2022-06-30 21:12 ` Daniel Borkmann 2022-07-05 16:00 ` Will Deacon 2022-07-05 16:00 ` Will Deacon 2022-07-05 18:34 ` KP Singh 2022-07-05 18:34 ` KP Singh 2022-07-07 3:35 ` Xu Kuohai 2022-07-07 3:35 ` Xu Kuohai 2022-07-06 16:08 ` Jean-Philippe Brucker 2022-07-06 16:08 ` Jean-Philippe Brucker 2022-07-06 16:11 ` Will Deacon 2022-07-06 16:11 ` Will Deacon 2022-07-07 2:56 ` Xu Kuohai 2022-07-07 2:56 ` Xu Kuohai
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=YsfptiexC0wFABFL@myrica \ --to=jean-philippe@linaro.org \ --cc=andrii@kernel.org \ --cc=ast@kernel.org \ --cc=bp@alien8.de \ --cc=bpf@vger.kernel.org \ --cc=catalin.marinas@arm.com \ --cc=daniel@iogearbox.net \ --cc=dave.hansen@linux.intel.com \ --cc=davem@davemloft.net \ --cc=dsahern@kernel.org \ --cc=hawk@kernel.org \ --cc=houtao1@huawei.com \ --cc=hpa@zytor.com \ --cc=james.morse@arm.com \ --cc=john.fastabend@gmail.com \ --cc=kafai@fb.com \ --cc=kpsingh@kernel.org \ --cc=kuba@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mingo@redhat.com \ --cc=netdev@vger.kernel.org \ --cc=rmk+kernel@armlinux.org.uk \ --cc=songliubraving@fb.com \ --cc=tglx@linutronix.de \ --cc=wangborong@cdjrlc.com \ --cc=will@kernel.org \ --cc=x86@kernel.org \ --cc=xukuohai@huawei.com \ --cc=yhs@fb.com \ --cc=yoshfuji@linux-ipv6.org \ --cc=zlim.lnx@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: 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.