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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 62B33C0219B for ; Mon, 10 Feb 2025 07:45:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E28D76B0085; Mon, 10 Feb 2025 02:45:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DD8D7280001; Mon, 10 Feb 2025 02:45:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA01D6B0089; Mon, 10 Feb 2025 02:45:58 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id AC2236B0085 for ; Mon, 10 Feb 2025 02:45:58 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 56EE280391 for ; Mon, 10 Feb 2025 07:45:58 +0000 (UTC) X-FDA: 83103251196.15.B125C7D Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf19.hostedemail.com (Postfix) with ESMTP id 8C8951A000E for ; Mon, 10 Feb 2025 07:45:56 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=none; spf=pass (imf19.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1739173556; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=p5t/qSQA2d3FSDSz3haQsw9RGnnePVfNejM9bi/AJzI=; b=l7werjp/GvDjAujfJrFUmLG/V3u/uiauqGbIDYa+hH03YxQI5YTj/PvaDmFkxrdsYzb48Q uDMQjhqAKMvzuptkSWO8c8rHUDNfRFzBKvV7COgeYjrAS7o4ex+tnnHvEBcJXW45UIMh9e ErBJBqFvg9dW0KO5J5BxZFmiPT3qpG8= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=none; spf=pass (imf19.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1739173556; a=rsa-sha256; cv=none; b=3isLygDJI81H3kPKILMS2CSidn8MSi13ALiBgodJaZDVXKFNFtlbe+R+Hx+WjSbexmThVo fSfwPDFdE/QmSTCT7I7vxL3hvCMT/og2v37RwMmUWwfv7kGzG4SgroQdPwid11RHGe66qN 4QYXQbzZcGVfvmJLv9FhUFQJD8mblHE= 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 8A2761BA8; Sun, 9 Feb 2025 23:46:17 -0800 (PST) Received: from [10.163.35.99] (unknown [10.163.35.99]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4EF743F58B; Sun, 9 Feb 2025 23:45:50 -0800 (PST) Message-ID: <43c256b6-7e44-41bb-b0c5-866fd4faab5c@arm.com> Date: Mon, 10 Feb 2025 13:15:50 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 15/16] mm: Generalize arch_sync_kernel_mappings() To: Ryan Roberts , Catalin Marinas , Will Deacon , Muchun Song , Pasha Tatashin , Andrew Morton , Uladzislau Rezki , Christoph Hellwig , Mark Rutland , Ard Biesheuvel , Dev Jain , Alexandre Ghiti , Steve Capper , Kevin Brodsky Cc: linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20250205151003.88959-1-ryan.roberts@arm.com> <20250205151003.88959-16-ryan.roberts@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <20250205151003.88959-16-ryan.roberts@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 8C8951A000E X-Stat-Signature: nziyhfs7g59fra5ucswq7911drygehy6 X-Rspam-User: X-HE-Tag: 1739173556-780809 X-HE-Meta: U2FsdGVkX19s6X+A8wJuw6XhViqeJLrYMgIsiLV5TD+1sZqxiniKZ1I405aE1mVr7iEARvmPSc2M2UdF1DdRKiDVbwC23UVtF3H/kd2A6Ay9VogUQ9RQnwGub0TaxsVFok9UYakEWSRw4KkW8/eWGknG+qyYkE9IsgW8XzlxZhTQ4bhIRKOXoQLtkYxJ3TOZDMZtuyLYzqn9nczf32uXYl+pt6W+ph67+JTKkreBvEDdSpgokyWMsHD+eoQT+aYxayoJhfCHXSdgTNCg9MWLfU++aIiOT6j5DN+m0WVCainf2vF401nQ3q/glfJETFQBi4bbxX04n7O2KqRXOqU8dJut+E7EuHI6JZoOs9yfxGfgkdh63FMX9F2beTzB4zlj5N0F5NZNk1ETFpehlR8uXuKrd8uGk/2JQB9uviBbQMS2a4HI8tZzap0VRrKuAVvuehW91LtwdaqEg3dvm+y+qBEGJVqBrS7e6E/wU0H8E3uRtQcFgiONrlPvn/GICdBxKssxJQtxIsYValrfHmGmUEzbJ5dPYvSelK4iYjmjRbOlI+sAWdWlZJtvd4um4PbBXQHmuFqqQHcaIFlb+hGM4nqLhDR2HbWrgzMakvXBNStyWCKBmOzkGmJ+jgR5sX1KLcYo0RDDRUwcbjXe5fuHvoz86VMcSPL/lhnYf+hJ6Y5LVlao3WxcK2J48i97utH8EVpaugPOkECP1fV/lTd1DwwNTZVfahehaS1xa/nFWrjIry9LLEi3eB8nHR3u5CCyAKlekEgfLgTIzXoXNBcUvvZcBPQDarYiig/1tpZSB4KqqIRBmKar7xlBRVly+7NkvUGlHT9GQUGXIlTM90pTpNREGFJaybmUJ+NbdgS4sttElde49/jVhNSJZH4mAu2j3kZqPmRP2n/s/hQPo1eW6wPOFXrQQf8FswCvG3k6/Vx2n+CPvtzyqjDCuPrTgkae25Oapuipo+ysda7AdyH ZsBshGR5 ppQuG2kRIVj3CPMrikYGqJIo/Lsuah0o82zevQPaVx/jVqgqFZu+NM4sIyqe44MgcQBcfUzcxNDZWLXMwQ3HAca9/XjLQB798p/FzV+yv/buk10R1jFqrKshYeSQzPtFQ1WyJEc3nu+f7fXJCc1QVq5Hp1GGSCnjPHp5lDJWTHxc+Enygvj750XQRnZNQja58Ob/jpAW4lkAcJ4eMlTthOrbMM7E3jp5ssVhUVjHYhH4HAcrYQdpW0jQv12O9JnRyrBv0 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2/5/25 20:39, Ryan Roberts wrote: > arch_sync_kernel_mappings() is an optional hook for arches to allow them > to synchonize certain levels of the kernel pgtables after modification. > But arm64 could benefit from a hook similar to this, paired with a call > prior to starting the batch of modifications. > > So let's introduce arch_update_kernel_mappings_begin() and > arch_update_kernel_mappings_end(). Both have a default implementation > which can be overridden by the arch code. The default for the former is > a nop, and the default for the latter is to call > arch_sync_kernel_mappings(), so the latter replaces previous > arch_sync_kernel_mappings() callsites. So by default, the resulting > behaviour is unchanged. > > To avoid include hell, the pgtbl_mod_mask type and it's associated > macros are moved to their own header. > > In a future patch, arm64 will opt-in to overriding both functions. > > Signed-off-by: Ryan Roberts > --- > include/linux/pgtable.h | 24 +---------------- > include/linux/pgtable_modmask.h | 32 ++++++++++++++++++++++ > include/linux/vmalloc.h | 47 +++++++++++++++++++++++++++++++++ > mm/memory.c | 5 ++-- > mm/vmalloc.c | 15 ++++++----- > 5 files changed, 92 insertions(+), 31 deletions(-) > create mode 100644 include/linux/pgtable_modmask.h > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > index 94d267d02372..7f70786a73b3 100644 > --- a/include/linux/pgtable.h > +++ b/include/linux/pgtable.h > @@ -4,6 +4,7 @@ > > #include > #include > +#include > > #define PMD_ORDER (PMD_SHIFT - PAGE_SHIFT) > #define PUD_ORDER (PUD_SHIFT - PAGE_SHIFT) > @@ -1786,29 +1787,6 @@ static inline bool arch_has_pfn_modify_check(void) > # define PAGE_KERNEL_EXEC PAGE_KERNEL > #endif > > -/* > - * Page Table Modification bits for pgtbl_mod_mask. > - * > - * These are used by the p?d_alloc_track*() set of functions an in the generic > - * vmalloc/ioremap code to track at which page-table levels entries have been > - * modified. Based on that the code can better decide when vmalloc and ioremap > - * mapping changes need to be synchronized to other page-tables in the system. > - */ > -#define __PGTBL_PGD_MODIFIED 0 > -#define __PGTBL_P4D_MODIFIED 1 > -#define __PGTBL_PUD_MODIFIED 2 > -#define __PGTBL_PMD_MODIFIED 3 > -#define __PGTBL_PTE_MODIFIED 4 > - > -#define PGTBL_PGD_MODIFIED BIT(__PGTBL_PGD_MODIFIED) > -#define PGTBL_P4D_MODIFIED BIT(__PGTBL_P4D_MODIFIED) > -#define PGTBL_PUD_MODIFIED BIT(__PGTBL_PUD_MODIFIED) > -#define PGTBL_PMD_MODIFIED BIT(__PGTBL_PMD_MODIFIED) > -#define PGTBL_PTE_MODIFIED BIT(__PGTBL_PTE_MODIFIED) > - > -/* Page-Table Modification Mask */ > -typedef unsigned int pgtbl_mod_mask; > - > #endif /* !__ASSEMBLY__ */ > > #if !defined(MAX_POSSIBLE_PHYSMEM_BITS) && !defined(CONFIG_64BIT) > diff --git a/include/linux/pgtable_modmask.h b/include/linux/pgtable_modmask.h > new file mode 100644 > index 000000000000..5a21b1bb8df3 > --- /dev/null > +++ b/include/linux/pgtable_modmask.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_PGTABLE_MODMASK_H > +#define _LINUX_PGTABLE_MODMASK_H > + > +#ifndef __ASSEMBLY__ > + > +/* > + * Page Table Modification bits for pgtbl_mod_mask. > + * > + * These are used by the p?d_alloc_track*() set of functions an in the generic > + * vmalloc/ioremap code to track at which page-table levels entries have been > + * modified. Based on that the code can better decide when vmalloc and ioremap > + * mapping changes need to be synchronized to other page-tables in the system. > + */ > +#define __PGTBL_PGD_MODIFIED 0 > +#define __PGTBL_P4D_MODIFIED 1 > +#define __PGTBL_PUD_MODIFIED 2 > +#define __PGTBL_PMD_MODIFIED 3 > +#define __PGTBL_PTE_MODIFIED 4 > + > +#define PGTBL_PGD_MODIFIED BIT(__PGTBL_PGD_MODIFIED) > +#define PGTBL_P4D_MODIFIED BIT(__PGTBL_P4D_MODIFIED) > +#define PGTBL_PUD_MODIFIED BIT(__PGTBL_PUD_MODIFIED) > +#define PGTBL_PMD_MODIFIED BIT(__PGTBL_PMD_MODIFIED) > +#define PGTBL_PTE_MODIFIED BIT(__PGTBL_PTE_MODIFIED) > + > +/* Page-Table Modification Mask */ > +typedef unsigned int pgtbl_mod_mask; > + > +#endif /* !__ASSEMBLY__ */ > + > +#endif /* _LINUX_PGTABLE_MODMASK_H */ > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h > index 16dd4cba64f2..cb5d8f1965a1 100644 > --- a/include/linux/vmalloc.h > +++ b/include/linux/vmalloc.h > @@ -11,6 +11,7 @@ > #include /* pgprot_t */ > #include > #include > +#include > > #include > > @@ -213,6 +214,26 @@ extern int remap_vmalloc_range(struct vm_area_struct *vma, void *addr, > int vmap_pages_range(unsigned long addr, unsigned long end, pgprot_t prot, > struct page **pages, unsigned int page_shift); > > +#ifndef arch_update_kernel_mappings_begin > +/** > + * arch_update_kernel_mappings_begin - A batch of kernel pgtable mappings are > + * about to be updated. > + * @start: Virtual address of start of range to be updated. > + * @end: Virtual address of end of range to be updated. > + * > + * An optional hook to allow architecture code to prepare for a batch of kernel > + * pgtable mapping updates. An architecture may use this to enter a lazy mode > + * where some operations can be deferred until the end of the batch. > + * > + * Context: Called in task context and may be preemptible. > + */ > +static inline void arch_update_kernel_mappings_begin(unsigned long start, > + unsigned long end) > +{ > +} > +#endif > + > +#ifndef arch_update_kernel_mappings_end > /* > * Architectures can set this mask to a combination of PGTBL_P?D_MODIFIED values > * and let generic vmalloc and ioremap code know when arch_sync_kernel_mappings() > @@ -229,6 +250,32 @@ int vmap_pages_range(unsigned long addr, unsigned long end, pgprot_t prot, > */ > void arch_sync_kernel_mappings(unsigned long start, unsigned long end); > > +/** > + * arch_update_kernel_mappings_end - A batch of kernel pgtable mappings have > + * been updated. > + * @start: Virtual address of start of range that was updated. > + * @end: Virtual address of end of range that was updated. > + * > + * An optional hook to inform architecture code that a batch update is complete. > + * This balances a previous call to arch_update_kernel_mappings_begin(). > + * > + * An architecture may override this for any purpose, such as exiting a lazy > + * mode previously entered with arch_update_kernel_mappings_begin() or syncing > + * kernel mappings to a secondary pgtable. The default implementation calls an > + * arch-provided arch_sync_kernel_mappings() if any arch-defined pgtable level > + * was updated. > + * > + * Context: Called in task context and may be preemptible. > + */ > +static inline void arch_update_kernel_mappings_end(unsigned long start, > + unsigned long end, > + pgtbl_mod_mask mask) > +{ > + if (mask & ARCH_PAGE_TABLE_SYNC_MASK) > + arch_sync_kernel_mappings(start, end); > +} One arch call back calling yet another arch call back sounds bit odd. Also should not ARCH_PAGE_TABLE_SYNC_MASK be checked both for __begin and __end callbacks in case a platform subscribes into this framework. Instead the following changes sound more reasonable, but will also require some more updates for the current platforms using arch_sync_kernel_mappings(). if (mask & ARCH_PAGE_TABLE_SYNC_MASK) arch_update_kernel_mappings_begin() if (mask & ARCH_PAGE_TABLE_SYNC_MASK) arch_update_kernel_mappings_end() Basically when any platform defines ARCH_PAGE_TABLE_SYNC_MASK and subscribes this framework, it will also provide arch_update_kernel_mappings_begin/end() callbacks as required. > +#endif > + > /* > * Lowlevel-APIs (not for driver use!) > */ > diff --git a/mm/memory.c b/mm/memory.c > index a15f7dd500ea..f80930bc19f6 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3035,6 +3035,8 @@ static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr, > if (WARN_ON(addr >= end)) > return -EINVAL; > > + arch_update_kernel_mappings_begin(start, end); > + > pgd = pgd_offset(mm, addr); > do { > next = pgd_addr_end(addr, end); > @@ -3055,8 +3057,7 @@ static int __apply_to_page_range(struct mm_struct *mm, unsigned long addr, > break; > } while (pgd++, addr = next, addr != end); > > - if (mask & ARCH_PAGE_TABLE_SYNC_MASK) > - arch_sync_kernel_mappings(start, start + size); > + arch_update_kernel_mappings_end(start, end, mask); > > return err; > } > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 50fd44439875..c5c51d86ef78 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -312,10 +312,10 @@ int vmap_page_range(unsigned long addr, unsigned long end, > pgtbl_mod_mask mask = 0; > int err; > > + arch_update_kernel_mappings_begin(addr, end); > err = vmap_range_noflush(addr, end, phys_addr, pgprot_nx(prot), > ioremap_max_page_shift, &mask); > - if (mask & ARCH_PAGE_TABLE_SYNC_MASK) > - arch_sync_kernel_mappings(addr, end); > + arch_update_kernel_mappings_end(addr, end, mask); > > flush_cache_vmap(addr, end); > if (!err) > @@ -463,6 +463,9 @@ void __vunmap_range_noflush(unsigned long start, unsigned long end) > pgtbl_mod_mask mask = 0; > > BUG_ON(addr >= end); > + > + arch_update_kernel_mappings_begin(start, end); > + > pgd = pgd_offset_k(addr); > do { > next = pgd_addr_end(addr, end); > @@ -473,8 +476,7 @@ void __vunmap_range_noflush(unsigned long start, unsigned long end) > vunmap_p4d_range(pgd, addr, next, &mask); > } while (pgd++, addr = next, addr != end); > > - if (mask & ARCH_PAGE_TABLE_SYNC_MASK) > - arch_sync_kernel_mappings(start, end); > + arch_update_kernel_mappings_end(start, end, mask); > } > > void vunmap_range_noflush(unsigned long start, unsigned long end) > @@ -625,6 +627,8 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > WARN_ON(page_shift < PAGE_SHIFT); > > + arch_update_kernel_mappings_begin(start, end); > + > if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMALLOC) || > page_shift == PAGE_SHIFT) { > err = vmap_small_pages_range_noflush(addr, end, prot, pages, > @@ -642,8 +646,7 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end, > } > } > > - if (mask & ARCH_PAGE_TABLE_SYNC_MASK) > - arch_sync_kernel_mappings(start, end); > + arch_update_kernel_mappings_end(start, end, mask); > > return err; > }