On Wed, Nov 29, 2023 at 10:10:22AM -0800, Stanislav Fomichev wrote: > On Wed, Nov 29, 2023 at 6:55 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote: > > > > On Wed, Nov 29, 2023 at 03:05:21PM +0100, Jiri Olsa wrote: > > > On Tue, Nov 28, 2023 at 01:24:14PM -0800, Stanislav Fomichev wrote: > > > > On 11/28, Jiri Olsa wrote: > > > > > We need to be able to skip ip address check for caller in following > > > > > changes. Adding checkip argument to allow that. > > > > > > > > > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx> > > > > > --- > > > > > arch/arm64/net/bpf_jit_comp.c | 3 ++- > > > > > arch/riscv/net/bpf_jit_comp64.c | 5 +++-- > > > > > arch/s390/net/bpf_jit_comp.c | 3 ++- > > > > > arch/x86/net/bpf_jit_comp.c | 24 +++++++++++++----------- > > > > > include/linux/bpf.h | 2 +- > > > > > kernel/bpf/arraymap.c | 8 ++++---- > > > > > kernel/bpf/core.c | 2 +- > > > > > kernel/bpf/trampoline.c | 12 ++++++------ > > > > > 8 files changed, 32 insertions(+), 27 deletions(-) > > > > > > > > > > diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c > > > > > index 7d4af64e3982..b52549d18730 100644 > > > > > --- a/arch/arm64/net/bpf_jit_comp.c > > > > > +++ b/arch/arm64/net/bpf_jit_comp.c > > > > > @@ -2167,7 +2167,8 @@ static int gen_branch_or_nop(enum aarch64_insn_branch_type type, void *ip, > > > > > * locations during the patching process, making the patching process easier. > > > > > */ > > > > > int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type, > > > > > - void *old_addr, void *new_addr) > > > > > + void *old_addr, void *new_addr, > > > > > > > > [..] > > > > > > > > > + bool checkip __maybe_unused) > > > > > > > > Any idea why only riscv and x86 do this check? > > > > > > so arm does the check as well, but needs the data from the lookup > > > to patch things properly.. but IIUC it does not suffer the same > > > issue because it does not implement direct tail calls [1] which > > > is used only on x86 > > > > > > > > > > > Asking because maybe it makes sense to move this check into some > > > > new generic bpf_text_poke and call it in the places where you currently > > > > call checkip=true (and keep using bpf_arch_text_poke for checkip=false > > > > case). > > > > > > > > (don't see any issues with the current approach btw, just interested..) > > > > > > I tried to add new function for that, but it did not look good for arm > > > because it needs to do the lookup anyway > > > > > > hm maybe we could use new arch function that would cover the single > > > tail call 'text poke' update in prog_array_map_poke_run and would be > > > implemented only on x86 ... using __bpf_arch_text_poke directly > > > > looks like below change would be enough, I'll test and send new version > > sg. I'm still not 100% sure why it's x86 only, I was (probably > wrongly?) assuming that at least arm64 jit is mostly on par with x86 > :-) AFAICS the direct tail calls are on x86, CI also seems to be ok with that change.. I'll send it as formal patch jirka
