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 EBA9CCAC583 for ; Thu, 11 Sep 2025 06:18:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=YlizTnzHI5Cy3BO440y/W3Cwr9p2XC32bOCkVOdLKeI=; b=0VzGyCZAUWd+kHJTnfLIvmKLwC ck37ncg2E9ggSVeok/MhkUen700z8xc9IsJhXGVhZk8AP9efBlJ+MClJ++FsctNe/4FLNfhoJUNxK KcFmTo3VHjscG2ej+7RtcbWSwjI+xAnwRLe3oQfRPKac1eno7/kov93Sdl5iW5zBwFyCugFWBE4gI DIVykAdRoSB2nzm+MWeQiuVCqQSwqvmp3BOA1AVKQnFZJXFOINabOyvnIMMtpAMmkvGwAu36rdjlS c9872aVetX1DaxVGemGoZhRChOZyvNHi8HTuFgkXAdIv0AE1s0LgtMvkQcGcIzl/zsJdhkgvjWQSJ dZ0QbNPw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uwadX-00000001C1b-2AeK; Thu, 11 Sep 2025 06:18:27 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uwadU-00000001C0b-3T32 for linux-arm-kernel@lists.infradead.org; Thu, 11 Sep 2025 06:18:26 +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 42351153B; Wed, 10 Sep 2025 23:18:15 -0700 (PDT) Received: from [10.164.18.53] (unknown [10.164.18.53]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E8A6F3F66E; Wed, 10 Sep 2025 23:18:19 -0700 (PDT) Message-ID: Date: Thu, 11 Sep 2025 11:48:17 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] arm64: mm: Move KPTI helpers to mmu.c To: Ard Biesheuvel Cc: Kevin Brodsky , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Catalin Marinas , Kees Cook , Mark Rutland , Ryan Roberts , Suzuki K Poulose , Will Deacon , Yeoreum Yun References: <20250910104454.317067-1-kevin.brodsky@arm.com> <41f3227e-b945-4303-90b7-732affb0a101@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250910_231824_953754_9A8A0A6C X-CRM114-Status: GOOD ( 23.35 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 11/09/25 11:38 AM, Ard Biesheuvel wrote: > On Thu, 11 Sept 2025 at 07:13, Anshuman Khandual > wrote: >> >> >> >> On 10/09/25 4:14 PM, Kevin Brodsky wrote: >>> create_kpti_ng_temp_pgd() is currently defined (as an alias) in >>> mmu.c without matching declaration in a header; instead cpufeature.c >>> makes its own declaration. This is clearly not pretty, and as commit >>> ceca927c86e6 ("arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc >>> function signature") showed, it also makes it very easy for the >>> prototypes to go out of sync. >>> >>> All this would be much simpler if kpti_install_ng_mappings() and >>> associated functions lived in mmu.c, where they logically belong. >>> This is what this patch does: >>> - Move kpti_install_ng_mappings() and associated functions from >>> cpufeature.c to mmu.c, add a declaration to >>> - Make create_kpti_ng_temp_pgd() a static function that simply calls >>> __create_pgd_mapping_locked() instead of aliasing it >>> - Mark all these functions __init >>> - Move __initdata after kpti_ng_temp_alloc (as suggested by >>> checkpatch) >>> >>> Signed-off-by: Kevin Brodsky >>> --- >>> Note: as things stand, create_kpti_ng_temp_pgd() could be removed, >>> but a separate patch [1] will make use of it to add an >>> assertion. >>> >>> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/ >>> --- >>> Cc: Anshuman Khandual >>> Cc: Ard Biesheuvel >>> Cc: Catalin Marinas >>> Cc: Kees Cook , >>> Cc: Mark Rutland >>> Cc: Ryan Roberts >>> Cc: Suzuki K Poulose >>> Cc: Will Deacon >>> Cc: Yeoreum Yun >>> --- >>> arch/arm64/include/asm/mmu.h | 6 ++ >>> arch/arm64/kernel/cpufeature.c | 97 ------------------------------ >>> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++--- >>> 3 files changed, 103 insertions(+), 106 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h >>> index 49f1a810df16..624edd6c4964 100644 >>> --- a/arch/arm64/include/asm/mmu.h >>> +++ b/arch/arm64/include/asm/mmu.h >>> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void) >>> return true; >>> } >>> >>> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0 >>> +void kpti_install_ng_mappings(void); >> >> Could the declarations be moved here instead ? > > Why? To avoid both typedef and external instance declaration in the C code even though there is just a single call site in there. Also is not bit cleaner as well ? > >> Otherwise LGTM. >> >> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h >> index 624edd6c4964..062465939192 100644 >> --- a/arch/arm64/include/asm/mmu.h >> +++ b/arch/arm64/include/asm/mmu.h >> @@ -106,6 +106,8 @@ static inline bool kaslr_requires_kpti(void) >> >> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 >> void kpti_install_ng_mappings(void); >> +typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long); >> +kpti_remap_fn idmap_kpti_install_ng_mappings; >> #else >> static inline void kpti_install_ng_mappings(void) {} >> #endif >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> index eff3295393ee..1b5c3c590e95 100644 >> --- a/arch/arm64/mm/mmu.c >> +++ b/arch/arm64/mm/mmu.c >> @@ -750,8 +750,6 @@ static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, >> >> static int __init __kpti_install_ng_mappings(void *__unused) >> { >> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long); >> - extern kpti_remap_fn idmap_kpti_install_ng_mappings; >> kpti_remap_fn *remap_fn; >> >> int cpu = smp_processor_id(); >> >>> +#else >>> +static inline void kpti_install_ng_mappings(void) {} >>> +#endif >>> + >>> #endif /* !__ASSEMBLY__ */ >>> #endif >>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c >>> index ef269a5a37e1..b99eaad48c14 100644 >>> --- a/arch/arm64/kernel/cpufeature.c >>> +++ b/arch/arm64/kernel/cpufeature.c >>> @@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope) >>> } >>> #endif >>> >>> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0 >>> -#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT)) >>> - >>> -extern >>> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt, >>> - phys_addr_t size, pgprot_t prot, >>> - phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags); >>> - >>> -static phys_addr_t __initdata kpti_ng_temp_alloc; >>> - >>> -static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type) >>> -{ >>> - kpti_ng_temp_alloc -= PAGE_SIZE; >>> - return kpti_ng_temp_alloc; >>> -} >>> - >>> -static int __init __kpti_install_ng_mappings(void *__unused) >>> -{ >>> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long); >>> - extern kpti_remap_fn idmap_kpti_install_ng_mappings; >>> - kpti_remap_fn *remap_fn; >>> - >>> - int cpu = smp_processor_id(); >>> - int levels = CONFIG_PGTABLE_LEVELS; >>> - int order = order_base_2(levels); >>> - u64 kpti_ng_temp_pgd_pa = 0; >>> - pgd_t *kpti_ng_temp_pgd; >>> - u64 alloc = 0; >>> - >>> - if (levels == 5 && !pgtable_l5_enabled()) >>> - levels = 4; >>> - else if (levels == 4 && !pgtable_l4_enabled()) >>> - levels = 3; >>> - >>> - remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings); >>> - >>> - if (!cpu) { >>> - alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order); >>> - kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE); >>> - kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd); >>> - >>> - // >>> - // Create a minimal page table hierarchy that permits us to map >>> - // the swapper page tables temporarily as we traverse them. >>> - // >>> - // The physical pages are laid out as follows: >>> - // >>> - // +--------+-/-------+-/------ +-/------ +-\\\--------+ >>> - // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] : >>> - // +--------+-\-------+-\------ +-\------ +-///--------+ >>> - // ^ >>> - // The first page is mapped into this hierarchy at a PMD_SHIFT >>> - // aligned virtual address, so that we can manipulate the PTE >>> - // level entries while the mapping is active. The first entry >>> - // covers the PTE[] page itself, the remaining entries are free >>> - // to be used as a ad-hoc fixmap. >>> - // >>> - create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc), >>> - KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL, >>> - kpti_ng_pgd_alloc, 0); >>> - } >>> - >>> - cpu_install_idmap(); >>> - remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA); >>> - cpu_uninstall_idmap(); >>> - >>> - if (!cpu) { >>> - free_pages(alloc, order); >>> - arm64_use_ng_mappings = true; >>> - } >>> - >>> - return 0; >>> -} >>> - >>> -static void __init kpti_install_ng_mappings(void) >>> -{ >>> - /* Check whether KPTI is going to be used */ >>> - if (!arm64_kernel_unmapped_at_el0()) >>> - return; >>> - >>> - /* >>> - * We don't need to rewrite the page-tables if either we've done >>> - * it already or we have KASLR enabled and therefore have not >>> - * created any global mappings at all. >>> - */ >>> - if (arm64_use_ng_mappings) >>> - return; >>> - >>> - stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask); >>> -} >>> - >>> -#else >>> -static inline void kpti_install_ng_mappings(void) >>> -{ >>> -} >>> -#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */ >>> - >>> static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap) >>> { >>> if (__this_cpu_read(this_cpu_vector) == vectors) { >>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >>> index 183801520740..eff3295393ee 100644 >>> --- a/arch/arm64/mm/mmu.c >>> +++ b/arch/arm64/mm/mmu.c >>> @@ -27,6 +27,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include >>> #include >>> @@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys, >>> mutex_unlock(&fixmap_lock); >>> } >>> >>> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0 >>> -extern __alias(__create_pgd_mapping_locked) >>> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt, >>> - phys_addr_t size, pgprot_t prot, >>> - phys_addr_t (*pgtable_alloc)(enum pgtable_type), >>> - int flags); >>> -#endif >>> - >>> static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm, >>> enum pgtable_type pgtable_type) >>> { >>> @@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma, >>> } >>> >>> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 >>> -static pgprot_t kernel_exec_prot(void) >>> +#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT)) >>> + >>> +static phys_addr_t kpti_ng_temp_alloc __initdata; >>> + >>> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type) >>> +{ >>> + kpti_ng_temp_alloc -= PAGE_SIZE; >>> + return kpti_ng_temp_alloc; >>> +} >>> + >>> +static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, >>> + unsigned long virt, phys_addr_t size, >>> + pgprot_t prot, >>> + phys_addr_t (*pgtable_alloc)(enum pgtable_type), >>> + int flags) >>> +{ >>> + __create_pgd_mapping_locked(pgdir, phys, virt, size, prot, >>> + pgtable_alloc, flags); >>> +} >>> + >>> +static int __init __kpti_install_ng_mappings(void *__unused) >>> +{ >>> + typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long); >>> + extern kpti_remap_fn idmap_kpti_install_ng_mappings; >>> + kpti_remap_fn *remap_fn; >>> + >>> + int cpu = smp_processor_id(); >>> + int levels = CONFIG_PGTABLE_LEVELS; >>> + int order = order_base_2(levels); >>> + u64 kpti_ng_temp_pgd_pa = 0; >>> + pgd_t *kpti_ng_temp_pgd; >>> + u64 alloc = 0; >>> + >>> + if (levels == 5 && !pgtable_l5_enabled()) >>> + levels = 4; >>> + else if (levels == 4 && !pgtable_l4_enabled()) >>> + levels = 3; >>> + >>> + remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings); >>> + >>> + if (!cpu) { >>> + alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order); >>> + kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE); >>> + kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd); >>> + >>> + // >>> + // Create a minimal page table hierarchy that permits us to map >>> + // the swapper page tables temporarily as we traverse them. >>> + // >>> + // The physical pages are laid out as follows: >>> + // >>> + // +--------+-/-------+-/------ +-/------ +-\\\--------+ >>> + // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] : >>> + // +--------+-\-------+-\------ +-\------ +-///--------+ >>> + // ^ >>> + // The first page is mapped into this hierarchy at a PMD_SHIFT >>> + // aligned virtual address, so that we can manipulate the PTE >>> + // level entries while the mapping is active. The first entry >>> + // covers the PTE[] page itself, the remaining entries are free >>> + // to be used as a ad-hoc fixmap. >>> + // >>> + create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc), >>> + KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL, >>> + kpti_ng_pgd_alloc, 0); >>> + } >>> + >>> + cpu_install_idmap(); >>> + remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA); >>> + cpu_uninstall_idmap(); >>> + >>> + if (!cpu) { >>> + free_pages(alloc, order); >>> + arm64_use_ng_mappings = true; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +void __init kpti_install_ng_mappings(void) >>> +{ >>> + /* Check whether KPTI is going to be used */ >>> + if (!arm64_kernel_unmapped_at_el0()) >>> + return; >>> + >>> + /* >>> + * We don't need to rewrite the page-tables if either we've done >>> + * it already or we have KASLR enabled and therefore have not >>> + * created any global mappings at all. >>> + */ >>> + if (arm64_use_ng_mappings) >>> + return; >>> + >>> + stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask); >>> +} >>> + >>> +static pgprot_t __init kernel_exec_prot(void) >>> { >>> return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC; >>> } >>> >>> base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c >>