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 X-Spam-Level: X-Spam-Status: No, score=-15.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3710C433B4 for ; Tue, 11 May 2021 13:45:05 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 375DD6134F for ; Tue, 11 May 2021 13:45:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 375DD6134F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; 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=+TnmVuu68kI7xFuF/WPYVPrDD/GCt5wvPVrV4BFiTtM=; b=TCJSOpuuJ+gmw9U3mu6H4YO8k GC2E9pKJ1yNOzUipX0Wknnv7b/diHCAmeRerDHlvruv3VJXEXMBXhj7T+zr2aURtyc+bzW2FrDpQe KgD3a7NvTG3a1ZMXqU3K9JrlD1RWAUvZz3/ooTq32uPWL6g4re2z1KZ32kgeRxsKiXok/Dfhh5Czg q35tNA0nq7d9iFGaCJN5PHDAnNwor42xsnM6R0fjfdv/LDyybTgo3sXdqTcpaME5XwDrlxtnDtUJI VyEuxtlEvHN7Gp4gXULAn4UiWOeVMqX/SoRNK0Yq5ZnUv/LVolf2vVGaapRFpFChD8EcNNPFX6vex LUzf7v3PA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lgSfV-000EXw-QI; Tue, 11 May 2021 13:43:26 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lgRtk-00HVWq-2Q for linux-arm-kernel@desiato.infradead.org; Tue, 11 May 2021 12:54:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=HSyDblXMMiPvDhqitBO3YZEy2frq7YxQow3eNkvsNSo=; b=KUEgqU7raq7yjgF/gLampVGiZI qzIIc/niXNCURDj73LiAUtofGg8i9OAjrrK6tI/niNexhKZ8LKEOcREYaIr0F/OzM+/yB9jjZIcHl DiCCaAbVv5unkhMcInS7sgF8ThG/LyVXXSYYRbR4ERBOfoH5IT3qgn/EMkzkdIlqOK6X4GjwTPt3k dTB+ytaudXesji90lDl1o5qZ5V7iGEESxLzQR8C2qQFM8u8ph0awGRKxfEkrfZO2ixHUJ3Sj++tSs nYbX3qmqRaVDByeD74UpWa3rahbU7vJobwsbuQo3KlhDt9bEWq1nM+Fbrq80ddKjvvIvLFyYi/Ugt /+m+1vxg==; Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lgRtf-009amc-1f for linux-arm-kernel@lists.infradead.org; Tue, 11 May 2021 12:54:02 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 0072661921; Tue, 11 May 2021 12:53:56 +0000 (UTC) Date: Tue, 11 May 2021 13:53:54 +0100 From: Catalin Marinas To: Peter Collingbourne Cc: Andrey Konovalov , Alexander Potapenko , Vincenzo Frascino , Andrew Morton , Evgenii Stepanov , linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/3] arm64: mte: handle tags zeroing at page allocation time Message-ID: <20210511125354.GC9799@arm.com> References: <20210511073108.138837-1-pcc@google.com> <20210511073108.138837-2-pcc@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210511073108.138837-2-pcc@google.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210511_055359_186865_E7527C9F X-CRM114-Status: GOOD ( 33.82 ) 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 Hi Peter, First of all, could you please add a cover letter to your series (in general) explaining the rationale for the patches, e.g. optimise tag initialisation for user pages? It makes it a lot easier to review if the overall picture is presented in the cover. On Tue, May 11, 2021 at 12:31:07AM -0700, Peter Collingbourne wrote: > Currently, on an anonymous page fault, the kernel allocates a zeroed > page and maps it in user space. If the mapping is tagged (PROT_MTE), > set_pte_at() additionally clears the tags. It is, however, more > efficient to clear the tags at the same time as zeroing the data on > allocation. To avoid clearing the tags on any page (which may not be > mapped as tagged), only do this if the vma flags contain VM_MTE. This > requires introducing a new GFP flag that is used to determine whether > to clear the tags. > > The DC GZVA instruction with a 0 top byte (and 0 tag) requires > top-byte-ignore. Set the TCR_EL1.{TBI1,TBID1} bits irrespective of > whether KASAN_HW is enabled. > > Signed-off-by: Peter Collingbourne > Co-developed-by: Catalin Marinas > Signed-off-by: Catalin Marinas > Link: https://linux-review.googlesource.com/id/Id46dc94e30fe11474f7e54f5d65e7658dbdddb26 This doesn't mention that the patch adds tag clearing on free as well. I'd actually leave this part out for a separate patch. It's not done for tags in current mainline when kasan is disabled, AFAICT. > diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h > index 012cffc574e8..a0bcaa5f735e 100644 > --- a/arch/arm64/include/asm/page.h > +++ b/arch/arm64/include/asm/page.h > @@ -13,6 +13,7 @@ > #ifndef __ASSEMBLY__ > > #include /* for READ_IMPLIES_EXEC */ > +#include /* for gfp_t */ > #include > > struct page; > @@ -28,10 +29,16 @@ void copy_user_highpage(struct page *to, struct page *from, > void copy_highpage(struct page *to, struct page *from); > #define __HAVE_ARCH_COPY_HIGHPAGE > > -#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \ > - alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr) > +struct page *__alloc_zeroed_user_highpage(gfp_t movableflags, > + struct vm_area_struct *vma, > + unsigned long vaddr); > #define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE > > +#define want_zero_tags_on_free() system_supports_mte() As I said above, unless essential to this patch, please move it to a separate one. Also, do we need this even when the kernel doesn't have kasan_hw? > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index 871c82ab0a30..8127e0c0b8fb 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -921,3 +921,28 @@ void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr, > debug_exception_exit(regs); > } > NOKPROBE_SYMBOL(do_debug_exception); > + > +/* > + * Used during anonymous page fault handling. > + */ > +struct page *__alloc_zeroed_user_highpage(gfp_t flags, > + struct vm_area_struct *vma, > + unsigned long vaddr) > +{ > + /* > + * If the page is mapped with PROT_MTE, initialise the tags at the > + * point of allocation and page zeroing as this is usually faster than > + * separate DC ZVA and STGM. > + */ > + if (vma->vm_flags & VM_MTE) > + flags |= __GFP_ZEROTAGS; > + > + return alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | flags, vma, vaddr); > +} > + > +void tag_clear_highpage(struct page *page) > +{ > + mte_zero_clear_page_tags(page_address(page)); > + page_kasan_tag_reset(page); > + set_bit(PG_mte_tagged, &page->flags); > +} Do we need the page_kasan_tag_reset() here? Maybe we do. Is it because kasan_alloc_pages() is no longer calls kasan_unpoison_pages() below? > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c > index 45e552cb9172..34362c8d0955 100644 > --- a/mm/kasan/hw_tags.c > +++ b/mm/kasan/hw_tags.c > @@ -242,7 +242,14 @@ void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags) > { > bool init = !want_init_on_free() && want_init_on_alloc(flags); > > - kasan_unpoison_pages(page, order, init); > + if (flags & __GFP_ZEROTAGS) { > + int i; > + > + for (i = 0; i != 1 << order; ++i) > + tag_clear_highpage(page + i); > + } else { > + kasan_unpoison_pages(page, order, init); > + } > } > > void kasan_free_pages(struct page *page, unsigned int order) > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 6e82a7f6fd6f..7ac0f0721d22 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1219,10 +1219,16 @@ static int free_tail_pages_check(struct page *head_page, struct page *page) > return ret; > } > > -static void kernel_init_free_pages(struct page *page, int numpages) > +static void kernel_init_free_pages(struct page *page, int numpages, bool zero_tags) > { > int i; > > + if (zero_tags) { > + for (i = 0; i < numpages; i++) > + tag_clear_highpage(page + i); > + return; > + } > + > /* s390's use of memset() could override KASAN redzones. */ > kasan_disable_current(); > for (i = 0; i < numpages; i++) { This function has another loop calling clear_highpage(). Do we end up zeroing the page twice? > @@ -1314,7 +1320,8 @@ static __always_inline bool free_pages_prepare(struct page *page, > bool init = want_init_on_free(); > > if (init) > - kernel_init_free_pages(page, 1 << order); > + kernel_init_free_pages(page, 1 << order, > + want_zero_tags_on_free()); > if (!skip_kasan_poison) > kasan_poison_pages(page, order, init); > } I think passing 'false' here to kernel_init_free_pages() matches the current mainline. You could make this dependent on kasan_hw being enabled rather than just system_supports_mte(). With kasan_hw disabled, the kernel accesses are not checked anyway, so it's pointless to erase the tags on free. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel