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 9ADDFEB64D7 for ; Wed, 21 Jun 2023 15:31:40 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NtsAwdCUsJdjZutgPBlovFiFOI5cJvyfExHw7bP/NtQ=; b=GHervUkJl+P/fd 1Csxev/CWDSbw9xkbiATuPyLvB+3Pyzp2npe7Yuu+R3DrWH6X4h7o3UovEbDtCw4wOEW0yK0aFeHd lhYexmoNb7k4jOiX0s50dga+XJIrgludVQEbRVnish8YWnhrrhZl9rRn6yxbtjAyRB+LXO3AUZxKh h0jqUN+4apMYxPuIkQU6m5lTeTNv3vJfvjeKautiyOm/BCikebs1pe5SF1Y/5DX69NEH3L4wEXkrG FvwIWXNqn4oaU8RwsqQZch7wrIvMglfcF4j+T7cdXcbqRWshJ9rbwBiRosqiXNtQrHdwd86vWdeGE 8g1+NulRna1ZlXovg2rw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qBznk-00Ewwh-25; Wed, 21 Jun 2023 15:31:20 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qBznh-00Ewvn-0I for linux-arm-kernel@lists.infradead.org; Wed, 21 Jun 2023 15:31:19 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 79BE31063; Wed, 21 Jun 2023 08:31:55 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.27.65]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C415C3F64C; Wed, 21 Jun 2023 08:31:09 -0700 (PDT) Date: Wed, 21 Jun 2023 16:31:00 +0100 From: Mark Rutland To: Puranjay Mohan Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, catalin.marinas@arm.com, bpf@vger.kernel.org, kpsingh@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH bpf-next v3 3/3] bpf, arm64: use bpf_jit_binary_pack_alloc Message-ID: References: <20230619100121.27534-1-puranjay12@gmail.com> <20230619100121.27534-4-puranjay12@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230619100121.27534-4-puranjay12@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230621_083117_223066_E1D6DB79 X-CRM114-Status: GOOD ( 23.86 ) 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 On Mon, Jun 19, 2023 at 10:01:21AM +0000, Puranjay Mohan wrote: > Use bpf_jit_binary_pack_alloc for memory management of JIT binaries in > ARM64 BPF JIT. The bpf_jit_binary_pack_alloc creates a pair of RW and RX > buffers. The JIT writes the program into the RW buffer. When the JIT is > done, the program is copied to the final RX buffer > with bpf_jit_binary_pack_finalize. > > Implement bpf_arch_text_copy() and bpf_arch_text_invalidate() for ARM64 > JIT as these functions are required by bpf_jit_binary_pack allocator. > > Signed-off-by: Puranjay Mohan >From a quick look, I don't beleive the I-cache maintenance is quite right -- explanation below. > @@ -1562,34 +1610,39 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) > > /* 3. Extra pass to validate JITed code. */ > if (validate_ctx(&ctx)) { > - bpf_jit_binary_free(header); > prog = orig_prog; > - goto out_off; > + goto out_free_hdr; > } > > /* And we're done. */ > if (bpf_jit_enable > 1) > bpf_jit_dump(prog->len, prog_size, 2, ctx.image); > > - bpf_flush_icache(header, ctx.image + ctx.idx); > + bpf_flush_icache(ro_header, ctx.ro_image + ctx.idx); I think this is too early; we haven't copied the instructions into the ro_header yet, so that still contains stale instructions. IIUC at the whole point of this is to pack multiple programs into shared ROX pages, and so there can be an executable mapping of the RO page at this point, and the CPU can fetch stale instructions throught that. Note that *regardless* of whether there is an executeable mapping at this point (and even if no executable mapping exists until after the copy), we at least need a data cache clean to the PoU *after* the copy (so fetches don't get a stale value from the PoU), and the I-cache maintenance has to happeon the VA the instrutions will be executed from (or VIPT I-caches can still contain stale instructions). Thanks, Mark. > > if (!prog->is_func || extra_pass) { > if (extra_pass && ctx.idx != jit_data->ctx.idx) { > pr_err_once("multi-func JIT bug %d != %d\n", > ctx.idx, jit_data->ctx.idx); > - bpf_jit_binary_free(header); > prog->bpf_func = NULL; > prog->jited = 0; > prog->jited_len = 0; > + goto out_free_hdr; > + } > + if (WARN_ON(bpf_jit_binary_pack_finalize(prog, ro_header, > + header))) { > + /* ro_header has been freed */ > + ro_header = NULL; > + prog = orig_prog; > goto out_off; > } > - bpf_jit_binary_lock_ro(header); > } else { > jit_data->ctx = ctx; > - jit_data->image = image_ptr; > + jit_data->ro_image = ro_image_ptr; > jit_data->header = header; > + jit_data->ro_header = ro_header; > } > - prog->bpf_func = (void *)ctx.image; > + prog->bpf_func = (void *)ctx.ro_image; > prog->jited = 1; > prog->jited_len = prog_size; > > @@ -1610,6 +1663,14 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) > bpf_jit_prog_release_other(prog, prog == orig_prog ? > tmp : orig_prog); > return prog; > + > +out_free_hdr: > + if (header) { > + bpf_arch_text_copy(&ro_header->size, &header->size, > + sizeof(header->size)); > + bpf_jit_binary_pack_free(ro_header, header); > + } > + goto out_off; > } > > bool bpf_jit_supports_kfunc_call(void) > @@ -1617,6 +1678,13 @@ bool bpf_jit_supports_kfunc_call(void) > return true; > } > > +void *bpf_arch_text_copy(void *dst, void *src, size_t len) > +{ > + if (!aarch64_insn_copy(dst, src, len)) > + return ERR_PTR(-EINVAL); > + return dst; > +} > + > u64 bpf_jit_alloc_exec_limit(void) > { > return VMALLOC_END - VMALLOC_START; > @@ -2221,3 +2289,27 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type, > > return ret; > } > + > +void bpf_jit_free(struct bpf_prog *prog) > +{ > + if (prog->jited) { > + struct arm64_jit_data *jit_data = prog->aux->jit_data; > + struct bpf_binary_header *hdr; > + > + /* > + * If we fail the final pass of JIT (from jit_subprogs), > + * the program may not be finalized yet. Call finalize here > + * before freeing it. > + */ > + if (jit_data) { > + bpf_jit_binary_pack_finalize(prog, jit_data->ro_header, > + jit_data->header); > + kfree(jit_data); > + } > + hdr = bpf_jit_binary_pack_hdr(prog); > + bpf_jit_binary_pack_free(hdr, NULL); > + WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(prog)); > + } > + > + bpf_prog_unlock_free(prog); > +} > -- > 2.40.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel