From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 16BF1C433F5 for ; Tue, 10 May 2022 11:46:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:In-reply-to: Date:Subject:Cc:To:From:References:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/L/DRpTKNvEBwbJ1hGA8Y1WV/rQ3a5qpDODNsispPnM=; b=EuUTK6HSQtZlBV wDL8RBSefPiIgn79vVluplFHtvO6CKZMAwk5MQMfqLV3GgH6RkOdVqicdU2+Ts3F4d/gPg1Q9IxXC 4QTxYeMtXSRNA3J78rXogxvhdg4CGcQKgoGMUU/vvJu3NEwX/c+8IkjVmPNpeENEckM/3KJCy5r4L 7f3QdilkAOCYEN1QxzpKmx5lIjI6hQBiCMGRpJprN6OZFnjEKI0Erd8oPwCySWSJywZkj/Q46Y8fG pI9iBZkYpaln1qukw29trNByEKGOhqYe4U3JPm40eyvCTm0uKlq+POnmiPusPVveIykJbFGsJ9K9o eaM7QLV6aXgLCXC9SKpQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1noOJB-001fwV-B4; Tue, 10 May 2022 11:45:41 +0000 Received: from mail-ej1-x632.google.com ([2a00:1450:4864:20::632]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1noOJ7-001ftQ-3A for linux-arm-kernel@lists.infradead.org; Tue, 10 May 2022 11:45:39 +0000 Received: by mail-ej1-x632.google.com with SMTP id l18so32342886ejc.7 for ; Tue, 10 May 2022 04:45:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloudflare.com; s=google; h=references:user-agent:from:to:cc:subject:date:in-reply-to :message-id:mime-version; bh=7CJgZK/Q3TXpdrc82+EsjW+w5ztoxNGdpsgGltu/dv8=; b=M6Kk70C20E34EyBk6iNsYOcQRG7DQcgLbFdyEulG2ocYzl7J0XnWxHqsZIlR6UUzud OggPoFqx5m1Honn9I0w1BZnKD7rmMc3Tfmd+LacWlFoMg4qkRs2ztQlOjdaifDQKDUFL PLpQc2nzaqrCYxqW7NDsqgcBWA3RhV/Oy2qFI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:references:user-agent:from:to:cc:subject:date :in-reply-to:message-id:mime-version; bh=7CJgZK/Q3TXpdrc82+EsjW+w5ztoxNGdpsgGltu/dv8=; b=BbLPaRnY5g6DzQB1VXrEgMyixPw0csbCJt/2SBzLUj2Ya6Ocd+aHz/RUQ92q+0OsZI kpBkogSvInB1AdxjWoEALnQnXbOTsw39mG735uoUan8BPLFP76EBPxZB+EFyyFJcTNZy JWitu37jHuKNmGJV2DtK7GXQq7QElYhKnFaGmdcflmuldpgadMiGxNxuIDclcxe+SBw2 h1nqyaybMDdKROuWGhD1d14gWjeEUF5Wc+q+Poz3U3xLd8AnXW4LCl21iZp+SyoqOj8b qo1kpOSEs1ftYPEpXZwpjW4limtgD7JR1y72KQbkJOthPc7WUlmQtwgeJ4j/Pa8kA2Si 0pzw== X-Gm-Message-State: AOAM531ftVoIyNBSqRzzmBKNp+k7/IS/786+RFcvB7ilWrgIhA4NUzLm eP+lQ+59zGLUmcBAPsfvbQBUkQ== X-Google-Smtp-Source: ABdhPJyLAw11BXqovb/hyRd3O52ps0BA+TwNNEhz5hf8CzWAXREk2pVUIGq9fB6HrslFKPlLwZUkkw== X-Received: by 2002:a17:907:6d25:b0:6f4:d753:f250 with SMTP id sa37-20020a1709076d2500b006f4d753f250mr19240326ejc.580.1652183131282; Tue, 10 May 2022 04:45:31 -0700 (PDT) Received: from cloudflare.com (79.184.139.106.ipv4.supernova.orange.pl. [79.184.139.106]) by smtp.gmail.com with ESMTPSA id n12-20020a1709065e0c00b006f3ef214e0bsm6127841eju.113.2022.05.10.04.45.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 May 2022 04:45:30 -0700 (PDT) References: <20220424154028.1698685-1-xukuohai@huawei.com> <20220424154028.1698685-6-xukuohai@huawei.com> User-agent: mu4e 1.6.10; emacs 27.2 From: Jakub Sitnicki To: Xu Kuohai Cc: bpf@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, linux-kselftest@vger.kernel.org, Catalin Marinas , Will Deacon , Steven Rostedt , Ingo Molnar , Daniel Borkmann , Alexei Starovoitov , Zi Shen Lim , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , "David S . Miller" , Hideaki YOSHIFUJI , David Ahern , Thomas Gleixner , Borislav Petkov , Dave Hansen , x86@kernel.org, hpa@zytor.com, Shuah Khan , Jakub Kicinski , Jesper Dangaard Brouer , Mark Rutland , Pasha Tatashin , Ard Biesheuvel , Daniel Kiss , Steven Price , Sudeep Holla , Marc Zyngier , Peter Collingbourne , Mark Brown , Delyan Kratunov , Kumar Kartikeya Dwivedi Subject: Re: [PATCH bpf-next v3 5/7] bpf, arm64: Support to poke bpf prog Date: Tue, 10 May 2022 11:36:59 +0200 In-reply-to: <20220424154028.1698685-6-xukuohai@huawei.com> Message-ID: <87ilqdobl1.fsf@cloudflare.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220510_044537_192206_1A1FFFAE X-CRM114-Status: GOOD ( 29.41 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Thanks for incorporating the attach to BPF progs bits into the series. I have a couple minor comments. Please see below. On Sun, Apr 24, 2022 at 11:40 AM -04, Xu Kuohai wrote: > 1. Set up the bpf prog entry in the same way as fentry to support > trampoline. Now bpf prog entry looks like this: > > bti c // if BTI enabled > mov x9, x30 // save lr > nop // to be replaced with jump instruction > paciasp // if PAC enabled > > 2. Update bpf_arch_text_poke() to poke bpf prog. If the instruction > to be poked is bpf prog's first instruction, skip to the nop > instruction in the prog entry. > > Signed-off-by: Xu Kuohai > --- > arch/arm64/net/bpf_jit.h | 1 + > arch/arm64/net/bpf_jit_comp.c | 41 +++++++++++++++++++++++++++-------- > 2 files changed, 33 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/net/bpf_jit.h b/arch/arm64/net/bpf_jit.h > index 194c95ccc1cf..1c4b0075a3e2 100644 > --- a/arch/arm64/net/bpf_jit.h > +++ b/arch/arm64/net/bpf_jit.h > @@ -270,6 +270,7 @@ > #define A64_BTI_C A64_HINT(AARCH64_INSN_HINT_BTIC) > #define A64_BTI_J A64_HINT(AARCH64_INSN_HINT_BTIJ) > #define A64_BTI_JC A64_HINT(AARCH64_INSN_HINT_BTIJC) > +#define A64_NOP A64_HINT(AARCH64_INSN_HINT_NOP) > > /* DMB */ > #define A64_DMB_ISH aarch64_insn_gen_dmb(AARCH64_INSN_MB_ISH) > diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c > index 3f9bdfec54c4..293bdefc5d0c 100644 > --- a/arch/arm64/net/bpf_jit_comp.c > +++ b/arch/arm64/net/bpf_jit_comp.c > @@ -237,14 +237,23 @@ static bool is_lsi_offset(int offset, int scale) > return true; > } > > -/* Tail call offset to jump into */ > -#if IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) || \ > - IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL) > -#define PROLOGUE_OFFSET 9 > +#if IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) > +#define BTI_INSNS 1 > +#else > +#define BTI_INSNS 0 > +#endif > + > +#if IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL) > +#define PAC_INSNS 1 > #else > -#define PROLOGUE_OFFSET 8 > +#define PAC_INSNS 0 > #endif Above can be folded into: #define BTI_INSNS (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) ? 1 : 0) #define PAC_INSNS (IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL) ? 1 : 0) > > +/* Tail call offset to jump into */ > +#define PROLOGUE_OFFSET (BTI_INSNS + 2 + PAC_INSNS + 8) > +/* Offset of nop instruction in bpf prog entry to be poked */ > +#define POKE_OFFSET (BTI_INSNS + 1) > + > static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf) > { > const struct bpf_prog *prog = ctx->prog; > @@ -281,12 +290,15 @@ static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf) > * > */ > > + if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL)) > + emit(A64_BTI_C, ctx); I'm no arm64 expert, but this looks like a fix for BTI. Currently we never emit BTI because ARM64_BTI_KERNEL depends on ARM64_PTR_AUTH_KERNEL, while BTI must be the first instruction for the jump target [1]. Am I following correctly? [1] https://lwn.net/Articles/804982/ > + > + emit(A64_MOV(1, A64_R(9), A64_LR), ctx); > + emit(A64_NOP, ctx); > + > /* Sign lr */ > if (IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL)) > emit(A64_PACIASP, ctx); > - /* BTI landing pad */ > - else if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL)) > - emit(A64_BTI_C, ctx); > > /* Save FP and LR registers to stay align with ARM64 AAPCS */ > emit(A64_PUSH(A64_FP, A64_LR, A64_SP), ctx); > @@ -1552,9 +1564,11 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type, > u32 old_insn; > u32 new_insn; > u32 replaced; > + unsigned long offset = ~0UL; > enum aarch64_insn_branch_type branch_type; > + char namebuf[KSYM_NAME_LEN]; > > - if (!is_bpf_text_address((long)ip)) > + if (!__bpf_address_lookup((unsigned long)ip, NULL, &offset, namebuf)) > /* Only poking bpf text is supported. Since kernel function > * entry is set up by ftrace, we reply on ftrace to poke kernel > * functions. For kernel funcitons, bpf_arch_text_poke() is only > @@ -1565,6 +1579,15 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type, > */ > return -EINVAL; > > + /* bpf entry */ > + if (offset == 0UL) > + /* skip to the nop instruction in bpf prog entry: > + * bti c // if BTI enabled > + * mov x9, x30 > + * nop > + */ > + ip = (u32 *)ip + POKE_OFFSET; This is very much personal preference, however, I find the use pointer arithmetic too clever here. Would go for a more verbose: offset = POKE_OFFSET * AARCH64_INSN_SIZE; ip = (void *)((unsigned long)ip + offset); > + > if (poke_type == BPF_MOD_CALL) > branch_type = AARCH64_INSN_BRANCH_LINK; > else I think it'd make more sense to merge this patch with patch 4 (the preceding one). Initial implementation of of bpf_arch_text_poke() from patch 4 is not fully functional, as it will always fail for bpf_arch_text_poke(ip, BPF_MOD_CALL, ...) calls. At least, I find it a bit confusing. Otherwise than that: Reviewed-by: Jakub Sitnicki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel