From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6B0F913ADF for ; Thu, 22 Jun 2023 08:23:51 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id E2E6E2102; Thu, 22 Jun 2023 01:23:24 -0700 (PDT) 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 18E791042; Thu, 22 Jun 2023 01:24:08 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.25.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 67F413F663; Thu, 22 Jun 2023 01:23:22 -0700 (PDT) Date: Thu, 22 Jun 2023 09:23:16 +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> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net On Wed, Jun 21, 2023 at 10:57:20PM +0200, Puranjay Mohan wrote: > On Wed, Jun 21, 2023 at 5:31 PM Mark Rutland wrote: > > On Mon, Jun 19, 2023 at 10:01:21AM +0000, Puranjay Mohan wrote: > > > @@ -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 for catching this, It is a big miss from my side. > > I was able to reproduce the boot issue in the other thread on my > raspberry pi. I think it is connected to the > wrong I-cache handling done by me. > > As you rightly pointed out: We need to do bpf_flush_icache() after > copying the instructions to the ro_header or the CPU can run > incorrect instructions. > > When I move the call to bpf_flush_icache() after > bpf_jit_binary_pack_finalize() (this does the copy to ro_header), the > boot issue > is fixed. Would this change be enough to make this work or I would > need to do more with the data cache as well to catch other > edge cases? AFAICT, bpf_flush_icache() calls flush_icache_range(). Despite its name, flush_icache_range() has d-cache maintenance, i-cache maintenance, and context synchronization (i.e. it does everything necessary). As long as you call that with the VAs the code will be executed from, that should be sufficient, and you don't need to do any other work. Thanks, Mark. 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 A40B5EB64DA for ; Thu, 22 Jun 2023 08:24:03 +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=4n7h7vrkOAwZRnNHjpHO3i6QO+BUr3oqwdFhZxfv0Tc=; b=btHQevlwgpnk03 dj8qKEv4mbOwtl3DyCzZKKMDiVk9hDLjy15iALS662w3UeDdHH9uaPoBELtvbubN3M5/NT1jYrpLb /+mBaoEJ/EZnDfNbOsq+TYdtnGhizdANifdUpersmdM4zruQeJd9hVjg90dt642lpPT+EkpMID5Yw meZI1ZwXR5xHg1eZYFOGFGF4cCPG73zKT37Yw9mTK7RkIlW6bRQJ9plX9M23uku4Z6CGSWhcrqGaa dmgGXtEqmGZ08sLqrwPUjh303qn6BKlXkbfQQnt/MoJ8WNCWlOSZVuwUI9M7SiR/yB9bTIDYBNMMq xZF2qq44Cbwk07J5CtJQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qCFbH-00HP8B-1y; Thu, 22 Jun 2023 08:23:31 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qCFbE-00HP6A-2x for linux-arm-kernel@lists.infradead.org; Thu, 22 Jun 2023 08:23:30 +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 18E791042; Thu, 22 Jun 2023 01:24:08 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.25.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 67F413F663; Thu, 22 Jun 2023 01:23:22 -0700 (PDT) Date: Thu, 22 Jun 2023 09:23:16 +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: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230622_012329_003455_93CC90B8 X-CRM114-Status: GOOD ( 30.94 ) 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="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org T24gV2VkLCBKdW4gMjEsIDIwMjMgYXQgMTA6NTc6MjBQTSArMDIwMCwgUHVyYW5qYXkgTW9oYW4g d3JvdGU6Cj4gT24gV2VkLCBKdW4gMjEsIDIwMjMgYXQgNTozMeKAr1BNIE1hcmsgUnV0bGFuZCA8 bWFyay5ydXRsYW5kQGFybS5jb20+IHdyb3RlOgo+ID4gT24gTW9uLCBKdW4gMTksIDIwMjMgYXQg MTA6MDE6MjFBTSArMDAwMCwgUHVyYW5qYXkgTW9oYW4gd3JvdGU6Cj4gPiA+IEBAIC0xNTYyLDM0 ICsxNjEwLDM5IEBAIHN0cnVjdCBicGZfcHJvZyAqYnBmX2ludF9qaXRfY29tcGlsZShzdHJ1Y3Qg YnBmX3Byb2cgKnByb2cpCj4gPiA+Cj4gPiA+ICAgICAgIC8qIDMuIEV4dHJhIHBhc3MgdG8gdmFs aWRhdGUgSklUZWQgY29kZS4gKi8KPiA+ID4gICAgICAgaWYgKHZhbGlkYXRlX2N0eCgmY3R4KSkg ewo+ID4gPiAtICAgICAgICAgICAgIGJwZl9qaXRfYmluYXJ5X2ZyZWUoaGVhZGVyKTsKPiA+ID4g ICAgICAgICAgICAgICBwcm9nID0gb3JpZ19wcm9nOwo+ID4gPiAtICAgICAgICAgICAgIGdvdG8g b3V0X29mZjsKPiA+ID4gKyAgICAgICAgICAgICBnb3RvIG91dF9mcmVlX2hkcjsKPiA+ID4gICAg ICAgfQo+ID4gPgo+ID4gPiAgICAgICAvKiBBbmQgd2UncmUgZG9uZS4gKi8KPiA+ID4gICAgICAg aWYgKGJwZl9qaXRfZW5hYmxlID4gMSkKPiA+ID4gICAgICAgICAgICAgICBicGZfaml0X2R1bXAo cHJvZy0+bGVuLCBwcm9nX3NpemUsIDIsIGN0eC5pbWFnZSk7Cj4gPiA+Cj4gPiA+IC0gICAgIGJw Zl9mbHVzaF9pY2FjaGUoaGVhZGVyLCBjdHguaW1hZ2UgKyBjdHguaWR4KTsKPiA+ID4gKyAgICAg YnBmX2ZsdXNoX2ljYWNoZShyb19oZWFkZXIsIGN0eC5yb19pbWFnZSArIGN0eC5pZHgpOwo+ID4K PiA+IEkgdGhpbmsgdGhpcyBpcyB0b28gZWFybHk7IHdlIGhhdmVuJ3QgY29waWVkIHRoZSBpbnN0 cnVjdGlvbnMgaW50byB0aGUKPiA+IHJvX2hlYWRlciB5ZXQsIHNvIHRoYXQgc3RpbGwgY29udGFp bnMgc3RhbGUgaW5zdHJ1Y3Rpb25zLgo+ID4KPiA+IElJVUMgYXQgdGhlIHdob2xlIHBvaW50IG9m IHRoaXMgaXMgdG8gcGFjayBtdWx0aXBsZSBwcm9ncmFtcyBpbnRvIHNoYXJlZCBST1gKPiA+IHBh Z2VzLCBhbmQgc28gdGhlcmUgY2FuIGJlIGFuIGV4ZWN1dGFibGUgbWFwcGluZyBvZiB0aGUgUk8g cGFnZSBhdCB0aGlzIHBvaW50LAo+ID4gYW5kIHRoZSBDUFUgY2FuIGZldGNoIHN0YWxlIGluc3Ry dWN0aW9ucyB0aHJvdWdodCB0aGF0Lgo+ID4KPiA+IE5vdGUgdGhhdCAqcmVnYXJkbGVzcyogb2Yg d2hldGhlciB0aGVyZSBpcyBhbiBleGVjdXRlYWJsZSBtYXBwaW5nIGF0IHRoaXMgcG9pbnQKPiA+ IChhbmQgZXZlbiBpZiBubyBleGVjdXRhYmxlIG1hcHBpbmcgZXhpc3RzIHVudGlsIGFmdGVyIHRo ZSBjb3B5KSwgd2UgYXQgbGVhc3QKPiA+IG5lZWQgYSBkYXRhIGNhY2hlIGNsZWFuIHRvIHRoZSBQ b1UgKmFmdGVyKiB0aGUgY29weSAoc28gZmV0Y2hlcyBkb24ndCBnZXQgYQo+ID4gc3RhbGUgdmFs dWUgZnJvbSB0aGUgUG9VKSwgYW5kIHRoZSBJLWNhY2hlIG1haW50ZW5hbmNlIGhhcyB0byBoYXBw ZW9uIHRoZSBWQQo+ID4gdGhlIGluc3RydXRpb25zIHdpbGwgYmUgZXhlY3V0ZWQgZnJvbSAob3Ig VklQVCBJLWNhY2hlcyBjYW4gc3RpbGwgY29udGFpbiBzdGFsZQo+ID4gaW5zdHJ1Y3Rpb25zKS4K PiAKPiBUaGFua3MgZm9yIGNhdGNoaW5nIHRoaXMsIEl0IGlzIGEgYmlnIG1pc3MgZnJvbSBteSBz aWRlLgo+IAo+IEkgd2FzIGFibGUgdG8gcmVwcm9kdWNlIHRoZSBib290IGlzc3VlIGluIHRoZSBv dGhlciB0aHJlYWQgb24gbXkKPiByYXNwYmVycnkgcGkuIEkgdGhpbmsgaXQgaXMgY29ubmVjdGVk IHRvIHRoZQo+IHdyb25nIEktY2FjaGUgaGFuZGxpbmcgZG9uZSBieSBtZS4KPiAKPiBBcyB5b3Ug cmlnaHRseSBwb2ludGVkIG91dDogV2UgbmVlZCB0byBkbyBicGZfZmx1c2hfaWNhY2hlKCkgYWZ0 ZXIKPiBjb3B5aW5nIHRoZSBpbnN0cnVjdGlvbnMgdG8gdGhlIHJvX2hlYWRlciBvciB0aGUgQ1BV IGNhbiBydW4KPiBpbmNvcnJlY3QgaW5zdHJ1Y3Rpb25zLgo+IAo+IFdoZW4gSSBtb3ZlIHRoZSBj YWxsIHRvIGJwZl9mbHVzaF9pY2FjaGUoKSBhZnRlcgo+IGJwZl9qaXRfYmluYXJ5X3BhY2tfZmlu YWxpemUoKSAodGhpcyBkb2VzIHRoZSBjb3B5IHRvIHJvX2hlYWRlciksIHRoZQo+IGJvb3QgaXNz dWUKPiBpcyBmaXhlZC4gV291bGQgdGhpcyBjaGFuZ2UgYmUgZW5vdWdoIHRvIG1ha2UgdGhpcyB3 b3JrIG9yIEkgd291bGQKPiBuZWVkIHRvIGRvIG1vcmUgd2l0aCB0aGUgZGF0YSBjYWNoZSBhcyB3 ZWxsIHRvIGNhdGNoIG90aGVyCj4gZWRnZSBjYXNlcz8KCkFGQUlDVCwgYnBmX2ZsdXNoX2ljYWNo ZSgpIGNhbGxzIGZsdXNoX2ljYWNoZV9yYW5nZSgpLiBEZXNwaXRlIGl0cyBuYW1lLApmbHVzaF9p Y2FjaGVfcmFuZ2UoKSBoYXMgZC1jYWNoZSBtYWludGVuYW5jZSwgaS1jYWNoZSBtYWludGVuYW5j ZSwgYW5kIGNvbnRleHQKc3luY2hyb25pemF0aW9uIChpLmUuIGl0IGRvZXMgZXZlcnl0aGluZyBu ZWNlc3NhcnkpLgoKQXMgbG9uZyBhcyB5b3UgY2FsbCB0aGF0IHdpdGggdGhlIFZBcyB0aGUgY29k ZSB3aWxsIGJlIGV4ZWN1dGVkIGZyb20sIHRoYXQKc2hvdWxkIGJlIHN1ZmZpY2llbnQsIGFuZCB5 b3UgZG9uJ3QgbmVlZCB0byBkbyBhbnkgb3RoZXIgd29yay4KClRoYW5rcywKTWFyay4KCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFybS1rZXJu ZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRw Oi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK